Flush sample buffers when reading new segments in Media Source BMFF parser.

media/mp4/mp4_stream_parser.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "media/mp4/mp4_stream_parser.h"
 
 #include "base/callback.h"
 #include "base/logging.h"
 #include "base/time.h"
 #include "media/base/audio_decoder_config.h"
@@ skipping 58 matching lines @@
 
   queue_.Push(buf, size);
 
   BufferQueue audio_buffers;
   BufferQueue video_buffers;
 
   bool result, err = false;
 
   do {
     if (state_ == kParsingBoxes) {
+      // Flush any buffers we've gotten in this chunk so that buffers don't

[acolwell GONE FROM CHROMIUM, 2012/06/25 21:22:16]

It might be clearer to place this logic in EnqueueSample() where you transition
to kParsingBoxes. WDYT? Doesn't that correspond to the case you are trying to
capture? 

[strobe_, 2012/06/26 00:37:59]

Done.

+      // cross NewSegment() calls
+      if (!audio_buffers.empty()) {

[acolwell GONE FROM CHROMIUM, 2012/06/25 21:22:16]

This isn't sufficient. You need to do all the same checks that are done below.
I'd suggest moving that logic into a helper function and call it from here and
below.

[strobe_, 2012/06/26 00:37:59]

What checks are missing here as compared to below?

[acolwell GONE FROM CHROMIUM, 2012/06/26 15:25:14]

Sorry. The CHECK/DCHECK here was actually inappropriate. It should have been an
RCHECK(). Calls from JavaScript should not trigger a tab crash. I need to pay
more attention. Now that you are using the common helper function this is taken
care of.

+        CHECK(!audio_cb_.is_null());

[acolwell GONE FROM CHROMIUM, 2012/06/25 21:22:16]

Use DCHECK here and below. Let the null pointer exception below cause the crash
in release.

[strobe_, 2012/06/26 00:37:59]

Done. (well, moved into the shared error conditions)

+        RCHECK(audio_cb_.Run(audio_buffers));
+        audio_buffers.clear();
+      }
+      if (!video_buffers.empty()) {
+        CHECK(!video_cb_.is_null());
+        RCHECK(video_cb_.Run(video_buffers));
+        video_buffers.clear();
+      }
+
       if (mdat_tail_ > queue_.head()) {
         result = queue_.Trim(mdat_tail_);
       } else {
         result = ParseBox(&err);
       }
     } else {
       DCHECK_EQ(kEmittingSamples, state_);
       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));
       }
     }
   } while (result && !err);
 
   if (err) {
     DLOG(ERROR) << "Unknown error while parsing MP4";
     queue_.Reset();
     moov_.reset();
     ChangeState(kError);
     return false;
   }
 
   if (!audio_buffers.empty() &&
       (audio_cb_.is_null() || !audio_cb_.Run(audio_buffers)))
     return false;

[acolwell GONE FROM CHROMIUM, 2012/06/25 21:22:16]

You should have a ChangeState(kError) here and below since it means we've
received unexpected data and we shouldn't accept  data anymore.

[strobe_, 2012/06/26 00:37:59]

Done.

   if (!video_buffers.empty() &&
       (video_cb_.is_null() || !video_cb_.Run(video_buffers)))
     return false;
 
   return true;
 }
 
 bool MP4StreamParser::ParseBox(bool* err) {
   const uint8* buf;
   int size;
@@ skipping 220 matching lines @@
   return true;
 }
 
 void MP4StreamParser::ChangeState(State new_state) {
   DVLOG(2) << "Changing state: " << new_state;
   state_ = new_state;
 }
 
 }  // namespace mp4
 }  // namespace media