Hi Mauro, On Tuesday 16 September 2014 07:58:12 Mauro Carvalho Chehab wrote: > Em Tue, 16 Sep 2014 13:15:27 +0300 Laurent Pinchart escreveu: > > On Tuesday 16 September 2014 07:01:29 Mauro Carvalho Chehab wrote: > > > Em Tue, 16 Sep 2014 12:09:01 +0300 Laurent Pinchart escreveu: > > > > On Monday 15 September 2014 21:10:55 Mauro Carvalho Chehab wrote: > > > > > This reverts commit 9241650d62f79a3da01f1d5e8ebd195083330b75. > > > > > > > > > > The commit 9241650d62f7 was meant to solve an issue with Gstreamer > > > > > version 0.10 with libv4l 1.2, where a fixup patch for DQBUF exposed > > > > > a bad behavior ag Gstreamer. > > > > > > > > That's not correct. The patch was created to solve an issue observed > > > > with the Gstreamer 0.10 v4l2src element accessing the video device > > > > directly, *without* libv4l. > > > > > > Ok. From the discussions we took yesterday on the thread, I got the > > > wrong impression from Nicolas comments that this happens only with > > > gst < 1.4 and libv4l >= 1.2. > > > > My understanding is that recent gst versions worked around the problem, > > and the above combination of versions might be problematic, but gst 0.10 > > is definitely affected. > > > > > > The V4L2 specification documents poll() as follows. > > > > > > > > "When the application did not call VIDIOC_QBUF or VIDIOC_STREAMON yet > > > > the poll() function succeeds, but sets the POLLERR flag in the revents > > > > field." > > > > > > > > The vb2 poll implementation didn't conform with that, as it returned > > > > POLLERR when the buffer list was empty due to a transient buffer > > > > underrun, even if both VIDIOC_STREAMON and VIDIOC_QBUF have been > > > > called. > > > > > > > > The commit thus brought the vb2 poll implementation in line with the > > > > specification. If we really want to revert it to its broken behaviour, > > > > then it would be fair to explain this in the revert message, > > > > > > Ok, I'll rewrite the text. We likely want to fix the documentation too, > > > in order to reflect the way it is. > > > > > > > and I want to know how you propose fixing this properly, as the revert > > > > really causes issues for userspace. > > > > > > This patch simply broke all VBI applications. So, it should be reverted. > > > > > > From what you're saying, using Gst 0.10 with a kernel before 3.16 and > > > VB2 was always broken, right? > > > > Correct. And not only gst 0.10, any userspace application that doesn't > > specifically handles transient buffer underruns will be affected. > > > > vb2 doesn't conform to the V4L2 specification, and I believe the > > specification is right in this case. Reverting this patch will push the > > problem to userspace, where all applications will have to handle buffer > > underruns manually. > > What happens with VB1? How is it solved there? > > I don't generally use gst 0.10, but I don't remember a single error > report about gst 0.10 and VB1-based drivers.
I haven't tried VB1, but a quick look at the source code shows it to be affected as well. The problem with gst 0.10 is only noticeable when a buffer underrun occurs (when you don't requeue buffers fast enough and the queue buffers list becomes temporarily empty), so it can very well go unnoticed for a long time. > > > And with VB1, is it also broken? If so, then this is a Gst 0.10 bug, > > > and the fix should be a patch for it, or a recommendation to upgrade > > > to a newer version without such bug. > > > > As explained above, this isn't a gst bug. > > > > > If, otherwise, it works with VB1, then we need to patch VB2 to have > > > exactly the same behavior as VB1 with that regards, as VBI works > > > with VB1. > > > > One option would be to have implement a different poll behaviour for VBI > > and video. > > That would be a nightmare. I don't like it much either. Hans has posted a proposal to fix the problem an hour ago, let's discuss it. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
