On Thu, 2009-09-03 at 13:20 +0200, Hans de Goede wrote: > Hans Verkuil, > > I think we have found a bug in the read() implementation of the cx18 > driver, see below. > > > Hi all, > > On 09/03/2009 12:37 PM, Simon Farnsworth wrote: > > Simon Farnsworth wrote: > >> Hans de Goede wrote: > >>> Ok, > >>> > >>> That was even easier then I thought it would be. Attached is a > >>> patch (against libv4l-0.6.1), which implements 1) and 3) from > >>> above. > >>> > >> I applied it to a clone of your HG repository, and had to make a > >> minor change to get it to compile. I've attached the updated patch. > >> > >> It looks like the read() from the card isn't reading entire frames > >> ata a time - I'm using a piece of test gear that I have to return in > >> a couple of hours to send colourbars to it, and I'm seeing bad > >> colour, and the picture moving across the screen. I'll try and chase > >> this, see whether there's something obviously wrong. > >> > > There is indeed something obviously wrong; at line 315 of libv4l2.c, we > > expand the buffer we read into, then ask for that many bytes. > > Hans de Goede, > Ah, actually this is a driver bug, I agree. > not a libv4l2 bug, but I'll fix things > in libv4l to work around it for now. OK, thanks. > read() should always return an entire frame (or as much of it as will fit I agree > and throw away the rest). That sounds fine to me. Hans and Hans, The V4L2 spec for the read() call seems unlcear to me: "Return Value On success, the number of bytes read is returned. It is not an error if this number is smaller than the number of bytes requested, or the amount of data required for one frame. This may happen for example because read() was interrupted by a signal. On error, -1 is returned, and the errno variable is set appropriately. In this case the next read will start at the beginning of a new frame." To me, the spec only says the remainder of a frame is thrown away when read() exits with an error. BTW, should select() return "data available", if less than one whole frame is available? It can happen if the buffers we give to the CX23418 firmware don't exactly match the YUV framesize. The V4l2 spec for the read() call seems to imply that read() should block (or return with EAGAIN) until at least one whole frame is available. Is that correct? Regards, Andy > Think for example of jpeg streams, where the > exact size of the image isn't known by the client (as it differs from frame > to frame). dest_fmt.fmt.pix.sizeimage purely is an upper limit, and so > is the value passed in to read(), the driver itself should clamp it so > that it returns exactly one frame (for formats which are frame based). > > The page alignment (2 pages on i386 / one on x86_64) is done because some > drivers internally use page aligned buffer sizes and thus for example with > jpeg streams, can have frames queued for read() slightly bigger then > dest_fmt.fmt.pix.sizeimage, but when this happens that is really a driver bug, > because as said dest_fmt.fmt.pix.sizeimage should report an upper limit > of the the frame sizes to be expected. I'll remove the align workaround, as > that bug is much less likely to be hit (and probably easier to fix at the > driver level) then the issue we're now seeing with read(). > > Regards, > > Hans > -- > 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 > -- 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
