Re: [REVIEW] au0828-video.c

[Hans Verkuil <hverkuil@xs4all.nl>]

On 12/12/2014 04:52 PM, Devin Heitmueller wrote:
>> No, tvtime no longer hangs if no frames arrive, so there is no need for
>> this timeout handling. I'd strip it out, which can be done in a separate
>> patch.
> 
> Did you actually try it?

Mauro tried it, not me. I'm not sure if he looked at whether the user
interface is blocked when waiting for a frame.

> Do you have some patches to tvtime which
> aren't upstream?
> 
> I wrote the comment in question (and added the associated code).  The
> issue is that tvtime does *everything* in a single thread (except the
> recent ALSA audio work), that includes servicing the video/vbi devices
> as well as the user interface.  That thread blocks on a DQBUF ioctl
> until data arrives, and thus if frames are not being delivered it will
> hang the entire tvtime user interface.
> 
> Now you can certainly argue that is a bad design decision, but it's
> been that way for 15+ years, so we can't break it now.  Hence why I
> generate dummy frames on a timeout if the decoder isn't delivering
> video.  Unfortunately the au8522 doesn't have a free running mode
> (i.e. blue screen if no video), which is why most of the other devices
> work fine (decoders by Conexant, NXP, Trident, etc all have such
> functionality).
> 
> Don't get me wrong - I *hate* that I had to put that timer crap in the
> driver, but it was necessary to be compatible with one of the most
> popular applications out there.
> 
> In short, that code cannot be removed.

Sure it can. I just tried tvtime and you are right, it blocks the GUI.
But the fix is very easy as well. So now I've updated tvtime so that
it timeouts and gives the GUI time to update itself.

No more need for such an ugly hack in au0828. The au0828 isn't the only
driver that can block, others do as well. Admittedly, they aren't very
common, but they do exist. So it is much better to fix the application
than adding application workarounds in the kernel.

Regards,

	Hans