Index | Thread | Search

From:
Kirill A. Korinsky <kirill@korins.ky>
Subject:
Re: sys/uvideo: support colorformat from device
To:
tech@openbsd.org, mglocker@openbsd.org
Date:
Sun, 23 Feb 2025 11:58:41 +0100

Download raw body.

Thread
On Thu, 20 Feb 2025 01:17:38 +0100,
Kirill A. Korinsky <kirill@korins.ky> wrote:
> 
> On Mon, 17 Feb 2025 01:51:30 +0100,
> Kirill A. Korinsky <kirill@korins.ky> wrote:
> > 
> > tech@,
> > 
> > I'd like to add support for the color profile of the webcam when it
> > announces it. My Jabra PanaCast 20 does this for mjpeg with non-default
> > luma and chroma values profile: Rec 609 vs 709, and this diff improves
> > the quality of the picture, and I look alive :), and not a bit green or
> > blue.
> > 
> 
> Here an improved version of this diff which appliy colorformat to all
> previous pixelformats, if it the same, instead of the last one.
>

Here a version which never overwrite colorformats when a device sends
descripts similar to that flow:
- some supported format
- some frames
- colorformat
- some unsupported format and frames
- colorformat

It also forward to consumer colorformat only when device sent it, if it
doesn't send anything, nothing forwarded.

