MSE: Make WebMClusterParser hold back buffers at or beyond buffer missing duration

media/formats/webm/webm_cluster_parser.cc

 // Copyright 2014 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/formats/webm/webm_cluster_parser.h"
 
 #include <vector>
 
 #include "base/logging.h"
 #include "base/sys_byteorder.h"
@@ skipping 24 matching lines @@
       parser_(kWebMIdCluster, this),
       last_block_timecode_(-1),
       block_data_size_(-1),
       block_duration_(-1),
       block_add_id_(-1),
       block_additional_data_size_(-1),
       discard_padding_(-1),
       cluster_timecode_(-1),
       cluster_start_time_(kNoTimestamp()),
       cluster_ended_(false),
-      audio_(audio_track_num, false, audio_default_duration),
-      video_(video_track_num, true, video_default_duration),
+      audio_(audio_track_num, false, audio_default_duration, log_cb),
+      video_(video_track_num, true, video_default_duration, log_cb),
+      ready_buffer_upper_bound_(kNoTimestamp()),
       log_cb_(log_cb) {
   for (WebMTracksParser::TextTracks::const_iterator it = text_tracks.begin();
        it != text_tracks.end();
        ++it) {
     text_track_map_.insert(std::make_pair(
-        it->first, Track(it->first, false, kNoTimestamp())));
+        it->first, Track(it->first, false, kNoTimestamp(), log_cb_)));
   }
 }
 
 WebMClusterParser::~WebMClusterParser() {}
 
 void WebMClusterParser::Reset() {
   last_block_timecode_ = -1;
   cluster_timecode_ = -1;
   cluster_start_time_ = kNoTimestamp();
   cluster_ended_ = false;
   parser_.Reset();
   audio_.Reset();
   video_.Reset();
   ResetTextTracks();
+  ready_buffer_upper_bound_ = kNoTimestamp();
 }
 
 int WebMClusterParser::Parse(const uint8* buf, int size) {
-  audio_.ClearBuffersButKeepLastIfMissingDuration();
-  video_.ClearBuffersButKeepLastIfMissingDuration();
-  ResetTextTracks();
+  audio_.ClearReadyBuffers();
+  video_.ClearReadyBuffers();
+  ClearTextTrackReadyBuffers();
+  ready_buffer_upper_bound_ = kNoTimestamp();
 
   int result = parser_.Parse(buf, size);
 
   if (result < 0) {
     cluster_ended_ = false;
     return result;
   }
 
   cluster_ended_ = parser_.IsParsingComplete();
   if (cluster_ended_) {
@@ skipping 15 matching lines @@
     parser_.Reset();
 
     last_block_timecode_ = -1;
     cluster_timecode_ = -1;
   }
 
   return result;
 }
 
 const WebMClusterParser::BufferQueue& WebMClusterParser::GetAudioBuffers() {
-  if (cluster_ended_)
-    audio_.ApplyDurationEstimateIfNeeded();
-  return audio_.buffers();
+  if (ready_buffer_upper_bound_ == kNoTimestamp())
+    UpdateReadyBuffers();
+
+  return audio_.ready_buffers();
 }
 
 const WebMClusterParser::BufferQueue& WebMClusterParser::GetVideoBuffers() {
-  if (cluster_ended_)
-    video_.ApplyDurationEstimateIfNeeded();
-  return video_.buffers();
+  if (ready_buffer_upper_bound_ == kNoTimestamp())
+    UpdateReadyBuffers();
+
+  return video_.ready_buffers();
 }
 
 const WebMClusterParser::TextBufferQueueMap&
 WebMClusterParser::GetTextBuffers() {
-  // Translate our |text_track_map_| into |text_buffers_map_|, inserting rows in
-  // the output only for non-empty text buffer queues in |text_track_map_|.
-  text_buffers_map_.clear();

[acolwell GONE FROM CHROMIUM, 2014/04/23 22:36:39]

Why move this code? It seems like this is more consistant with the audio & video
code.

[wolenetz, 2014/04/25 20:04:21]

This was just an optimization to iterate through the text track list just once
to setup the text map per each Parse...Get{*}Buffers... cycle.
UpdateReadyBuffers() needs to occur at most once per cycle, so that calculating
the upper bound and moving buffers into ready_buffers is done consistently
across all tracks. I could retain iteration here (to reconstruct the map, but
not redo extraction), and also retain iteration in UpdateReadyBuffers() (to
perform the once-per-cycle extraction). WDYT? (I've made this change to clarify
what this looks like.)

-  for (TextTrackMap::const_iterator itr = text_track_map_.begin();
-       itr != text_track_map_.end();
-       ++itr) {
-    // Per OnBlock(), all text buffers should already have valid durations, so
-    // there is no need to call
-    // itr->second.ApplyDurationEstimateIfNeeded() here.
-    const BufferQueue& text_buffers = itr->second.buffers();
-    if (!text_buffers.empty())
-      text_buffers_map_.insert(std::make_pair(itr->first, text_buffers));
-  }
+  if (ready_buffer_upper_bound_ == kNoTimestamp())
+    UpdateReadyBuffers();
 
   return text_buffers_map_;
 }
 
 WebMParserClient* WebMClusterParser::OnListStart(int id) {
   if (id == kWebMIdCluster) {
     cluster_timecode_ = -1;
     cluster_start_time_ = kNoTimestamp();
   } else if (id == kWebMIdBlockGroup) {
     block_data_.reset();
@@ skipping 262 matching lines @@
   }
 
   if (discard_padding != 0) {
     buffer->set_discard_padding(base::TimeDelta::FromMicroseconds(
                                     discard_padding / 1000));
   }
 
   return track->AddBuffer(buffer);
 }
 
