feat(uvc): Thread safety

host/class/uvc/usb_host_uvc_2/TODO

@@ -23,7 +23,7 @@ To-Do list:
 ## Added to first release after reviews:
 - [x] Allow floating point FPS
 - [x] Consider making uvc_host_stream_pause/unpause private!
-- [ ] Organize members of uvc_host_stream_s into dynamic/constant/mux_protected/single_thread sub-structures
+- [x] Organize members of uvc_host_stream_s into dynamic/constant/mux_protected/single_thread sub-structures
 
 ## Future releases
 - [x] Check if MPS fits into IN FIFO (on S2 and S3 our buffer is not big enough to hold 1 ISOC frame!)

host/class/uvc/usb_host_uvc_2/examples/basic_uvc_stream/main/basic_uvc_stream.c

@@ -107,7 +107,7 @@ static void frame_handling_task(void *arg)
 {
     const uvc_host_stream_config_t *stream_config = (const uvc_host_stream_config_t *)arg;
     QueueHandle_t frame_q = *((QueueHandle_t *)(stream_config->user_ctx));
-    const int uvc_index = stream_config->usb.uvc_function_index;
+    const int uvc_index = stream_config->usb.uvc_stream_index;
 
     while (true) {
         uvc_host_stream_hdl_t uvc_stream = NULL;
@@ -167,7 +167,7 @@ static const uvc_host_stream_config_t stream_mjpeg_config = {
     .user_ctx = &rx_frames_queue[0],
     .usb.vid = EXAMPLE_USB_DEVICE_VID,
     .usb.pid = EXAMPLE_USB_DEVICE_PID,
-    .usb.uvc_function_index = 0,
+    .usb.uvc_stream_index = 0,
     .vs_format.h_res = 720,
     .vs_format.v_res = 1280,
     .vs_format.fps = 15,
@@ -184,7 +184,7 @@ static const uvc_host_stream_config_t stream_h265_config = {
     .user_ctx = &rx_frames_queue[1],
     .usb.vid = EXAMPLE_USB_DEVICE_VID,
     .usb.pid = EXAMPLE_USB_DEVICE_PID, // Customer's device
-    .usb.uvc_function_index = 1,
+    .usb.uvc_stream_index = 1,
     .vs_format.h_res = 1280,
     .vs_format.v_res = 720,
     .vs_format.fps = 15,

host/class/uvc/usb_host_uvc_2/examples/camera_display/main/camera_display.c

@@ -215,7 +215,7 @@ void app_main(void)
         .usb = {
             .vid = 0,
             .pid = 0,
-            .uvc_function_index = 0,
+            .uvc_stream_index = 0,
         },
         .vs_format = {
             .h_res = FRAME_H_RES,

host/class/uvc/usb_host_uvc_2/host_test/main/streaming/test_streaming.cpp

@@ -36,16 +36,16 @@ void run_streaming_frame_reconstruction_scenario(void)
     // Variables reused in all tests
     constexpr int user_arg = 0x12345678;
     uvc_stream_t stream = {}; // Define mock stream
-    stream.cb_arg = (void *)&user_arg;
+    stream.constant.cb_arg = (void *)&user_arg;
     // @todo instead of doing this we can
     // - uvc_transfers_allocate   to allocate all transfers
     // - uvc_host_stream_unpause  to start the stream
 
-    stream.current_frame_id = 2; // Start with invalid frame ID
-    stream.stream_cb = [](const uvc_host_stream_event_data_t *event, void *user_ctx) {
+    stream.single_thread.current_frame_id = 2; // Start with invalid frame ID
+    stream.constant.stream_cb = [](const uvc_host_stream_event_data_t *event, void *user_ctx) {
         return stream_callback(event, user_ctx);
     };
-    stream.frame_cb = [](const uvc_host_frame_t *frame, void *user_ctx) -> bool {
+    stream.constant.frame_cb = [](const uvc_host_frame_t *frame, void *user_ctx) -> bool {
         // We cannot have catching lambdas here, so we must call this std::function...
         return frame_callback(frame, user_ctx);
     };
@@ -60,8 +60,8 @@ void run_streaming_frame_reconstruction_scenario(void)
     };
 
     GIVEN("Streaming enabled") {
-        stream.streaming = true;
-        stream.vs_format = {
+        stream.dynamic.streaming = true;
+        stream.constant.vs_format = {
             .h_res = 46,
             .v_res = 46,
             .fps = 15,
@@ -75,12 +75,12 @@ void run_streaming_frame_reconstruction_scenario(void)
             frame_callback = [&](const uvc_host_frame_t *frame, void *user_ctx) -> bool {
                 frame_callback_called++;
 
-                REQUIRE(user_ctx == stream.cb_arg); // Sanity check
+                REQUIRE(user_ctx == stream.constant.cb_arg); // Sanity check
                 REQUIRE(frame->data_len == logo_jpg.size());
-                REQUIRE(frame->vs_format.h_res == stream.vs_format.h_res);
-                REQUIRE(frame->vs_format.v_res == stream.vs_format.v_res);
-                REQUIRE(frame->vs_format.fps == stream.vs_format.fps);
-                REQUIRE(frame->vs_format.format == stream.vs_format.format);
+                REQUIRE(frame->vs_format.h_res == stream.constant.vs_format.h_res);
+                REQUIRE(frame->vs_format.v_res == stream.constant.vs_format.v_res);
+                REQUIRE(frame->vs_format.fps == stream.constant.vs_format.fps);
+                REQUIRE(frame->vs_format.format == stream.constant.vs_format.format);
 
                 std::vector<uint8_t> frame_data(frame->data, frame->data + frame->data_len);
                 std::vector<uint8_t> original_data(logo_jpg.begin(), logo_jpg.end());
@@ -163,7 +163,7 @@ void run_streaming_frame_reconstruction_scenario(void)
             // We expect overflow stream event
             enum uvc_host_dev_event event_type = static_cast<enum uvc_host_dev_event>(-1); // Explicitly set to invalid value
             stream_callback = [&](const uvc_host_stream_event_data_t *event, void *user_ctx) {
-                REQUIRE(user_ctx == stream.cb_arg); // Sanity check
+                REQUIRE(user_ctx == stream.constant.cb_arg); // Sanity check
                 event_type = event->type;
             };
             REQUIRE(uvc_frame_allocate(&stream, 1, logo_jpg.size() - 100, 0) == ESP_OK);
@@ -183,7 +183,7 @@ void run_streaming_frame_reconstruction_scenario(void)
             // We expect overflow stream event
             enum uvc_host_dev_event event_type = static_cast<enum uvc_host_dev_event>(-1); // Explicitly set to invalid value
             stream_callback = [&](const uvc_host_stream_event_data_t *event, void *user_ctx) {
-                REQUIRE(user_ctx == stream.cb_arg); // Sanity check
+                REQUIRE(user_ctx == stream.constant.cb_arg); // Sanity check
                 event_type = event->type;
             };
 
@@ -207,7 +207,7 @@ void run_streaming_frame_reconstruction_scenario(void)
     }
 
     GIVEN("Streaming disabled") {
-        stream.streaming = false;
+        stream.dynamic.streaming = false;
 
         WHEN("New data is received") {
             usb_transfer_t transfer = {

host/class/uvc/usb_host_uvc_2/include/usb/uvc_host.h

@@ -116,7 +116,7 @@ typedef struct {
     struct {
         uint16_t vid;                     /**< Device's Vendor ID. Set to 0 for any */
         uint16_t pid;                     /**< Device's Product ID. Set to 0 for any */
-        uint8_t uvc_function_index;       /**< Index of UVC function you want to use. Set to 0 to use first available UVC function */
+        uint8_t uvc_stream_index;         /**< Index of UVC function you want to use. Set to 0 to use first available UVC function */
     } usb;
     uvc_host_stream_format_t vs_format;   /**< Video Stream format. Resolution, FPS and encoding */
     struct {

host/class/uvc/usb_host_uvc_2/private_include/uvc_critical_priv.h

@@ -6,9 +6,17 @@
 
 #pragma once
 
+#include <stdatomic.h>
+
 #include "freertos/FreeRTOS.h"
 #include "freertos/portmacro.h"
 
 extern portMUX_TYPE uvc_lock;
-#define UVC_ENTER_CRITICAL()   portENTER_CRITICAL(&uvc_lock)
-#define UVC_EXIT_CRITICAL()    portEXIT_CRITICAL(&uvc_lock)
+#define UVC_ENTER_CRITICAL()              portENTER_CRITICAL(&uvc_lock)
+#define UVC_EXIT_CRITICAL()               portEXIT_CRITICAL(&uvc_lock)
+
+#define UVC_ATOMIC_LOAD(x)                __atomic_load_n(&x, __ATOMIC_SEQ_CST)
+#define UVC_ATOMIC_SET_IF_NULL(x, new_x)  ({ \
+                                              int expected = 0; \
+                                              __atomic_compare_exchange_n(&(x), &expected, (new_x), false, __ATOMIC_SEQ_CST, __ATOMIC_SEQ_CST); \
+                                          })

host/class/uvc/usb_host_uvc_2/private_include/uvc_types_priv.h

@@ -28,31 +28,36 @@ typedef enum {
 } uvc_stream_bulk_packet_type_t;
 
 struct uvc_host_stream_s {
-    // UVC driver related members
-    uvc_host_stream_callback_t stream_cb; // User's callback for stream events
-    uvc_host_frame_callback_t frame_cb;   // User's frame callback
-    void *cb_arg;                         // Common argument for user's callbacks
     SLIST_ENTRY(uvc_host_stream_s) list_entry;
 
-    // Constant USB descriptor values
-    uint16_t bcdUVC;                      // Version of UVC specs this device implements
-    uint8_t bInterfaceNumber;             // USB Video Streaming interface claimed by this stream. Needed for ISOC Stream start and CTRL transfers
-    uint8_t bAlternateSetting;            // Alternate setting for selected interface. Needed for ISOC Stream start
-    uint8_t bEndpointAddress;             // Streaming endpoint address. Needed for BULK Stream stop
-
-    // USB host related members
-    usb_device_handle_t dev_hdl;          // USB device handle
-    unsigned num_of_xfers;                // Number of USB transfers
-    usb_transfer_t **xfers;               // Pointer to array of USB transfers. Accessible only by the UVC driver
-
-    // Frame buffers
-    bool streaming;                       // Flag whether the stream is on/off
-    uvc_host_stream_format_t vs_format;   // Format of the video stream
-    QueueHandle_t empty_fb_queue;         // Queue of empty framebuffers
-    uvc_host_frame_t *current_frame;      // Frame that is being written to
-    bool skip_current_frame;              // Flag to skip this frame. An error has occurred during fetch
-    uint8_t current_frame_id;             // Current frame ID. Used for start of frame detection
-
-    // Bulk only
-    uvc_stream_bulk_packet_type_t next_bulk_packet; // Simple state machine: next expected packet
+    struct {
+        // UVC driver related members
+        uvc_host_stream_callback_t stream_cb; // User's callback for stream events
+        uvc_host_frame_callback_t frame_cb;   // User's frame callback
+        void *cb_arg;                         // Common argument for user's callbacks
+        uvc_host_stream_format_t vs_format;   // Format of the video stream (Runtime format change of opened stream is not supported)
+        QueueHandle_t empty_fb_queue;         // Queue of empty framebuffers
+
+        // Constant USB descriptor values
+        uint16_t bcdUVC;                      // Version of UVC specs this device implements
+        uint8_t  bInterfaceNumber;            // USB Video Streaming interface claimed by this stream. Needed for ISOC Stream start and CTRL transfers
+        uint8_t  bAlternateSetting;           // Alternate setting for selected interface. Needed for ISOC Stream start
+        uint8_t  bEndpointAddress;            // Streaming endpoint address. Needed for BULK Stream stop
+
+        // USB host related members
+        usb_device_handle_t dev_hdl;          // USB device handle
+        unsigned num_of_xfers;                // Number of USB transfers
+        usb_transfer_t **xfers;               // Pointer to array of USB transfers. Accessible only by the UVC driver
+    } constant; // Constant members do no change after installation thus do not require a critical section
+
+    struct {
+        uvc_host_frame_t *current_frame;      // Frame that is being written to
+        bool streaming;                       // Flag whether stream is on/off
+    } dynamic; // Dynamic members require a critical section
+
+    struct {
+        uvc_stream_bulk_packet_type_t next_bulk_packet; // Bulk only: next expected packet
+        bool skip_current_frame;                        // Flag to skip current frame. An error has occurred in the stream
+        uint8_t current_frame_id;                       // Frame ID can be only 0 or 1. But we also allow setting it to invalid value = 2.
+    } single_thread; // Single thread members are only accessed from 1 thread, so they do not need protection
 };