Re: [PATCH 0/6] UVC Gadget: Extend color matching support

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Dan,

Thank you for the series.

On Tue, Dec 13, 2022 at 08:37:30AM +0000, Daniel Scally wrote:
> The current UVC gadget implementation hardcodes a single color matching
> descriptor and transmits it a single time following all the format and frame

I'm not sure I would use "transmits" in this context.  Descriptors are
for sure transmitted over the wire, but all in one go, not as individual
units (at least within a configuration descriptor).  Maybe "includes"
would be a better term ? This is nitpicking for the cover letter, but
the comment applies more importantly to commit messages and code for the
whole series.

> descriptors. This is inflexible, and additionally applies only to the _last_
> format in the array of descriptors.
> 
> This series extends the support such that the default descriptor can be amended
> and is transmitted once-per-format instead of once-only, it then adds the ability
> to create new color matching descriptors and associate them with particular formats.
> The default color matching descriptor is retained and used where the user does not
> link a new color matching descriptor to the format, so the default interaction
> with userspace is unchanged from the current implementation.

I wonder if we shouldn't drop the default descriptor. If userspace
doesn't specify one, then we really can't know what colorimetry data
applies to the frames. Instead of providing a default to the host, not
providing any colorimetry information would be better.

> Daniel Scally (6):
>   usb: gadget: usb: Remove "default" from color matching attributes
>   usb: gadget: uvc: Add struct for color matching in configs
>   usb: gadget: uvc: Copy color matching descriptor for each frame
>   usb: gadget: uvc: Remove the hardcoded default color matching
>   usb: gadget: uvc: Make color matching attributes read/write
>   usb: gadget: uvc: Allow creating new color matching descriptors
> 
>  .../ABI/testing/configfs-usb-gadget-uvc       |   6 +-
>  drivers/usb/gadget/function/f_uvc.c           |   9 -
>  drivers/usb/gadget/function/u_uvc.h           |   1 -
>  drivers/usb/gadget/function/uvc_configfs.c    | 247 +++++++++++++++---
>  drivers/usb/gadget/function/uvc_configfs.h    |   9 +
>  5 files changed, 228 insertions(+), 44 deletions(-)

-- 
Regards,

Laurent Pinchart