Re: Question about libv4lconvert.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, 15 Dec 2010 21:10:52 +0100
Hans de Goede <hdegoede@xxxxxxxxxx> wrote:

> Hi,
>

Hi Hans, thanks for the quick reply.
 
> On 12/15/2010 05:11 PM, Antonio Ospite wrote:
> > Hi,
> >
> > I am taking a look at libv4lconvert, and I have a question about the
> > logic in v4lconvert_convert_pixfmt(), in some conversion switches there
> > is code like this:
> >
> > 	case V4L2_PIX_FMT_GREY:
> > 		switch (dest_pix_fmt) {
> > 		case V4L2_PIX_FMT_RGB24:
> > 	        case V4L2_PIX_FMT_BGR24:
> > 			v4lconvert_grey_to_rgb24(src, dest, width, height);
> > 			break;
> > 		case V4L2_PIX_FMT_YUV420:
> > 		case V4L2_PIX_FMT_YVU420:
> > 			v4lconvert_grey_to_yuv420(src, dest, fmt);
> > 			break;
> > 		}
> > 		if (src_size<  (width * height)) {
> > 			V4LCONVERT_ERR("short grey data frame\n");
> > 			errno = EPIPE;
> > 			result = -1;
> > 		}
> > 		break;
> >
> > However the conversion routines which are going to be called seem to
> > assume that the buffers, in particular the source buffer, are of the
> > correct full frame size when looping over them.
> >
> 
> Correct, because they trust that the kernel drivers have allocated large
> enough buffers to hold a valid frame which is a safe assumption.
>

Maybe this was the piece I was missing: even a partial (useful) frame
comes in a full size buffer, right? If so then the current logic is sane
indeed; and if the current assumption in conversion routines is
contradicted then it must be due to a defect related to the kernel
driver.

>  > My question is: shouldn't the size check now at the end of the case
>  > block be at the _beginning_ of it instead, so to detect a short frame
>  > before conversion and avoid a possible out of bound access inside the
>  > conversion routine?
> 
> This is done this way deliberately, this has to do with how the EPIPE
> errno variable is used in a special way.
> 
> An error return from v4lconvert_convert with an errno of EPIPE means
> I managed to get some data for you but not an entire frame. The upper
> layers of libv4l will respond to this by retrying (getting another frame),
> but only a limited number of times. Once the retries are exceeded they
> will simply pass along whatever they did manage to get.
>

Ah, that's why you do the conversion even on partial frames, I think I
see now.

> The reason for this is that there can be bus errors or vsync issues (*),
> which lead to a short frame, which are intermittent errors. So detecting
> them and getting another frame is a good thing to do because usually the
> next frame will be fine. However sometimes there are cases where every
> single frame is a short frame, for example the zc3xx driver used to
> deliver jpeg's with only 224 lines of data when running at 320x240 with
> some sensors. Now one can argue that this is a driver issue, and it is
> but it still happens in this case it is much better to pass along
> the 224 lines which we did get, then to make this a fatal error.
> 
> Note that due to the retries the user will get a much lower framerate,
> which together with the missing lines at the bottom + printing
> of error messages will hopefully be enough for the user to report
> a bug to us, despite him/her getting some picture.
> 
> I hope this explains.
>

It does, thanks a lot.

> Regards,
> 
> Hans
> 
> 
> *) While starting the stream it may take several frames for vsync to
> properly lock in some cases.
> 

Regards,
   Antonio.


-- 
Antonio Ospite
http://ao2.it

PGP public key ID: 0x4553B001

A: Because it messes up the order in which people normally read text.
   See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?

Attachment: pgp1Lj__y97Rr.pgp
Description: PGP signature


[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux