Bug 1401071 - move fallible memory allocation github instead of including source and other nits. r=kinetik
authorAlfredo.Yang <ayang@mozilla.com>
Mon, 25 Sep 2017 09:14:00 +0800
changeset 670579 edae6c684c0e29c8ab2dda551fa8af7a5bdaa304
parent 670578 c17cf6e4ee95f168cf7006b6f739e9be673da243
child 670580 8acabce73863e53c985427c0f73861dade3a8189
push id81678
push userkgupta@mozilla.com
push dateTue, 26 Sep 2017 17:28:53 +0000
reviewerskinetik
bugs1401071
milestone58.0a1
Bug 1401071 - move fallible memory allocation github instead of including source and other nits. r=kinetik MozReview-Commit-ID: 40Yg8wUqZ2g
media/libstagefright/binding/MP4Metadata.cpp
media/libstagefright/binding/include/mp4parse.h
media/libstagefright/binding/mp4parse-cargo.patch
media/libstagefright/binding/mp4parse/Cargo.toml
media/libstagefright/binding/mp4parse/src/lib.rs
media/libstagefright/binding/mp4parse/src/tests.rs
media/libstagefright/binding/mp4parse_capi/Cargo.toml
media/libstagefright/binding/mp4parse_capi/src/lib.rs
media/libstagefright/binding/mp4parse_fallible/README
media/libstagefright/binding/update-rust.sh
--- a/media/libstagefright/binding/MP4Metadata.cpp
+++ b/media/libstagefright/binding/MP4Metadata.cpp
@@ -758,23 +758,21 @@ MP4MetadataRust::Init()
   mp4parse_io io = { read_source, &mRustSource };
   mRustParser.reset(mp4parse_new(&io));
   MOZ_ASSERT(mRustParser);
 
   if (MOZ_LOG_TEST(sLog, LogLevel::Debug)) {
     mp4parse_log(true);
   }
 
-  mp4parse_fallible_allocation(true);
-
   mp4parse_status rv = mp4parse_read(mRustParser.get());
   MOZ_LOG(sLog, LogLevel::Debug, ("rust parser returned %d\n", rv));
   Telemetry::Accumulate(Telemetry::MEDIA_RUST_MP4PARSE_SUCCESS,
                         rv == mp4parse_status_OK);
