Add more tests to verify FFI pointers are stable

mp4parse_capi/src/lib.rs

@@ -35,9 +35,11 @@
 // file, You can obtain one at https://mozilla.org/MPL/2.0/.
 
 use byteorder::WriteBytesExt;
+use fallible_collections::TryReserveError;
 use mp4parse::unstable::rational_scale;
 use std::convert::TryFrom;
 use std::convert::TryInto;
+use std::hash::Hash;
 
 use std::io::Read;
 
@@ -435,6 +437,24 @@ pub struct Mp4parseAvifParser {
     sample_table: TryHashMap<u32, TryVec<Indice>>,
 }
 
+trait CacheInsertExt<K, V> {
+    fn insert_cache_entry(&mut self, key: K, value: V) -> Result<(), TryReserveError>;
+}
+
+impl<K, V> CacheInsertExt<K, V> for TryHashMap<K, V>
+where
+    K: Eq + Hash,
+{
+    fn insert_cache_entry(&mut self, key: K, value: V) -> Result<(), TryReserveError> {
+        let replaced = self.insert(key, value)?;
+        debug_assert!(
+            replaced.is_none(),
+            "cache entries must never be replaced once published"
+        );
+        Ok(())
+    }
+}
+
 impl Mp4parseAvifParser {
     fn context(&self) -> &AvifContext {
         &self.context
@@ -854,7 +874,7 @@ fn get_track_audio_info(
                         return Err(Mp4parseStatus::Invalid);
                     }
                     Ok(_) => {
-                        opus_header.insert((track_index, desc_i), v)?;
+                        opus_header.insert_cache_entry((track_index, desc_i), v)?;
                         if let Some(v) = opus_header.get(&(track_index, desc_i)) {
                             if v.len() > u32::MAX as usize {
                                 return Err(Mp4parseStatus::Invalid);
@@ -913,7 +933,7 @@ fn get_track_audio_info(
 
     parser
         .audio_track_sample_descriptions
-        .insert(track_index, audio_sample_infos)?;
+        .insert_cache_entry(track_index, audio_sample_infos)?;
     match parser.audio_track_sample_descriptions.get(&track_index) {
         Some(sample_info) => {
             if sample_info.len() > u32::MAX as usize {
@@ -1105,7 +1125,7 @@ fn mp4parse_get_track_video_info_safe(
 
     parser
         .video_track_sample_descriptions
-        .insert(track_index, video_sample_infos)?;
+        .insert_cache_entry(track_index, video_sample_infos)?;
     match parser.video_track_sample_descriptions.get(&track_index) {
         Some(sample_info) => {
             if sample_info.len() > u32::MAX as usize {
@@ -1481,7 +1501,7 @@ fn get_indice_table(
 
     if let Some(v) = create_sample_table(track, offset_time) {
         indices.set_indices(&v);
-        sample_table_cache.insert(track_id, v)?;
+        sample_table_cache.insert_cache_entry(track_id, v)?;
         return Ok(());
     }
 

mp4parse_capi/tests/test_avis.rs

@@ -1,3 +1,4 @@
+use mp4parse::unstable::Indice;
 use mp4parse_capi::*;
 use num_traits::ToPrimitive;
 use std::io::Read;
@@ -11,19 +12,8 @@ extern "C" fn buf_read(buf: *mut u8, size: usize, userdata: *mut std::os::raw::c
     }
 }
 
-unsafe fn parse_file_and_get_info(path: &str) -> (*mut Mp4parseAvifParser, Mp4parseAvifInfo) {
-    let mut file = std::fs::File::open(path).expect("Unknown file");
-    let io = Mp4parseIo {
-        read: Some(buf_read),
-        userdata: &mut file as *mut _ as *mut std::os::raw::c_void,
-    };
-
-    let mut parser = std::ptr::null_mut();
-    let mut rv = mp4parse_avif_new(&io, ParseStrictness::Normal, &mut parser);
-    assert_eq!(rv, Mp4parseStatus::Ok);
-    assert!(!parser.is_null());
-
-    let mut info = Mp4parseAvifInfo {
+fn default_avif_info() -> Mp4parseAvifInfo {
+    Mp4parseAvifInfo {
         premultiplied_alpha: Default::default(),
         major_brand: Default::default(),
         unsupported_features_bitfield: Default::default(),
@@ -44,12 +34,44 @@ unsafe fn parse_file_and_get_info(path: &str) -> (*mut Mp4parseAvifParser, Mp4pa
         color_track_bit_depth: Default::default(),
         alpha_track_id: Default::default(),
         alpha_track_bit_depth: Default::default(),
+    }
+}
+
+fn default_avif_image() -> Mp4parseAvifImage {
+    Mp4parseAvifImage {
+        primary_image: Default::default(),
+        alpha_image: Default::default(),
+    }
+}
+
+unsafe fn parse_file(path: &str) -> *mut Mp4parseAvifParser {
+    let mut file = std::fs::File::open(path).expect("Unknown file");
+    let io = Mp4parseIo {
+        read: Some(buf_read),
+        userdata: &mut file as *mut _ as *mut std::os::raw::c_void,
     };
-    rv = mp4parse_avif_get_info(parser, &mut info);
+
+    let mut parser = std::ptr::null_mut();
+    let rv = mp4parse_avif_new(&io, ParseStrictness::Normal, &mut parser);
+    assert_eq!(rv, Mp4parseStatus::Ok);
+    assert!(!parser.is_null());
+
+    parser
+}
+
+unsafe fn parse_file_and_get_info(path: &str) -> (*mut Mp4parseAvifParser, Mp4parseAvifInfo) {
+    let parser = parse_file(path);
+    let mut info = default_avif_info();
+    let rv = mp4parse_avif_get_info(parser, &mut info);
     assert_eq!(rv, Mp4parseStatus::Ok);
     (parser, info)
 }
 
+unsafe fn assert_slice_pointer_is_readable<T>(ptr: *const T, len: usize) {
+    let slice = std::slice::from_raw_parts(ptr, len);
+    assert_eq!(slice.len(), len);
+}
+
 fn check_loop_count(path: &str, expected_loop_count: i64) {
     let (parser, info) = unsafe { parse_file_and_get_info(path) };
     match info.loop_mode {
@@ -111,3 +133,143 @@ fn check_timescales() {
     check_timescale("tests/loop_forever.avif", 2);
     check_timescale("tests/no_edts.avif", 16384);
 }
+
+#[test]
+fn repeated_get_info_returns_stable_pointers() {
+    let (parser, info1) = unsafe { parse_file_and_get_info("tests/loop_1.avif") };
+
+    unsafe {
+        let mut info2 = default_avif_info();
+        let rv = mp4parse_avif_get_info(parser, &mut info2);
+        assert_eq!(rv, Mp4parseStatus::Ok);
+
+        assert_eq!(info1.spatial_extents, info2.spatial_extents);
+        assert!(!info1.spatial_extents.is_null());
+        assert_slice_pointer_is_readable(info1.spatial_extents, 1);
+
+        assert_eq!(info1.nclx_colour_information, info2.nclx_colour_information);
+        if !info1.nclx_colour_information.is_null() {
+            assert_slice_pointer_is_readable(info1.nclx_colour_information, 1);
+        }
+
+        assert_eq!(
+            info1.icc_colour_information.length,
+            info2.icc_colour_information.length
+        );
+        assert_eq!(
+            info1.icc_colour_information.data,
+            info2.icc_colour_information.data
+        );
+        if info1.icc_colour_information.length == 0 {
+            assert!(info1.icc_colour_information.data.is_null());
+        } else {
+            assert_slice_pointer_is_readable(
+                info1.icc_colour_information.data,
+                info1.icc_colour_information.length,
+            );
+        }
+
+        assert_eq!(info1.image_mirror, info2.image_mirror);
+        if !info1.image_mirror.is_null() {
+            assert_slice_pointer_is_readable(info1.image_mirror, 1);
+        }
+
+        assert_eq!(info1.pixel_aspect_ratio, info2.pixel_aspect_ratio);
+        if !info1.pixel_aspect_ratio.is_null() {
+            assert_slice_pointer_is_readable(info1.pixel_aspect_ratio, 1);
+        }
+
+        mp4parse_avif_free(parser);
+    }
+}
+
+#[test]
+fn repeated_get_image_returns_stable_pointers() {
+    let parser = unsafe { parse_file("tests/loop_1.avif") };
+
+    unsafe {
+        let mut image1 = default_avif_image();
+        let rv = mp4parse_avif_get_image(parser, &mut image1);
+        assert_eq!(rv, Mp4parseStatus::Ok);
+        assert!(image1.primary_image.length > 0);
+        assert!(!image1.primary_image.data.is_null());
+        assert!(image1.alpha_image.length > 0);
+        assert!(!image1.alpha_image.data.is_null());
+
+        let mut image2 = default_avif_image();
+        let rv = mp4parse_avif_get_image(parser, &mut image2);
+        assert_eq!(rv, Mp4parseStatus::Ok);
+
+        assert_eq!(image1.primary_image.length, image2.primary_image.length);
+        assert_eq!(image1.primary_image.data, image2.primary_image.data);
+        assert_eq!(image1.alpha_image.length, image2.alpha_image.length);
+        assert_eq!(image1.alpha_image.data, image2.alpha_image.data);
+
+        assert_slice_pointer_is_readable(image1.primary_image.data, image1.primary_image.length);
+        assert_slice_pointer_is_readable(image1.alpha_image.data, image1.alpha_image.length);
+
+        mp4parse_avif_free(parser);
+    }
+}
+
+#[test]
+fn repeated_get_indice_table_returns_stable_pointer() {
+    let (parser, info) = unsafe { parse_file_and_get_info("tests/loop_1.avif") };
+
+    unsafe {
+        let mut indices1 = Mp4parseByteData::default();
+        let mut timescale1: u64 = 0;
+        let rv = mp4parse_avif_get_indice_table(
+            parser,
+            info.color_track_id,
+            &mut indices1,
+            &mut timescale1,
+        );
+        assert_eq!(rv, Mp4parseStatus::Ok);
+        assert!(indices1.length > 0);
+        assert!(!indices1.indices.is_null());
+
+        let mut indices2 = Mp4parseByteData::default();
+        let mut timescale2: u64 = 0;
+        let rv = mp4parse_avif_get_indice_table(
+            parser,
+            info.color_track_id,
+            &mut indices2,
+            &mut timescale2,
+        );
+        assert_eq!(rv, Mp4parseStatus::Ok);
+
+        assert_eq!(timescale1, timescale2);
+        assert_eq!(indices1.length, indices2.length);
+        assert_eq!(indices1.indices, indices2.indices);
+        assert_slice_pointer_is_readable(indices1.indices, indices1.length);
+
+        let first1: &[Indice] = std::slice::from_raw_parts(indices1.indices, indices1.length);
+        let first2: &[Indice] = std::slice::from_raw_parts(indices2.indices, indices2.length);
+        assert_eq!(first1[0], first2[0]);
+
+        mp4parse_avif_free(parser);
+    }
+}
+
+#[test]
+fn empty_avif_byte_slices_use_null_pointers() {
+    let (parser, info) = unsafe { parse_file_and_get_info("tests/no_edts.avif") };
+
+    unsafe {
+        let mut image = default_avif_image();
+        let rv = mp4parse_avif_get_image(parser, &mut image);
+        assert_eq!(rv, Mp4parseStatus::Ok);
+
+        assert!(image.primary_image.length > 0);
+        assert!(!image.primary_image.data.is_null());
+        assert_eq!(image.alpha_image.length, 0);
+        assert!(image.alpha_image.data.is_null());
+
+        if info.icc_colour_information.length == 0 {
+            assert!(info.icc_colour_information.data.is_null());
+        }
+
+        mp4parse_avif_free(parser);
+    }
+}

mp4parse_capi/tests/test_encryption.rs

@@ -70,6 +70,7 @@ fn parse_cenc() {
         assert_eq!(protected_data.crypt_byte_block, 0);
         assert_eq!(protected_data.skip_byte_block, 0);
         assert_eq!(protected_data.constant_iv.length, 0);
+        assert!(protected_data.constant_iv.data.is_null());
 
         // Verify audio track and crypto information
         let mut audio = Mp4parseTrackAudioInfo::default();
@@ -98,6 +99,7 @@ fn parse_cenc() {
         assert_eq!(protected_data.crypt_byte_block, 0);
         assert_eq!(protected_data.skip_byte_block, 0);
         assert_eq!(protected_data.constant_iv.length, 0);
+        assert!(protected_data.constant_iv.data.is_null());
     }
 }
 
@@ -246,9 +248,11 @@ fn parse_unencrypted() {
         assert_eq!(protected_data.is_encrypted, 0x00);
         assert_eq!(protected_data.iv_size, 0);
         assert_eq!(protected_data.kid.length, 0);
+        assert!(protected_data.kid.data.is_null());
         assert_eq!(protected_data.crypt_byte_block, 0);
         assert_eq!(protected_data.skip_byte_block, 0);
         assert_eq!(protected_data.constant_iv.length, 0);
+        assert!(protected_data.constant_iv.data.is_null());
     }
 }
 

mp4parse_capi/tests/test_fragment.rs

@@ -103,6 +103,7 @@ fn parse_opus_fragment() {
         assert_eq!((*audio.sample_info).bit_depth, 16);
         assert_eq!((*audio.sample_info).sample_rate, 48000);
         assert_eq!((*audio.sample_info).extra_data.length, 0);
+        assert!((*audio.sample_info).extra_data.data.is_null());
         assert_eq!((*audio.sample_info).codec_specific_config.length, 19);
 
         let mut is_fragmented_file: u8 = 0;