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 <me@josuah.net>

Author: josuah

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);
+
 /**
  * @}
  */

samples/subsys/usb/uvc/src/main.c

@@ -17,15 +17,72 @@
 
 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)
+{
+	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 +93,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 +120,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 +135,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);