Morning Laurent
On 18/12/2022 18:12, Laurent Pinchart wrote:
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.
I've used the same term I think in the series yes. No problem; I'll swap
to includes.
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.
According to the spec:
"In the absence of this descriptor, or in the case of "Unspecified"
values within the descriptor, color matching defaults will be assumed.
The color matching defaults are compliant with sRGB since the BT.709
transfer function and the sRGB transfer function are very similar"
And it goes on to identify the default values for each of the
descriptor's fields...which happen to be the values that are set in our
default descriptor. So I think that including that default descriptor
shouldn't change the host's behaviour, but does give userspace an easy
way to see what's set...I think it's fine to keep.
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(-)