-  if (rv != mp4parse_status_OK && rv != mp4parse_status_TABLE_TOO_LARGE) {
+  if (rv != mp4parse_status_OK && rv != mp4parse_status_OOM) {
     MOZ_LOG(sLog, LogLevel::Info, ("Rust mp4 parser fails to parse this stream."));
     MOZ_ASSERT(rv > 0);
     Telemetry::Accumulate(Telemetry::MEDIA_RUST_MP4PARSE_ERROR_CODE, rv);
     return false;
   }
 
   UpdateCrypto();
 
--- a/media/libstagefright/binding/include/mp4parse.h
+++ b/media/libstagefright/binding/include/mp4parse.h
@@ -18,18 +18,17 @@ extern "C" {
 
 typedef enum mp4parse_status {
 	mp4parse_status_OK = 0,
 	mp4parse_status_BAD_ARG = 1,
 	mp4parse_status_INVALID = 2,
 	mp4parse_status_UNSUPPORTED = 3,
 	mp4parse_status_EOF = 4,
 	mp4parse_status_IO = 5,
-	mp4parse_status_TABLE_TOO_LARGE = 6,
-	mp4parse_status_OOM = 7,
+	mp4parse_status_OOM = 6,
 } mp4parse_status;
 
 typedef enum mp4parse_track_type {
 	mp4parse_track_type_VIDEO = 0,
 	mp4parse_track_type_AUDIO = 1,
 } mp4parse_track_type;
 
 typedef enum mp4parse_codec {
@@ -114,18 +113,16 @@ typedef struct mp4parse_io {
 mp4parse_parser* mp4parse_new(mp4parse_io const* io);
 
 /// Free an `mp4parse_parser*` allocated by `mp4parse_new()`.
 void mp4parse_free(mp4parse_parser* parser);
 
 /// Enable `mp4_parser` log.
 void mp4parse_log(bool enable);
 
-void mp4parse_fallible_allocation(bool enable);
-
 /// Run the `mp4parse_parser*` allocated by `mp4parse_new()` until EOF or error.
 mp4parse_status mp4parse_read(mp4parse_parser* parser);
 
 /// Return the number of tracks parsed by previous `mp4parse_read()` call.
 mp4parse_status mp4parse_get_track_count(mp4parse_parser const* parser, uint32_t* count);
 
 /// Fill the supplied `mp4parse_track_info` with metadata for `track`.
 mp4parse_status mp4parse_get_track_info(mp4parse_parser* parser, uint32_t track_index, mp4parse_track_info* info);
--- a/media/libstagefright/binding/mp4parse-cargo.patch
+++ b/media/libstagefright/binding/mp4parse-cargo.patch
@@ -1,57 +1,57 @@
 diff --git a/media/libstagefright/binding/mp4parse/Cargo.toml b/media/libstagefright/binding/mp4parse/Cargo.toml
 index ff9422c..814c4c6 100644
 --- a/media/libstagefright/binding/mp4parse/Cargo.toml
 +++ b/media/libstagefright/binding/mp4parse/Cargo.toml
-@@ -20,20 +20,12 @@ exclude = [
+@@ -20,19 +20,12 @@ exclude = [
  ]
  
 -[badges]
 -travis-ci = { repository = "https://github.com/mozilla/mp4parse-rust" }
  
  [dependencies]
--byteorder = "1.0.0"
+ byteorder = "1.0.0"
 -afl = { version = "0.1.1", optional = true }
 -afl-plugin = { version = "0.1.1", optional = true }
 -abort_on_panic = { version = "1.0.0", optional = true }
--bitreader = { version = "0.3.0" }
--num-traits = "0.1.37"
--mp4parse_fallible = { path = "../mp4parse_fallible" }
-+byteorder = "1.0.0"
-+bitreader = { version = "0.3.0" }
-+num-traits = "0.1.37"
-+mp4parse_fallible = { path = "../mp4parse_fallible" }
+ bitreader = { version = "0.3.0" }
+ num-traits = "0.1.37"
+-mp4parse_fallible = { git = "https://github.com/alfredoyang/mp4parse_fallible", optional = true }
++mp4parse_fallible = { path = "../mp4parse_fallible", optional = true }
  
  [dev-dependencies]
  test-assembler = "0.1.2"
  
 -[features]
 -fuzz = ["afl", "afl-plugin", "abort_on_panic"]
--
  # Somewhat heavy-handed, but we want at least -Z force-overflow-checks=on.
  [profile.release]
  debug-assertions = true
 diff --git a/media/libstagefright/binding/mp4parse_capi/Cargo.toml b/media/libstagefright/binding/mp4parse_capi/Cargo.toml
 index a30e045..a965f06 100644
 --- a/media/libstagefright/binding/mp4parse_capi/Cargo.toml
 +++ b/media/libstagefright/binding/mp4parse_capi/Cargo.toml
-@@ -18,19 +18,10 @@ exclude = [
+@@ -18,22 +18,13 @@ exclude = [
    "*.mp4",
  ]
 
 -build = "build.rs"
 -
 -[badges]
 -travis-ci = { repository = "https://github.com/mozilla/mp4parse-rust" }
 +build = false
 
  [dependencies]
  byteorder = "1.0.0"
- mp4parse = {version = "0.8.0", path = "../mp4parse"}
+
+ # To enable fallible memory allocation, add 'features = ["mp4parse_fallible"]'
+ # in mp4parse brace.
+-mp4parse = {version = "0.8.0", path = "../mp4parse"}
++mp4parse = {version = "0.8.0", path = "../mp4parse", features = ["mp4parse_fallible"]}
  num-traits = "0.1.37"
 
 -[build-dependencies]
 -moz-cheddar = "0.4.0"
 -
 -[features]
 -fuzz = ["mp4parse/fuzz"]
 -
--- a/media/libstagefright/binding/mp4parse/Cargo.toml
+++ b/media/libstagefright/binding/mp4parse/Cargo.toml
@@ -19,13 +19,13 @@ exclude = [
   "*.mp4",
 ]
 
 
 [dependencies]
 byteorder = "1.0.0"
 bitreader = { version = "0.3.0" }
 num-traits = "0.1.37"
-mp4parse_fallible = { path = "../mp4parse_fallible" }
+mp4parse_fallible = { path = "../mp4parse_fallible", optional = true }
 
 [dev-dependencies]
 test-assembler = "0.1.2"
 
--- a/media/libstagefright/binding/mp4parse/src/lib.rs
+++ b/media/libstagefright/binding/mp4parse/src/lib.rs
@@ -6,23 +6,27 @@
 #![cfg_attr(feature = "fuzz", feature(plugin))]
 #![cfg_attr(feature = "fuzz", plugin(afl_plugin))]
 #[cfg(feature = "fuzz")]
 extern crate afl;
 
 extern crate byteorder;
 extern crate bitreader;
 extern crate num_traits;
-extern crate mp4parse_fallible;
 use byteorder::{ReadBytesExt, WriteBytesExt};
 use bitreader::{BitReader, ReadInto};
 use std::io::{Read, Take};
 use std::io::Cursor;
 use std::cmp;
 use num_traits::Num;
+
+#[cfg(feature = "mp4parse_fallible")]
+extern crate mp4parse_fallible;
+
+#[cfg(feature = "mp4parse_fallible")]
 use mp4parse_fallible::FallibleVec;
 
 mod boxes;
 use boxes::{BoxType, FourCC};
 
 // Unit tests.
 #[cfg(test)]
 mod tests;
@@ -31,26 +35,16 @@ mod tests;
 const BUF_SIZE_LIMIT: usize = 1024 * 1024;
 
 // Max table length. Calculating in worth case for one week long video, one
 // frame per table entry in 30 fps.
 const TABLE_SIZE_LIMIT: u32 = 30 * 60 * 60 * 24 * 7;
 
 static DEBUG_MODE: std::sync::atomic::AtomicBool = std::sync::atomic::ATOMIC_BOOL_INIT;
 
-static FALLIBLE_ALLOCATION: std::sync::atomic::AtomicBool = std::sync::atomic::ATOMIC_BOOL_INIT;
-
-pub fn set_fallible_allocation_mode(fallible: bool) {
-    FALLIBLE_ALLOCATION.store(fallible, std::sync::atomic::Ordering::SeqCst);
-}
-
-fn get_fallible_allocation_mode() -> bool {
-    FALLIBLE_ALLOCATION.load(std::sync::atomic::Ordering::Relaxed)
-}
-
 pub fn set_debug_mode(mode: bool) {
     DEBUG_MODE.store(mode, std::sync::atomic::Ordering::SeqCst);
 }
 
 #[inline(always)]
 fn get_debug_mode() -> bool {
     DEBUG_MODE.load(std::sync::atomic::Ordering::Relaxed)
 }
@@ -60,36 +54,42 @@ macro_rules! log {
         if get_debug_mode() {
             println!( $( $args )* );
         }
     )
 }
 
 // TODO: vec_push() and vec_reserve() needs to be replaced when Rust supports
 // fallible memory allocation in raw_vec.
+#[allow(unreachable_code)]
 pub fn vec_push<T>(vec: &mut Vec<T>, val: T) -> std::result::Result<(), ()> {
-    if get_fallible_allocation_mode() {
+    #[cfg(feature = "mp4parse_fallible")]
+    {
         return vec.try_push(val);
     }
 
     vec.push(val);
     Ok(())
 }
 
+#[allow(unreachable_code)]
 pub fn vec_reserve<T>(vec: &mut Vec<T>, size: usize) -> std::result::Result<(), ()> {
-    if get_fallible_allocation_mode() {
+    #[cfg(feature = "mp4parse_fallible")]
+    {
         return vec.try_reserve(size);
     }
 
     vec.reserve(size);
     Ok(())
 }
 
-fn reserve_read_buf(size: usize) -> std::result::Result<Vec<u8>, ()> {
-    if get_fallible_allocation_mode() {
+#[allow(unreachable_code)]
+fn allocate_read_buf(size: usize) -> std::result::Result<Vec<u8>, ()> {
+    #[cfg(feature = "mp4parse_fallible")]
+    {
         let mut buf: Vec<u8> = Vec::new();
         buf.try_reserve(size)?;
         unsafe { buf.set_len(size); }
         return Ok(buf);
     }
 
     Ok(vec![0; size])
 }
@@ -105,18 +105,16 @@ pub enum Error {
     /// Parse error caused by limited parser support rather than invalid data.
     Unsupported(&'static str),
     /// Reflect `std::io::ErrorKind::UnexpectedEof` for short data.
     UnexpectedEOF,
     /// Propagate underlying errors from `std::io`.
     Io(std::io::Error),
     /// read_mp4 terminated without detecting a moov box.
     NoMoov,
-    /// Parse error caused by table size is over limitation.
-    TableTooLarge,
     /// Out of memory
     OutOfMemory,
 }
 
 impl From<bitreader::BitReaderError> for Error {
     fn from(_: bitreader::BitReaderError) -> Error {
         Error::InvalidData("invalid data")
     }
@@ -2056,17 +2054,17 @@ fn skip<T: Read>(src: &mut T, mut bytes:
     Ok(())
 }
 
 /// Read size bytes into a Vector or return error.
 fn read_buf<T: ReadBytesExt>(src: &mut T, size: usize) -> Result<Vec<u8>> {
     if size > BUF_SIZE_LIMIT {
         return Err(Error::InvalidData("read_buf size exceeds BUF_SIZE_LIMIT"));
     }
-    if let Ok(mut buf) = reserve_read_buf(size) {
+    if let Ok(mut buf) = allocate_read_buf(size) {
         let r = src.read(&mut buf)?;
         if r != size {
           return Err(Error::InvalidData("failed buffer read"));
         }
         return Ok(buf);
     }
 
     Err(Error::OutOfMemory)
@@ -2097,17 +2095,17 @@ fn be_u24<T: ReadBytesExt>(src: &mut T) 
 fn be_u32<T: ReadBytesExt>(src: &mut T) -> Result<u32> {
     src.read_u32::<byteorder::BigEndian>().map_err(From::from)
 }
 
 /// Using in reading table size and return error if it exceeds limitation.
 fn be_u32_with_limit<T: ReadBytesExt>(src: &mut T) -> Result<u32> {
     be_u32(src).and_then(|v| {
         if v > TABLE_SIZE_LIMIT {
-            return Err(Error::TableTooLarge);
+            return Err(Error::OutOfMemory);
         }
         Ok(v)
     })
 }
 
 fn be_u64<T: ReadBytesExt>(src: &mut T) -> Result<u64> {
     src.read_u64::<byteorder::BigEndian>().map_err(From::from)
 }
--- a/media/libstagefright/binding/mp4parse/src/tests.rs
+++ b/media/libstagefright/binding/mp4parse/src/tests.rs
@@ -1018,17 +1018,17 @@ fn max_table_limit() {
     }).into_inner();
     let mut stream = make_box(BoxSize::Auto, b"edts", |s| {
         s.append_bytes(elst.as_slice())
     });
     let mut iter = super::BoxIter::new(&mut stream);
     let mut stream = iter.next_box().unwrap().unwrap();
     let mut track = super::Track::new(0);
     match super::read_edts(&mut stream, &mut track) {
-        Err(Error::TableTooLarge) => (),
+        Err(Error::OutOfMemory) => (),
         Ok(_) => panic!("expected an error result"),
         _ => panic!("expected a different error result"),
     }
 }
 
 #[test]
 fn unknown_video_sample_entry() {
     let unknown_codec = make_box(BoxSize::Auto, b"yyyy", |s| {
--- a/media/libstagefright/binding/mp4parse_capi/Cargo.toml
+++ b/media/libstagefright/binding/mp4parse_capi/Cargo.toml
@@ -17,11 +17,14 @@ repository = "https://github.com/mozilla
 exclude = [
   "*.mp4",
 ]
 
 build = false
 
 [dependencies]
 byteorder = "1.0.0"
-mp4parse = {version = "0.8.0", path = "../mp4parse"}
+
+# To enable fallible memory allocation, add 'features = ["mp4parse_fallible"]'
+# in mp4parse brace.
+mp4parse = {version = "0.8.0", path = "../mp4parse", features = ["mp4parse_fallible"]}
 num-traits = "0.1.37"
 
--- a/media/libstagefright/binding/mp4parse_capi/src/lib.rs
+++ b/media/libstagefright/binding/mp4parse_capi/src/lib.rs
@@ -65,18 +65,17 @@ use mp4parse::vec_push;
 #[derive(PartialEq, Debug)]
 pub enum mp4parse_status {
     OK = 0,
     BAD_ARG = 1,
     INVALID = 2,
     UNSUPPORTED = 3,
     EOF = 4,
     IO = 5,
-    TABLE_TOO_LARGE = 6,
-    OOM = 7,
+    OOM = 6,
 }
 
 #[allow(non_camel_case_types)]
 #[repr(C)]
 #[derive(PartialEq, Debug)]
 pub enum mp4parse_track_type {
     VIDEO = 0,
     AUDIO = 1,
@@ -305,21 +304,16 @@ pub unsafe extern fn mp4parse_free(parse
 }
 
 /// Enable `mp4_parser` log.
 #[no_mangle]
 pub unsafe extern fn mp4parse_log(enable: bool) {
     mp4parse::set_debug_mode(enable);
 }
 
-#[no_mangle]
-pub unsafe extern fn mp4parse_fallible_allocation(enable: bool) {
-    mp4parse::set_fallible_allocation_mode(enable);
-}
-
 /// Run the `mp4parse_parser*` allocated by `mp4parse_new()` until EOF or error.
 #[no_mangle]
 pub unsafe extern fn mp4parse_read(parser: *mut mp4parse_parser) -> mp4parse_status {
     // Validate arguments from C.
     if parser.is_null() || (*parser).poisoned() {
         return mp4parse_status::BAD_ARG;
     }
 
@@ -339,17 +333,16 @@ pub unsafe extern fn mp4parse_read(parse
         Err(Error::Io(_)) => {
             // Block further calls after a read failure.
             // Getting std::io::ErrorKind::UnexpectedEof is normal
             // but our From trait implementation should have converted
             // those to our Error::UnexpectedEOF variant.
             (*parser).set_poisoned(true);
             mp4parse_status::IO
         },
-        Err(Error::TableTooLarge) => mp4parse_status::TABLE_TOO_LARGE,
         Err(Error::OutOfMemory) => mp4parse_status::OOM,
     }
 }
 
 /// Return the number of tracks parsed by previous `mp4parse_read()` call.
 #[no_mangle]
 pub unsafe extern fn mp4parse_get_track_count(parser: *const mp4parse_parser, count: *mut u32) -> mp4parse_status {
     // Validate arguments from C.
@@ -920,20 +913,19 @@ fn create_sample_table(track: &Track, tr
                 return None;
             }
         }
     }
 
     // Mark the sync sample in sample_table according to 'stss'.
     if let Some(ref v) = track.stss {
         for iter in &v.samples {
-            if let Some(elem) = sample_table.get_mut((iter - 1) as usize) {
-                elem.sync = true;
-            } else {
-                return None;
+            match iter.checked_sub(1).and_then(|idx| { sample_table.get_mut(idx as usize) }) {
+                Some(elem) => elem.sync = true,
+                _ => return None,
             }
         }
     }
 
     let ctts_iter = match track.ctts {
         Some(ref v) => Some(v.samples.as_slice().iter()),
         _ => None,
     };
--- a/media/libstagefright/binding/mp4parse_fallible/README
+++ b/media/libstagefright/binding/mp4parse_fallible/README
@@ -1,2 +1,7 @@
 This is from https://github.com/servo/servo/tree/master/components/fallible
 with modificaion for mp4 demuxer.
+
+The purpose of this crate is to solve infallible memory allocation problem
+which causes OOM easily on win32. This is more like a temporary solution.
+Once rust supports fallible memory allocation in its stdlib, this can be
+retired.
--- a/media/libstagefright/binding/update-rust.sh
+++ b/media/libstagefright/binding/update-rust.sh
@@ -1,22 +1,23 @@
 #!/bin/sh -e
 # Script to update mp4parse-rust sources to latest upstream
 
 # Default version.
-VER=295fd9fac60f1c0627750695a4218e32425b0f66
+VER=7df7df72865226640de6aed05de2cb42751cd501
 
 # Accept version or commit from the command line.
 if test -n "$1"; then
   VER=$1
 fi
 
 echo "Fetching sources..."
 rm -rf _upstream
 git clone https://github.com/mozilla/mp4parse-rust _upstream/mp4parse
+git clone https://github.com/alfredoyang/mp4parse_fallible _upstream/mp4parse_fallible
 pushd _upstream/mp4parse
 git checkout ${VER}
 echo "Verifying sources..."
 pushd mp4parse
 cargo test
 popd
 echo "Constructing C api header..."
 pushd mp4parse_capi
@@ -35,17 +36,17 @@ cp _upstream/mp4parse/mp4parse/tests/*.m
 rm -rf mp4parse_capi
 mkdir -p mp4parse_capi/src
 cp _upstream/mp4parse/mp4parse_capi/Cargo.toml mp4parse_capi/
 cp _upstream/mp4parse/mp4parse_capi/build.rs mp4parse_capi/
 cp _upstream/mp4parse/mp4parse_capi/include/mp4parse.h include/
 cp _upstream/mp4parse/mp4parse_capi/src/*.rs mp4parse_capi/src/
 rm -rf mp4parse_fallible
 mkdir -p mp4parse_fallible
-cp _upstream/mp4parse/mp4parse_fallible/* mp4parse_fallible/
+cp _upstream/mp4parse_fallible/* mp4parse_fallible/
 
 echo "Applying patches..."
 patch -p4 < mp4parse-cargo.patch
 
 echo "Cleaning up..."
 rm -rf _upstream
 
 echo "Updating gecko Cargo.lock..."