Quantcast

[Zbarw-devel] how to fix proc_video_thread break

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[Zbarw-devel] how to fix proc_video_thread break

jarekczek
Administrator
Klaus, I need to fix the "break" in proc_video_thread. Currently it is
definitely out of place, until this moment we agree.

In line 211 we have 4 possibilities:

1. img
2. !img && proc->streaming, because img was temporarily unavailable
3. !img && proc->streaming, because the processor is being switched off
4. !img && !proc->streaming, because the processor is being switched off

What to do:
1. all ok
4. break, because the processor is being switched off. However if the
rest of code is ok, continue should also work.

The problematic cases 2 and 3 are difficult, because we don't know
whether the processor is being switched off or not. So I suggest to
"continue".

What do you think about it?

Additional info:
In r118 spadix made a change, uncommented of course. Before the change
things were really simple:
116:         if(!img)
116:             /* FIXME could abort streaming and keep running? */
116:             break;
So at that time he didn't consider case 2 at all. Later he felt the need
to "continue" under some conditions, but what pushed him in this
direction? I don't know. My hypothesis is that he made a mistake with !

Jarek

------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and
threat landscape has changed and how IT managers can respond. Discussions
will include endpoint security, mobile security and the latest in malware
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
_______________________________________________
Zbarw-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/zbarw-devel
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [Zbarw-devel] how to fix proc_video_thread break

klaus triendl
Am Sat, 09 Jun 2012 17:41:18 +0200
schrieb Jarek Czekalski <[hidden email]>:

> Klaus, I need to fix the "break" in proc_video_thread. Currently it
> is definitely out of place, until this moment we agree.

Great, then we have a common goal at least :)


> In line 211 we have 4 possibilities:
>
> 1. img
> 2. !img && proc->streaming, because img was temporarily unavailable
> 3. !img && proc->streaming, because the processor is being switched
> off 4. !img && !proc->streaming, because the processor is being
> switched off
>
> What to do:
> 1. all ok
> 4. break, because the processor is being switched off. However if the
> rest of code is ok, continue should also work.
>
> The problematic cases 2 and 3 are difficult, because we don't know
> whether the processor is being switched off or not. So I suggest to
> "continue".
>
> What do you think about it?

Never break at all. We can't tell at this point why we don't get an
image. Breaking means ending the video thread which leads to video
freezing.
We must not end the video thread.
If it's a temporary problem it will recover from itself.
Otherwise there is another big problem going on anyway, which has to be
examined.

The patch I've submitted originally sets proc->streaming=0. I agree with
you that there's no need to disable streaming at this point because
zbar_processor_set_active will do this anyway in a controlled
environment.
We could think of it as something like the middle way between
break/continue conditions. However, we would end the streaming if an
image would be just temporarily unavailable.

therefore, in code:
if (!img)
    continue;


> Additional info:
> In r118 spadix made a change, uncommented of course. Before the
> change things were really simple:
> 116:         if(!img)
> 116:             /* FIXME could abort streaming and keep running? */
> 116:             break;
> So at that time he didn't consider case 2 at all. Later he felt the
> need to "continue" under some conditions, but what pushed him in this
> direction? I don't know. My hypothesis is that he made a mistake
> with !

He has chosen this solution because the video thread shouldn't be
stopped when the streaming has been disabled e.g. by closing the video
window.

What he didn't assume, but is happening:
1) img temporarily unavailable
2) multithreading issues



Klaus

------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and
threat landscape has changed and how IT managers can respond. Discussions
will include endpoint security, mobile security and the latest in malware
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
_______________________________________________
Zbarw-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/zbarw-devel
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [Zbarw-devel] how to fix proc_video_thread break

