diff --git a/dolby_vision/CHANGELOG.md b/dolby_vision/CHANGELOG.md index 0931fd9..59296fb 100644 --- a/dolby_vision/CHANGELOG.md +++ b/dolby_vision/CHANGELOG.md @@ -5,6 +5,7 @@ - `GenerateConfig`: list encoding helpers now return iterators instead of a `Vec`. - The iterators do not filter out errors anymore. `collect_encoded_rpus` was added for convenience to reproduce previous behaviour. +- `From for DoviMappingMethod` was replaced by `TryFrom`. ## 3.3.2 - `rpu`: fix `write_rpu_data` allocated capacity. Now static and 512 bytes. diff --git a/dolby_vision/src/c_structs/buffers.rs b/dolby_vision/src/c_structs/buffers.rs index bfb9d97..6be648c 100644 --- a/dolby_vision/src/c_structs/buffers.rs +++ b/dolby_vision/src/c_structs/buffers.rs @@ -267,8 +267,10 @@ impl From> for U16Data { impl Freeable for Data { unsafe fn free(&self) { - unsafe { - Vec::from_raw_parts(self.data as *mut u8, self.len, self.len); + if !self.data.is_null() { + unsafe { + Vec::from_raw_parts(self.data as *mut u8, self.len, self.len); + } } } } @@ -295,8 +297,10 @@ impl Freeable for U64Data { impl Freeable for I64Data { unsafe fn free(&self) { - unsafe { - Vec::from_raw_parts(self.data as *mut i64, self.len, self.len); + if !self.data.is_null() { + unsafe { + Vec::from_raw_parts(self.data as *mut i64, self.len, self.len); + } } } } diff --git a/dolby_vision/src/c_structs/extension_metadata.rs b/dolby_vision/src/c_structs/extension_metadata.rs index d067ae6..316247c 100644 --- a/dolby_vision/src/c_structs/extension_metadata.rs +++ b/dolby_vision/src/c_structs/extension_metadata.rs @@ -129,24 +129,43 @@ impl DmData { } /// # Safety + /// All non-null pointers must have been allocated by Box::into_raw. pub unsafe fn free(&self) { unsafe { - drop(Box::from_raw(self.level1 as *mut ExtMetadataBlockLevel1)); + if !self.level1.is_null() { + drop(Box::from_raw(self.level1 as *mut ExtMetadataBlockLevel1)); + } self.level2.free(); - drop(Box::from_raw(self.level3 as *mut ExtMetadataBlockLevel3)); - drop(Box::from_raw(self.level4 as *mut ExtMetadataBlockLevel4)); - drop(Box::from_raw(self.level5 as *mut ExtMetadataBlockLevel5)); - drop(Box::from_raw(self.level6 as *mut ExtMetadataBlockLevel6)); + if !self.level3.is_null() { + drop(Box::from_raw(self.level3 as *mut ExtMetadataBlockLevel3)); + } + if !self.level4.is_null() { + drop(Box::from_raw(self.level4 as *mut ExtMetadataBlockLevel4)); + } + if !self.level5.is_null() { + drop(Box::from_raw(self.level5 as *mut ExtMetadataBlockLevel5)); + } + if !self.level6.is_null() { + drop(Box::from_raw(self.level6 as *mut ExtMetadataBlockLevel6)); + } self.level8.free(); - drop(Box::from_raw(self.level9 as *mut ExtMetadataBlockLevel9)); + if !self.level9.is_null() { + drop(Box::from_raw(self.level9 as *mut ExtMetadataBlockLevel9)); + } self.level10.free(); - drop(Box::from_raw(self.level11 as *mut ExtMetadataBlockLevel11)); - drop(Box::from_raw( - self.level254 as *mut ExtMetadataBlockLevel254, - )); - drop(Box::from_raw( - self.level255 as *mut ExtMetadataBlockLevel255, - )); + if !self.level11.is_null() { + drop(Box::from_raw(self.level11 as *mut ExtMetadataBlockLevel11)); + } + if !self.level254.is_null() { + drop(Box::from_raw( + self.level254 as *mut ExtMetadataBlockLevel254, + )); + } + if !self.level255.is_null() { + drop(Box::from_raw( + self.level255 as *mut ExtMetadataBlockLevel255, + )); + } } } } diff --git a/dolby_vision/src/c_structs/rpu.rs b/dolby_vision/src/c_structs/rpu.rs index e611166..d5c90c6 100644 --- a/dolby_vision/src/c_structs/rpu.rs +++ b/dolby_vision/src/c_structs/rpu.rs @@ -27,19 +27,23 @@ pub struct RpuOpaqueList { } impl RpuOpaque { - pub(crate) fn new(rpu: Option, error: Option) -> Self { - Self { rpu, error } + pub(crate) fn new(rpu: Option) -> Self { + Self { rpu, error: None } + } + + pub(crate) fn invalid_with_error(msg: &str) -> Self { + Self { + rpu: None, + error: CString::new(msg).ok(), + } } } impl From> for RpuOpaque { fn from(res: Result) -> Self { match res { - Ok(rpu) => Self::new(Some(rpu), None), - Err(e) => Self::new( - None, - Some(CString::new(format!("Failed parsing RPU: {e}")).unwrap()), - ), + Ok(rpu) => Self::new(Some(rpu)), + Err(e) => Self::invalid_with_error(&format!("Failed parsing RPU: {e}")), } } } diff --git a/dolby_vision/src/capi.rs b/dolby_vision/src/capi.rs index f34f2c5..a4a82bd 100644 --- a/dolby_vision/src/capi.rs +++ b/dolby_vision/src/capi.rs @@ -19,7 +19,11 @@ use super::c_structs::*; /// Adds an error if the parsing fails. #[unsafe(no_mangle)] pub unsafe extern "C" fn dovi_parse_rpu(buf: *const u8, len: size_t) -> *mut RpuOpaque { - assert!(!buf.is_null()); + if buf.is_null() { + return Box::into_raw(Box::new(RpuOpaque::invalid_with_error( + "dovi_parse_rpu: null buffer pointer", + ))); + } let data = unsafe { slice::from_raw_parts(buf, len) }; let res = DoviRpu::parse_rpu(data); @@ -37,7 +41,11 @@ pub unsafe extern "C" fn dovi_parse_itu_t35_dovi_metadata_obu( buf: *const u8, len: size_t, ) -> *mut RpuOpaque { - assert!(!buf.is_null()); + if buf.is_null() { + return Box::into_raw(Box::new(RpuOpaque::invalid_with_error( + "dovi_parse_itu_t35_dovi_metadata_obu: null buffer pointer", + ))); + } let data = unsafe { slice::from_raw_parts(buf, len) }; let res = DoviRpu::parse_itu_t35_dovi_metadata_obu(data); @@ -52,7 +60,11 @@ pub unsafe extern "C" fn dovi_parse_itu_t35_dovi_metadata_obu( /// Adds an error if the parsing fails. #[unsafe(no_mangle)] pub unsafe extern "C" fn dovi_parse_unspec62_nalu(buf: *const u8, len: size_t) -> *mut RpuOpaque { - assert!(!buf.is_null()); + if buf.is_null() { + return Box::into_raw(Box::new(RpuOpaque::invalid_with_error( + "dovi_parse_unspec62_nalu: null buffer pointer", + ))); + } let data = unsafe { slice::from_raw_parts(buf, len) }; let res = DoviRpu::parse_unspec62_nalu(data); @@ -124,8 +136,7 @@ pub unsafe extern "C" fn dovi_write_rpu(ptr: *mut RpuOpaque) -> *const Data { match rpu.write_rpu() { Ok(buf) => Box::into_raw(Box::new(Data::from(buf))), Err(e) => { - opaque.error = - Some(CString::new(format!("Failed writing byte buffer: {e}")).unwrap()); + opaque.error = CString::new(format!("Failed writing byte buffer: {e}")).ok(); null_mut() } } @@ -151,8 +162,7 @@ pub unsafe extern "C" fn dovi_write_unspec62_nalu(ptr: *mut RpuOpaque) -> *const match rpu.write_hevc_unspec62_nalu() { Ok(buf) => Box::into_raw(Box::new(Data::from(buf))), Err(e) => { - opaque.error = - Some(CString::new(format!("Failed writing byte buffer: {e}")).unwrap()); + opaque.error = CString::new(format!("Failed writing byte buffer: {e}")).ok(); null_mut() } } @@ -191,7 +201,7 @@ pub unsafe extern "C" fn dovi_convert_rpu_with_mode(ptr: *mut RpuOpaque, mode: u Ok(_) => 0, Err(e) => { opaque.error = - Some(CString::new(format!("Failed converting with mode {mode}: {e}")).unwrap()); + CString::new(format!("Failed converting with mode {mode}: {e}")).ok(); -1 } } @@ -337,7 +347,7 @@ pub unsafe extern "C" fn dovi_parse_rpu_bin_file(path: *const c_char) -> *const let opaque_list: Vec<*mut RpuOpaque> = rpus .into_iter() - .map(|rpu| Box::into_raw(Box::new(RpuOpaque::new(Some(rpu), None)))) + .map(|rpu| Box::into_raw(Box::new(RpuOpaque::new(Some(rpu))))) .collect(); rpu_list.list = @@ -355,8 +365,8 @@ pub unsafe extern "C" fn dovi_parse_rpu_bin_file(path: *const c_char) -> *const Some("parse_rpu_bin_file: Failed parsing the input path as a string".to_string()); } - if let Some(err) = error { - rpu_list.error = CString::new(err).unwrap().into_raw(); + if let Some(err) = error.and_then(|err| CString::new(err).ok()) { + rpu_list.error = err.into_raw(); } return Box::into_raw(Box::new(rpu_list)); @@ -403,7 +413,7 @@ pub unsafe extern "C" fn dovi_rpu_set_active_area_offsets( Ok(_) => 0, Err(e) => { opaque.error = - Some(CString::new(format!("Failed editing active area offsets: {e}")).unwrap()); + CString::new(format!("Failed editing active area offsets: {e}")).ok(); -1 } } @@ -452,8 +462,7 @@ pub unsafe extern "C" fn dovi_write_av1_rpu_metadata_obu_t35_payload( match rpu.write_av1_rpu_metadata_obu_t35_payload() { Ok(buf) => Box::into_raw(Box::new(Data::from(buf))), Err(e) => { - opaque.error = - Some(CString::new(format!("Failed writing byte buffer: {e}")).unwrap()); + opaque.error = CString::new(format!("Failed writing byte buffer: {e}")).ok(); null_mut() } } @@ -481,8 +490,7 @@ pub unsafe extern "C" fn dovi_write_av1_rpu_metadata_obu_t35_complete( match rpu.write_av1_rpu_metadata_obu_t35_complete() { Ok(buf) => Box::into_raw(Box::new(Data::from(buf))), Err(e) => { - opaque.error = - Some(CString::new(format!("Failed writing byte buffer: {e}")).unwrap()); + opaque.error = CString::new(format!("Failed writing byte buffer: {e}")).ok(); null_mut() } } diff --git a/dolby_vision/src/rpu/dovi_rpu.rs b/dolby_vision/src/rpu/dovi_rpu.rs index 5e346e0..873b1f4 100644 --- a/dolby_vision/src/rpu/dovi_rpu.rs +++ b/dolby_vision/src/rpu/dovi_rpu.rs @@ -116,6 +116,13 @@ impl DoviRpu { // Ignore trailing bytes let rpu_end = data.len() - trailing_zeroes; + + // Minimum: 1 prefix byte + at least 1 byte payload + 4 CRC32 bytes + 1 final byte = 7 + ensure!( + rpu_end >= 7, + "RPU data too short: {rpu_end} bytes after trimming trailing zeroes" + ); + let last_byte = data[rpu_end - 1]; // Minus 4 bytes for the CRC32, 1 for the 0x80 ending byte diff --git a/dolby_vision/src/rpu/rpu_data_mapping.rs b/dolby_vision/src/rpu/rpu_data_mapping.rs index 34ca2fd..02b16fc 100644 --- a/dolby_vision/src/rpu/rpu_data_mapping.rs +++ b/dolby_vision/src/rpu/rpu_data_mapping.rs @@ -150,7 +150,7 @@ impl RpuDataMapping { let num_pieces = (curve.num_pivots_minus2 + 1) as usize; for _ in 0..num_pieces { - let mapping_idc = DoviMappingMethod::from(reader.read_ue()?); + let mapping_idc = DoviMappingMethod::try_from(reader.read_ue()?)?; curve.mapping_idc = mapping_idc; // MAPPING_POLYNOMIAL @@ -525,12 +525,14 @@ impl DoviMMRCurve { } } -impl From for DoviMappingMethod { - fn from(value: u64) -> Self { +impl TryFrom for DoviMappingMethod { + type Error = anyhow::Error; + + fn try_from(value: u64) -> Result { match value { - 0 => Self::Polynomial, - 1 => Self::MMR, - _ => unreachable!(), + 0 => Ok(Self::Polynomial), + 1 => Ok(Self::MMR), + _ => bail!("Invalid mapping_idc value: {value}"), } } }