diff mbox

uvc: Fix bytesperline calculation for planar YUV

Message ID 1452199428-3513-1-git-send-email-nicolas.dufresne@collabora.com (mailing list archive)
State Accepted, archived
Delegated to: Laurent Pinchart
Headers

Commit Message

Nicolas Dufresne Jan. 7, 2016, 8:43 p.m. UTC 
  The formula used to calculate bytesperline only works for packed format.
So far, all planar format we support have their bytesperline equal to
the image width (stride of the Y plane or a line of Y for M420).

Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
---
 drivers/media/usb/uvc/uvc_v4l2.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

Comments

Laurent Pinchart April 13, 2016, 2:36 p.m. UTC | #1 
Hi Nicolas,

Thank you for the patch.

On Thursday 07 Jan 2016 15:43:48 Nicolas Dufresne wrote:
> The formula used to calculate bytesperline only works for packed format.
> So far, all planar format we support have their bytesperline equal to
> the image width (stride of the Y plane or a line of Y for M420).
> 
> Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
> ---
>  drivers/media/usb/uvc/uvc_v4l2.c | 18 ++++++++++++++++--
>  1 file changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/usb/uvc/uvc_v4l2.c
> b/drivers/media/usb/uvc/uvc_v4l2.c index d7723ce..ceb1d1b 100644
> --- a/drivers/media/usb/uvc/uvc_v4l2.c
> +++ b/drivers/media/usb/uvc/uvc_v4l2.c
> @@ -142,6 +142,20 @@ static __u32 uvc_try_frame_interval(struct uvc_frame
> *frame, __u32 interval) return interval;
>  }
> 
> +static __u32 uvc_v4l2_get_bytesperline(struct uvc_format *format,
> +	struct uvc_frame *frame)

I'd make the two parameters const.

