Em Wed, 30 Jan 2013 10:49:25 +0100
Hans Verkuil <hverk...@xs4all.nl> escreveu:
> On Wed 30 January 2013 10:40:30 Mauro Carvalho Chehab wrote:
> > Em Wed, 30 Jan 2013 09:01:22 +0100
> > Hans Verkuil <hverk...@xs4all.nl> escreveu:
> >
> > > 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.
> > >
> > > Changes since v1:
> > >
> > > - v1 still miscalculated the bytesperline and imagesize values (they were
> > > too large).
> > > - G_FMT had the same calculation bug.
> > >
> > > Tested with my em28xx.
> > >
> > > Regards,
> > >
> > > Hans
> > >
> > > The bytesperline calculation was incorrect: it used the old width instead
> > > of
> > > the provided width in the case of TRY_FMT, and it miscalculated the
> > > bytesperline
> > > value for the depth == 12 (planar YUV 4:1:1) case. For planar formats the
> > > bytesperline value should be the bytesperline of the widest plane, which
> > > is
> > > the Y plane which has 8 bits per pixel, not 12.
> > >
> > > Signed-off-by: Hans Verkuil <hans.verk...@cisco.com>
> > > ---
> > > drivers/media/usb/em28xx/em28xx-video.c | 8 ++++----
> > > 1 file changed, 4 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/media/usb/em28xx/em28xx-video.c
> > > b/drivers/media/usb/em28xx/em28xx-video.c
> > > index 2eabf2a..6ced426 100644
> > > --- a/drivers/media/usb/em28xx/em28xx-video.c
> > > +++ b/drivers/media/usb/em28xx/em28xx-video.c
> > > @@ -837,8 +837,8 @@ static int vidioc_g_fmt_vid_cap(struct file *file,
> > > void *priv,
> > > f->fmt.pix.width = dev->width;
> > > f->fmt.pix.height = dev->height;
> > > f->fmt.pix.pixelformat = dev->format->fourcc;
> > > - f->fmt.pix.bytesperline = (dev->width * dev->format->depth + 7) >> 3;
> > > - f->fmt.pix.sizeimage = f->fmt.pix.bytesperline * dev->height;
> > > + f->fmt.pix.bytesperline = dev->width * (dev->format->depth >> 3);
> >
> > Why did you remove the round up here?
>
> Because that would give the wrong result. Depth can be 8, 12 or 16. The YUV
> 4:1:1
> planar format is the one with depth 12. But for the purposes of the
> bytesperline
> calculation only the depth of the largest plane counts, which is the luma
> plane
> with a depth of 8. So for a width of 720 the value of bytesperline should be:
>
> depth=8 -> bytesperline = 720
> depth=12 -> bytesperline = 720
With depth=12, it should be, instead, 1080, as 2 pixels need 3 bytes.
> depth=16 -> bytesperline = 1440
Well,
depth=8 -> bytesperline = (720 * 8) + 7) / 8 = 720
depth=12 -> bytesperline = (720 * 12) + 7) / 8 = 1080
depth=16 -> bytesperline = (720 * 16) + 7) / 8 = 1440
So, this sounds perfectly OK on my eyes:
f->fmt.pix.bytesperline = (dev->width * dev->format->depth + 7) >> 3;
Regards,
Mauro
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
