fix(host/uvc): Fixed negotiation for some non-conforming UVC devices

Some UVC devices require to set alternate setting to 0 before the negotiation starts. Closes espressif/esp-idf#9868
espressif · Jan 30, 2025 · 477ab5c · 477ab5c
1 parent 43cc64e
commit 477ab5c
Show file tree

Hide file tree

Showing 4 changed files with 98 additions and 69 deletions.
diff --git a/host/class/uvc/usb_host_uvc/CHANGELOG.md b/host/class/uvc/usb_host_uvc/CHANGELOG.md
@@ -1,3 +1,7 @@
+## 2.0.1
+
+- Fixed negotiation for some non-conforming UVC devices (https://github.com/espressif/esp-idf/issues/9868)
+
 ## 2.0.0
 
 - New version of the driver, native to Espressif's USB Host Library

diff --git a/host/class/uvc/usb_host_uvc/idf_component.yml b/host/class/uvc/usb_host_uvc/idf_component.yml
@@ -1,5 +1,5 @@
 ## IDF Component Manager Manifest File
-version: "2.0.0"
+version: "2.0.1"
 description: USB Host UVC driver
 url: https://github.com/espressif/esp-usb/tree/master/host/class/uvc/usb_host_uvc
 dependencies:

diff --git a/host/class/uvc/usb_host_uvc/uvc_control.c b/host/class/uvc/usb_host_uvc/uvc_control.c
@@ -1,11 +1,12 @@
 /*
- * SPDX-FileCopyrightText: 2024 Espressif Systems (Shanghai) CO LTD
+ * SPDX-FileCopyrightText: 2024-2025 Espressif Systems (Shanghai) CO LTD
  *
  * SPDX-License-Identifier: Apache-2.0
  */
 // This file will contain all Class-Specific request from USB UVC specification chapter 4
 
 #include <string.h> // For memset
+#include <math.h>   // fabs for float comparison
 
 #include "esp_check.h"
 
@@ -16,6 +17,8 @@
 #include "uvc_descriptors_priv.h"
 #include "uvc_check_priv.h"
 
+#define FLOAT_EQUAL(a, b) (fabs(a - b) < 0.0001f) // For comparing float values with acceptable difference (epsilon value)
+
 static const char *TAG = "uvc-control";
 
 static uint16_t uvc_vs_control_size(uint16_t uvc_version)
@@ -112,7 +115,7 @@ static inline bool uvc_is_vs_format_equal(const uvc_host_stream_format_t *a, con
 {
     if (a->h_res == b->h_res &&
             a->v_res == b->v_res &&
-            a->fps == b->fps &&
+            FLOAT_EQUAL(a->fps, b->fps) &&
             a->format == b->format) {
         return true;
     }
@@ -128,27 +131,35 @@ esp_err_t uvc_host_stream_control_negotiate(uvc_host_stream_hdl_t stream_hdl, co
     UVC_CHECK(stream_hdl && vs_format, ESP_ERR_INVALID_ARG);
     esp_err_t ret = ESP_ERR_NOT_FOUND;
     uvc_vs_ctrl_t vs_result = {0};
+    uvc_vs_ctrl_t vs_result_ignored = {0};
+    uvc_host_stream_format_t format_set, format_ignored;
 
     // Try 2x. Some camera may return error on first try
-    uvc_host_stream_format_t set_format, fake_format;
     for (int i = 0; i < 2; i++) {
-        // We do this to mimic Windows driver
-        uvc_host_stream_control_probe_get(stream_hdl, &vs_result, &fake_format);
-        uvc_host_stream_control_probe_get_max(stream_hdl, &vs_result, &fake_format);
-        uvc_host_stream_control_probe_get_min(stream_hdl, &vs_result, &fake_format);
-
-        // The real negotiation starts here. Zeroize the vs_result struct and start over
-        memset(&vs_result, 0, sizeof(uvc_vs_ctrl_t));
+        // Notes: Some cameras require 'probe_get' to be the 1st negotiation call
+        // It returns 'default' format and frame settings. It is ignored for now.
+        // We can reuse the returned value, if we wanted to implement 'negotiate default frame format' feature.
+        uvc_host_stream_control_probe_get(stream_hdl, &vs_result, &format_ignored);
+        uvc_host_stream_control_probe_set(stream_hdl, &vs_result, vs_format);       // Set the desired frame format
+        uvc_host_stream_control_probe_get(stream_hdl, &vs_result, &format_ignored); // Get back the format (not checked yet)
+
+        // We do this to mimic Windows driver: The Min/Max values are ignored by this driver.
+        // These values can be used if we wanted to negotiate advanced parameters, such as wCompQuality to select JPEG encoding quality
+        uvc_host_stream_control_probe_get_max(stream_hdl, &vs_result_ignored, &format_ignored);
+        uvc_host_stream_control_probe_get_min(stream_hdl, &vs_result_ignored, &format_ignored);
+
+        // Probe that the camera accepts our format before committing
         ret = uvc_host_stream_control_probe_set(stream_hdl, &vs_result, vs_format);
-        ret |= uvc_host_stream_control_probe_get(stream_hdl, &vs_result, &set_format);
+        ret |= uvc_host_stream_control_probe_get(stream_hdl, &vs_result, &format_set);
         UVC_CHECK(ret == ESP_OK, ret);
-        if (uvc_is_vs_format_equal(&set_format, vs_format)) {
+        if (uvc_is_vs_format_equal(&format_set, vs_format)) {
+            // If the 'set format' equals 'get format', the camera accepts our format and we can commit it
             break;
         }
     }
 
     ESP_LOGD(TAG, "Frame format negotiation:\n\tRequested: %dx%d@%2.1fFPS\n\tGot: %dx%d@%2.1fFPS",
-             vs_format->h_res, vs_format->v_res, vs_format->fps, set_format.h_res, set_format.v_res, set_format.fps);
+             vs_format->h_res, vs_format->v_res, vs_format->fps, format_set.h_res, format_set.v_res, format_set.fps);
 
     // Commit the negotiated format
     ret = uvc_host_stream_control_commit(stream_hdl, &vs_result, vs_format);

diff --git a/host/class/uvc/usb_host_uvc/uvc_host.c b/host/class/uvc/usb_host_uvc/uvc_host.c
@@ -1,5 +1,5 @@
 /*
- * SPDX-FileCopyrightText: 2024 Espressif Systems (Shanghai) CO LTD
+ * SPDX-FileCopyrightText: 2024-2025 Espressif Systems (Shanghai) CO LTD
  *
  * SPDX-License-Identifier: Apache-2.0
  */
@@ -321,11 +321,49 @@ static esp_err_t uvc_find_and_open_usb_device(uint16_t vid, uint16_t pid, TickTy
     return ESP_ERR_NOT_FOUND;
 }
 
-static esp_err_t uvc_find_streaming_intf(uvc_stream_t *uvc_stream, uint8_t uvc_index, const uvc_host_stream_format_t *vs_format)
+/**
+ * @brief Send SetInterface USB command to the camera
+ *
+ * @note Only for ISOC streams
+ * @param[in] stream_hdl UVC stream handle
+ * @param[in] stream_on  true: Set streaming alternate interface. false: Set alternative setting to 0
+ * @return
+ *     - ESP_OK: Success
+ *     - Other:  CTRL transfer error
+ */
+static esp_err_t uvc_set_interface(uvc_host_stream_hdl_t stream_hdl, bool stream_on)
+{
+    uvc_stream_t *uvc_stream = (uvc_stream_t *)stream_hdl;
+    return uvc_host_usb_ctrl(
+               stream_hdl,
+               USB_BM_REQUEST_TYPE_DIR_OUT | USB_BM_REQUEST_TYPE_TYPE_STANDARD | USB_BM_REQUEST_TYPE_RECIP_INTERFACE,
+               USB_B_REQUEST_SET_INTERFACE,
+               stream_on ? uvc_stream->constant.bAlternateSetting : 0,
+               uvc_stream->constant.bInterfaceNumber,
+               0,
+               NULL);
+}
+
+/**
+ * @brief Find and claim interface for selected frame format
+ *
+ * @param[in]  uvc_stream  Pointer to UVC stream
+ * @param[in]  uvc_index   Index of UVC function you want to use
+ * @param[in]  vs_format   Desired frame format
+ * @param[out] ep_desc_ret EP descriptor for this stream
+ * @return
+ *     - ESP_OK:              Success, interface found and claimed
+ *     - ESP_ERR_INVALID_ARG: Input parameter is NULL
+ *     - ESP_ERR_NOT_FOUND:   Selected format was not found
+ *     - Other:               Error during interface claim
+ */
+static esp_err_t uvc_claim_interface(uvc_stream_t *uvc_stream, uint8_t uvc_index, const uvc_host_stream_format_t *vs_format, const usb_ep_desc_t **ep_desc_ret)
 {
-    UVC_CHECK(uvc_stream && vs_format, ESP_ERR_INVALID_ARG);
+    UVC_CHECK(uvc_stream && vs_format && ep_desc_ret, ESP_ERR_INVALID_ARG);
 
     const usb_config_desc_t *cfg_desc;
+    const usb_intf_desc_t *intf_desc;
+    const usb_ep_desc_t *ep_desc;
     ESP_ERROR_CHECK(usb_host_get_active_config_descriptor(uvc_stream->constant.dev_hdl, &cfg_desc));
 
     // Find UVC USB function with desired index
@@ -337,39 +375,23 @@ static esp_err_t uvc_find_streaming_intf(uvc_stream_t *uvc_stream, uint8_t uvc_i
         TAG, "Could not find frame format %dx%d@%2.1fFPS",
         vs_format->h_res, vs_format->v_res, vs_format->fps);
 
-    // Here we only save the interface number that can meet our format requirement
-    // bAlternateSetting and bEndpointAddress are saved during interface claim
-    uvc_stream->constant.bInterfaceNumber = bInterfaceNumber;
-    uvc_stream->constant.bcdUVC = bcdUVC;
-    return ESP_OK;
-}
-
-/**
- * @brief Claim streaming interface
- *
- * @param[in]  uvc_stream       UVC stream handle
- * @param[out] ep_desc_ret      Pointer of associated streaming endpoint
- * @return
- *     - ESP_OK: Success - interface claimed
- *     - Else: Error
- */
-static esp_err_t uvc_claim_interface(uvc_stream_t *uvc_stream, const usb_ep_desc_t **ep_desc_ret)
-{
-    const usb_intf_desc_t *intf_desc;
-    const usb_ep_desc_t *ep_desc;
-    const usb_config_desc_t *cfg_desc;
-    ESP_ERROR_CHECK(usb_host_get_active_config_descriptor(uvc_stream->constant.dev_hdl, &cfg_desc));
-
     ESP_RETURN_ON_ERROR(
-        uvc_desc_get_streaming_intf_and_ep(cfg_desc, uvc_stream->constant.bInterfaceNumber, MAX_MPS_IN, &intf_desc, &ep_desc),
-        TAG, "Could not find Streaming interface %d", uvc_stream->constant.bInterfaceNumber);
+        uvc_desc_get_streaming_intf_and_ep(cfg_desc, bInterfaceNumber, MAX_MPS_IN, &intf_desc, &ep_desc),
+        TAG, "Could not find Streaming interface %d", bInterfaceNumber);
 
-    // Save all required parameters
+    // Save all constant information about the UVC stream
+    uvc_stream->constant.bInterfaceNumber  = bInterfaceNumber;
+    uvc_stream->constant.bcdUVC            = bcdUVC;
     uvc_stream->constant.bAlternateSetting = intf_desc->bAlternateSetting;
     uvc_stream->constant.bEndpointAddress  = ep_desc->bEndpointAddress;
-    *ep_desc_ret = ep_desc;
-
-    return usb_host_interface_claim(p_uvc_host_driver->usb_client_hdl, uvc_stream->constant.dev_hdl, intf_desc->bInterfaceNumber, intf_desc->bAlternateSetting);
+    *ep_desc_ret                           = ep_desc;
+
+    // Claim the interface in USB Host Lib
+    return usb_host_interface_claim(
+               p_uvc_host_driver->usb_client_hdl,
+               uvc_stream->constant.dev_hdl,
+               intf_desc->bInterfaceNumber,
+               intf_desc->bAlternateSetting);
 }
 
 esp_err_t uvc_host_install(const uvc_host_driver_config_t *driver_config)
@@ -535,24 +557,29 @@ esp_err_t uvc_host_stream_open(const uvc_host_stream_config_t *stream_config, in
         goto not_found;
     }
 
-    // Find the streaming interface
+    // Find the streaming interface and endpoint and claim it
+    const usb_ep_desc_t *ep_desc;
     ESP_GOTO_ON_ERROR(
-        uvc_find_streaming_intf(uvc_stream, stream_config->usb.uvc_stream_index, &stream_config->vs_format),
-        err, TAG, "Could not find streaming interface");
+        uvc_claim_interface(uvc_stream, stream_config->usb.uvc_stream_index, &stream_config->vs_format, &ep_desc),
+        claim_err, TAG, "Could not find/claim streaming interface");
+    ESP_LOGD(TAG, "Claimed interface index %d with MPS %d", uvc_stream->constant.bInterfaceNumber, USB_EP_DESC_GET_MPS(ep_desc));
+
+    /*
+    * Although not strictly required by the UVC specification, some UVC ISOC
+    * cameras require explicitly entering the NOT STREAMING state by setting
+    * the interface's Alternate Setting to 0.
+    */
+    if (uvc_stream->constant.bAlternateSetting != 0) {
+        // We do not check return code here on purpose. We can silently continue
+        uvc_set_interface(uvc_stream, false);
+    }
 
     // Negotiate the frame format
     uvc_vs_ctrl_t vs_result;
     ESP_GOTO_ON_ERROR(
         uvc_host_stream_control_negotiate(uvc_stream, &stream_config->vs_format, &vs_result),
         err, TAG, "Failed to negotiate requested Video Stream format");
 
-    // Claim Video Streaming interface
-    const usb_ep_desc_t *ep_desc;
-    ESP_GOTO_ON_ERROR(
-        uvc_claim_interface(uvc_stream, &ep_desc),
-        claim_err, TAG, "Could not claim Streaming interface");
-    ESP_LOGD(TAG, "Claimed interface index %d with MPS %d", uvc_stream->constant.bInterfaceNumber, USB_EP_DESC_GET_MPS(ep_desc));
-
     // Allocate USB transfers
     ESP_GOTO_ON_ERROR(
         uvc_transfers_allocate(uvc_stream, stream_config->advanced.number_of_urbs, stream_config->advanced.urb_size, ep_desc),
@@ -652,19 +679,6 @@ esp_err_t uvc_host_stream_close(uvc_host_stream_hdl_t stream_hdl)
     return ret;
 }
 
-static esp_err_t uvc_set_interface(uvc_host_stream_hdl_t stream_hdl, bool stream_on)
-{
-    uvc_stream_t *uvc_stream = (uvc_stream_t *)stream_hdl;
-    return uvc_host_usb_ctrl(
-               stream_hdl,
-               USB_BM_REQUEST_TYPE_DIR_OUT | USB_BM_REQUEST_TYPE_TYPE_STANDARD | USB_BM_REQUEST_TYPE_RECIP_INTERFACE,
-               USB_B_REQUEST_SET_INTERFACE,
-               stream_on ? uvc_stream->constant.bAlternateSetting : 0,
-               uvc_stream->constant.bInterfaceNumber,
-               0,
-               NULL);
-}
-
 static esp_err_t uvc_clear_endpoint_feature(uvc_host_stream_hdl_t stream_hdl)
 {
     uvc_stream_t *uvc_stream = (uvc_stream_t *)stream_hdl;