From 413afdc2cd3984711cc57049e7c925a6f0f7bc05 Mon Sep 17 00:00:00 2001 From: Josuah Demangeon Date: Mon, 13 Oct 2025 12:45:27 +0000 Subject: [PATCH 1/5] usb: device_next: uvc: propagate descriptor error to the application When running out of descriptor, return an error instead of ignoring it. The application need to make sure to adjust the Kconfig macros to have enough descriptors for all formats to add. Signed-off-by: Josuah Demangeon --- subsys/usb/device_next/class/usbd_uvc.c | 57 ++++++++++++++++++++----- 1 file changed, 46 insertions(+), 11 deletions(-) diff --git a/subsys/usb/device_next/class/usbd_uvc.c b/subsys/usb/device_next/class/usbd_uvc.c index 9038a8e0d3f15..c158299d7c8ee 100644 --- a/subsys/usb/device_next/class/usbd_uvc.c +++ b/subsys/usb/device_next/class/usbd_uvc.c @@ -1374,7 +1374,7 @@ static int uvc_assign_desc(const struct device *dev, void *const desc, return 0; err: - LOG_ERR("Out of descriptor pointers, raise CONFIG_USBD_VIDEO_MAX_FORMATS above %u", + LOG_WRN("Out of descriptors, raise CONFIG_USBD_VIDEO_MAX_FORMATS above %u", CONFIG_USBD_VIDEO_MAX_FORMATS); return -ENOMEM; } @@ -1506,8 +1506,9 @@ static int uvc_add_vs_frame_interval(struct uvc_frame_discrete_descriptor *const int i = desc->bFrameIntervalType; if (i >= CONFIG_USBD_VIDEO_MAX_FRMIVAL) { - LOG_WRN("Out of frame interval fields"); - return -ENOSPC; + LOG_WRN("Out of descriptors, raise CONFIG_USBD_VIDEO_MAX_FRMIVAL above %u", + CONFIG_USBD_VIDEO_MAX_FRMIVAL); + return -ENOMEM; } desc->dwFrameInterval[i] = sys_cpu_to_le32(video_frmival_nsec(frmival) / 100); @@ -1533,6 +1534,7 @@ static int uvc_add_vs_frame_desc(const struct device *dev, .width = w, .height = h, .pitch = p}; struct video_frmival_enum fie = {.format = &fmt}; uint32_t max_size = MAX(p, w) * h; + int ret; __ASSERT_NO_MSG(data->video_dev != NULL); __ASSERT_NO_MSG(format_desc != NULL); @@ -1561,12 +1563,26 @@ static int uvc_add_vs_frame_desc(const struct device *dev, switch (fie.type) { case VIDEO_FRMIVAL_TYPE_DISCRETE: LOG_DBG("Adding discrete frame interval %u", fie.index); - uvc_add_vs_frame_interval(desc, &fie.discrete, &fmt); + + ret = uvc_add_vs_frame_interval(desc, &fie.discrete, &fmt); + if (ret != 0) { + return ret; + } + break; case VIDEO_FRMIVAL_TYPE_STEPWISE: LOG_DBG("Adding stepwise frame interval %u", fie.index); - uvc_add_vs_frame_interval(desc, &fie.stepwise.min, &fmt); - uvc_add_vs_frame_interval(desc, &fie.stepwise.max, &fmt); + + ret = uvc_add_vs_frame_interval(desc, &fie.stepwise.min, &fmt); + if (ret != 0) { + return ret; + } + + ret = uvc_add_vs_frame_interval(desc, &fie.stepwise.max, &fmt); + if (ret != 0) { + return ret; + } + break; default: CODE_UNREACHABLE; @@ -1578,7 +1594,10 @@ static int uvc_add_vs_frame_desc(const struct device *dev, if (desc->bFrameIntervalType == 0) { struct video_frmival frmival = {.numerator = 1, .denominator = 30}; - uvc_add_vs_frame_interval(desc, &frmival, &fmt); + ret = uvc_add_vs_frame_interval(desc, &frmival, &fmt); + if (ret != 0) { + return ret; + } } /* UVC requires the frame intervals to be sorted, but not Zephyr */ @@ -1671,7 +1690,11 @@ static int uvc_init(struct usbd_class_data *const c_data) if (prev_pixfmt != cap->pixelformat) { if (prev_pixfmt != 0) { cfg->desc->if1_hdr.wTotalLength += cfg->desc->if1_color.bLength; - uvc_assign_desc(dev, &cfg->desc->if1_color, true, true); + + ret = uvc_assign_desc(dev, &cfg->desc->if1_color, true, true); + if (ret != 0) { + return ret; + } } ret = uvc_add_vs_format_desc(dev, &format_desc, cap); @@ -1696,9 +1719,21 @@ static int uvc_init(struct usbd_class_data *const c_data) } cfg->desc->if1_hdr.wTotalLength += cfg->desc->if1_color.bLength; - uvc_assign_desc(dev, &cfg->desc->if1_color, true, true); - uvc_assign_desc(dev, &cfg->desc->if1_ep_fs, true, false); - uvc_assign_desc(dev, &cfg->desc->if1_ep_hs, false, true); + + ret = uvc_assign_desc(dev, &cfg->desc->if1_color, true, true); + if (ret != 0) { + return ret; + } + + ret = uvc_assign_desc(dev, &cfg->desc->if1_ep_fs, true, false); + if (ret != 0) { + return ret; + } + + ret = uvc_assign_desc(dev, &cfg->desc->if1_ep_hs, false, true); + if (ret != 0) { + return ret; + } cfg->desc->if1_hdr.wTotalLength = sys_cpu_to_le16(cfg->desc->if1_hdr.wTotalLength); From a44abdec7b44a7ddfe4397662c20ac1c3975132c Mon Sep 17 00:00:00 2001 From: Josuah Demangeon Date: Mon, 13 Oct 2025 18:42:41 +0000 Subject: [PATCH 2/5] usb: device_next: uvc: use fmt->size instead of computing it every time Make use of the recently merged fmt->size to know the maximum size of the frame to be allocated, which works for both compressed and uncompressed video. Signed-off-by: Josuah Demangeon --- subsys/usb/device_next/class/usbd_uvc.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/subsys/usb/device_next/class/usbd_uvc.c b/subsys/usb/device_next/class/usbd_uvc.c index c158299d7c8ee..ce854e9d14430 100644 --- a/subsys/usb/device_next/class/usbd_uvc.c +++ b/subsys/usb/device_next/class/usbd_uvc.c @@ -577,7 +577,7 @@ static int uvc_get_vs_probe_max_size(const struct device *dev, struct uvc_probe { struct uvc_data *data = dev->data; struct video_format *fmt = &data->video_fmt; - uint32_t max_frame_size = MAX(fmt->pitch, fmt->width) * fmt->height; + uint32_t max_frame_size = fmt->size; uint32_t max_payload_size = max_frame_size + UVC_MAX_HEADER_LENGTH; switch (request) { @@ -633,9 +633,8 @@ static int uvc_get_vs_format_from_desc(const struct device *dev, struct video_fo /* Fill the format according to what the host selected */ fmt->width = frame_desc->wWidth; fmt->height = frame_desc->wHeight; - fmt->pitch = fmt->width * video_bits_per_pixel(fmt->pixelformat) / BITS_PER_BYTE; - return 0; + return video_estimate_fmt_size(fmt); } static int uvc_get_vs_probe_struct(const struct device *dev, struct uvc_probe *const probe, @@ -1480,8 +1479,7 @@ static void uvc_set_vs_bitrate_range(struct uvc_frame_discrete_descriptor *const uint32_t bitrate_max = sys_le32_to_cpu(desc->dwMaxBitRate); uint32_t bitrate; - /* Multiplication/division in this order to avoid overflow */ - bitrate = MAX(fmt->pitch, fmt->width) * frmival_nsec / (NSEC_PER_SEC / 100) * fmt->height; + bitrate = (uint64_t)fmt->size * frmival_nsec / (NSEC_PER_SEC / 100); /* Extend the min/max value to include the bitrate of this format */ bitrate_min = MIN(bitrate_min, bitrate); @@ -1533,7 +1531,6 @@ static int uvc_add_vs_frame_desc(const struct device *dev, struct video_format fmt = {.pixelformat = cap->pixelformat, .width = w, .height = h, .pitch = p}; struct video_frmival_enum fie = {.format = &fmt}; - uint32_t max_size = MAX(p, w) * h; int ret; __ASSERT_NO_MSG(data->video_dev != NULL); @@ -1552,7 +1549,7 @@ static int uvc_add_vs_frame_desc(const struct device *dev, desc->bFrameIndex = format_desc->bNumFrameDescriptors + 1; desc->wWidth = sys_cpu_to_le16(w); desc->wHeight = sys_cpu_to_le16(h); - desc->dwMaxVideoFrameBufferSize = sys_cpu_to_le32(max_size); + desc->dwMaxVideoFrameBufferSize = sys_cpu_to_le32(fmt.size); desc->bDescriptorSubtype = (format_desc->bDescriptorSubtype == UVC_VS_FORMAT_UNCOMPRESSED) ? UVC_VS_FRAME_UNCOMPRESSED : UVC_VS_FRAME_MJPEG; desc->dwMinBitRate = sys_cpu_to_le32(UINT32_MAX); From 6e7b3cdf410380eb3fff9a05304c1eed35aa8c59 Mon Sep 17 00:00:00 2001 From: Josuah Demangeon Date: Tue, 15 Jul 2025 22:33:54 +0000 Subject: [PATCH 3/5] usb: device_next: uvc: remove application decisions from the UVC class The UVC class was deciding itself which formats were sent to the host. Remove this logic out of the UVC class and introduce uvc_add_format() to give the application the freedom of which format to list. Signed-off-by: Josuah Demangeon --- include/zephyr/usb/class/usbd_uvc.h | 31 ++- samples/subsys/usb/uvc/src/main.c | 105 ++++++++++- subsys/usb/device_next/class/usbd_uvc.c | 238 ++++++++++++------------ 3 files changed, 234 insertions(+), 140 deletions(-) diff --git a/include/zephyr/usb/class/usbd_uvc.h b/include/zephyr/usb/class/usbd_uvc.h index bd5562389206b..9cd3ad1d1931b 100644 --- a/include/zephyr/usb/class/usbd_uvc.h +++ b/include/zephyr/usb/class/usbd_uvc.h @@ -26,20 +26,37 @@ */ /** - * @brief Set the video device that a UVC instance will use. + * @brief Set the video device that a UVC instance will use for control requests. * - * It will query its supported controls, formats and frame rates, and use this information to - * generate USB descriptors sent to the host. - * - * At runtime, it will forward all USB controls from the host to this device. + * It will query its supported video controls and frame intervals and use this information to + * generate the USB descriptors presented to the host. In addition, for every supported UVC control + * request from the host to this @p uvc_dev instance, it will issue a matching video API control + * request to @p video_dev. * * @note This function must be called before @ref usbd_enable. * - * @param uvc_dev The UVC device - * @param video_dev The video device that this UVC instance controls + * @param uvc_dev Pointer to the UVC device to configure + * @param video_dev Pointer to the video device to which controls requests are sent */ void uvc_set_video_dev(const struct device *uvc_dev, const struct device *video_dev); +/** + * @brief Add a video format that a UVC instance will present to the host. + * + * This information will be used to generate USB descriptors. + * The particular format selected by the host can be queried with @ref video_get_format. + * + * @note This function must be called before @ref usbd_enable. + * + * @note The @p fmt struct field @c size must be set prior to call this function, + * such as calling @ref video_set_format(). + * + * @param uvc_dev Pointer to the UVC device to configure + * @param fmt The video format to add to this UVC instance + * @return 0 on success, negative value on error + */ +int uvc_add_format(const struct device *const uvc_dev, const struct video_format *const fmt); + /** * @} */ diff --git a/samples/subsys/usb/uvc/src/main.c b/samples/subsys/usb/uvc/src/main.c index 454bed7bdd9c5..1342e6e3e3b0f 100644 --- a/samples/subsys/usb/uvc/src/main.c +++ b/samples/subsys/usb/uvc/src/main.c @@ -17,15 +17,74 @@ LOG_MODULE_REGISTER(uvc_sample, LOG_LEVEL_INF); -const struct device *const uvc_dev = DEVICE_DT_GET(DT_NODELABEL(uvc)); -const struct device *const video_dev = DEVICE_DT_GET(DT_CHOSEN(zephyr_camera)); +const static struct device *const uvc_dev = DEVICE_DT_GET(DT_NODELABEL(uvc)); +const static struct device *const video_dev = DEVICE_DT_GET(DT_CHOSEN(zephyr_camera)); + +/* Format capabilities of video_dev, used everywhere through the sample */ +static struct video_caps video_caps = {.type = VIDEO_BUF_TYPE_OUTPUT}; + +static int app_add_format(uint32_t pixfmt, uint32_t width, uint32_t height) +{ + struct video_format fmt = { + .pixelformat = pixfmt, + .width = width, + .height = height, + .type = VIDEO_BUF_TYPE_OUTPUT, + }; + int ret; + + /* Set the format to get the size */ + ret = video_set_format(video_dev, &fmt); + if (ret != 0) { + LOG_ERR("Could not set the format of %s to %s %ux%u (size %u)", + video_dev->name, VIDEO_FOURCC_TO_STR(fmt.pixelformat), + fmt.width, fmt.height, fmt.size); + return ret; + } + + if (fmt.size > CONFIG_VIDEO_BUFFER_POOL_SZ_MAX) { + LOG_WRN("Skipping format %ux%u", fmt.width, fmt.height); + return 0; + } + + ret = uvc_add_format(uvc_dev, &fmt); + if (ret == -ENOMEM) { + /* If there are too many formats, ignore the error, just list fewer formats */ + return 0; + } + return ret; +} + +/* Submit to UVC only the formats expected to be working (enough memory for the size, etc.) */ +static int app_add_filtered_formats(void) +{ + int ret; + + for (int i = 0; video_caps.format_caps[i].pixelformat != 0; i++) { + const struct video_format_cap *vcap = &video_caps.format_caps[i]; + + ret = app_add_format(vcap->pixelformat, vcap->width_min, vcap->height_min); + if (ret != 0) { + return ret; + } + + if (vcap->width_min != vcap->width_max || vcap->height_min != vcap->height_max) { + ret = app_add_format(vcap->pixelformat, vcap->width_max, vcap->height_max); + if (ret != 0) { + return ret; + } + } + } + + return 0; +} int main(void) { struct usbd_context *sample_usbd; struct video_buffer *vbuf; struct video_format fmt = {0}; - struct video_caps caps; + struct video_frmival frmival = {0}; struct k_poll_signal sig; struct k_poll_event evt[1]; k_timeout_t timeout = K_FOREVER; @@ -36,16 +95,21 @@ int main(void) return -ENODEV; } - caps.type = VIDEO_BUF_TYPE_OUTPUT; - - if (video_get_caps(video_dev, &caps)) { + ret = video_get_caps(video_dev, &video_caps); + if (ret != 0) { LOG_ERR("Unable to retrieve video capabilities"); return 0; } - /* Must be done before initializing USB */ + /* Must be called before usb_enable() */ uvc_set_video_dev(uvc_dev, video_dev); + /* Must be called before usb_enable() */ + ret = app_add_filtered_formats(); + if (ret != 0) { + return ret; + } + sample_usbd = sample_usbd_init_device(NULL); if (sample_usbd == NULL) { return -ENODEV; @@ -58,7 +122,6 @@ int main(void) LOG_INF("Waiting the host to select the video format"); - /* Get the video format once it is selected by the host */ while (true) { fmt.type = VIDEO_BUF_TYPE_INPUT; @@ -74,9 +137,31 @@ int main(void) k_sleep(K_MSEC(10)); } - LOG_INF("The host selected format '%s' %ux%u, preparing %u buffers of %u bytes", + ret = video_get_frmival(uvc_dev, &frmival); + if (ret != 0) { + LOG_ERR("Failed to get the video frame interval"); + return ret; + } + + LOG_INF("The host selected format '%s' %ux%u at frame interval %u/%u", VIDEO_FOURCC_TO_STR(fmt.pixelformat), fmt.width, fmt.height, - CONFIG_VIDEO_BUFFER_POOL_NUM_MAX, fmt.pitch * fmt.height); + frmival.numerator, frmival.denominator); + + fmt.type = VIDEO_BUF_TYPE_OUTPUT; + + ret = video_set_format(video_dev, &fmt); + if (ret != 0) { + LOG_ERR("Could not set the format of %s to %s %ux%u (size %u)", + video_dev->name, VIDEO_FOURCC_TO_STR(fmt.pixelformat), + fmt.width, fmt.height, fmt.size); + } + + ret = video_set_frmival(video_dev, &frmival); + if (ret != 0) { + LOG_WRN("Could not set the framerate of %s", video_dev->name); + } + + LOG_INF("Preparing %u buffers of %u bytes", CONFIG_VIDEO_BUFFER_POOL_NUM_MAX, fmt.size); for (int i = 0; i < CONFIG_VIDEO_BUFFER_POOL_NUM_MAX; i++) { vbuf = video_buffer_alloc(fmt.size, K_NO_WAIT); diff --git a/subsys/usb/device_next/class/usbd_uvc.c b/subsys/usb/device_next/class/usbd_uvc.c index ce854e9d14430..35d575a3fd13d 100644 --- a/subsys/usb/device_next/class/usbd_uvc.c +++ b/subsys/usb/device_next/class/usbd_uvc.c @@ -111,6 +111,10 @@ struct uvc_data { struct video_frmival video_frmival; /* Signal to alert video devices of buffer-related evenets */ struct k_poll_signal *video_sig; + /* Last pixel format that was added by uvc_add_format() */ + uint32_t last_pix_fmt; + /* Last format descriptor that was added by uvc_add_format() */ + struct uvc_format_descriptor *last_format_desc; /* Makes sure flushing the stream only happens in one context at a time */ struct k_mutex mutex; /* Zero Length packet used to reset a stream when restarted */ @@ -179,15 +183,6 @@ UDC_BUF_POOL_VAR_DEFINE(uvc_buf_pool, UVC_TOTAL_BUFS, UVC_TOTAL_BUFS * USBD_MAX_ static void uvc_flush_queue(const struct device *dev); -/* UVC public API */ - -void uvc_set_video_dev(const struct device *const dev, const struct device *const video_dev) -{ - struct uvc_data *data = dev->data; - - data->video_dev = video_dev; -} - /* UVC helper functions */ static const struct uvc_guid_quirk uvc_guid_quirks[] = { @@ -792,8 +787,8 @@ static int uvc_get_vs_commit(const struct device *dev, struct net_buf *const buf static int uvc_set_vs_commit(const struct device *dev, const struct net_buf *const buf) { struct uvc_data *data = dev->data; - struct video_format fmt = data->video_fmt; - struct video_frmival frmival = data->video_frmival; + struct video_format *fmt = &data->video_fmt; + struct video_frmival *frmival = &data->video_frmival; int ret; __ASSERT_NO_MSG(data->video_dev != NULL); @@ -803,27 +798,9 @@ static int uvc_set_vs_commit(const struct device *dev, const struct net_buf *con return ret; } - LOG_INF("Ready to transfer, setting source format to '%s' %ux%u", - VIDEO_FOURCC_TO_STR(fmt.pixelformat), fmt.width, fmt.height); - - fmt.type = VIDEO_BUF_TYPE_OUTPUT; - - ret = video_set_format(data->video_dev, &fmt); - if (ret != 0) { - LOG_ERR("Could not set the format of %s", data->video_dev->name); - return ret; - } - - LOG_DBG("Setting frame interval of %s to %u/%u", - data->video_dev->name, - data->video_frmival.numerator, data->video_frmival.denominator); - - ret = video_set_frmival(data->video_dev, &frmival); - if (ret != 0) { - LOG_WRN("Could not set the framerate of %s", data->video_dev->name); - } - - LOG_DBG("UVC device ready, %s can now be started", data->video_dev->name); + LOG_INF("Host selected format '%s' %ux%u, frame interval %u/%u", + VIDEO_FOURCC_TO_STR(fmt->pixelformat), fmt->width, fmt->height, + frmival->numerator, frmival->denominator); if (atomic_test_bit(&data->state, UVC_STATE_STREAM_READY)) { atomic_set_bit(&data->state, UVC_STATE_STREAM_RESTART); @@ -1408,13 +1385,13 @@ static union uvc_fmt_desc *uvc_new_fmt_desc(const struct device *dev) static int uvc_add_vs_format_desc(const struct device *dev, struct uvc_format_descriptor **const format_desc, - const struct video_format_cap *const cap) + uint32_t fourcc) { const struct uvc_config *cfg = dev->config; __ASSERT_NO_MSG(format_desc != NULL); - if (cap->pixelformat == VIDEO_PIX_FMT_JPEG) { + if (fourcc == VIDEO_PIX_FMT_JPEG) { struct uvc_format_mjpeg_descriptor *desc; LOG_INF("Adding format descriptor #%u for MJPEG", @@ -1437,7 +1414,7 @@ static int uvc_add_vs_format_desc(const struct device *dev, struct uvc_format_uncomp_descriptor *desc; LOG_INF("Adding format descriptor #%u for '%s'", - cfg->desc->if1_hdr.bNumFormats + 1, VIDEO_FOURCC_TO_STR(cap->pixelformat)); + cfg->desc->if1_hdr.bNumFormats + 1, VIDEO_FOURCC_TO_STR(fourcc)); desc = &uvc_new_fmt_desc(dev)->fmt_uncomp; if (desc == NULL) { @@ -1448,8 +1425,8 @@ static int uvc_add_vs_format_desc(const struct device *dev, desc->bFormatIndex = cfg->desc->if1_hdr.bNumFormats + 1; desc->bLength = sizeof(*desc); desc->bDescriptorSubtype = UVC_VS_FORMAT_UNCOMPRESSED; - uvc_fourcc_to_guid(desc->guidFormat, cap->pixelformat); - desc->bBitsPerPixel = video_bits_per_pixel(cap->pixelformat); + uvc_fourcc_to_guid(desc->guidFormat, fourcc); + desc->bBitsPerPixel = video_bits_per_pixel(fourcc); desc->bDefaultFrameIndex = 1; cfg->desc->if1_hdr.bNumFormats++; cfg->desc->if1_hdr.wTotalLength += desc->bLength; @@ -1473,7 +1450,8 @@ static int uvc_compare_frmival_desc(const void *const a, const void *const b) } static void uvc_set_vs_bitrate_range(struct uvc_frame_discrete_descriptor *const desc, - const uint64_t frmival_nsec, struct video_format *const fmt) + const uint64_t frmival_nsec, + const struct video_format *const fmt) { uint32_t bitrate_min = sys_le32_to_cpu(desc->dwMinBitRate); uint32_t bitrate_max = sys_le32_to_cpu(desc->dwMaxBitRate); @@ -1499,7 +1477,7 @@ static void uvc_set_vs_bitrate_range(struct uvc_frame_discrete_descriptor *const static int uvc_add_vs_frame_interval(struct uvc_frame_discrete_descriptor *const desc, const struct video_frmival *const frmival, - struct video_format *const fmt) + const struct video_format *const fmt) { int i = desc->bFrameIntervalType; @@ -1520,24 +1498,19 @@ static int uvc_add_vs_frame_interval(struct uvc_frame_discrete_descriptor *const static int uvc_add_vs_frame_desc(const struct device *dev, struct uvc_format_descriptor *const format_desc, - const struct video_format_cap *const cap, const bool min) + const struct video_format *const fmt) { const struct uvc_config *cfg = dev->config; struct uvc_data *data = dev->data; struct uvc_frame_discrete_descriptor *desc; - uint16_t w = min ? cap->width_min : cap->width_max; - uint16_t h = min ? cap->height_min : cap->height_max; - uint16_t p = MAX(video_bits_per_pixel(cap->pixelformat), 8) * w / BITS_PER_BYTE; - struct video_format fmt = {.pixelformat = cap->pixelformat, - .width = w, .height = h, .pitch = p}; - struct video_frmival_enum fie = {.format = &fmt}; + struct video_frmival_enum fie = {.format = fmt}; int ret; __ASSERT_NO_MSG(data->video_dev != NULL); __ASSERT_NO_MSG(format_desc != NULL); LOG_INF("Adding frame descriptor #%u for %ux%u", - format_desc->bNumFrameDescriptors + 1, w, h); + format_desc->bNumFrameDescriptors + 1, fmt->width, fmt->height); desc = &uvc_new_fmt_desc(dev)->frm_disc; if (desc == NULL) { @@ -1547,9 +1520,9 @@ static int uvc_add_vs_frame_desc(const struct device *dev, desc->bLength = sizeof(*desc) - CONFIG_USBD_VIDEO_MAX_FRMIVAL * sizeof(uint32_t); desc->bDescriptorType = USB_DESC_CS_INTERFACE; desc->bFrameIndex = format_desc->bNumFrameDescriptors + 1; - desc->wWidth = sys_cpu_to_le16(w); - desc->wHeight = sys_cpu_to_le16(h); - desc->dwMaxVideoFrameBufferSize = sys_cpu_to_le32(fmt.size); + desc->wWidth = sys_cpu_to_le16(fmt->width); + desc->wHeight = sys_cpu_to_le16(fmt->height); + desc->dwMaxVideoFrameBufferSize = sys_cpu_to_le32(fmt->size); desc->bDescriptorSubtype = (format_desc->bDescriptorSubtype == UVC_VS_FORMAT_UNCOMPRESSED) ? UVC_VS_FRAME_UNCOMPRESSED : UVC_VS_FRAME_MJPEG; desc->dwMinBitRate = sys_cpu_to_le32(UINT32_MAX); @@ -1561,7 +1534,7 @@ static int uvc_add_vs_frame_desc(const struct device *dev, case VIDEO_FRMIVAL_TYPE_DISCRETE: LOG_DBG("Adding discrete frame interval %u", fie.index); - ret = uvc_add_vs_frame_interval(desc, &fie.discrete, &fmt); + ret = uvc_add_vs_frame_interval(desc, &fie.discrete, fmt); if (ret != 0) { return ret; } @@ -1570,12 +1543,12 @@ static int uvc_add_vs_frame_desc(const struct device *dev, case VIDEO_FRMIVAL_TYPE_STEPWISE: LOG_DBG("Adding stepwise frame interval %u", fie.index); - ret = uvc_add_vs_frame_interval(desc, &fie.stepwise.min, &fmt); + ret = uvc_add_vs_frame_interval(desc, &fie.stepwise.min, fmt); if (ret != 0) { return ret; } - ret = uvc_add_vs_frame_interval(desc, &fie.stepwise.max, &fmt); + ret = uvc_add_vs_frame_interval(desc, &fie.stepwise.max, fmt); if (ret != 0) { return ret; } @@ -1591,7 +1564,7 @@ static int uvc_add_vs_frame_desc(const struct device *dev, if (desc->bFrameIntervalType == 0) { struct video_frmival frmival = {.numerator = 1, .denominator = 30}; - ret = uvc_add_vs_frame_interval(desc, &frmival, &fmt); + ret = uvc_add_vs_frame_interval(desc, &frmival, fmt); if (ret != 0) { return ret; } @@ -1636,10 +1609,6 @@ static int uvc_init(struct usbd_class_data *const c_data) const struct device *dev = usbd_class_get_private(c_data); const struct uvc_config *cfg = dev->config; struct uvc_data *data = dev->data; - struct uvc_format_descriptor *format_desc = NULL; - struct video_caps caps; - uint32_t prev_pixfmt = 0; - uint32_t mask = 0; int ret; __ASSERT_NO_MSG(data->video_dev != NULL); @@ -1649,9 +1618,51 @@ static int uvc_init(struct usbd_class_data *const c_data) return 0; } - cfg->desc->if0_hdr.baInterfaceNr[0] = cfg->desc->if1.bInterfaceNumber; + cfg->desc->if1_hdr.wTotalLength += cfg->desc->if1_color.bLength; - /* Generating VideoControl descriptors (interface 0) */ + ret = uvc_assign_desc(dev, &cfg->desc->if1_color, true, true); + if (ret != 0) { + return ret; + } + + ret = uvc_assign_desc(dev, &cfg->desc->if1_ep_fs, true, false); + if (ret != 0) { + return ret; + } + + ret = uvc_assign_desc(dev, &cfg->desc->if1_ep_hs, false, true); + if (ret != 0) { + return ret; + } + + cfg->desc->if1_hdr.wTotalLength = sys_cpu_to_le16(cfg->desc->if1_hdr.wTotalLength); + + /* Generating the default probe message now that descriptors are complete */ + + ret = uvc_get_vs_probe_struct(dev, &data->default_probe, UVC_GET_CUR); + if (ret != 0) { + LOG_ERR("init: failed to query the default probe"); + return ret; + } + + atomic_set_bit(&data->state, UVC_STATE_INITIALIZED); + + return 0; +} + +/* UVC public API */ + +void uvc_set_video_dev(const struct device *const dev, const struct device *const video_dev) +{ + struct uvc_data *data = dev->data; + const struct uvc_config *cfg = dev->config; + uint32_t mask = 0; + + data->video_dev = video_dev; + + /* Generate VideoControl descriptors (interface 0) */ + + cfg->desc->if0_hdr.baInterfaceNr[0] = cfg->desc->if1.bInterfaceNumber; mask = uvc_get_mask(data->video_dev, uvc_control_map_ct, ARRAY_SIZE(uvc_control_map_ct)); cfg->desc->if0_ct.bmControls[0] = mask >> 0; @@ -1668,81 +1679,60 @@ static int uvc_init(struct usbd_class_data *const c_data) cfg->desc->if0_xu.bmControls[1] = mask >> 8; cfg->desc->if0_xu.bmControls[2] = mask >> 16; cfg->desc->if0_xu.bmControls[3] = mask >> 24; +} - /* Generating VideoStreaming descriptors (interface 1) */ - - caps.type = VIDEO_BUF_TYPE_OUTPUT; +int uvc_add_format(const struct device *const dev, const struct video_format *const fmt) +{ + struct uvc_data *data = dev->data; + const struct uvc_config *cfg = dev->config; + int ret; - ret = video_get_caps(data->video_dev, &caps); - if (ret != 0) { - LOG_ERR("Could not load %s video format list", data->video_dev->name); - return ret; + if (data->video_dev == NULL) { + LOG_ERR("Video device not yet configured into UVC"); + return -EINVAL; } - cfg->desc->if1_hdr.wTotalLength = sys_le16_to_cpu(cfg->desc->if1_hdr.wTotalLength); + if (fmt->size == 0) { + LOG_ERR("The format size must be set prior to add it to UVC"); + return -EINVAL; + } - for (int i = 0; caps.format_caps[i].pixelformat != 0; i++) { - const struct video_format_cap *cap = &caps.format_caps[i]; + if (data->last_pix_fmt != fmt->pixelformat && + data->fmt_desc_idx + 2 > CONFIG_USBD_VIDEO_MAX_FORMATS) { + LOG_WRN("Not enough format descriptors to add descriptors for '%s' and %ux%u", + VIDEO_FOURCC_TO_STR(fmt->pixelformat), fmt->width, fmt->height); + return -ENOMEM; + } - if (prev_pixfmt != cap->pixelformat) { - if (prev_pixfmt != 0) { - cfg->desc->if1_hdr.wTotalLength += cfg->desc->if1_color.bLength; + if (data->last_pix_fmt == fmt->pixelformat && + data->fmt_desc_idx + 1 > CONFIG_USBD_VIDEO_MAX_FORMATS) { + LOG_WRN("Not enough format descriptors to add descriptors %ux%u", + fmt->width, fmt->height); + return -ENOMEM; + } - ret = uvc_assign_desc(dev, &cfg->desc->if1_color, true, true); - if (ret != 0) { - return ret; - } - } + if (data->last_pix_fmt != fmt->pixelformat) { + if (data->last_pix_fmt != 0) { + cfg->desc->if1_hdr.wTotalLength += cfg->desc->if1_color.bLength; - ret = uvc_add_vs_format_desc(dev, &format_desc, cap); + ret = uvc_assign_desc(dev, &cfg->desc->if1_color, true, true); if (ret != 0) { return ret; } } - ret = uvc_add_vs_frame_desc(dev, format_desc, cap, true); + ret = uvc_add_vs_format_desc(dev, &data->last_format_desc, fmt->pixelformat); if (ret != 0) { return ret; } - - if (cap->width_min != cap->width_max || cap->height_min != cap->height_max) { - ret = uvc_add_vs_frame_desc(dev, format_desc, cap, false); - if (ret != 0) { - return ret; - } - } - - prev_pixfmt = cap->pixelformat; - } - - cfg->desc->if1_hdr.wTotalLength += cfg->desc->if1_color.bLength; - - ret = uvc_assign_desc(dev, &cfg->desc->if1_color, true, true); - if (ret != 0) { - return ret; - } - - ret = uvc_assign_desc(dev, &cfg->desc->if1_ep_fs, true, false); - if (ret != 0) { - return ret; } - ret = uvc_assign_desc(dev, &cfg->desc->if1_ep_hs, false, true); + ret = uvc_add_vs_frame_desc(dev, data->last_format_desc, fmt); if (ret != 0) { return ret; } - cfg->desc->if1_hdr.wTotalLength = sys_cpu_to_le16(cfg->desc->if1_hdr.wTotalLength); - - /* Generating the default probe message now that descriptors are complete */ - - ret = uvc_get_vs_probe_struct(dev, &data->default_probe, UVC_GET_CUR); - if (ret != 0) { - LOG_ERR("init: failed to query the default probe"); - return ret; - } - - atomic_set_bit(&data->state, UVC_STATE_INITIALIZED); + data->last_pix_fmt = fmt->pixelformat; return 0; } @@ -2087,26 +2077,27 @@ static int uvc_dequeue(const struct device *dev, struct video_buffer **const vbu static int uvc_get_format(const struct device *dev, struct video_format *const fmt) { struct uvc_data *data = dev->data; - struct video_format tmp_fmt = {0}; - int ret; - - __ASSERT_NO_MSG(data->video_dev != NULL); if (!atomic_test_bit(&data->state, UVC_STATE_ENABLED) || !atomic_test_bit(&data->state, UVC_STATE_STREAM_READY)) { return -EAGAIN; } - LOG_DBG("Querying the format from %s", data->video_dev->name); + *fmt = data->video_fmt; - tmp_fmt.type = VIDEO_BUF_TYPE_OUTPUT; + return 0; +} - ret = video_get_format(data->video_dev, &tmp_fmt); - if (ret != 0) { - return ret; +static int uvc_get_frmival(const struct device *dev, struct video_frmival *const frmival) +{ + struct uvc_data *data = dev->data; + + if (!atomic_test_bit(&data->state, UVC_STATE_ENABLED) || + !atomic_test_bit(&data->state, UVC_STATE_STREAM_READY)) { + return -EAGAIN; } - *fmt = tmp_fmt; + *frmival = data->video_frmival; return 0; } @@ -2139,6 +2130,7 @@ static int uvc_set_signal(const struct device *dev, struct k_poll_signal *const static DEVICE_API(video, uvc_video_api) = { .get_format = uvc_get_format, + .get_frmival = uvc_get_frmival, .set_stream = uvc_set_stream, .enqueue = uvc_enqueue, .dequeue = uvc_dequeue, From 121d8b15746bc94529f09e2d07ca3c121bfc03c3 Mon Sep 17 00:00:00 2001 From: Josuah Demangeon Date: Wed, 16 Jul 2025 07:24:20 +0000 Subject: [PATCH 4/5] samples: usb: uvc: add filtering of the format The UVC class now lets the application select the format list sent to the host. Leverage this in the sample to filter out any format that is not expected to work (buffer too big, rarely supported formats). Signed-off-by: Josuah Demangeon --- samples/subsys/usb/uvc/src/main.c | 35 ++++++++++++++++++++++++++++--- 1 file changed, 32 insertions(+), 3 deletions(-) diff --git a/samples/subsys/usb/uvc/src/main.c b/samples/subsys/usb/uvc/src/main.c index 1342e6e3e3b0f..7830c3a9427b9 100644 --- a/samples/subsys/usb/uvc/src/main.c +++ b/samples/subsys/usb/uvc/src/main.c @@ -23,7 +23,28 @@ const static struct device *const video_dev = DEVICE_DT_GET(DT_CHOSEN(zephyr_cam /* Format capabilities of video_dev, used everywhere through the sample */ static struct video_caps video_caps = {.type = VIDEO_BUF_TYPE_OUTPUT}; -static int app_add_format(uint32_t pixfmt, uint32_t width, uint32_t height) +/* Pixel formats present in one of the UVC 1.5 standard */ +static bool app_is_supported_format(uint32_t pixfmt) +{ + return pixfmt == VIDEO_PIX_FMT_JPEG || + pixfmt == VIDEO_PIX_FMT_YUYV || + pixfmt == VIDEO_PIX_FMT_NV12; +} + +static bool app_has_supported_format(void) +{ + const struct video_format_cap *fmts = video_caps.format_caps; + + for (int i = 0; fmts[i].pixelformat != 0; i++) { + if (app_is_supported_format(fmts[i].pixelformat)) { + return true; + } + } + + return false; +} + +static int app_add_format(uint32_t pixfmt, uint32_t width, uint32_t height, bool has_sup_fmts) { struct video_format fmt = { .pixelformat = pixfmt, @@ -33,6 +54,11 @@ static int app_add_format(uint32_t pixfmt, uint32_t width, uint32_t height) }; int ret; + /* If the system has any standard pixel format, only propose them to the host */ + if (has_sup_fmts && !app_is_supported_format(pixfmt)) { + return 0; + } + /* Set the format to get the size */ ret = video_set_format(video_dev, &fmt); if (ret != 0) { @@ -58,18 +84,21 @@ static int app_add_format(uint32_t pixfmt, uint32_t width, uint32_t height) /* Submit to UVC only the formats expected to be working (enough memory for the size, etc.) */ static int app_add_filtered_formats(void) { + const bool has_sup_fmts = app_has_supported_format(); int ret; for (int i = 0; video_caps.format_caps[i].pixelformat != 0; i++) { const struct video_format_cap *vcap = &video_caps.format_caps[i]; - ret = app_add_format(vcap->pixelformat, vcap->width_min, vcap->height_min); + ret = app_add_format(vcap->pixelformat, vcap->width_min, vcap->height_min, + has_sup_fmts); if (ret != 0) { return ret; } if (vcap->width_min != vcap->width_max || vcap->height_min != vcap->height_max) { - ret = app_add_format(vcap->pixelformat, vcap->width_max, vcap->height_max); + ret = app_add_format(vcap->pixelformat, vcap->width_max, vcap->height_max, + has_sup_fmts); if (ret != 0) { return ret; } From d6d97c6898741af8a663e228dd339b52c62be191 Mon Sep 17 00:00:00 2001 From: Josuah Demangeon Date: Thu, 24 Jul 2025 11:04:50 +0000 Subject: [PATCH 5/5] doc: release: 4.3: add UVC changes Add USB UVC device's new uvc_add_format() function to the release note, and document the semantic changes of UVC APIs in the migration guide. Signed-off-by: Josuah Demangeon --- doc/releases/migration-guide-4.3.rst | 6 ++++++ doc/releases/release-notes-4.3.rst | 6 ++++++ 2 files changed, 12 insertions(+) diff --git a/doc/releases/migration-guide-4.3.rst b/doc/releases/migration-guide-4.3.rst index cc4cdb88cfeeb..9369e5c2db23f 100644 --- a/doc/releases/migration-guide-4.3.rst +++ b/doc/releases/migration-guide-4.3.rst @@ -175,6 +175,12 @@ Stepper * :dtcompatible:`zephyr,gpio-stepper` has been replaced by :dtcompatible:`zephyr,h-bridge-stepper`. +USB +=== + +* The USB Video Class was configuring the framerate and format of the source video device. + This is now to be done by the application after the host selected the format (:github:`93192`). + .. zephyr-keep-sorted-stop Bluetooth diff --git a/doc/releases/release-notes-4.3.rst b/doc/releases/release-notes-4.3.rst index 6d300cfea7a30..114848a9fb7d1 100644 --- a/doc/releases/release-notes-4.3.rst +++ b/doc/releases/release-notes-4.3.rst @@ -309,6 +309,12 @@ New APIs and options * :c:macro:`__deprecated_version` +* USB + + * Video + + * :c:func:`uvc_add_format` + * Video * :c:member:`video_format.size` field