Message-Id: <20200828032752.3229698-1-agoode@google.com>
Date:   Thu, 27 Aug 2020 23:27:52 -0400
From:   Adam Goode <agoode@...gle.com>
To:     Laurent Pinchart <laurent.pinchart@...asonboard.com>,
        Mauro Carvalho Chehab <mchehab@...nel.org>
Cc:     linux-media@...r.kernel.org, linux-kernel@...r.kernel.org,
        Adam Goode <agoode@...gle.com>
Subject: [PATCH v2] media: uvcvideo: Convey full colorspace information to V4L2

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@...gle.com>
---

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

Powered by blists - more mailing lists