> +{
> +	switch (format->fcc) {
> +	case V4L2_PIX_FMT_NV12:
> +	case V4L2_PIX_FMT_YVU420:
> +	case V4L2_PIX_FMT_YUV420:
> +	case V4L2_PIX_FMT_M420:
> +		return frame->wWidth;
> +	default:
> +		return format->bpp * frame->wWidth / 8;
> +	}
> +}
> +
>  static int uvc_v4l2_try_format(struct uvc_streaming *stream,
>  	struct v4l2_format *fmt, struct uvc_streaming_control *probe,
>  	struct uvc_format **uvc_format, struct uvc_frame **uvc_frame)
> @@ -245,7 +259,7 @@ static int uvc_v4l2_try_format(struct uvc_streaming
> *stream, fmt->fmt.pix.width = frame->wWidth;
>  	fmt->fmt.pix.height = frame->wHeight;
>  	fmt->fmt.pix.field = V4L2_FIELD_NONE;
> -	fmt->fmt.pix.bytesperline = format->bpp * frame->wWidth / 8;
> +	fmt->fmt.pix.bytesperline = uvc_v4l2_get_bytesperline(format, frame);
>  	fmt->fmt.pix.sizeimage = probe->dwMaxVideoFrameSize;
>  	fmt->fmt.pix.colorspace = format->colorspace;
>  	fmt->fmt.pix.priv = 0;
> @@ -282,7 +296,7 @@ static int uvc_v4l2_get_format(struct uvc_streaming
> *stream, fmt->fmt.pix.width = frame->wWidth;
>  	fmt->fmt.pix.height = frame->wHeight;
>  	fmt->fmt.pix.field = V4L2_FIELD_NONE;
> -	fmt->fmt.pix.bytesperline = format->bpp * frame->wWidth / 8;
> +	fmt->fmt.pix.bytesperline = uvc_v4l2_get_bytesperline(format, frame);
>  	fmt->fmt.pix.sizeimage = stream->ctrl.dwMaxVideoFrameSize;
>  	fmt->fmt.pix.colorspace = format->colorspace;
>  	fmt->fmt.pix.priv = 0;

This looks good to me otherwise.

If it's fine with you I can fix the above issue while applying.
Nicolas Dufresne April 13, 2016, 3:57 p.m. UTC | #2 
Le mercredi 13 avril 2016 à 17:36 +0300, Laurent Pinchart a écrit :
> Hi Nicolas,
> 
> Thank you for the patch.
> 
> On Thursday 07 Jan 2016 15:43:48 Nicolas Dufresne wrote:
> > 
> > The formula used to calculate bytesperline only works for packed
> > format.
> > So far, all planar format we support have their bytesperline equal
> > to
> > the image width (stride of the Y plane or a line of Y for M420).
> > 
> > Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
> > ---
> >  drivers/media/usb/uvc/uvc_v4l2.c | 18 ++++++++++++++++--
> >  1 file changed, 16 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/media/usb/uvc/uvc_v4l2.c
> > b/drivers/media/usb/uvc/uvc_v4l2.c index d7723ce..ceb1d1b 100644
> > --- a/drivers/media/usb/uvc/uvc_v4l2.c
> > +++ b/drivers/media/usb/uvc/uvc_v4l2.c
> > @@ -142,6 +142,20 @@ static __u32 uvc_try_frame_interval(struct
> > uvc_frame
> > *frame, __u32 interval) return interval;
> >  }
> > 
> > +static __u32 uvc_v4l2_get_bytesperline(struct uvc_format *format,
> > +	struct uvc_frame *frame)
> I'd make the two parameters const.

I agree.

> 
> > 
> > +{
> > +	switch (format->fcc) {
> > +	case V4L2_PIX_FMT_NV12:
> > +	case V4L2_PIX_FMT_YVU420:
> > +	case V4L2_PIX_FMT_YUV420:
> > +	case V4L2_PIX_FMT_M420:
> > +		return frame->wWidth;
> > +	default:
> > +		return format->bpp * frame->wWidth / 8;
> > +	}
> > +}
> > +
> >  static int uvc_v4l2_try_format(struct uvc_streaming *stream,
> >  	struct v4l2_format *fmt, struct uvc_streaming_control
> > *probe,
> >  	struct uvc_format **uvc_format, struct uvc_frame
> > **uvc_frame)
> > @@ -245,7 +259,7 @@ static int uvc_v4l2_try_format(struct
> > uvc_streaming
> > *stream, fmt->fmt.pix.width = frame->wWidth;
> >  	fmt->fmt.pix.height = frame->wHeight;
> >  	fmt->fmt.pix.field = V4L2_FIELD_NONE;
> > -	fmt->fmt.pix.bytesperline = format->bpp * frame->wWidth /
> > 8;
> > +	fmt->fmt.pix.bytesperline =
> > uvc_v4l2_get_bytesperline(format, frame);
> >  	fmt->fmt.pix.sizeimage = probe->dwMaxVideoFrameSize;
> >  	fmt->fmt.pix.colorspace = format->colorspace;
> >  	fmt->fmt.pix.priv = 0;
> > @@ -282,7 +296,7 @@ static int uvc_v4l2_get_format(struct
> > uvc_streaming
> > *stream, fmt->fmt.pix.width = frame->wWidth;
> >  	fmt->fmt.pix.height = frame->wHeight;
> >  	fmt->fmt.pix.field = V4L2_FIELD_NONE;
> > -	fmt->fmt.pix.bytesperline = format->bpp * frame->wWidth /
> > 8;
> > +	fmt->fmt.pix.bytesperline =
> > uvc_v4l2_get_bytesperline(format, frame);
> >  	fmt->fmt.pix.sizeimage = stream->ctrl.dwMaxVideoFrameSize;
> >  	fmt->fmt.pix.colorspace = format->colorspace;
> >  	fmt->fmt.pix.priv = 0;
> This looks good to me otherwise.
> 
> If it's fine with you I can fix the above issue while applying.

That would be really nice.

thanks,
Nicolas
Laurent Pinchart April 14, 2016, 4:32 p.m. UTC | #3 
Hi Nicolas,

On Wednesday 13 Apr 2016 11:57:34 Nicolas Dufresne wrote:
> Le mercredi 13 avril 2016 à 17:36 +0300, Laurent Pinchart a écrit :
> > Hi Nicolas,
> > 
> > Thank you for the patch.
> > 
> > On Thursday 07 Jan 2016 15:43:48 Nicolas Dufresne wrote:
> > > The formula used to calculate bytesperline only works for packed
> > > format.
> > > So far, all planar format we support have their bytesperline equal
> > > to
> > > the image width (stride of the Y plane or a line of Y for M420).
> > > 
> > > Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
> > > ---
> > >  drivers/media/usb/uvc/uvc_v4l2.c | 18 ++++++++++++++++--
> > >  1 file changed, 16 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/media/usb/uvc/uvc_v4l2.c
> > > b/drivers/media/usb/uvc/uvc_v4l2.c index d7723ce..ceb1d1b 100644
> > > --- a/drivers/media/usb/uvc/uvc_v4l2.c
> > > +++ b/drivers/media/usb/uvc/uvc_v4l2.c
> > > @@ -142,6 +142,20 @@ static __u32 uvc_try_frame_interval(struct
> > > uvc_frame
> > > *frame, __u32 interval) return interval;
> > >  }
> > > 
> > > +static __u32 uvc_v4l2_get_bytesperline(struct uvc_format *format,
> > > +	struct uvc_frame *frame)
> > 
> > I'd make the two parameters const.
> 
> I agree.
> 
> > > +{
> > > +	switch (format->fcc) {
> > > +	case V4L2_PIX_FMT_NV12:
> > > +	case V4L2_PIX_FMT_YVU420:
> > > +	case V4L2_PIX_FMT_YUV420:
> > > +	case V4L2_PIX_FMT_M420:
> > > +		return frame->wWidth;
> > > +	default:
> > > +		return format->bpp * frame->wWidth / 8;
> > > +	}
> > > +}
> > > +
> > >  static int uvc_v4l2_try_format(struct uvc_streaming *stream,
> > >  	struct v4l2_format *fmt, struct uvc_streaming_control
> > > *probe,
> > >  	struct uvc_format **uvc_format, struct uvc_frame
> > > **uvc_frame)
> > > @@ -245,7 +259,7 @@ static int uvc_v4l2_try_format(struct
> > > uvc_streaming
> > > *stream, fmt->fmt.pix.width = frame->wWidth;
> > >  	fmt->fmt.pix.height = frame->wHeight;
> > >  	fmt->fmt.pix.field = V4L2_FIELD_NONE;
> > > -	fmt->fmt.pix.bytesperline = format->bpp * frame->wWidth /
> > > 8;
> > > +	fmt->fmt.pix.bytesperline =
> > > uvc_v4l2_get_bytesperline(format, frame);
> > >  	fmt->fmt.pix.sizeimage = probe->dwMaxVideoFrameSize;
> > >  	fmt->fmt.pix.colorspace = format->colorspace;
> > >  	fmt->fmt.pix.priv = 0;
> > > @@ -282,7 +296,7 @@ static int uvc_v4l2_get_format(struct
> > > uvc_streaming
> > > *stream, fmt->fmt.pix.width = frame->wWidth;
> > >  	fmt->fmt.pix.height = frame->wHeight;
> > >  	fmt->fmt.pix.field = V4L2_FIELD_NONE;
> > > -	fmt->fmt.pix.bytesperline = format->bpp * frame->wWidth /
> > > 8;
> > > +	fmt->fmt.pix.bytesperline =
> > > uvc_v4l2_get_bytesperline(format, frame);
> > >  	fmt->fmt.pix.sizeimage = stream->ctrl.dwMaxVideoFrameSize;
> > >  	fmt->fmt.pix.colorspace = format->colorspace;
> > >  	fmt->fmt.pix.priv = 0;
> > 
> > This looks good to me otherwise.
> > 
> > If it's fine with you I can fix the above issue while applying.
> 
> That would be really nice.

Applied to my tree with the above changes, thank you.
diff mbox

Patch

diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c
index d7723ce..ceb1d1b 100644
--- a/drivers/media/usb/uvc/uvc_v4l2.c
+++ b/drivers/media/usb/uvc/uvc_v4l2.c
@@ -142,6 +142,20 @@  static __u32 uvc_try_frame_interval(struct uvc_frame *frame, __u32 interval)
 	return interval;
 }
 
+static __u32 uvc_v4l2_get_bytesperline(struct uvc_format *format,
+	struct uvc_frame *frame)
+{
+	switch (format->fcc) {
+	case V4L2_PIX_FMT_NV12:
+	case V4L2_PIX_FMT_YVU420:
+	case V4L2_PIX_FMT_YUV420:
+	case V4L2_PIX_FMT_M420:
+		return frame->wWidth;
+	default:
+		return format->bpp * frame->wWidth / 8;
+	}
+}
+
 static int uvc_v4l2_try_format(struct uvc_streaming *stream,
 	struct v4l2_format *fmt, struct uvc_streaming_control *probe,
 	struct uvc_format **uvc_format, struct uvc_frame **uvc_frame)
@@ -245,7 +259,7 @@  static int uvc_v4l2_try_format(struct uvc_streaming *stream,
 	fmt->fmt.pix.width = frame->wWidth;
 	fmt->fmt.pix.height = frame->wHeight;
 	fmt->fmt.pix.field = V4L2_FIELD_NONE;
-	fmt->fmt.pix.bytesperline = format->bpp * frame->wWidth / 8;
+	fmt->fmt.pix.bytesperline = uvc_v4l2_get_bytesperline(format, frame);
 	fmt->fmt.pix.sizeimage = probe->dwMaxVideoFrameSize;
 	fmt->fmt.pix.colorspace = format->colorspace;
 	fmt->fmt.pix.priv = 0;
@@ -282,7 +296,7 @@  static int uvc_v4l2_get_format(struct uvc_streaming *stream,
 	fmt->fmt.pix.width = frame->wWidth;
 	fmt->fmt.pix.height = frame->wHeight;
 	fmt->fmt.pix.field = V4L2_FIELD_NONE;
-	fmt->fmt.pix.bytesperline = format->bpp * frame->wWidth / 8;
+	fmt->fmt.pix.bytesperline = uvc_v4l2_get_bytesperline(format, frame);
 	fmt->fmt.pix.sizeimage = stream->ctrl.dwMaxVideoFrameSize;
 	fmt->fmt.pix.colorspace = format->colorspace;
 	fmt->fmt.pix.priv = 0;