Bug 1301065 - Update rust mp4parse to v0.5.1. r?kinetik draft
authorRalph Giles <giles@mozilla.com>
Tue, 13 Sep 2016 09:16:44 -0700
changeset 413124 f654116501867bc567d23be3ee38624b12edd851
parent 413105 b89a2196fcc75cd0d6464670ad675bba5a52f83d
child 531150 8215616a0ac85e689b973c8242a71f108f42ce7e
push id29349
push userbmo:giles@thaumas.net
push dateTue, 13 Sep 2016 17:48:19 +0000
reviewerskinetik
bugs1301065
milestone51.0a1
Bug 1301065 - Update rust mp4parse to v0.5.1. r?kinetik Result of running the update script, followed by `cargo update` in tookit/library/rust/. MozReview-Commit-ID: LNdvuOqVx9a
media/libstagefright/binding/mp4parse/Cargo.toml
media/libstagefright/binding/mp4parse/src/lib.rs
media/libstagefright/binding/mp4parse_capi/Cargo.toml
media/libstagefright/binding/mp4parse_capi/src/lib.rs
toolkit/library/rust/Cargo.lock
--- a/media/libstagefright/binding/mp4parse/Cargo.toml
+++ b/media/libstagefright/binding/mp4parse/Cargo.toml
@@ -1,11 +1,11 @@
 [package]
 name = "mp4parse"
-version = "0.5.0"
+version = "0.5.1"
 authors = [
   "Ralph Giles <giles@mozilla.com>",
   "Matthew Gregan <kinetik@flim.org>",
 ]
 
 description = "Parser for ISO base media file format (mp4)"
 documentation = "https://mp4parse-docs.surge.sh/mp4parse/"
 license = "MPL-2.0"
--- a/media/libstagefright/binding/mp4parse/src/lib.rs
+++ b/media/libstagefright/binding/mp4parse/src/lib.rs
@@ -286,29 +286,31 @@ pub enum TrackType {
     Video,
     Unknown,
 }
 
 impl Default for TrackType {
     fn default() -> Self { TrackType::Unknown }
 }
 
-/// The media's global (mvhd) timescale.
+/// The media's global (mvhd) timescale in units per second.
 #[derive(Debug, Copy, Clone, PartialEq)]
 pub struct MediaTimeScale(pub u64);
 
-/// A time scaled by the media's global (mvhd) timescale.
+/// A time to be scaled by the media's global (mvhd) timescale.
 #[derive(Debug, Copy, Clone, PartialEq)]
 pub struct MediaScaledTime(pub u64);
 
 /// The track's local (mdhd) timescale.
+/// Members are timescale units per second and the track id.
 #[derive(Debug, Copy, Clone, PartialEq)]
 pub struct TrackTimeScale(pub u64, pub usize);
 
-/// A time scaled by the track's local (mdhd) timescale.
+/// A time to be scaled by the track's local (mdhd) timescale.
+/// Members are time in scale units and the track id.
 #[derive(Debug, Copy, Clone, PartialEq)]
 pub struct TrackScaledTime(pub u64, pub usize);
 
 /// A fragmented file contains no sample data in stts, stsc, and stco.
 #[derive(Debug, Default)]
 pub struct EmptySampleTableBoxes {
     pub empty_stts : bool,
     pub empty_stsc : bool,
--- a/media/libstagefright/binding/mp4parse_capi/Cargo.toml
+++ b/media/libstagefright/binding/mp4parse_capi/Cargo.toml
@@ -1,11 +1,11 @@
 [package]
 name = "mp4parse_capi"
-version = "0.5.0"
+version = "0.5.1"
 authors = [
   "Ralph Giles <giles@mozilla.com>",
   "Matthew Gregan <kinetik@flim.org>",
 ]
 
 description = "Parser for ISO base media file format (mp4)"
 documentation = "https://mp4parse-docs.surge.sh/mp4parse/"
 license = "MPL-2.0"
@@ -13,16 +13,16 @@ license = "MPL-2.0"
 repository = "https://github.com/mozilla/mp4parse-rust"
 
 # Avoid complaints about trying to package test files.
 exclude = [
   "*.mp4",
 ]
 
 [dependencies]
-"mp4parse" = {version = "0.5.0", path = "../mp4parse"}
+"mp4parse" = {version = "0.5.1", path = "../mp4parse"}
 
 [features]
 fuzz = ["mp4parse/fuzz"]
 
 # Somewhat heavy-handed, but we want at least -Z force-overflow-checks=on.
 [profile.release]
 debug-assertions = true
--- a/media/libstagefright/binding/mp4parse_capi/src/lib.rs
+++ b/media/libstagefright/binding/mp4parse_capi/src/lib.rs
@@ -273,25 +273,46 @@ pub unsafe extern fn mp4parse_get_track_
     // Make sure the track count fits in a u32.
     if context.tracks.len() > u32::max_value() as usize {
         return MP4PARSE_ERROR_INVALID;
     }
     *count = context.tracks.len() as u32;
     MP4PARSE_OK
 }
 
