Skip to content

Commit 1bb9221

Browse files
committed
fix(uvc): add mjpeg and uvc payload header checks
1 parent f697348 commit 1bb9221

File tree

14 files changed

+155
-63
lines changed

14 files changed

+155
-63
lines changed

host/class/uvc/usb_host_uvc/CHANGELOG.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,14 @@ All notable changes to this project will be documented in this file.
55
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),
66
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
77

8+
## [2.4.1] - 2025-11-27
9+
10+
### Added
11+
12+
- Added SOI check to prevent output of corrupted MJPEG frames
13+
- Moved `usb_types_uvc.h` to `private_include` directory
14+
- Fixed bulk transfer EOF handling to improve robustness
15+
816
## [2.4.0] - 2025-11-21
917

1018
### Added

host/class/uvc/usb_host_uvc/host_test/main/streaming/test_streaming_helpers.hpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
#include <cstdint>
1313

1414
#include "usb/usb_types_stack.h"
15-
#include "usb/usb_types_uvc.h"
15+
#include "usb_types_uvc.h"
1616

1717
extern "C" {
1818
#include "Mockusb_host.h"

host/class/uvc/usb_host_uvc/idf_component.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
## IDF Component Manager Manifest File
2-
version: "2.4.0"
2+
version: "2.4.1"
33
description: USB Host UVC driver
44
url: https://github.com/espressif/esp-usb/tree/master/host/class/uvc/usb_host_uvc
55
dependencies:

host/class/uvc/usb_host_uvc/include/esp_private/uvc_control.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
#pragma once
88

99
#include "usb/uvc_host.h"
10+
#include "usb_types_uvc.h"
1011

1112
#ifdef __cplusplus
1213
extern "C" {

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88

99
#include <stdbool.h>
1010
#include "usb/usb_host.h"
11-
#include "usb/usb_types_uvc.h"
1211
#include "esp_err.h"
1312

1413
// Use this macros for opening a UVC stream with any VID or PID

host/class/uvc/usb_host_uvc/private_include/uvc_descriptors_priv.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
// So we include only files with USB specification definitions
1111
// This interface is also used in host_tests
1212
#include "usb/usb_types_ch9.h"
13-
#include "usb/usb_types_uvc.h"
13+
#include "usb_types_uvc.h"
1414

1515
#define UVC_DESC_FPS_TO_DWFRAMEINTERVAL(fps) (((fps) != 0) ? 10000000.0f / (fps) : 0)
1616
#define UVC_DESC_DWFRAMEINTERVAL_TO_FPS(dwFrameInterval) (((dwFrameInterval) != 0) ? 10000000.0f / ((float)(dwFrameInterval)) : 0)

host/class/uvc/usb_host_uvc/private_include/uvc_frame_priv.h

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,17 @@ static inline void uvc_frame_reset(uvc_host_frame_t *frame)
9090
*/
9191
void uvc_frame_format_update(uvc_stream_t *uvc_stream, const uvc_host_stream_format_t *vs_format);
9292

93+
/**
94+
* @brief Check if UVC payload header is valid
95+
*
96+
* @param[in] hdr Pointer to UVC payload header
97+
* @param[in] packet_len Length of packet in bytes
98+
* @return
99+
* - true: Header is valid
100+
* - false: Header is invalid
101+
*/
102+
bool uvc_frame_payload_header_validate(const uvc_payload_header_t *hdr, size_t packet_len);
103+
93104
#ifdef __cplusplus
94105
}
95106
#endif

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212

1313
#include "usb/usb_host.h"
1414
#include "usb/uvc_host.h"
15+
#include "usb_types_uvc.h"
1516

1617
#include "freertos/FreeRTOS.h"
1718
#include "freertos/queue.h"

host/class/uvc/usb_host_uvc/uvc_bulk.c

Lines changed: 59 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* SPDX-FileCopyrightText: 2024 Espressif Systems (Shanghai) CO LTD
2+
* SPDX-FileCopyrightText: 2024-2025 Espressif Systems (Shanghai) CO LTD
33
*
44
* SPDX-License-Identifier: Apache-2.0
55
*/
@@ -81,50 +81,36 @@ void bulk_transfer_callback(usb_transfer_t *transfer)
8181
// Consequently, it becomes impossible to distinguish between the last data packet and the EoF header.
8282
// To address this, the hack discards all frames whose last data packet has the MPS size.
8383
switch (uvc_stream->single_thread.next_bulk_packet) {
84-
case UVC_STREAM_BULK_PACKET_EOF: {
85-
const uvc_payload_header_t *payload_header = (const uvc_payload_header_t *)payload;
86-
uvc_stream->single_thread.next_bulk_packet = UVC_STREAM_BULK_PACKET_SOF;
87-
88-
if (payload_header->bmHeaderInfo.end_of_frame) {
89-
// Just skip the current frame if the frame_id does not match, we plan to include a better fix in #308
90-
// Due to the SOF of the next frame may not be handled, this may result in dropping two consecutive frames.
91-
if (payload_header->bmHeaderInfo.error || payload_header->bmHeaderInfo.frame_id != uvc_stream->single_thread.current_frame_id) {
92-
uvc_stream->single_thread.skip_current_frame = true;
93-
}
94-
95-
// Get the current frame being processed and clear it from the stream,
96-
// so no more data is written to this frame after the end of frame
97-
UVC_ENTER_CRITICAL(); // Enter critical section to safely check and modify the stream state.
98-
uvc_host_frame_t *this_frame = uvc_stream->dynamic.current_frame;
99-
uvc_stream->dynamic.current_frame = NULL;
100-
101-
// Determine if we should invoke the frame callback:
102-
// Only invoke the callback if streaming is active, a frame callback exists,
103-
// and we have a valid frame to pass to the user.
104-
const bool invoke_fb_callback = (uvc_stream->dynamic.streaming && uvc_stream->constant.frame_cb && this_frame && !uvc_stream->single_thread.skip_current_frame);
105-
UVC_EXIT_CRITICAL();
106-
107-
bool return_frame = true; // Default to returning the frame in case streaming has been stopped
108-
if (invoke_fb_callback) {
109-
// Call the user's frame callback. If the callback returns false,
110-
// we do not return the frame to the empty queue (i.e., the user wants to keep it for processing)
111-
return_frame = uvc_stream->constant.frame_cb(this_frame, uvc_stream->constant.cb_arg);
112-
}
113-
if (return_frame) {
114-
// If the user has processed the frame (or the stream is stopped), return it to the empty frame queue
115-
uvc_host_frame_return(uvc_stream, this_frame);
116-
}
117-
break;
118-
}
119-
120-
__attribute__((fallthrough)); // Fall through! This is not EoF but SoF!
121-
}
12284
case UVC_STREAM_BULK_PACKET_SOF: {
12385
const uvc_payload_header_t *payload_header = (const uvc_payload_header_t *)payload;
12486

12587
// We detected start of new frame. Update Frame ID and start fetching this frame
12688
uvc_stream->single_thread.current_frame_id = payload_header->bmHeaderInfo.frame_id;
12789
uvc_stream->single_thread.skip_current_frame = payload_header->bmHeaderInfo.error; // Check for error flag
90+
// Validate header before accessing it
91+
if (!uvc_frame_payload_header_validate(payload_header, transfer->actual_num_bytes)) {
92+
ESP_LOGD(TAG, "invalid UVC payload header, %02x, %02x, len:%d", payload[0], payload[1], transfer->actual_num_bytes);
93+
uvc_stream->single_thread.skip_current_frame = true;
94+
goto skip_sof;
95+
}
96+
payload_data += payload_header->bHeaderLength; // Pointer arithmetic!
97+
payload_data_len -= payload_header->bHeaderLength;
98+
99+
// Drop frame if device signals error in header
100+
if (payload_header->bmHeaderInfo.error) {
101+
ESP_LOGW(TAG, "frame error flag set");
102+
uvc_stream->single_thread.skip_current_frame = true;
103+
goto skip_sof;
104+
}
105+
106+
// Check mjpeg frame start
107+
if (uvc_stream->dynamic.vs_format.format == UVC_VS_FORMAT_MJPEG &&
108+
(payload_data[0] != 0xff || payload_data[1] != 0xd8)) {
109+
// We received frame with invalid frame, skip this frame
110+
uvc_stream->single_thread.skip_current_frame = true;
111+
ESP_LOGW(TAG, "invalid MJPEG SOI");
112+
goto skip_sof;
113+
}
128114

129115
// Get free frame buffer for this new frame
130116
UVC_ENTER_CRITICAL();
@@ -150,17 +136,11 @@ void bulk_transfer_callback(usb_transfer_t *transfer)
150136
uvc_frame_reset(uvc_stream->dynamic.current_frame);
151137
UVC_EXIT_CRITICAL();
152138
}
153-
154-
payload_data += payload_header->bHeaderLength; // Pointer arithmetic!
155-
payload_data_len -= payload_header->bHeaderLength;
139+
skip_sof:
156140
uvc_stream->single_thread.next_bulk_packet = UVC_STREAM_BULK_PACKET_DATA;
157141
__attribute__((fallthrough)); // Fall through! There can be data after SoF!
158142
}
159143
case UVC_STREAM_BULK_PACKET_DATA: {
160-
// We got short packet in data section, next packet is EoF
161-
if (transfer->data_buffer_size > transfer->actual_num_bytes) {
162-
uvc_stream->single_thread.next_bulk_packet = UVC_STREAM_BULK_PACKET_EOF;
163-
}
164144
// Add received data to frame buffer
165145
if (!uvc_stream->single_thread.skip_current_frame) {
166146
uvc_host_frame_t *current_frame = UVC_ATOMIC_LOAD(uvc_stream->dynamic.current_frame);
@@ -179,7 +159,40 @@ void bulk_transfer_callback(usb_transfer_t *transfer)
179159
}
180160
}
181161
}
162+
// We got short packet in data section, this packet is EoF
163+
if (transfer->data_buffer_size > transfer->actual_num_bytes) {
164+
uvc_stream->single_thread.next_bulk_packet = UVC_STREAM_BULK_PACKET_EOF;
165+
} else {
166+
break; //only break if we have got full packet
167+
}
168+
}
169+
case UVC_STREAM_BULK_PACKET_EOF: {
170+
uvc_stream->single_thread.next_bulk_packet = UVC_STREAM_BULK_PACKET_SOF;
171+
172+
// Get the current frame being processed and clear it from the stream,
173+
// so no more data is written to this frame after the end of frame
174+
UVC_ENTER_CRITICAL(); // Enter critical section to safely check and modify the stream state.
175+
uvc_host_frame_t *this_frame = uvc_stream->dynamic.current_frame;
176+
uvc_stream->dynamic.current_frame = NULL;
177+
178+
// Determine if we should invoke the frame callback:
179+
// Only invoke the callback if streaming is active, a frame callback exists,
180+
// and we have a valid frame to pass to the user.
181+
const bool invoke_fb_callback = (uvc_stream->dynamic.streaming && uvc_stream->constant.frame_cb && this_frame && !uvc_stream->single_thread.skip_current_frame);
182+
UVC_EXIT_CRITICAL();
183+
184+
bool return_frame = true; // Default to returning the frame in case streaming has been stopped
185+
if (invoke_fb_callback) {
186+
// Call the user's frame callback. If the callback returns false,
187+
// we do not return the frame to the empty queue (i.e., the user wants to keep it for processing)
188+
return_frame = uvc_stream->constant.frame_cb(this_frame, uvc_stream->constant.cb_arg);
189+
}
190+
if (return_frame) {
191+
// If the user has processed the frame (or the stream is stopped), return it to the empty frame queue
192+
uvc_host_frame_return(uvc_stream, this_frame);
193+
}
182194
break;
195+
183196
}
184197
default: abort();
185198
}

0 commit comments

Comments
 (0)