#67 video streaming doesn't restart after processing a barcode

version_0.10
open
nobody
None
5
2013-02-02
2012-06-01
klaus triendl
No

On slow computers - e.g. Windows 7 tablet with Intel Atom, a multi-threading issue shows up:

When multiple barcodes are processed (e.g. by calling zbar_process_one()) with the same processor instance then it happens that sooner or later the processing doesn't start anymore - the preview window stays at the zbar logo. For it to happen it doesn't matter whether the user closes the window or a barcode has been processed.

The procedure that causes this issue is proc_video_thread() which aborts processing when it doesn't get a sample image anymore but when the streaming flag is still active (proc->streaming == true).
Because proc->streaming isn't reset processing another barcode won't start anymore.
(Because of the similarity of the patches I want to mention that this issue has nothing to do with bug "vfw camera hangs in small resolutions - ID: 3337972" (https://sourceforge.net/tracker/?func=detail&aid=3337972&group_id=189236&atid=928515))

The simplest thing we could do is to always abort processing when no image is available (see patch). I don't know whether this is feasible but it works for me.
A better fix would require a different handling of the proc->streaming and vdo->active flags.

Detailed comment (from the provided patch) for the case that proc_video_thread() doesn't get a sample image:

there's one plausible reason we've got no image:
video streaming has been aborted (e.g. window has been closed by user).
However it's not wise to just test for !proc->streaming in a multi-threaded environment,
and to abort all other cases as problems that should end this thread (if proc->streaming is still true).

reason: there are two flags that determine whether streaming is active:
vdo->active and proc->streaming, both of them are set independently in zbar_processor_set_active().

real case szenario on a windows 7 tablet with intel atom:
1) main thread calls zbar_process_one(), streaming is set up and proc_video_thread() does its work
2) then e.g. user closes the preview window and zbar_process_one() in turn calls zbar_processor_set_active(proc, 0).
zbar_processor_set_active() in turn calls zbar_video_enable() which sets vdo->active=false.
3) now, before zbar_processor_set_active() is able to set proc->streaming=false proc_video_thread() runs its next loop iteration, calls zbar_video_next_image() above which immediately returns because vdo->active==false, but proc->streaming is still true

Discussion

  • Klaus, why not just:

    diff --git a/zbar/processor.c b/zbar/processor.c
    --- a/zbar/processor.c
    +++ b/zbar/processor.c
    @@ -208,7 +208,7 @@
    zbar_image_t *img = zbar_video_next_image(proc->video);
    _zbar_mutex_lock(&proc->mutex);

    - if(!img && !proc->streaming)
    + if(!img && proc->streaming)
    continue;
    else if(!img)
    /* FIXME could abort streaming and keep running? */

    There is simply a mistake in the condition. This is the patch I run with for months.

     
  • klaus triendl
    klaus triendl
    2012-06-04

    I'm afraid that your patch wouldn't solve the problem.

    An important note before I elaborate: my sentence "Because proc->streaming isn't reset processing another barcode won't start anymore." isn't true, which I've just realized.

    The real problem is that the processor's thread has ended and isn't restarted anymore, only if we would call zbar_processor_init() again (which starts the thread).
    The way I'm using zbar is to have one processor instance, initialize it once, and then repeatedly call zbar_process_one().

    Now, why wouldn't your patch solve the problem?
    Remember, we're in a multi-threaded environment. In case we don't get an image and zbar_processor_set_active() has already set proc->streaming to false then the thread would be ended as well, yielding the same problem as described by this bug report.
    It's really a multi-threading issue.

    In the meantime I'm thinking that the processor thread shouldn't be ended at all except of course 'normally' by zbar_processor_destroy().

    Also I don't think that it's a mistake in the condition, otherwise the comment 'FIXME could abort streaming...' doesn't make sense.

     
  • Let's have a look at the original code to decide when the comment makes sense:

    if(!img && !proc->streaming)
    continue;
    else if(!img)
    /* FIXME could abort streaming and keep running? */
    break;

    Here the comment "could abort streaming (...)?" describes the condition: "!img && proc->streaming". Why talking about "aborting" if proc->streaming is true? That doesn't make sense. But it starts to make sense when the condition is inverted into "!img && !proc->streaming". This is "aborting" and this is a "fixme" you aim for.

    I understand why you don't want "break". That's right. But there's no reason to do "proc->streaming = 0". In your patch you say:

    + // 3) now, before zbar_processor_set_active() is able to set proc->streaming=false
    + // proc_video_thread() runs its next loop iteration, calls zbar_video_next_image()
    + // above which immediately returns because vdo->active==false, but proc->streaming
    + // is still true

    I have a question. Will zbar_processor_set_active never reach this statement?
    proc->streaming = active;
    I think it will reach that statement and this will finish processing in a proper way. So there's no need to do proc->streaming=0 in another place.

     
  • I admit that my understanding of things happening in proc_video_thread and this is visible in my previous comment. I just found a quick workaround for my problems, but it does not cover all the cases. I confirm it's still possible to get into video freeze, so something needs to be done. I think we need to insert somewhere simulated computations to slow down the process and easily reproduce both issues:
    1. hanging of zbarcam
    2. inability to properly close down the processor