Fix MP4StreamParser discard behavior when retaining 'moof'.

media/mp4/mp4_stream_parser.cc

Index: media/mp4/mp4_stream_parser.cc
diff --git a/media/mp4/mp4_stream_parser.cc b/media/mp4/mp4_stream_parser.cc
index d153d69161e00df969ab933bd26ebfd07c020a3b..60630b0f9b9256c351822c9b47a7f6651c97069b 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);
@@ -438,9 +437,8 @@ bool MP4StreamParser::SendAndFlushSamples(BufferQueue* audio_buffers,
   return !err;
 }
 
-bool MP4StreamParser::ReadMDATsUntil(const int64 tgt_offset) {
-  DCHECK(tgt_offset <= queue_.tail());

[acolwell GONE FROM CHROMIUM, 2012/08/01 16:39:02]

Why remove this DCHECK?

[strobe_, 2012/08/01 19:01:33]

It was in fact always too narrow. It worked before because samples were stored
contiguously in runs (as mandated by BMFF) *and* runs were stored contiguously
in the 'mdat' (which isn't mandated). When aux info was moved to the 'moof', the
gap between aux info and the first sample would cause this to fire for any
append range that included all aux info but not the first frame in the sequence.

-
+bool MP4StreamParser::ReadAndDiscardMDATsUntil(const int64 tgt_offset) {
+  bool err = false;
   while (mdat_tail_ < tgt_offset) {
     const uint8* buf;
     int size;
@@ -448,19 +446,17 @@ bool MP4StreamParser::ReadMDATsUntil(const int64 tgt_offset) {
 
     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_, tgt_offset));

[acolwell GONE FROM CHROMIUM, 2012/08/01 16:39:02]

Why do you unconditionally trim now?

[strobe_, 2012/08/01 19:01:33]

If multiple boxes are consumed in this loop, but the appended data ends in the
middle of a box header, we still want to clear all boxes up to the start of that
box header. Also, if user appends data on segment boundaries, mdat_tail_ will
equal tgt_offset on the last frame, so we want to clear  that data even though
the loop doesn't run.

A false return from queue_.Trim() is also not an error anymore for the same
reason that the DCHECK was removed.

+  return !err;
 }
 
 void MP4StreamParser::ChangeState(State new_state) {