-WebMClusterParser::Track::Track(int track_num, bool is_video,
-                                base::TimeDelta default_duration)
+WebMClusterParser::Track::Track(int track_num,
+                                bool is_video,
+                                base::TimeDelta default_duration,
+                                const LogCB& log_cb)
     : track_num_(track_num),
       is_video_(is_video),
       default_duration_(default_duration),
-      estimated_next_frame_duration_(kNoTimestamp()) {
+      estimated_next_frame_duration_(kNoTimestamp()),
+      log_cb_(log_cb) {
   DCHECK(default_duration_ == kNoTimestamp() ||
          default_duration_ > base::TimeDelta());
 }
 
 WebMClusterParser::Track::~Track() {}
 
+base::TimeDelta WebMClusterParser::Track::GetReadyUpperBound() {
+  DCHECK(ready_buffers_.empty());
+  if (last_added_buffer_missing_duration_)
+    return last_added_buffer_missing_duration_->GetDecodeTimestamp();
+
+  return kInfiniteDuration();
+}
+
+const WebMClusterParser::BufferQueue&
+WebMClusterParser::Track::ExtractReadyBuffers(
+    const base::TimeDelta before_timestamp) {
+  DCHECK(ready_buffers_.empty());
+  DCHECK(base::TimeDelta() <= before_timestamp);
+  DCHECK(kNoTimestamp() != before_timestamp);
+
+  if (!buffers_.empty()) {

[acolwell GONE FROM CHROMIUM, 2014/04/23 22:36:39]

nit: reverse condition and early return.

[wolenetz, 2014/04/25 20:04:21]

Done.

+    if (buffers_.back()->GetDecodeTimestamp() < before_timestamp) {
+      // All of |buffers_| are ready.
+      ready_buffers_.swap(buffers_);
+      DVLOG(3) << __FUNCTION__ << " : " << track_num_ << " All "
+               << ready_buffers_.size() << " are ready: before upper bound ts "
+               << before_timestamp.InSecondsF();

[acolwell GONE FROM CHROMIUM, 2014/04/23 22:36:39]

nit:early return.

[wolenetz, 2014/04/25 20:04:21]

Done.

+    } else {
+      // Not all of |buffers_| are ready yet. Move any that are ready to
+      // |ready_buffers_|.
+      while (true) {
+        const scoped_refptr<StreamParserBuffer>& buffer = buffers_.front();
+        if (buffer->GetDecodeTimestamp() >= before_timestamp)
+          break;
+        ready_buffers_.push_back(buffer);
+        buffers_.pop_front();
+        DCHECK(!buffers_.empty());
+      }
+      DVLOG(3) << __FUNCTION__ << " : " << track_num_ << " Only "
+               << ready_buffers_.size() << " are ready, " << buffers_.size()
+               << " are at or after upper bound ts "
+               << before_timestamp.InSecondsF();
+    }
+  }
+
+  return ready_buffers_;

[acolwell GONE FROM CHROMIUM, 2014/04/23 22:36:39]

nit: looks like this is valuable at only one call site. Mind just calling
ready_buffers() there instead of returning here?

[wolenetz, 2014/04/25 20:04:21]

Done.

+}
+
 bool WebMClusterParser::Track::AddBuffer(
     const scoped_refptr<StreamParserBuffer>& buffer) {
   DVLOG(2) << "AddBuffer() : " << track_num_
            << " ts " << buffer->timestamp().InSecondsF()
            << " dur " << buffer->duration().InSecondsF()
            << " kf " << buffer->IsKeyframe()
            << " size " << buffer->data_size();
 
   if (last_added_buffer_missing_duration_) {
     base::TimeDelta derived_duration =
@@ skipping 36 matching lines @@
            << last_added_buffer_missing_duration_->duration().InSecondsF()
            << " kf " << last_added_buffer_missing_duration_->IsKeyframe()
            << " size " << last_added_buffer_missing_duration_->data_size();
 
   // Don't use the applied duration as a future estimation (don't use
   // QueueBuffer() here.)
   buffers_.push_back(last_added_buffer_missing_duration_);
   last_added_buffer_missing_duration_ = NULL;
 }
 
-void WebMClusterParser::Track::ClearBuffersButKeepLastIfMissingDuration() {
-  // Note that |estimated_next_frame_duration_| is not reset, so it can be
-  // reused on subsequent buffers added to this instance.
-  buffers_.clear();
+void WebMClusterParser::Track::ClearReadyBuffers() {
+  // Note that |buffers_| are kept and |estimated_next_frame_duration_| is not
+  // reset here.
+  ready_buffers_.clear();
 }
 
 void WebMClusterParser::Track::Reset() {
-  ClearBuffersButKeepLastIfMissingDuration();
+  ClearReadyBuffers();
+  buffers_.clear();
   last_added_buffer_missing_duration_ = NULL;
 }
 
 bool WebMClusterParser::Track::IsKeyframe(const uint8* data, int size) const {
   // For now, assume that all blocks are keyframes for datatypes other than
   // video. This is a valid assumption for Vorbis, WebVTT, & Opus.
   if (!is_video_)
     return true;
 
   // Make sure the block is big enough for the minimal keyframe header size.
   if (size < 7)
     return false;
 
   // The LSb of the first byte must be a 0 for a keyframe.
   // http://tools.ietf.org/html/rfc6386 Section 19.1
   if ((data[0] & 0x01) != 0)
     return false;
 
   // Verify VP8 keyframe startcode.
   // http://tools.ietf.org/html/rfc6386 Section 19.1
   if (data[3] != 0x9d || data[4] != 0x01 || data[5] != 0x2a)
     return false;
 
   return true;
 }
 
 bool WebMClusterParser::Track::QueueBuffer(
     const scoped_refptr<StreamParserBuffer>& buffer) {
   DCHECK(!last_added_buffer_missing_duration_);
+
+  // WebMClusterParser::OnBlock() gives MEDIA_LOG and parse error on decreasing
+  // block timecode detection within a cluster. Therefore, we should not see
+  // those here.
+  base::TimeDelta previous_buffers_timestamp = buffers_.empty() ?
+      base::TimeDelta() : buffers_.back()->GetDecodeTimestamp();
+  CHECK(previous_buffers_timestamp <= buffer->GetDecodeTimestamp());
+
   base::TimeDelta duration = buffer->duration();
   if (duration < base::TimeDelta() || duration == kNoTimestamp()) {
-    DVLOG(2) << "QueueBuffer() : Invalid buffer duration: "
-             << duration.InSecondsF();
+    MEDIA_LOG(log_cb_) << "Invalid buffer duration: " << duration.InSecondsF();
     return false;
   }
 
   // The estimated frame duration is the minimum non-zero duration since the
   // last initialization segment.  The minimum is used to ensure frame durations
   // aren't overestimated.
   if (duration > base::TimeDelta()) {
     if (estimated_next_frame_duration_ == kNoTimestamp()) {
       estimated_next_frame_duration_ = duration;
     } else {
@@ skipping 19 matching lines @@
       duration = base::TimeDelta::FromMilliseconds(
           kDefaultAudioBufferDurationInMs);
     }
   }
 
   DCHECK(duration > base::TimeDelta());
   DCHECK(duration != kNoTimestamp());
   return duration;
 }
 
+void WebMClusterParser::ClearTextTrackReadyBuffers() {
+  text_buffers_map_.clear();
+  for (TextTrackMap::iterator it = text_track_map_.begin();
+       it != text_track_map_.end();
+       ++it) {
+    it->second.ClearReadyBuffers();
+  }
+}
+
 void WebMClusterParser::ResetTextTracks() {
-  text_buffers_map_.clear();
+  ClearTextTrackReadyBuffers();
   for (TextTrackMap::iterator it = text_track_map_.begin();
        it != text_track_map_.end();
        ++it) {
     it->second.Reset();
   }
 }
 
+void WebMClusterParser::UpdateReadyBuffers() {
+  DCHECK(ready_buffer_upper_bound_ == kNoTimestamp());
+  DCHECK(text_buffers_map_.empty());
+
+  if (cluster_ended_) {
+    audio_.ApplyDurationEstimateIfNeeded();
+    video_.ApplyDurationEstimateIfNeeded();
+    // Per OnBlock(), all text buffers should already have valid durations, so
+    // there is no need to call ApplyDurationEstimateIfNeeded() on text tracks
+    // here.
+    ready_buffer_upper_bound_ = kInfiniteDuration();
+    DCHECK(ready_buffer_upper_bound_ == audio_.GetReadyUpperBound());
+    DCHECK(ready_buffer_upper_bound_ == video_.GetReadyUpperBound());
+  } else {
+    ready_buffer_upper_bound_ = std::min(audio_.GetReadyUpperBound(),
+                                         video_.GetReadyUpperBound());
+    DCHECK(base::TimeDelta() <= ready_buffer_upper_bound_);
+    DCHECK(kNoTimestamp() != ready_buffer_upper_bound_);
+  }
+
+  // Prepare each track's ready buffers for retrieval.
+  audio_.ExtractReadyBuffers(ready_buffer_upper_bound_);
+  video_.ExtractReadyBuffers(ready_buffer_upper_bound_);
+  for (TextTrackMap::iterator itr = text_track_map_.begin();
+       itr != text_track_map_.end();
+       ++itr) {
+    const BufferQueue& text_buffers = itr->second.ExtractReadyBuffers(
+        ready_buffer_upper_bound_);
+    if (!text_buffers.empty())
+      text_buffers_map_.insert(std::make_pair(itr->first, text_buffers));

[acolwell GONE FROM CHROMIUM, 2014/04/23 22:36:39]

Why are these staged here? I would expect these to be extracted at the same time
audio_ & video_ would.

[wolenetz, 2014/04/25 20:04:21]

Done (see my reply to GetTextBuffers() comment, above.)

+  }
+}
+
 WebMClusterParser::Track*
 WebMClusterParser::FindTextTrack(int track_num) {
   const TextTrackMap::iterator it = text_track_map_.find(track_num);
 
   if (it == text_track_map_.end())
     return NULL;
 
   return &it->second;
 }
 
 }  // namespace media