| Message ID | 20230323-uvc-gadget-cleanup-v1-7-e41f0c5d9d8e@pengutronix.de |
|---|---|
| State | New |
| Headers | show |
| Series | usb: gadget: uvc: fix errors reported by v4l2-compliance | expand |
Hi Michael, Thank you for the patch. On Thu, Mar 23, 2023 at 12:41:15PM +0100, Michael Tretter wrote: > Store the values of the configured color space in the driver. This > allows the user space to set the color space and re-read the set values > later. > > UVC allows to announce the colorspace in the USB descriptors. The values > of the descriptors are not evaluated by the driver, yet. Thus, the > default is always the default specified by UVC and not the configured > default. As with format handling and frame interval handling, I don't like this. It's useless code that userspace won't exercise. > Signed-off-by: Michael Tretter <m.tretter@pengutronix.de> > --- > drivers/usb/gadget/function/uvc.h | 4 ++++ > drivers/usb/gadget/function/uvc_v4l2.c | 32 ++++++++++++++++++++++++++++++-- > 2 files changed, 34 insertions(+), 2 deletions(-) > > diff --git a/drivers/usb/gadget/function/uvc.h b/drivers/usb/gadget/function/uvc.h > index 100475b1363e..6b4ab3e07173 100644 > --- a/drivers/usb/gadget/function/uvc.h > +++ b/drivers/usb/gadget/function/uvc.h > @@ -96,6 +96,10 @@ struct uvc_video { > unsigned int width; > unsigned int height; > unsigned int imagesize; > + enum v4l2_colorspace colorspace; > + enum v4l2_ycbcr_encoding ycbcr_enc; > + enum v4l2_quantization quantization; > + enum v4l2_xfer_func xfer_func; > struct mutex mutex; /* protects frame parameters */ > > unsigned int uvc_num_requests; > diff --git a/drivers/usb/gadget/function/uvc_v4l2.c b/drivers/usb/gadget/function/uvc_v4l2.c > index c5983bb0a8d1..673532ff0faa 100644 > --- a/drivers/usb/gadget/function/uvc_v4l2.c > +++ b/drivers/usb/gadget/function/uvc_v4l2.c > @@ -180,6 +180,11 @@ void uvc_init_default_format(struct uvc_device *uvc) > video->height = 240; > video->imagesize = 320 * 240 * 2; > > + video->colorspace = V4L2_COLORSPACE_SRGB; > + video->quantization = V4L2_QUANTIZATION_FULL_RANGE; > + video->xfer_func = V4L2_XFER_FUNC_SRGB; > + video->ycbcr_enc = V4L2_YCBCR_ENC_601; > + > return; > } > > @@ -196,6 +201,14 @@ void uvc_init_default_format(struct uvc_device *uvc) > video->width = uframe->frame.w_width; > video->height = uframe->frame.w_height; > video->imagesize = uvc_get_frame_size(uformat, uframe); > + > + if (uformat->type == UVCG_UNCOMPRESSED) > + video->colorspace = V4L2_COLORSPACE_SRGB; > + else > + video->colorspace = V4L2_COLORSPACE_JPEG; > + video->quantization = V4L2_QUANTIZATION_FULL_RANGE; > + video->xfer_func = V4L2_XFER_FUNC_SRGB; > + video->ycbcr_enc = V4L2_YCBCR_ENC_601; > } > > static struct uvcg_frame *find_closest_frame_by_size(struct uvc_device *uvc, > @@ -294,7 +307,12 @@ uvc_v4l2_get_format(struct file *file, void *fh, struct v4l2_format *fmt) > fmt->fmt.pix.field = V4L2_FIELD_NONE; > fmt->fmt.pix.bytesperline = video->bpp * video->width / 8; > fmt->fmt.pix.sizeimage = video->imagesize; > - fmt->fmt.pix.colorspace = V4L2_COLORSPACE_SRGB; > + > + fmt->fmt.pix.colorspace = video->colorspace; > + fmt->fmt.pix.ycbcr_enc = video->ycbcr_enc; > + fmt->fmt.pix.quantization = video->quantization; > + fmt->fmt.pix.xfer_func = video->xfer_func; > + > fmt->fmt.pix.priv = 0; > > return 0; > @@ -335,7 +353,12 @@ uvc_v4l2_try_format(struct file *file, void *fh, struct v4l2_format *fmt) > fmt->fmt.pix.bytesperline = uvc_v4l2_get_bytesperline(uformat, uframe); > fmt->fmt.pix.sizeimage = uvc_get_frame_size(uformat, uframe); > fmt->fmt.pix.pixelformat = to_uvc_format(uformat)->fcc; > - fmt->fmt.pix.colorspace = V4L2_COLORSPACE_SRGB; > + > + fmt->fmt.pix.colorspace = video->colorspace; > + fmt->fmt.pix.ycbcr_enc = video->ycbcr_enc; > + fmt->fmt.pix.quantization = video->quantization; > + fmt->fmt.pix.xfer_func = video->xfer_func; > + > fmt->fmt.pix.priv = 0; > > return 0; > @@ -359,6 +382,11 @@ uvc_v4l2_set_format(struct file *file, void *fh, struct v4l2_format *fmt) > video->height = fmt->fmt.pix.height; > video->imagesize = fmt->fmt.pix.sizeimage; > > + video->colorspace = fmt->fmt.pix.colorspace; > + video->ycbcr_enc = fmt->fmt.pix.ycbcr_enc; > + video->quantization = fmt->fmt.pix.quantization; > + video->xfer_func = fmt->fmt.pix.xfer_func; > + > return ret; > } >
diff --git a/drivers/usb/gadget/function/uvc.h b/drivers/usb/gadget/function/uvc.h index 100475b1363e..6b4ab3e07173 100644 --- a/drivers/usb/gadget/function/uvc.h +++ b/drivers/usb/gadget/function/uvc.h @@ -96,6 +96,10 @@ struct uvc_video { unsigned int width; unsigned int height; unsigned int imagesize; + enum v4l2_colorspace colorspace; + enum v4l2_ycbcr_encoding ycbcr_enc; + enum v4l2_quantization quantization; + enum v4l2_xfer_func xfer_func; struct mutex mutex; /* protects frame parameters */ unsigned int uvc_num_requests; diff --git a/drivers/usb/gadget/function/uvc_v4l2.c b/drivers/usb/gadget/function/uvc_v4l2.c index c5983bb0a8d1..673532ff0faa 100644 --- a/drivers/usb/gadget/function/uvc_v4l2.c +++ b/drivers/usb/gadget/function/uvc_v4l2.c @@ -180,6 +180,11 @@ void uvc_init_default_format(struct uvc_device *uvc) video->height = 240; video->imagesize = 320 * 240 * 2; + video->colorspace = V4L2_COLORSPACE_SRGB; + video->quantization = V4L2_QUANTIZATION_FULL_RANGE; + video->xfer_func = V4L2_XFER_FUNC_SRGB; + video->ycbcr_enc = V4L2_YCBCR_ENC_601; + return; } @@ -196,6 +201,14 @@ void uvc_init_default_format(struct uvc_device *uvc) video->width = uframe->frame.w_width; video->height = uframe->frame.w_height; video->imagesize = uvc_get_frame_size(uformat, uframe); + + if (uformat->type == UVCG_UNCOMPRESSED) + video->colorspace = V4L2_COLORSPACE_SRGB; + else + video->colorspace = V4L2_COLORSPACE_JPEG; + video->quantization = V4L2_QUANTIZATION_FULL_RANGE; + video->xfer_func = V4L2_XFER_FUNC_SRGB; + video->ycbcr_enc = V4L2_YCBCR_ENC_601; } static struct uvcg_frame *find_closest_frame_by_size(struct uvc_device *uvc, @@ -294,7 +307,12 @@ uvc_v4l2_get_format(struct file *file, void *fh, struct v4l2_format *fmt) fmt->fmt.pix.field = V4L2_FIELD_NONE; fmt->fmt.pix.bytesperline = video->bpp * video->width / 8; fmt->fmt.pix.sizeimage = video->imagesize; - fmt->fmt.pix.colorspace = V4L2_COLORSPACE_SRGB; + + fmt->fmt.pix.colorspace = video->colorspace; + fmt->fmt.pix.ycbcr_enc = video->ycbcr_enc; + fmt->fmt.pix.quantization = video->quantization; + fmt->fmt.pix.xfer_func = video->xfer_func; + fmt->fmt.pix.priv = 0; return 0; @@ -335,7 +353,12 @@ uvc_v4l2_try_format(struct file *file, void *fh, struct v4l2_format *fmt) fmt->fmt.pix.bytesperline = uvc_v4l2_get_bytesperline(uformat, uframe); fmt->fmt.pix.sizeimage = uvc_get_frame_size(uformat, uframe); fmt->fmt.pix.pixelformat = to_uvc_format(uformat)->fcc; - fmt->fmt.pix.colorspace = V4L2_COLORSPACE_SRGB; + + fmt->fmt.pix.colorspace = video->colorspace; + fmt->fmt.pix.ycbcr_enc = video->ycbcr_enc; + fmt->fmt.pix.quantization = video->quantization; + fmt->fmt.pix.xfer_func = video->xfer_func; + fmt->fmt.pix.priv = 0; return 0; @@ -359,6 +382,11 @@ uvc_v4l2_set_format(struct file *file, void *fh, struct v4l2_format *fmt) video->height = fmt->fmt.pix.height; video->imagesize = fmt->fmt.pix.sizeimage; + video->colorspace = fmt->fmt.pix.colorspace; + video->ycbcr_enc = fmt->fmt.pix.ycbcr_enc; + video->quantization = fmt->fmt.pix.quantization; + video->xfer_func = fmt->fmt.pix.xfer_func; + return ret; }
Store the values of the configured color space in the driver. This allows the user space to set the color space and re-read the set values later. UVC allows to announce the colorspace in the USB descriptors. The values of the descriptors are not evaluated by the driver, yet. Thus, the default is always the default specified by UVC and not the configured default. Signed-off-by: Michael Tretter <m.tretter@pengutronix.de> --- drivers/usb/gadget/function/uvc.h | 4 ++++ drivers/usb/gadget/function/uvc_v4l2.c | 32 ++++++++++++++++++++++++++++++-- 2 files changed, 34 insertions(+), 2 deletions(-)