| Message ID | 4A9F9C5F.9000007@onelan.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
Simon Farnsworth wrote: > I appear to lose colour after a few seconds of capture, which I shall chase > further. And resolved - I was off-frequency by 2MHz, which leaves me surprised that I got picture. Only thing left for me to sort out is audio support.
On Thu, 2009-09-03 at 11:56 +0100, Simon Farnsworth wrote: > Simon Farnsworth wrote: > > I appear to lose colour after a few seconds of capture, which I shall chase > > further. > > And resolved - I was off-frequency by 2MHz, which leaves me surprised that > I got picture. Only thing left for me to sort out is audio support. I've got a start at cx18-alsa support in http://linuxtv.org/hg/~awalls/mc-lab (IIRC) But it's only really the non-working skeleton of things to get ALSA device nodes. The behind the scenes work of actually opening the PCM stream for ALSA PCM nodes and proper locking with the /dev/video24 PCM stream nodes is not there. You'll have to use /dev/video24 for now. Regards, Andy -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. > Ah, actually this is a driver bug, not a libv4l2 bug, but I'll fix things in libv4l to work around it for now. read() should always return an entire frame (or as much of it as will fit and throw away the rest). 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. > > > > Ah, actually this is a driver bug, not a libv4l2 bug, but I'll fix things > in libv4l to work around it for now. > > read() should always return an entire frame (or as much of it as will fit > and throw away the rest). 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(). Hans and Hans, I'll have time to look into this on Friday and see what can be done. Hans de Goede, I may ask for more information/explanation later. Regards, Andy > Regards, > > Hans -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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@vger.kernel.org > 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, On 09/04/2009 05:14 AM, Andy Walls wrote: > 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. > It does seem to say that yes, as said in a previous mail of me, this part of the spec needs some fixing. It seems to try and describe the queuing that happens inside the driver in too much detail and leaves out other much me important bits about how read() on video devices behaves. > > 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? I agree that waiting until at least one whole frame is available. Is the correct behaviour. Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff -r c51a90c0f62f v4l2-apps/libv4l/libv4l2/libv4l2.c --- a/v4l2-apps/libv4l/libv4l2/libv4l2.c Tue Sep 01 10:03:27 2009 +0200 +++ b/v4l2-apps/libv4l/libv4l2/libv4l2.c Thu Sep 03 11:32:40 2009 +0100 @@ -326,7 +326,7 @@ } do { - result = SYS_READ(devices[index].fd, devices[index].readbuf, buf_size); + result = SYS_READ(devices[index].fd, devices[index].readbuf, devices[index].dest_fmt.fmt.pix.sizeimage); if (result <= 0) { if (result && errno != EAGAIN) { int saved_err = errno;