Resolve issues in C FFI and RPU parsing

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<u64> for DoviMappingMethod` was replaced by `TryFrom`.
 
 ## 3.3.2
 - `rpu`: fix `write_rpu_data` allocated capacity. Now static and 512 bytes.

dolby_vision/src/c_structs/buffers.rs

@@ -267,8 +267,10 @@ impl<const N: usize> From<Option<[u16; N]>> 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);
+            }
         }
     }
 }

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,
+                ));
+            }
         }
     }
 }

dolby_vision/src/c_structs/rpu.rs

@@ -27,19 +27,23 @@ pub struct RpuOpaqueList {
 }
 
 impl RpuOpaque {
-    pub(crate) fn new(rpu: Option<DoviRpu>, error: Option<CString>) -> Self {
-        Self { rpu, error }
+    pub(crate) fn new(rpu: Option<DoviRpu>) -> Self {
+        Self { rpu, error: None }
+    }
+
+    pub(crate) fn invalid_with_error(msg: &str) -> Self {
+        Self {
+            rpu: None,
+            error: CString::new(msg).ok(),
+        }
     }
 }
 
 impl From<Result<DoviRpu, anyhow::Error>> for RpuOpaque {
     fn from(res: Result<DoviRpu, anyhow::Error>) -> 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}")),
         }
     }
 }

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()
             }
         }

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

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<u64> for DoviMappingMethod {
-    fn from(value: u64) -> Self {
+impl TryFrom<u64> for DoviMappingMethod {
+    type Error = anyhow::Error;
+
+    fn try_from(value: u64) -> Result<Self> {
         match value {
-            0 => Self::Polynomial,
-            1 => Self::MMR,
-            _ => unreachable!(),
+            0 => Ok(Self::Polynomial),
+            1 => Ok(Self::MMR),
+            _ => bail!("Invalid mapping_idc value: {value}"),
         }
     }
 }