From 3155a8d6a86deffc9e27a3e6721c123ee39adc7c Mon Sep 17 00:00:00 2001 From: Muntasir Mamun Date: Fri, 16 Oct 2026 22:10:20 +0600 Subject: [PATCH] [FIX] Widen avc cc_count to uint16_t/u16 and bound caption buffer (#2379) The 8-bit type of avc_ctx->cc_count (unsigned char in C, u8 in Rust) predates commit c1043ca7 (#2359). In #2359, MAX_CC_COUNT and HDCC_MAX_TRIPLETS_PER_SLOT were raised from 31 to 310 to allow H.264 access units with multiple SEI messages to buffer their caption triplets. Raising the buffer cap to 310 made the wrapped range reachable for valid frames whenever accumulated triplets exceed 255 (e.g. 240 + 20 = 260 triplets): - In C, ctx->cc_count wrapped modulo 256, overwriting the beginning of the buffer and corrupting cc_data. - In Rust, ctx.cc_count += new_cc_count as u8 panicked in debug mode due to overflow and wrapped in release mode. Furthermore, widening cc_count allows frames with 11+ full SEIs (>310 triplets) to exceed the 931-byte storage capacity of a store_hdcc() slot (HDCC_MAX_TRIPLETS_PER_SLOT * 3 + 1), causing a row buffer overrun and leading ccxr_process_cc_data to discard the frame. Fixes: - src/lib_ccx/avc_functions.h: widen avc_ctx.cc_count from unsigned char to uint16_t. - src/lib_ccx/avc_functions.c: bound cc_count accumulation to HDCC_MAX_TRIPLETS_PER_SLOT (310) in copy_ccdata_to_buffer() and write 0xFF terminator. - src/lib_ccx/sequencing.c: clamp cc_count to HDCC_MAX_TRIPLETS_PER_SLOT in store_hdcc(), copying cc_count * 3 bytes and writing 0xFF terminator to prevent row overrun. - src/rust/src/avc/common_types.rs: widen AvcContextRust.cc_count from u8 to u16. - src/rust/src/lib.rs: export MAX_CC_COUNT (310) to crate. - src/rust/src/avc/core.rs: bound accumulation to MAX_CC_COUNT in copy_ccdata_to_buffer(), write 0xFF terminator, and add unit tests covering >255 triplets and 310 capping. --- src/lib_ccx/avc_functions.c | 7 ++++- src/lib_ccx/avc_functions.h | 4 ++- src/lib_ccx/sequencing.c | 5 +++- src/rust/src/avc/common_types.rs | 2 +- src/rust/src/avc/core.rs | 51 ++++++++++++++++++++++++++++---- src/rust/src/lib.rs | 2 +- 6 files changed, 61 insertions(+), 10 deletions(-) diff --git a/src/lib_ccx/avc_functions.c b/src/lib_ccx/avc_functions.c index 4588587a8..1acea69fb 100644 --- a/src/lib_ccx/avc_functions.c +++ b/src/lib_ccx/avc_functions.c @@ -447,8 +447,13 @@ void copy_ccdata_to_buffer(struct avc_ctx *ctx, char *source, int new_cc_count) mprint("Warning: Probably loss of CC data, unsaved buffer being rewritten, trailing end might get lost\n"); ctx->ccblocks_in_avc_lost++; } - memcpy(ctx->cc_data + ctx->cc_count * 3, source, new_cc_count * 3 + 1); + if (ctx->cc_count >= HDCC_MAX_TRIPLETS_PER_SLOT) + return; + if (ctx->cc_count + new_cc_count > HDCC_MAX_TRIPLETS_PER_SLOT) + new_cc_count = HDCC_MAX_TRIPLETS_PER_SLOT - ctx->cc_count; + memcpy(ctx->cc_data + ctx->cc_count * 3, source, new_cc_count * 3); ctx->cc_count += new_cc_count; + ctx->cc_data[ctx->cc_count * 3] = 0xFF; ctx->cc_buffer_saved = CCX_FALSE; } diff --git a/src/lib_ccx/avc_functions.h b/src/lib_ccx/avc_functions.h index e46ceae6d..2e3ca5dc6 100644 --- a/src/lib_ccx/avc_functions.h +++ b/src/lib_ccx/avc_functions.h @@ -1,9 +1,11 @@ #ifndef AVC_FUNCTION_H #define AVC_FUNCTION_H +#include + struct avc_ctx { - unsigned char cc_count; + uint16_t cc_count; // buffer to hold cc data unsigned char *cc_data; int64_t cc_databufsize; diff --git a/src/lib_ccx/sequencing.c b/src/lib_ccx/sequencing.c index d064dbf1b..b3dde38e2 100644 --- a/src/lib_ccx/sequencing.c +++ b/src/lib_ccx/sequencing.c @@ -67,7 +67,10 @@ void store_hdcc(struct encoder_ctx *enc_ctx, struct lib_cc_decode *dec_ctx, unsi } dec_ctx->cc_fts[seq_index] = current_fts_now; // CFS: Maybe do even if there's no data? dec_ctx->cc_data_count[seq_index] = 0; - memcpy(dec_ctx->cc_data_pkts[seq_index] + dec_ctx->cc_data_count[seq_index] * 3, cc_data, cc_count * 3 + 1); + if (cc_count > HDCC_MAX_TRIPLETS_PER_SLOT) + cc_count = HDCC_MAX_TRIPLETS_PER_SLOT; + memcpy(dec_ctx->cc_data_pkts[seq_index] + dec_ctx->cc_data_count[seq_index] * 3, cc_data, cc_count * 3); + dec_ctx->cc_data_pkts[seq_index][cc_count * 3] = 0xFF; } dec_ctx->cc_data_count[seq_index] += cc_count; } diff --git a/src/rust/src/avc/common_types.rs b/src/rust/src/avc/common_types.rs index 4fcb4e5ea..755099227 100644 --- a/src/rust/src/avc/common_types.rs +++ b/src/rust/src/avc/common_types.rs @@ -4,7 +4,7 @@ pub const MAXBFRAMES: i32 = 50; #[derive(Debug, Clone)] pub struct AvcContextRust { - pub cc_count: u8, + pub cc_count: u16, pub cc_data: Vec, pub cc_databufsize: usize, pub cc_buffer_saved: bool, diff --git a/src/rust/src/avc/core.rs b/src/rust/src/avc/core.rs index e4d68ab2b..8286cf54a 100644 --- a/src/rust/src/avc/core.rs +++ b/src/rust/src/avc/core.rs @@ -5,7 +5,7 @@ use crate::bindings::{cc_subtitle, encoder_ctx, lib_cc_decode}; use crate::ctorust::FromCType; use crate::ffi_alloc::realloc; use crate::libccxr_exports::time::ccxr_set_fts; -use crate::{anchor_hdcc, current_fps, process_hdcc, store_hdcc, MPEG_CLOCK_FREQ}; +use crate::{anchor_hdcc, current_fps, process_hdcc, store_hdcc, MAX_CC_COUNT, MPEG_CLOCK_FREQ}; use lib_ccxr::common::AvcNalType; use lib_ccxr::util::log::DebugMessageFlag; use lib_ccxr::{debug, info}; @@ -436,13 +436,24 @@ pub fn copy_ccdata_to_buffer(ctx: &mut AvcContextRust, source: &[u8], new_cc_cou ctx.ccblocks_in_avc_lost += 1; } + if ctx.cc_count as usize >= MAX_CC_COUNT { + return; + } + + let to_copy = if (ctx.cc_count as usize) + new_cc_count > MAX_CC_COUNT { + MAX_CC_COUNT - ctx.cc_count as usize + } else { + new_cc_count + }; + // Copy the cc data let start_idx = ctx.cc_count as usize * 3; - let copy_len = new_cc_count * 3 + 1; + let copy_bytes = to_copy * 3; - if start_idx + copy_len <= ctx.cc_data.len() { - ctx.cc_data[start_idx..start_idx + copy_len].copy_from_slice(&source[..copy_len]); - ctx.cc_count += new_cc_count as u8; + if start_idx + copy_bytes < ctx.cc_data.len() { + ctx.cc_data[start_idx..start_idx + copy_bytes].copy_from_slice(&source[..copy_bytes]); + ctx.cc_count += to_copy as u16; + ctx.cc_data[ctx.cc_count as usize * 3] = 0xFF; ctx.cc_buffer_saved = false; } } @@ -796,4 +807,34 @@ mod tests { let buf = [0x00, 0x00, 0x02, 0x00, 0x00, 0x01, 0x65]; assert_eq!(find_nal_start_code(&buf), Some(5)); } + + #[test] + fn test_copy_ccdata_to_buffer_beyond_u8_max() { + let mut ctx = AvcContextRust::default(); + ctx.cc_data = vec![0u8; 1024]; + ctx.cc_databufsize = 1024; + ctx.cc_count = 250; + + let dummy_source = vec![0xFA; 64]; // 20 triplets = 60 + 1 = 61 bytes + copy_ccdata_to_buffer(&mut ctx, &dummy_source, 20); + + // 250 + 20 = 270 triplets. If it were u8, it would wrap to 14. + assert_eq!(ctx.cc_count, 270); + assert_eq!(ctx.cc_data[270 * 3], 0xFF); + } + + #[test] + fn test_copy_ccdata_to_buffer_caps_at_max_cc_count() { + let mut ctx = AvcContextRust::default(); + ctx.cc_data = vec![0u8; 1024]; + ctx.cc_databufsize = 1024; + ctx.cc_count = 300; + + let dummy_source = vec![0xFA; 64]; // 20 triplets + copy_ccdata_to_buffer(&mut ctx, &dummy_source, 20); + + // 300 + 20 = 320, but must be capped at MAX_CC_COUNT (310). + assert_eq!(ctx.cc_count, MAX_CC_COUNT as u16); + assert_eq!(ctx.cc_data[MAX_CC_COUNT * 3], 0xFF); + } } diff --git a/src/rust/src/lib.rs b/src/rust/src/lib.rs index 42f39ec1a..31ed44403 100644 --- a/src/rust/src/lib.rs +++ b/src/rust/src/lib.rs @@ -395,7 +395,7 @@ pub extern "C" fn ccxr_dtvcc_set_active(dtvcc_ptr: *mut std::ffi::c_void, active /// /// This is not the 5-bit cc_count wire field (max 31) of a single CEA-708 /// cc_data header: buffered frames (e.g. MP4) can carry more triplets than that. -const MAX_CC_COUNT: usize = 310; +pub(crate) const MAX_CC_COUNT: usize = 310; /// Process cc_data ///