diff --git a/src/rust/src/decoder/mod.rs b/src/rust/src/decoder/mod.rs index ff1439722..9cbd90ee4 100644 --- a/src/rust/src/decoder/mod.rs +++ b/src/rust/src/decoder/mod.rs @@ -24,8 +24,10 @@ const CCX_DTVCC_MAX_PACKET_LENGTH: u8 = 128; const CCX_DTVCC_NO_LAST_SEQUENCE: i32 = -1; const CCX_DTVCC_SCREENGRID_ROWS: u8 = 75; const CCX_DTVCC_SCREENGRID_COLUMNS: u8 = 210; -const CCX_DTVCC_MAX_ROWS: u8 = 15; -const CCX_DTVCC_MAX_COLUMNS: u8 = 32 * 2; +pub(crate) const CCX_DTVCC_MAX_ROWS: u8 = 15; +pub(crate) const CCX_DTVCC_MAX_COLUMNS: u8 = 32 * 2; + +pub use window::{alloc_row, dealloc_row}; /// Context required for processing 708 data pub struct Dtvcc<'a> { diff --git a/src/rust/src/decoder/window.rs b/src/rust/src/decoder/window.rs index 8c290cd51..3c34be2fb 100644 --- a/src/rust/src/decoder/window.rs +++ b/src/rust/src/decoder/window.rs @@ -15,6 +15,29 @@ use std::{ ptr::copy_nonoverlapping, }; +/// Get the layout for a single caption row +fn row_layout() -> Layout { + Layout::array::(CCX_DTVCC_MAX_COLUMNS as usize).expect("row layout calculation") +} + +/// Allocate a new zero-initialized caption row +/// +/// # Safety +/// The returned pointer must be freed using `dealloc_row` +pub unsafe fn alloc_row() -> *mut dtvcc_symbol { + alloc_zeroed(row_layout()) as *mut dtvcc_symbol +} + +/// Deallocate a caption row +/// +/// # Safety +/// `ptr` must have been allocated by `alloc_row` or `alloc_zeroed` with `row_layout` +pub unsafe fn dealloc_row(ptr: *mut dtvcc_symbol) { + if !ptr.is_null() { + dealloc(ptr as *mut u8, row_layout()); + } +} + use super::timing::get_time_str; use super::{ CCX_DTVCC_MAX_COLUMNS, CCX_DTVCC_MAX_ROWS, CCX_DTVCC_SCREENGRID_COLUMNS, @@ -161,23 +184,15 @@ impl dtvcc_window { pub fn clear_row(&mut self, row_index: usize) { if is_true(self.memory_reserved) { unsafe { - let layout = Layout::array::(CCX_DTVCC_MAX_COLUMNS as usize); - if let Err(e) = layout { - error!("clear_row: Incorrect Layout, {e}"); - } else { - let layout = layout.unwrap(); - // deallocate previous memory - if !self.rows[row_index].is_null() { - dealloc(self.rows[row_index] as *mut u8, layout); - } - - // allocate new zero initialized memory - let ptr = alloc_zeroed(layout); - if ptr.is_null() { - error!("clear_row: Not enough memory",); - } - self.rows[row_index] = ptr as *mut dtvcc_symbol; + // deallocate previous memory + dealloc_row(self.rows[row_index]); + + // allocate new zero initialized memory + let ptr = alloc_row(); + if ptr.is_null() { + error!("clear_row: Not enough memory",); } + self.rows[row_index] = ptr; } for col in 0..CCX_DTVCC_MAX_COLUMNS as usize { // Set pen color and attributes to default value diff --git a/src/rust/src/demuxer/common_types.rs b/src/rust/src/demuxer/common_types.rs index 0f7bfe20c..d359db764 100644 --- a/src/rust/src/demuxer/common_types.rs +++ b/src/rust/src/demuxer/common_types.rs @@ -1,13 +1,9 @@ use crate::bindings::{lib_ccx_ctx, list_head}; +use crate::ffi_alloc; use lib_ccxr::common::{Codec, Decoder608Report, DecoderDtvccReport, StreamMode, StreamType}; use lib_ccxr::time::Timestamp; -use std::os::raw::c_void; use std::ptr::null_mut; -extern "C" { - fn free(ptr: *mut c_void); -} - // Size of the Startbytes Array in CcxDemuxer - const 1MB pub(crate) const ARRAY_SIZE: usize = 1024 * 1024; @@ -284,19 +280,15 @@ impl Drop for CcxDemuxer<'_> { // Free all non-null PSIBuffer pointers. // These are freed using C's free to be compatible with memory that might be allocated by C. for ptr in self.pid_buffers.drain(..) { - if !ptr.is_null() { - unsafe { - free(ptr as *mut c_void); - } + unsafe { + ffi_alloc::c_free(ptr); } } // Free all non-null PMTEntry pointers. // These are freed using C's free to be compatible with memory that might be allocated by C. for ptr in self.pids_programs.drain(..) { - if !ptr.is_null() { - unsafe { - free(ptr as *mut c_void); - } + unsafe { + ffi_alloc::c_free(ptr); } } } diff --git a/src/rust/src/demuxer/stream_functions.rs b/src/rust/src/demuxer/stream_functions.rs index ed1298de8..59422725e 100644 --- a/src/rust/src/demuxer/stream_functions.rs +++ b/src/rust/src/demuxer/stream_functions.rs @@ -1,5 +1,6 @@ use crate::bindings::ccx_demuxer; use crate::demuxer::common_types::{CcxDemuxer, CcxStreamMp4Box, STARTBYTESLENGTH}; +use crate::ffi_alloc; use crate::file_functions::file::{buffered_read_opt, return_to_buffer}; use crate::libccxr_exports::demuxer::{alloc_new_demuxer, copy_demuxer_from_rust_to_c}; use cfg_if::cfg_if; @@ -145,7 +146,7 @@ unsafe fn detect_stream_type_common(ctx: &mut CcxDemuxer, ccx_options: &mut Opti copy_demuxer_from_rust_to_c(demuxer, ctx); let private = ccx_gxf_init(demuxer); ctx.private_data = private as *mut core::ffi::c_void; - drop(Box::from_raw(demuxer)); + ffi_alloc::c_free(demuxer); } // WTV check @@ -269,7 +270,7 @@ unsafe fn detect_stream_type_common(ctx: &mut CcxDemuxer, ccx_options: &mut Opti let private = ccx_mxf_init(demuxer); ctx.private_data = private as *mut core::ffi::c_void; } - drop(Box::from_raw(demuxer)); + ffi_alloc::c_free(demuxer); } // Still not found diff --git a/src/rust/src/encoder/simplexml.rs b/src/rust/src/encoder/simplexml.rs index cb5c9856c..6b8afb07c 100644 --- a/src/rust/src/encoder/simplexml.rs +++ b/src/rust/src/encoder/simplexml.rs @@ -1,6 +1,7 @@ use crate::bindings::{cc_subtitle, ccx_encoding_type_CCX_ENC_ASCII, eia608_screen, encoder_ctx}; use crate::encoder::ccxr_get_str_basic; use crate::encoder::common::write_raw; +use crate::ffi_alloc; use lib_ccxr::common::CCX_DECODER_608_SCREEN_WIDTH; use lib_ccxr::info; use std::os::raw::{c_int, c_void}; @@ -79,8 +80,7 @@ pub fn write_cc_bitmap_as_simplexml(sub: &mut cc_subtitle, _context: &mut encode if !sub.data.is_null() { unsafe { - let data_box = Box::from_raw(sub.data as *mut u8); - drop(data_box); + ffi_alloc::c_free(sub.data); } sub.data = ptr::null_mut(); } diff --git a/src/rust/src/ffi_alloc.rs b/src/rust/src/ffi_alloc.rs new file mode 100644 index 000000000..9a1a0b54c --- /dev/null +++ b/src/rust/src/ffi_alloc.rs @@ -0,0 +1,18 @@ +use std::os::raw::c_void; + +extern "C" { + fn free(ptr: *mut c_void); +} +/// Frees memory allocated by C code (malloc/calloc) +/// +/// # Safety +/// +/// The pointer must have been allocated by C's malloc/calloc/realloc and +/// not yet freed. Calling this function with a pointer that was not +/// allocated by the C allocator is undefined behavior. +#[inline] +pub unsafe fn c_free(ptr: *mut T) { + if !ptr.is_null() { + free(ptr as *mut c_void); + } +} diff --git a/src/rust/src/lib.rs b/src/rust/src/lib.rs index f4b838cda..3801b11af 100644 --- a/src/rust/src/lib.rs +++ b/src/rust/src/lib.rs @@ -21,6 +21,7 @@ pub mod decoder; pub mod demuxer; pub mod encoder; pub mod es; +pub mod ffi_alloc; pub mod file_functions; #[cfg(feature = "hardsubx_ocr")] pub mod hardsubx; @@ -272,10 +273,8 @@ pub extern "C" fn ccxr_dtvcc_free(dtvcc_ptr: *mut std::ffi::c_void) { for window in decoder.windows.iter() { if is_true(window.memory_reserved) { for row_ptr in window.rows.iter() { - if !row_ptr.is_null() { - unsafe { - drop(Box::from_raw(*row_ptr)); - } + unsafe { + decoder::dealloc_row(*row_ptr); } } } diff --git a/src/rust/src/libccxr_exports/demuxer.rs b/src/rust/src/libccxr_exports/demuxer.rs index 6a875c7d5..5963553a6 100755 --- a/src/rust/src/libccxr_exports/demuxer.rs +++ b/src/rust/src/libccxr_exports/demuxer.rs @@ -18,13 +18,14 @@ const POISON_PTR_PATTERN: usize = 0xcdcdcdcdcdcdcdcd; const POISON_PTR_PATTERN: usize = 0xcdcdcdcd; extern "C" { - fn activity_input_file_closed(); - fn close(fd: c_int) -> c_int; - fn malloc(size: usize) -> *mut c_void; - fn free(ptr: *mut c_void); - fn calloc(nmemb: usize, size: usize) -> *mut c_void; + pub(crate) fn activity_input_file_closed(); + pub(crate) fn close(fd: c_int) -> c_int; + pub(crate) fn malloc(size: usize) -> *mut c_void; + pub(crate) fn calloc(nmemb: usize, size: usize) -> *mut c_void; } +use crate::ffi_alloc; + pub fn copy_c_array_to_rust_vec( c_bytes: &[u8; crate::demuxer::common_types::ARRAY_SIZE], ) -> Vec { @@ -110,7 +111,7 @@ pub unsafe fn copy_demuxer_from_rust_to_c(c_demuxer: *mut ccx_demuxer, rust_demu // We also check for POISON_PTR_PATTERN for safety in debug builds. if !c.PID_buffers[i].is_null() && c.PID_buffers[i] as usize != POISON_PTR_PATTERN { unsafe { - free(c.PID_buffers[i] as *mut c_void); + ffi_alloc::c_free(c.PID_buffers[i]); c.PID_buffers[i] = std::ptr::null_mut(); } } @@ -152,7 +153,7 @@ pub unsafe fn copy_demuxer_from_rust_to_c(c_demuxer: *mut ccx_demuxer, rust_demu // SAFETY: We use C's free to be compatible with memory that might be allocated by C. if !c.PIDs_programs[i].is_null() && c.PIDs_programs[i] as usize != POISON_PTR_PATTERN { unsafe { - free(c.PIDs_programs[i] as *mut c_void); + ffi_alloc::c_free(c.PIDs_programs[i]); c.PIDs_programs[i] = std::ptr::null_mut(); } } diff --git a/src/rust/src/parser.rs b/src/rust/src/parser.rs index 47573a996..2bca48bbd 100644 --- a/src/rust/src/parser.rs +++ b/src/rust/src/parser.rs @@ -1663,6 +1663,7 @@ pub mod tests { common::{MkvLangFilter, OutputFormat, SelectCodec, StreamMode, StreamType}, util::{encoding::Encoding, log::DebugMessageFlag}, }; + use serial_test::serial; /// # Safety /// @@ -2318,18 +2319,21 @@ pub mod tests { } #[test] + #[serial] fn test_buffersize_with_k_suffix() { let (_, _) = parse_args(&["--buffersize", "64K"]); assert_eq!(get_file_buffer_size(), 64 * 1024); } #[test] + #[serial] fn test_buffersize_with_m_suffix() { let (_, _) = parse_args(&["--buffersize", "2M"]); assert_eq!(get_file_buffer_size(), 2 * 1024 * 1024); } #[test] + #[serial] fn test_buffersize_with_numeric_value() { let (_, _) = parse_args(&["--buffersize", "8192"]); assert_eq!(get_file_buffer_size(), 8192);