Download raw body.
sys/uvideo: bypass unknown pixelformat to consumer
On Sat, Jan 11, 2025 at 10:33:03AM GMT, Kirill A. Korinsky wrote:
> On Sat, 11 Jan 2025 09:38:21 +0100,
> Marcus Glocker <marcus@nazgul.ch> wrote:
> >
> > On Fri, Jan 10, 2025 at 11:43:40PM GMT, Kirill A. Korinsky wrote:
> >
> > > + for (j = 0; j < nent; j++) {
> > > + if (!memcmp(sc->sc_fmtgrp[i].format->u.uc.guidFormat,
> > > + uvideo_map_fmts[j].guidFormat, 16)) {
> > > + sc->sc_fmtgrp[i].pixelformat =
> > > + uvideo_map_fmts[j].pixelformat;
> > > + break;
> > > + }
> > > }
> >
> > What I find odd here, is that we first copy the pixel format, and then
> > we only check if we need to map it, and if yes, we need to overwrite
> > the pixel format again.
> >
> > At the end of the loop you could just check if a mapping was required
> > by
> >
> > if (j == ent) /* no mapping was done */
> >
> > and only execute the default copy in that 'if' branch if no mapping was
> > done.
> >
> > Something like:
> >
> > /* but some GUID should be mapped to other pixelformats */
> > for (j = 0, nent = nitems(uvideo_map_fmts); j < nent; j++) {
> > if (!memcmp(sc->sc_fmtgrp[i].format->u.uc.guidFormat,
> > uvideo_map_fmts[j].guidFormat, 16)) {
> > sc->sc_fmtgrp[i].pixelformat =
> > uvideo_map_fmts[j].pixelformat;
> > break;
> > }
> > }
> > if (j == nent) {
> > /* GUID is started by null-terminated pixelformat, usually */
> > memcpy(&sc->sc_fmtgrp[i].pixelformat,
> > sc->sc_fmtgrp[i].format->u.uc.guidFormat, sizeof(uint32_t));
> > }
> >
> > And adapt the comments a bit to the new sequence.
> >
>
> sure, I not sure which way is cleaner but it makes sense.
>
> Here an updated diff. I've tried to keep comments short and clear.
Thanks, looks good to me now. I have regression tested this patch
quickly with two of my cams.
ok mglocker@
> Index: sys/dev/usb/uvideo.c
> ===================================================================
> RCS file: /home/cvs/src/sys/dev/usb/uvideo.c,v
> diff -u -p -u -p -r1.233 uvideo.c
> --- sys/dev/usb/uvideo.c 1 Jan 2025 11:42:07 -0000 1.233
> +++ sys/dev/usb/uvideo.c 11 Jan 2025 09:22:18 -0000
> @@ -383,6 +383,18 @@ const struct uvideo_devs {
> #define uvideo_lookup(v, p) \
> ((const struct uvideo_devs *)usb_lookup(uvideo_devs, v, p))
>
> +const struct uvideo_map_fmts {
> + uint8_t guidFormat[16];
> + uint32_t pixelformat;
> +} uvideo_map_fmts[] = {
> + { UVIDEO_FORMAT_GUID_YUY2, V4L2_PIX_FMT_YUYV },
> + { UVIDEO_FORMAT_GUID_YV12, V4L2_PIX_FMT_YVU420 },
> + { UVIDEO_FORMAT_GUID_I420, V4L2_PIX_FMT_YUV420 },
> + { UVIDEO_FORMAT_GUID_Y800, V4L2_PIX_FMT_GREY },
> + { UVIDEO_FORMAT_GUID_Y8, V4L2_PIX_FMT_GREY },
> + { UVIDEO_FORMAT_GUID_KSMEDIA_L8_IR, V4L2_PIX_FMT_GREY },
> +};
> +
> int
> uvideo_open(void *addr, int flags, int *size, uint8_t *buffer,
> void (*intr)(void *), void *arg)
> @@ -1048,8 +1060,7 @@ uvideo_vs_parse_desc_format_uncompressed
> const usb_descriptor_t *desc)
> {
> struct usb_video_format_uncompressed_desc *d;
> - uint8_t guid_8bit_ir[16] = UVIDEO_FORMAT_GUID_KSMEDIA_L8_IR;
> - int i;
> + int i, j, nent;
>
> d = (struct usb_video_format_uncompressed_desc *)(uint8_t *)desc;
>
> @@ -1074,19 +1085,23 @@ uvideo_vs_parse_desc_format_uncompressed
> sc->sc_fmtgrp[sc->sc_fmtgrp_idx].format_dfidx =
> d->bDefaultFrameIndex;
> }
> +
> i = sc->sc_fmtgrp_idx;
> - if (!strcmp(sc->sc_fmtgrp[i].format->u.uc.guidFormat, "YUY2")) {
> - sc->sc_fmtgrp[i].pixelformat = V4L2_PIX_FMT_YUYV;
> - } else if (!strcmp(sc->sc_fmtgrp[i].format->u.uc.guidFormat, "NV12")) {
> - sc->sc_fmtgrp[i].pixelformat = V4L2_PIX_FMT_NV12;
> - } else if (!strcmp(sc->sc_fmtgrp[i].format->u.uc.guidFormat, "UYVY")) {
> - sc->sc_fmtgrp[i].pixelformat = V4L2_PIX_FMT_UYVY;
> - } else if (!memcmp(sc->sc_fmtgrp[i].format->u.uc.guidFormat,
> - guid_8bit_ir, 16)) {
> - sc->sc_fmtgrp[i].pixelformat = V4L2_PIX_FMT_GREY;
> - } else {
> - sc->sc_fmtgrp[i].pixelformat = 0;
> +
> + /* map GUID to pixel format if a matching entry is found */
> + for (j = 0, nent = nitems(uvideo_map_fmts); j < nent; j++) {
> + if (!memcmp(sc->sc_fmtgrp[i].format->u.uc.guidFormat,
> + uvideo_map_fmts[j].guidFormat, 16)) {
> + sc->sc_fmtgrp[i].pixelformat =
> + uvideo_map_fmts[j].pixelformat;
> + break;
> + }
> }
> + /* default to using GUID start as the pixel format */
> + if (j == nent)
> + memcpy(&sc->sc_fmtgrp[i].pixelformat,
> + sc->sc_fmtgrp[i].format->u.uc.guidFormat,
> + sizeof(uint32_t));
>
> if (sc->sc_fmtgrp_cur == NULL)
> /* set UNCOMPRESSED format */
> @@ -2992,26 +3007,10 @@ uvideo_enum_fmt(void *v, struct v4l2_fmt
> break;
> case UDESCSUB_VS_FORMAT_UNCOMPRESSED:
> fmtdesc->flags = 0;
> - if (sc->sc_fmtgrp[idx].pixelformat ==
> - V4L2_PIX_FMT_YUYV) {
> - (void)strlcpy(fmtdesc->description, "YUYV",
> - sizeof(fmtdesc->description));
> - fmtdesc->pixelformat = V4L2_PIX_FMT_YUYV;
> - } else if (sc->sc_fmtgrp[idx].pixelformat ==
> - V4L2_PIX_FMT_NV12) {
> - (void)strlcpy(fmtdesc->description, "NV12",
> - sizeof(fmtdesc->description));
> - fmtdesc->pixelformat = V4L2_PIX_FMT_NV12;
> - } else if (sc->sc_fmtgrp[idx].pixelformat ==
> - V4L2_PIX_FMT_UYVY) {
> - (void)strlcpy(fmtdesc->description, "UYVY",
> - sizeof(fmtdesc->description));
> - fmtdesc->pixelformat = V4L2_PIX_FMT_UYVY;
> - } else {
> - (void)strlcpy(fmtdesc->description, "Unknown UC Format",
> - sizeof(fmtdesc->description));
> - fmtdesc->pixelformat = 0;
> - }
> + fmtdesc->pixelformat = sc->sc_fmtgrp[idx].pixelformat;
> + (void)strlcpy(fmtdesc->description,
> + (char *) &fmtdesc->pixelformat,
> + sizeof(fmtdesc->description));
> bzero(fmtdesc->reserved, sizeof(fmtdesc->reserved));
> break;
> default:
> Index: sys/dev/usb/uvideo.h
> ===================================================================
> RCS file: /home/cvs/src/sys/dev/usb/uvideo.h,v
> diff -u -p -u -p -r1.61 uvideo.h
> --- sys/dev/usb/uvideo.h 22 Dec 2024 20:30:04 -0000 1.61
> +++ sys/dev/usb/uvideo.h 11 Jan 2025 08:25:58 -0000
> @@ -297,15 +297,23 @@ struct usb_video_probe_commit {
> */
> /* Table 2-1: Compression Formats */
> #define UVIDEO_FORMAT_GUID_YUY2 { \
> - 0x59, 0x55, 0x59, 0x32, 0x00, 0x00, 0x10, 0x00, \
> + 'Y', 'U', 'Y', '2', '\0', 0x00, 0x10, 0x00, \
> 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71 }
>
> -#define UVIDEO_FORMAT_GUID_NV12 { \
> - 0x4e, 0x56, 0x31, 0x32, 0x00, 0x00, 0x10, 0x00, \
> +#define UVIDEO_FORMAT_GUID_YV12 { \
> + 'Y', 'V', '1', '2', '\0', 0x00, 0x10, 0x00, \
> 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71 }
>
> -#define UVIDEO_FORMAT_GUID_UYVY { \
> - 0x55, 0x59, 0x56, 0x59, 0x00, 0x00, 0x10, 0x00, \
> +#define UVIDEO_FORMAT_GUID_I420 { \
> + 'I', '4', '2', '0', '\0', 0x00, 0x10, 0x00, \
> + 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71 }
> +
> +#define UVIDEO_FORMAT_GUID_Y800 { \
> + 'Y', '8', '0', '0', '\0', 0x00, 0x10, 0x00, \
> + 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71 }
> +
> +#define UVIDEO_FORMAT_GUID_Y8 { \
> + 'Y', '8', ' ', ' ', '\0', 0x00, 0x10, 0x00, \
> 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71 }
>
> #define UVIDEO_FORMAT_GUID_KSMEDIA_L8_IR { \
>
>
> --
> wbr, Kirill
>
sys/uvideo: bypass unknown pixelformat to consumer