Re: [RFC PATCH] em28xx: fix bytesperline calculation in TRY_FMT

["Frank Schäfer" <fschaefer.oss@googlemail.com>]

Am 29.01.2013 10:49, schrieb Hans Verkuil:
> This was part of my original em28xx patch series. That particular patch
> combined two things: this fix and the change where TRY_FMT would no
> longer return -EINVAL for unsupported pixelformats. The latter change was
> rejected (correctly), but we all forgot about the second part of the patch
> which fixed a real bug. I'm reposting just that fix.
>
> Regards,
>
> 	Hans
>
> The bytesperline calculation was incorrect: it used the old width instead
> of the provided width, and it miscalculated the bytesperline value for the
> depth == 12 case.
>
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> ---
>  drivers/media/usb/em28xx/em28xx-video.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/media/usb/em28xx/em28xx-video.c b/drivers/media/usb/em28xx/em28xx-video.c
> index 2eabf2a..070506d 100644
> --- a/drivers/media/usb/em28xx/em28xx-video.c
> +++ b/drivers/media/usb/em28xx/em28xx-video.c
> @@ -906,7 +906,7 @@ static int vidioc_try_fmt_vid_cap(struct file *file, void *priv,
>  	f->fmt.pix.width = width;
>  	f->fmt.pix.height = height;
>  	f->fmt.pix.pixelformat = fmt->fourcc;
> -	f->fmt.pix.bytesperline = (dev->width * fmt->depth + 7) >> 3;
> +	f->fmt.pix.bytesperline = width * ((fmt->depth + 7) >> 3);
>  	f->fmt.pix.sizeimage = f->fmt.pix.bytesperline * height;
>  	f->fmt.pix.colorspace = V4L2_COLORSPACE_SMPTE170M;
>  	if (dev->progressive)

Hmm... how are 12 bit pixels stored ? Are padding bits used so that 2
bytes per pixel are needed ?
I wonder if V4L2_PIX_FMT_YUV411P has ever been tested (libv4lconvert
doesn't support it)...

While we are at it, we should check and fix the other size calculations,
too.
For example, in em28xx-video.c we have in

vidioc_g_fmt_vid_cap():
f->fmt.pix.bytesperline = (dev->width * dev->format->depth + 7) >> 3;

queue_setup():
size = (dev->width * dev->height * dev->format->depth + 7) >> 3;

buffer_prepare():
size = (dev->width * dev->height * dev->format->depth + 7) >> 3;

em28xx_copy_video():
int bytesperline = dev->width << 1;

and there are probably more places...


Regards,
Frank