Index: sys/dev/usb/uvideo.c
===================================================================
RCS file: /home/cvs/src/sys/dev/usb/uvideo.c,v
diff -u -p -r1.239 uvideo.c
--- sys/dev/usb/uvideo.c	23 Feb 2025 08:28:57 -0000	1.239
+++ sys/dev/usb/uvideo.c	23 Feb 2025 10:50:33 -0000
@@ -131,6 +131,8 @@ usbd_status	uvideo_vs_parse_desc(struct 
 usbd_status	uvideo_vs_parse_desc_input_header(struct uvideo_softc *,
 		    const usb_descriptor_t *);
 usbd_status	uvideo_vs_parse_desc_format(struct uvideo_softc *);
+usbd_status	uvideo_vs_parse_desc_colorformat(struct uvideo_softc *,
+		    const usb_descriptor_t *);
 usbd_status	uvideo_vs_parse_desc_format_mjpeg(struct uvideo_softc *,
 		    const usb_descriptor_t *);
 usbd_status	uvideo_vs_parse_desc_format_uncompressed(struct uvideo_softc *,
@@ -420,6 +422,35 @@ const struct uvideo_map_fmts {
 	{ UVIDEO_FORMAT_GUID_INVI, V4L2_PIX_FMT_Y10 },
 };
 
+const enum v4l2_colorspace uvideo_color_primaries[] = {
+	V4L2_COLORSPACE_SRGB,		/* Unspecified */
+	V4L2_COLORSPACE_SRGB,
+	V4L2_COLORSPACE_470_SYSTEM_M,
+	V4L2_COLORSPACE_470_SYSTEM_BG,
+	V4L2_COLORSPACE_SMPTE170M,
+	V4L2_COLORSPACE_SMPTE240M,
+};
+
+const enum v4l2_xfer_func uvideo_transfer_characteristics[] = {
+	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,
+};
+
+const enum v4l2_ycbcr_encoding uvideo_matrix_coefficients[] = {
+	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,
+};
+
 int
 uvideo_open(void *addr, int flags, int *size, uint8_t *buffer,
     void (*intr)(void *), void *arg)
@@ -1010,6 +1041,12 @@ uvideo_vs_parse_desc_format(struct uvide
 		}
 
 		switch (desc->bDescriptorSubtype) {
+		case UDESCSUB_VS_COLORFORMAT:
+			if (desc->bLength == 6) {
+				(void)uvideo_vs_parse_desc_colorformat(
+				    sc, desc);
+			}
+			break;
 		case UDESCSUB_VS_FORMAT_MJPEG:
 			if (desc->bLength == 11) {
 				(void)uvideo_vs_parse_desc_format_mjpeg(
@@ -1040,6 +1077,42 @@ uvideo_vs_parse_desc_format(struct uvide
 }
 
 usbd_status
+uvideo_vs_parse_desc_colorformat(struct uvideo_softc *sc,
+    const usb_descriptor_t *desc)
+{
+	int fmtidx;
+	struct usb_video_colorformat_desc *d;
+
+	d = (struct usb_video_colorformat_desc *)(uint8_t *)desc;
+
+	fmtidx = sc->sc_fmtgrp_idx - 1;
+	if (fmtidx < 0 || sc->sc_fmtgrp[fmtidx].has_colorformat)
+		return (USBD_INVAL);
+
+	if (d->bColorPrimaries < nitems(uvideo_color_primaries))
+		sc->sc_fmtgrp[fmtidx].colorspace =
+		    uvideo_color_primaries[d->bColorPrimaries];
+	else
+		sc->sc_fmtgrp[fmtidx].colorspace = V4L2_COLORSPACE_SRGB;
+
+	if (d->bTransferCharacteristics < nitems(uvideo_transfer_characteristics))
+		sc->sc_fmtgrp[fmtidx].xfer_func =
+		    uvideo_transfer_characteristics[d->bTransferCharacteristics];
+	else
+		sc->sc_fmtgrp[fmtidx].xfer_func = V4L2_XFER_FUNC_DEFAULT;
+
+	if (d->bMatrixCoefficients < nitems(uvideo_matrix_coefficients))
+		sc->sc_fmtgrp[fmtidx].ycbcr_enc =
+		    uvideo_matrix_coefficients[d->bMatrixCoefficients];
+	else
+		sc->sc_fmtgrp[fmtidx].ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
+
+	sc->sc_fmtgrp[fmtidx].has_colorformat = 1;
+
+	return (USBD_NORMAL_COMPLETION);
+}
+
+usbd_status
 uvideo_vs_parse_desc_format_mjpeg(struct uvideo_softc *sc,
     const usb_descriptor_t *desc)
 {
@@ -3251,6 +3324,17 @@ uvideo_g_fmt(void *v, struct v4l2_format
 	fmt->fmt.pix.width = UGETW(sc->sc_fmtgrp_cur->frame_cur->wWidth);
 	fmt->fmt.pix.height = UGETW(sc->sc_fmtgrp_cur->frame_cur->wHeight);
 	fmt->fmt.pix.sizeimage = UGETDW(sc->sc_desc_probe.dwMaxVideoFrameSize);
+
+	if (sc->sc_fmtgrp_cur->has_colorformat) {
+		fmt->fmt.pix.colorspace = sc->sc_fmtgrp_cur->colorspace;
+		fmt->fmt.pix.xfer_func = sc->sc_fmtgrp_cur->xfer_func;
+		fmt->fmt.pix.ycbcr_enc = sc->sc_fmtgrp_cur->ycbcr_enc;
+
+		DPRINTF(1, "%s: %s: use color format"
+		    " colorspace=%d, xfer_func=%d, ycbcr_enc=%d\n",
+		    DEVNAME(sc), __func__, fmt->fmt.pix.colorspace,
+		    fmt->fmt.pix.xfer_func, fmt->fmt.pix.ycbcr_enc);
+	}
 
 	DPRINTF(1, "%s: %s: current width=%d, height=%d\n",
 	    DEVNAME(sc), __func__, fmt->fmt.pix.width, fmt->fmt.pix.height);
Index: sys/dev/usb/uvideo.h
===================================================================
RCS file: /home/cvs/src/sys/dev/usb/uvideo.h,v
diff -u -p -r1.63 uvideo.h
--- sys/dev/usb/uvideo.h	16 Jan 2025 22:58:19 -0000	1.63
+++ sys/dev/usb/uvideo.h	23 Feb 2025 10:51:10 -0000
@@ -412,6 +412,16 @@ struct usb_video_stream_header {
 	/* TODO complete struct */
 } __packed;
 
+/* Table 3-19: Color Matching Descriptor */
+struct usb_video_colorformat_desc {
+	uByte	bLength;
+	uByte	bDescriptorType;
+	uByte	bDescriptorSubtype;
+	uByte	bColorPrimaries;
+	uByte	bTransferCharacteristics;
+	uByte	bMatrixCoefficients;
+} __packed;
+
 /* Table 3-1: Motion-JPEG Video Format Descriptor */
 struct usb_video_format_mjpeg_desc {
 	uByte	bLength;
@@ -551,6 +561,10 @@ typedef SIMPLEQ_HEAD(, uvideo_mmap) q_mm
 
 struct uvideo_format_group {
 	uint32_t				 pixelformat;
+	int					 has_colorformat;
+	uint32_t				 colorspace;
+	uint32_t				 xfer_func;
+	uint32_t				 ycbcr_enc;
 	uint8_t					 format_dfidx;
 	struct uvideo_format_desc		*format;
 	/* frame descriptors for mjpeg and uncompressed are identical */