From c1740d5016bd1da57e26066513e3eea5c154719a Mon Sep 17 00:00:00 2001
From: "strobe@google.com"
 <strobe@google.com@0039d316-1c4b-4281-b951-d872f2087c98>
Date: Wed, 1 Aug 2012 21:45:03 +0000
Subject: Fix MP4StreamParser discard behavior when retaining 'moof'.

Adding support for retaining the 'moof' atom (revision 148982) broke an
assumption in ReadMDATsUntil(). This removes the assumption, consolidates
logic, and adds a comment and a test.

BUG=
TEST=MP4StreamParserTest, manual tests


Review URL: https://chromiumcodereview.appspot.com/10834101

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@149499 0039d316-1c4b-4281-b951-d872f2087c98
---
 media/mp4/mp4_stream_parser.cc          | 18 +++++++-----------
 media/mp4/mp4_stream_parser.h           |  8 +++++++-
 media/mp4/mp4_stream_parser_unittest.cc | 10 ++++++++--
 3 files changed, 22 insertions(+), 14 deletions(-)

(limited to 'media/mp4')

diff --git a/media/mp4/mp4_stream_parser.cc b/media/mp4/mp4_stream_parser.cc
index c7a1c4d..5bc6fd4 100644
--- a/media/mp4/mp4_stream_parser.cc
+++ b/media/mp4/mp4_stream_parser.cc
@@ -86,8 +86,7 @@ bool MP4StreamParser::Parse(const uint8* buf, int size) {
       result = EnqueueSample(&audio_buffers, &video_buffers, &err);
       if (result) {
         int64 max_clear = runs_->GetMaxClearOffset() + moof_head_;
-        DCHECK(max_clear <= queue_.tail());
-        err = !(ReadMDATsUntil(max_clear) && queue_.Trim(max_clear));
+        err = !ReadAndDiscardMDATsUntil(max_clear);
       }
     }
   } while (result && !err);
@@ -435,29 +434,26 @@ bool MP4StreamParser::SendAndFlushSamples(BufferQueue* audio_buffers,
   return !err;
 }
 
-bool MP4StreamParser::ReadMDATsUntil(const int64 tgt_offset) {
-  DCHECK(tgt_offset <= queue_.tail());
-
-  while (mdat_tail_ < tgt_offset) {
+bool MP4StreamParser::ReadAndDiscardMDATsUntil(const int64 offset) {
+  bool err = false;
+  while (mdat_tail_ < offset) {
     const uint8* buf;
     int size;
     queue_.PeekAt(mdat_tail_, &buf, &size);
 
     FourCC type;
     int box_sz;
-    bool err;
     if (!BoxReader::StartTopLevelBox(buf, size, &type, &box_sz, &err))
-      return false;
+      break;
 
     if (type != FOURCC_MDAT) {
       DLOG(WARNING) << "Unexpected type while parsing MDATs: "
                     << FourCCToString(type);
     }
-
     mdat_tail_ += box_sz;
   }
-
-  return true;
+  queue_.Trim(std::min(mdat_tail_, offset));
+  return !err;
 }
 
 void MP4StreamParser::ChangeState(State new_state) {
diff --git a/media/mp4/mp4_stream_parser.h b/media/mp4/mp4_stream_parser.h
index b950af2..c27227e 100644
--- a/media/mp4/mp4_stream_parser.h
+++ b/media/mp4/mp4_stream_parser.h
@@ -50,7 +50,13 @@ class MEDIA_EXPORT MP4StreamParser : public StreamParser {
 
   bool EmitKeyNeeded(const TrackEncryption& track_encryption);
 
-  bool ReadMDATsUntil(const int64 tgt_tail);
+  // To retain proper framing, each 'mdat' atom must be read; to limit memory
+  // usage, the atom's data needs to be discarded incrementally as frames are
+  // extracted from the stream. This function discards data from the stream up
+  // to |offset|, updating the |mdat_tail_| value so that framing can be
+  // retained after all 'mdat' information has been read.
+  // Returns 'true' on success, 'false' if there was an error.
+  bool ReadAndDiscardMDATsUntil(const int64 offset);
 
   void ChangeState(State new_state);
 
diff --git a/media/mp4/mp4_stream_parser_unittest.cc b/media/mp4/mp4_stream_parser_unittest.cc
index 56f40df..6b4f4ca 100644
--- a/media/mp4/mp4_stream_parser_unittest.cc
+++ b/media/mp4/mp4_stream_parser_unittest.cc
@@ -120,14 +120,17 @@ class MP4StreamParserTest : public testing::Test {
   }
 };
 
-
-
 TEST_F(MP4StreamParserTest, TestUnalignedAppend) {
   // Test small, non-segment-aligned appends (small enough to exercise
   // incremental append system)
   ParseMP4File("bear.1280x720_dash.mp4", 512);
 }
 
+TEST_F(MP4StreamParserTest, TestBytewiseAppend) {
+  // Ensure no incremental errors occur when parsing
+  ParseMP4File("bear.1280x720_dash.mp4", 1);
+}
+
 TEST_F(MP4StreamParserTest, TestMultiFragmentAppend) {
   // Large size ensures multiple fragments are appended in one call (size is
   // larger than this particular test file)
@@ -147,5 +150,8 @@ TEST_F(MP4StreamParserTest, TestReinitialization) {
                                  512));
 }
 
+// TODO(strobe): Create and test media which uses CENC auxiliary info stored
+// inside a private box
+
 }  // namespace mp4
 }  // namespace media
-- 
cgit v1.1