NACK/Cmnt: [PATCH] media: uvcvideo: Fix pixel format change for Elgato Cam Link 4K

[Stefan Bader]

On 23.06.21 23:59, Benjamin Drung wrote:
> From: Benjamin Drung <bdrung at posteo.de>
> 
> BugLink: https://bugs.launchpad.net/bugs/1932367
> 
> The Elgato Cam Link 4K HDMI video capture card reports to support three
> different pixel formats, where the first format depends on the connected
> HDMI device.
> 
> ```

This looks to have been sent another time with a cover email. So NACKing this 
one to clarify things.

-Stefan

> $ v4l2-ctl -d /dev/video0 --list-formats-ext
> ioctl: VIDIOC_ENUM_FMT
> 	Type: Video Capture
> 
> 	[0]: 'NV12' (Y/CbCr 4:2:0)
> 		Size: Discrete 3840x2160
> 			Interval: Discrete 0.033s (29.970 fps)
> 	[1]: 'NV12' (Y/CbCr 4:2:0)
> 		Size: Discrete 3840x2160
> 			Interval: Discrete 0.033s (29.970 fps)
> 	[2]: 'YU12' (Planar YUV 4:2:0)
> 		Size: Discrete 3840x2160
> 			Interval: Discrete 0.033s (29.970 fps)
> ```
> 
> Changing the pixel format to anything besides the first pixel format
> does not work:
> 
> ```
> $ v4l2-ctl -d /dev/video0 --try-fmt-video pixelformat=YU12
> Format Video Capture:
> 	Width/Height      : 3840/2160
> 	Pixel Format      : 'NV12' (Y/CbCr 4:2:0)
> 	Field             : None
> 	Bytes per Line    : 3840
> 	Size Image        : 12441600
> 	Colorspace        : sRGB
> 	Transfer Function : Rec. 709
> 	YCbCr/HSV Encoding: Rec. 709
> 	Quantization      : Default (maps to Limited Range)
> 	Flags             :
> ```
> 
> User space applications like VLC might show an error message on the
> terminal in that case:
> 
> ```
> libv4l2: error set_fmt gave us a different result than try_fmt!
> ```
> 
> Depending on the error handling of the user space applications, they
> might display a distorted video, because they use the wrong pixel format
> for decoding the stream.
> 
> The Elgato Cam Link 4K responds to the USB video probe
> VS_PROBE_CONTROL/VS_COMMIT_CONTROL with a malformed data structure: The
> second byte contains bFormatIndex (instead of being the second byte of
> bmHint). The first byte is always zero. The third byte is always 1.
> 
> The firmware bug was reported to Elgato on 2020-12-01 and it was
> forwarded by the support team to the developers as feature request.
> There is no firmware update available since then. The latest firmware
> for Elgato Cam Link 4K as of 2021-03-23 has MCU 20.02.19 and FPGA 67.
> 
> Therefore correct the malformed data structure for this device. The
> change was successfully tested with VLC, OBS, and Chromium using
> different pixel formats (YUYV, NV12, YU12), resolutions (3840x2160,
> 1920x1080), and frame rates (29.970 and 59.940 fps).
> 
> Cc: stable at vger.kernel.org
> Signed-off-by: Benjamin Drung <bdrung at posteo.de>
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> (backported from commit 4c6e0976295add7f0ed94d276c04a3d6f1ea8f83 linux-next)
> Signed-off-by: Benjamin Drung <bdrung at posteo.de>
> ---
>   drivers/media/usb/uvc/uvc_video.c | 27 +++++++++++++++++++++++++++
>   1 file changed, 27 insertions(+)
> 
> diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c
> index a6a441d92b94..70b0d0f8026b 100644
> --- a/drivers/media/usb/uvc/uvc_video.c
> +++ b/drivers/media/usb/uvc/uvc_video.c
> @@ -124,10 +124,37 @@ int uvc_query_ctrl(struct uvc_device *dev, u8 query, u8 unit,
>   static void uvc_fixup_video_ctrl(struct uvc_streaming *stream,
>   	struct uvc_streaming_control *ctrl)
>   {
> +	static const struct usb_device_id elgato_cam_link_4k = {
> +		USB_DEVICE(0x0fd9, 0x0066)
> +	};
>   	struct uvc_format *format = NULL;
>   	struct uvc_frame *frame = NULL;
>   	unsigned int i;
>   
> +	/*
> +	 * The response of the Elgato Cam Link 4K is incorrect: The second byte
> +	 * contains bFormatIndex (instead of being the second byte of bmHint).
> +	 * The first byte is always zero. The third byte is always 1.
> +	 *
> +	 * The UVC 1.5 class specification defines the first five bits in the
> +	 * bmHint bitfield. The remaining bits are reserved and should be zero.
> +	 * Therefore a valid bmHint will be less than 32.
> +	 *
> +	 * Latest Elgato Cam Link 4K firmware as of 2021-03-23 needs this fix.
> +	 * MCU: 20.02.19, FPGA: 67
> +	 */
> +	if (usb_match_one_id(stream->dev->intf, &elgato_cam_link_4k) &&
> +	    ctrl->bmHint > 255) {
> +		u8 corrected_format_index = ctrl->bmHint >> 8;
> +
> +		uvc_trace(UVC_TRACE_VIDEO,
> +			"Correct USB video probe response from {bmHint: 0x%04x, bFormatIndex: %u} to {bmHint: 0x%04x, bFormatIndex: %u}\n",
> +			ctrl->bmHint, ctrl->bFormatIndex,
> +			1, corrected_format_index);
> +		ctrl->bmHint = 1;
> +		ctrl->bFormatIndex = corrected_format_index;
> +	}
> +
>   	for (i = 0; i < stream->nformats; ++i) {
>   		if (stream->format[i].index == ctrl->bFormatIndex) {
>   			format = &stream->format[i];
> 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20210624/53e05c91/attachment.sig>