jarekczek
Administrator
W dniu 2012-06-09 21:16, klaus triendl pisze:
> 2. !img&&  proc->streaming, because img was temporarily unavailable
I finally managed to rule this condition out, as denying the
documentation. See [#8], which is easy to fix.

That was the base for my previous statment, so now I don't feel the need
to change this code at all. At the time of starting this thread I was
not aware of zbar_video_next_image documentation. And even if I had
been, I didn't believe it would be possible to fix.

Jarek

------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and
threat landscape has changed and how IT managers can respond. Discussions
will include endpoint security, mobile security and the latest in malware
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
_______________________________________________
Zbarw-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/zbarw-devel
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [Zbarw-devel] how to fix proc_video_thread break

klaus triendl
Am Sun, 10 Jun 2012 12:50:28 +0200
schrieb Jarek Czekalski <[hidden email]>:

> W dniu 2012-06-09 21:16, klaus triendl pisze:
> > 2. !img&&  proc->streaming, because img was temporarily unavailable
> I finally managed to rule this condition out, as denying the
> documentation. See [#8], which is easy to fix.

I've just fixed it, can you look over the patch 'zbarw_ticke_8'?
We should apply this to vfw as well.


> That was the base for my previous statment, so now I don't feel the
> need to change this code at all. At the time of starting this thread
> I was not aware of zbar_video_next_image documentation. And even if I
> had been, I didn't believe it would be possible to fix.

Please apply patch 'delay_proc_deactivation' to reproduce my reported
bug. You need to have a test program that reuses the processor instance
- see attached C++ test program, which you can compile with g++.
Zbarcam might be enought, though - it doesn't matter whether you scan
barcodes multiple times or close the preview window by hand.


So, there are the following conditions left:

2. !img && proc->streaming, because of some sort of error (you called
that temporary error)
3. !img && proc->streaming, because the processor is being switched off
4. !img && !proc->streaming, because the processor is being switched off

2. is very unlikely after having fixed the video freeze. It can only
happen if waiting for the capture event fails for unknown reasons.

So we practically can reduce to 2. and 3. and I would fix that with:

if (!img)
    continue;

Even if some unknown error would happen in zbar_video_next_image or
dshow_dq, we would still be on the safe side.



I'd really like to fix that until tomorrow because I'm deploying my
barcode scanner app on wednesday...


Regards,
Klaus
------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and
threat landscape has changed and how IT managers can respond. Discussions
will include endpoint security, mobile security and the latest in malware
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
_______________________________________________
Zbarw-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/zbarw-devel

zbarw_ticket_8.patch (4K) Download Attachment
delay_proc_deactivation.patch (776 bytes) Download Attachment
zbar_reuse_proc.cpp (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [Zbarw-devel] how to fix proc_video_thread break

jarekczek
Administrator
Hi Klaus

If you are in a hurry you may consider applying the changes only to your
copy. However if you are sure you would do exactly the same without the
time pressure, go on with your changes. Only quick remarks from a glance
at the patch:

1. zbar already has a routine calling FormatMessage. Probably you can
use it instead of the new macro.
2. "Because of this the actually returned image might not be the
signaling one but already a newer sample" - I don't think it's necessary
in the function description. There is a comment about it in the body.
3. You not only fixed the problem, but also changed the behaviour, now
waiting for a signal even if the picture is available. This change is
not necessary and requires a separate justification.
4. If you think it's necessary to remove the break, do it. If we
discover (in the future) the conditions under which "break" is
necessary, we may insert it back and give the appropriate comment then.

Jarek


W dniu 2012-06-11 15:17, klaus triendl pisze:

> Am Sun, 10 Jun 2012 12:50:28 +0200
> schrieb Jarek Czekalski<[hidden email]>:
>
>> W dniu 2012-06-09 21:16, klaus triendl pisze:
>>> 2. !img&&   proc->streaming, because img was temporarily unavailable
>> I finally managed to rule this condition out, as denying the
>> documentation. See [#8], which is easy to fix.
> I've just fixed it, can you look over the patch 'zbarw_ticke_8'?
> We should apply this to vfw as well.
>
>
>> That was the base for my previous statment, so now I don't feel the
>> need to change this code at all. At the time of starting this thread
>> I was not aware of zbar_video_next_image documentation. And even if I
>> had been, I didn't believe it would be possible to fix.
> Please apply patch 'delay_proc_deactivation' to reproduce my reported
> bug. You need to have a test program that reuses the processor instance
> - see attached C++ test program, which you can compile with g++.
> Zbarcam might be enought, though - it doesn't matter whether you scan
> barcodes multiple times or close the preview window by hand.
>
>
> So, there are the following conditions left:
>
> 2. !img&&  proc->streaming, because of some sort of error (you called
> that temporary error)
> 3. !img&&  proc->streaming, because the processor is being switched off
> 4. !img&&  !proc->streaming, because the processor is being switched off
>
> 2. is very unlikely after having fixed the video freeze. It can only
> happen if waiting for the capture event fails for unknown reasons.
>
> So we practically can reduce to 2. and 3. and I would fix that with:
>
> if (!img)
>      continue;
>
> Even if some unknown error would happen in zbar_video_next_image or
> dshow_dq, we would still be on the safe side.
>
>
>
> I'd really like to fix that until tomorrow because I'm deploying my
> barcode scanner app on wednesday...
>
>
> Regards,
> Klaus

------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and
threat landscape has changed and how IT managers can respond. Discussions
will include endpoint security, mobile security and the latest in malware
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
_______________________________________________
Zbarw-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/zbarw-devel
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [Zbarw-devel] how to fix proc_video_thread break

klaus triendl
Am Mon, 11 Jun 2012 18:33:17 +0200
schrieb Jarek Czekalski <[hidden email]>:

> remarks from a glance at the patch:
>
> 1. zbar already has a routine calling FormatMessage. Probably you can
> use it instead of the new macro.

Thanks for the hint


> 3. You not only fixed the problem, but also changed the
> behaviour, now waiting for a signal even if the picture is available.
> This change is not necessary and requires a separate justification.

The idea was to tie state->image and state->captured together. The
image is only available if 'captured' is signaled. The video freeze bug
happened exactly because it wasn't like this.

But you're right, it's unnecessary because the lock is active, so we
can save the possibly costly `unlock - query event - lock` operation.




Klaus

------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and
threat landscape has changed and how IT managers can respond. Discussions
will include endpoint security, mobile security and the latest in malware
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
_______________________________________________
Zbarw-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/zbarw-devel
Loading...