On Wed, 23 Sep 2009, dean wrote:
> This seems ok. This portion of code was based on vivi.c, so that might be
> checked also.
Yes, after seeing the mention of vivi in this driver I looked at vivi.c
and saw the same construct there. Though I'm willing to bet that it's
just as incorrect there as it was here, I haven't tested or otherwise
used vivi so I wasn't prepared to recommend a patch for it as well.
Probably vivi should be fixed, since it is after all intended as a model
for other v4l driver developers. (And are there any other drivers based
on vivi which have inherited this bug as well?)
-Mike
>
>
>
> Mike Isely wrote:
> > # HG changeset patch
> > # User Mike Isely <isely@xxxxxxxxx>
> > # Date 1253739604 18000
> > # Node ID 522a74147753ba59c7f45e368439928090a286f2
> > # Parent e349075171ddf939381fad432c23c1269abc4899
> > s2255drv: Don't conditionalize video buffer completion on waiting processes
> >
> > From: Mike Isely <isely@xxxxxxxxx>
> >
> > The s2255 driver had logic which aborted processing of a video frame
> > if there was no process waiting on the video buffer in question. That
> > simply doesn't work when the application is doing things in an
> > asynchronous manner. If the application went to the trouble to queue
> > the buffer in the first place, then the driver should always attempt
> > to complete it - even if the application at that moment has its
> > attention turned elsewhere. Applications which always blocked waiting
> > for I/O on the capture device would not have been affected by this.
> > Applications which *mostly* blocked waiting for I/O on the capture
> > device probably only would have been somewhat affected (frame lossage,
> > at a rate which goes up as the application blocks less). Applications
> > which never blocked on the capture device (e.g. polling only) however
> > would never have been able to receive any video frames, since in that
> > case this "is anyone waiting on this?" check on the buffer never would
> > have evalutated true. This patch just deletes that harmful check
> > against the buffer's wait queue.
> >
> > Priority: high
> >
> > Signed-off-by: Mike Isely <isely@xxxxxxxxx>
> >
> > diff -r e349075171dd -r 522a74147753 linux/drivers/media/video/s2255drv.c
> > --- a/linux/drivers/media/video/s2255drv.c Mon Sep 21 10:42:22 2009 -0500
> > +++ b/linux/drivers/media/video/s2255drv.c Wed Sep 23 16:00:04 2009 -0500
> > @@ -599,11 +599,6 @@
> > buf = list_entry(dma_q->active.next,
> > struct s2255_buffer, vb.queue);
> > - if (!waitqueue_active(&buf->vb.done)) {
> > - /* no one active */
> > - rc = -1;
> > - goto unlock;
> > - }
> > list_del(&buf->vb.queue);
> > do_gettimeofday(&buf->vb.ts);
> > dprintk(100, "[%p/%d] wakeup\n", buf, buf->vb.i);
> >
> >
> >
>
>
--
Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
