Bug 1301065 - Update rust mp4parse to v0.5.1. r=kinetik
authorRalph Giles <giles@mozilla.com>
Tue, 13 Sep 2016 09:16:44 -0700
changeset 355189 3a61bfe089b2b2cbef1b0cad0b664c973773a0fb
parent 355188 18f34131ce3064d10281630d9b185cd057f54c92
child 355190 24cefbae2be9111f84604e18fbacfd2548ac26ed
push id6570
push userraliiev@mozilla.com
push dateMon, 14 Nov 2016 12:26:13 +0000
treeherdermozilla-beta@f455459b2ae5 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskinetik
bugs1301065
milestone51.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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",
 ]