-fn media_time_to_ms(time: MediaScaledTime, scale: MediaTimeScale) -> u64 {
-    assert!(scale.0 != 0);
-    time.0 * 1000000 / scale.0
+/// Calculate numerator * scale / denominator, if possible.
+///
+/// Applying the associativity of integer arithmetic, we divide first
+/// and add the remainder after multiplying each term separately
+/// to preserve precision while leaving more headroom. That is,
+/// (n * s) / d is split into floor(n / d) * s + (n % d) * s / d.
+///
+/// Return None on overflow or if the denominator is zero.
+fn rational_scale(numerator: u64, denominator: u64, scale: u64) -> Option<u64> {
+    if denominator == 0 {
+        return None;
+    }
+    let integer = numerator / denominator;
+    let remainder = numerator % denominator;
+    match integer.checked_mul(scale) {
+        Some(integer) => remainder.checked_mul(scale)
+            .and_then(|remainder| (remainder/denominator).checked_add(integer)),
+        None => None,
+    }
 }
 
-fn track_time_to_ms(time: TrackScaledTime, scale: TrackTimeScale) -> u64 {
+fn media_time_to_us(time: MediaScaledTime, scale: MediaTimeScale) -> Option<u64> {
+    let microseconds_per_second = 1000000;
+    rational_scale(time.0, scale.0, microseconds_per_second)
+}
+
+fn track_time_to_us(time: TrackScaledTime, scale: TrackTimeScale) -> Option<u64> {
     assert!(time.1 == scale.1);
-    assert!(scale.0 != 0);
-    time.0 * 1000000 / scale.0
+    let microseconds_per_second = 1000000;
+    rational_scale(time.0, scale.0, microseconds_per_second)
 }
 
 /// Fill the supplied `mp4parse_track_info` with metadata for `track`.
 #[no_mangle]
 pub unsafe extern fn mp4parse_get_track_info(parser: *mut mp4parse_parser, track_index: u32, info: *mut mp4parse_track_info) -> mp4parse_error {
     if parser.is_null() || info.is_null() || (*parser).poisoned() {
         return MP4PARSE_ERROR_BADARG;
     }
@@ -328,23 +349,34 @@ pub unsafe extern fn mp4parse_get_track_
 
     let track = &context.tracks[track_index];
 
     if let (Some(track_timescale),
             Some(context_timescale),
             Some(track_duration)) = (track.timescale,
                                      context.timescale,
                                      track.duration) {
-        info.media_time = track.media_time.map_or(0, |media_time| {
-            track_time_to_ms(media_time, track_timescale) as i64
-        }) - track.empty_duration.map_or(0, |empty_duration| {
-            media_time_to_ms(empty_duration, context_timescale) as i64
-        });
+        let media_time =
+            match track.media_time.map_or(Some(0), |media_time| {
+                    track_time_to_us(media_time, track_timescale) }) {
+                Some(time) => time as i64,
+                None => return MP4PARSE_ERROR_INVALID,
+            };
+        let empty_duration =
+            match track.empty_duration.map_or(Some(0), |empty_duration| {
+                    media_time_to_us(empty_duration, context_timescale) }) {
+                Some(time) => time as i64,
+                None => return MP4PARSE_ERROR_INVALID,
+            };
+        info.media_time = media_time - empty_duration;
 
-        info.duration = track_time_to_ms(track_duration, track_timescale);
+        match track_time_to_us(track_duration, track_timescale) {
+            Some(duration) => info.duration = duration,
+            None => return MP4PARSE_ERROR_INVALID,
+        }
     } else {
         return MP4PARSE_ERROR_INVALID
     }
 
     info.track_id = match track.track_id {
         Some(track_id) => track_id,
         None => return MP4PARSE_ERROR_INVALID,
     };
@@ -742,8 +774,34 @@ fn arg_validation_with_data() {
         assert_eq!(MP4PARSE_ERROR_BADARG, mp4parse_get_track_audio_info(parser, 3, &mut audio));
         assert_eq!(audio.channels, 0);
         assert_eq!(audio.bit_depth, 0);
         assert_eq!(audio.sample_rate, 0);
 
         mp4parse_free(parser);
     }
 }
+
+#[test]
+fn rational_scale_overflow() {
+    assert_eq!(rational_scale(17, 3, 1000), Some(5666));
+    let large = 0x4000_0000_0000_0000;
+    assert_eq!(rational_scale(large, 2, 2), Some(large));
+    assert_eq!(rational_scale(large, 4, 4), Some(large));
+    assert_eq!(rational_scale(large, 2, 8), None);
+    assert_eq!(rational_scale(large, 8, 4), Some(large/2));
+    assert_eq!(rational_scale(large + 1, 4, 4), Some(large+1));
+    assert_eq!(rational_scale(large, 40, 1000), None);
+}
+
+#[test]
+fn media_time_overflow() {
+  let scale = MediaTimeScale(90000);
+  let duration = MediaScaledTime(9007199254710000);
+  assert_eq!(media_time_to_us(duration, scale), Some(100079991719000000));
+}
+
+#[test]
+fn track_time_overflow() {
+  let scale = TrackTimeScale(44100, 0);
+  let duration = TrackScaledTime(4413527634807900, 0);
+  assert_eq!(track_time_to_us(duration, scale), Some(100079991719000000));
+}
--- a/toolkit/library/rust/Cargo.lock
+++ b/toolkit/library/rust/Cargo.lock
@@ -1,25 +1,25 @@
 [root]
 name = "gkrust"
 version = "0.1.0"
 dependencies = [
- "mp4parse_capi 0.5.0",
+ "mp4parse_capi 0.5.1",
 ]
 
 [[package]]
 name = "byteorder"
 version = "0.5.3"
 
 [[package]]
 name = "mp4parse"
-version = "0.5.0"
+version = "0.5.1"
 dependencies = [
  "byteorder 0.5.3",
 ]
 
 [[package]]
 name = "mp4parse_capi"
-version = "0.5.0"
+version = "0.5.1"
 dependencies = [
- "mp4parse 0.5.0",
+ "mp4parse 0.5.1",
 ]