On Thu, Aug 27, 2020 at 11:28 PM Adam Goode <agoode@xxxxxxxxxx> wrote: > > The Color Matching Descriptor has been present in USB cameras since > the original version of UVC, but it has never been fully exposed > in Linux. > > This change informs V4L2 of all of the UVC colorspace parameters: > color primaries, transfer characteristics, and YCbCr encoding. > videodev2.h doesn't have values for all the possible UVC color settings, > so it is mapped as closely as possible. > > Additionally, this patch overrides the default setting for quantization > for UVC MJPEG. By default, V4L2 assumes that MJPEG is full range encoded, > which is not correct for UVC. > > JPEG itself does not specify YCbCr encoding information, this is > left to some other metadata. For typical JPEG images (those that conform to > JFIF, see https://www.w3.org/Graphics/JPEG/jfif3.pdf), the colorspace is > specified as YCbCr CCIR 601 with full range. The use of this variant > on the 601 standard in JFIF is the reason that V4L2 defaults to full range > for JPEG. > > A JPEG image isn't a JFIF unless it contains an APP0 tag with 'JFIF', and > the UVC standard is clear that APP0 is optional for its MJPEG payload. > It does not mention JFIF at all. Moreover, it provides color metadata > in the Color Matching Descriptor, all using limited range as of UVC 1.5. > > Note that web browsers such as Chrome and Firefox already ignore V4L2's > quantization for USB devices and use the correct limited range, but > other programs such as qv4l2 will incorrectly interpret the encoding of > MJPEG from USB cameras without this change. > > Since there are many YUV and non-YUV formats supported by UVC cameras (but > not mentioned in the official specifications), and the quantization is > also not specified for these formats, I am not changing that behavior: all > formats besides MJPEG will stay at V4L2_QUANTIZATION_DEFAULT as before. > > Signed-off-by: Adam Goode <agoode@xxxxxxxxxx> > --- > > Changes in v2: > - Apply quantization override only for MJPEG. > - Provide more comments and background information about JPEG vs JFIF. > - Explain the substitutions for xfer func and ycbcr encoding. > > drivers/media/usb/uvc/uvc_driver.c | 87 ++++++++++++++++++++++++++++-- > drivers/media/usb/uvc/uvc_v4l2.c | 6 +++ > drivers/media/usb/uvc/uvcvideo.h | 6 ++- > 3 files changed, 94 insertions(+), 5 deletions(-) > > diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c > index 431d86e1c94b..4e530a4bf976 100644 > --- a/drivers/media/usb/uvc/uvc_driver.c > +++ b/drivers/media/usb/uvc/uvc_driver.c > @@ -248,10 +248,10 @@ static struct uvc_format_desc *uvc_format_by_guid(const u8 guid[16]) > return NULL; > } > > -static u32 uvc_colorspace(const u8 primaries) > +static enum v4l2_colorspace uvc_colorspace(const u8 primaries) > { > - static const u8 colorprimaries[] = { > - 0, > + static const enum v4l2_colorspace colorprimaries[] = { > + V4L2_COLORSPACE_DEFAULT, /* Unspecified */ > V4L2_COLORSPACE_SRGB, > V4L2_COLORSPACE_470_SYSTEM_M, > V4L2_COLORSPACE_470_SYSTEM_BG, > @@ -262,7 +262,61 @@ static u32 uvc_colorspace(const u8 primaries) > if (primaries < ARRAY_SIZE(colorprimaries)) > return colorprimaries[primaries]; > > - return 0; > + return V4L2_COLORSPACE_DEFAULT; /* Reserved */ > +} > + > +static enum v4l2_xfer_func uvc_xfer_func(const u8 transfer_characteristics) > +{ > + /* V4L2 currently does not currently have definitions for all > + * possible values of UVC transfer characteristics. If > + * v4l2_xfer_func is extended with new values, the mapping > + * below should be updated. > + * > + * Substitutions are taken from the mapping given for > + * V4L2_XFER_FUNC_DEFAULT documented in videodev2.h. > + */ > + static const enum v4l2_xfer_func xfer_funcs[] = { > + V4L2_XFER_FUNC_DEFAULT, /* Unspecified */ > + V4L2_XFER_FUNC_709, > + V4L2_XFER_FUNC_709, /* Substitution for BT.470-2 M */ > + V4L2_XFER_FUNC_709, /* Substitution for BT.470-2 B, G */ > + V4L2_XFER_FUNC_709, /* Substitution for SMPTE 170M */ > + V4L2_XFER_FUNC_SMPTE240M, > + V4L2_XFER_FUNC_NONE, > + V4L2_XFER_FUNC_SRGB, > + }; > + > + if (transfer_characteristics < ARRAY_SIZE(xfer_funcs)) > + return xfer_funcs[transfer_characteristics]; > + > + return V4L2_XFER_FUNC_DEFAULT; /* Reserved */ > +} > + > +static enum v4l2_ycbcr_encoding uvc_ycbcr_enc(const u8 matrix_coefficients) > +{ > + /* V4L2 currently does not currently have definitions for all > + * possible values of UVC matrix coefficients. If > + * v4l2_ycbcr_encoding is extended with new values, the > + * mapping below should be updated. > + * > + * Substitutions are taken from the mapping given for > + * V4L2_YCBCR_ENC_DEFAULT documented in videodev2.h. > + * > + * FCC is assumed to be close enough to 601. > + */ > + static const enum v4l2_ycbcr_encoding ycbcr_encs[] = { > + V4L2_YCBCR_ENC_DEFAULT, /* Unspecified */ > + V4L2_YCBCR_ENC_709, > + V4L2_YCBCR_ENC_601, /* Substitution for FCC */ > + V4L2_YCBCR_ENC_601, /* Substitution for BT.470-2 B, G */ > + V4L2_YCBCR_ENC_601, > + V4L2_YCBCR_ENC_SMPTE240M, > + }; > + > + if (matrix_coefficients < ARRAY_SIZE(ycbcr_encs)) > + return ycbcr_encs[matrix_coefficients]; > + > + return V4L2_YCBCR_ENC_DEFAULT; /* Reserved */ > } > > /* Simplify a fraction using a simple continued fraction decomposition. The > @@ -533,6 +587,29 @@ static int uvc_parse_format(struct uvc_device *dev, > > strscpy(format->name, "MJPEG", sizeof(format->name)); > format->fcc = V4L2_PIX_FMT_MJPEG; > + /* > + * JPEG does not define a color encoding and relies on > + * external metadata for this purpose. The most common > + * metadata for this in JPEG is the JFIF > + * specification, which defines a full-range variant > + * of CCIR 601. Because of this, the default > + * quantization for JPEG images in V4L2 is full range. > + * > + * But while most JPEG files are JFIF (identified by > + * the presence of an APP0 tag starting with 'JFIF'), > + * MJPEG frames from UVC are typically not. The MJPEG > + * UVC standard declares APP0 tags as optional and > + * makes no mention of JFIF. > + * > + * For UVC MJPEG, the color encoding metadata comes > + * from the Color Matching descriptor, which uses the > + * standard limited range YCbCr encodings and not the > + * variant JFIF full range encoding. > + * > + * The default for JPEG in V4L2 is full range, so we need > + * to override the default here. > + */ > + format->quantization = V4L2_QUANTIZATION_LIM_RANGE; > format->flags = UVC_FMT_FLAG_COMPRESSED; > format->bpp = 0; > ftype = UVC_VS_FRAME_MJPEG; > @@ -704,6 +781,8 @@ static int uvc_parse_format(struct uvc_device *dev, > } > > format->colorspace = uvc_colorspace(buffer[3]); > + format->xfer_func = uvc_xfer_func(buffer[4]); > + format->ycbcr_enc = uvc_ycbcr_enc(buffer[5]); > > buflen -= buffer[0]; > buffer += buffer[0]; > diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c > index 0335e69b70ab..8795f53a6306 100644 > --- a/drivers/media/usb/uvc/uvc_v4l2.c > +++ b/drivers/media/usb/uvc/uvc_v4l2.c > @@ -253,6 +253,9 @@ static int uvc_v4l2_try_format(struct uvc_streaming *stream, > 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.xfer_func = format->xfer_func; > + fmt->fmt.pix.ycbcr_enc = format->ycbcr_enc; > + fmt->fmt.pix.quantization = format->quantization; > > if (uvc_format != NULL) > *uvc_format = format; > @@ -289,6 +292,9 @@ static int uvc_v4l2_get_format(struct uvc_streaming *stream, > 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.xfer_func = format->xfer_func; > + fmt->fmt.pix.ycbcr_enc = format->ycbcr_enc; > + fmt->fmt.pix.quantization = format->quantization; > > done: > mutex_unlock(&stream->mutex); > diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h > index 6ab972c643e3..e49cc671b0d3 100644 > --- a/drivers/media/usb/uvc/uvcvideo.h > +++ b/drivers/media/usb/uvc/uvcvideo.h > @@ -272,6 +272,7 @@ struct uvc_format_desc { > char *name; > u8 guid[16]; > u32 fcc; > + enum v4l2_quantization quantization; > }; > > /* The term 'entity' refers to both UVC units and UVC terminals. > @@ -370,7 +371,10 @@ struct uvc_format { > u8 type; > u8 index; > u8 bpp; > - u8 colorspace; > + enum v4l2_colorspace colorspace; > + enum v4l2_xfer_func xfer_func; > + enum v4l2_ycbcr_encoding ycbcr_enc; > + enum v4l2_quantization quantization; > u32 fcc; > u32 flags; > > -- > 2.26.2 > As mentioned elsewhere, the quantization change here is incorrect. I will follow up with a new patch. Adam
