MSE: Populate WebM missing duration with DefaultDuration, derived, or default

media/formats/webm/webm_cluster_parser.cc

Index: media/formats/webm/webm_cluster_parser.cc
diff --git a/media/formats/webm/webm_cluster_parser.cc b/media/formats/webm/webm_cluster_parser.cc
index a762e2c40b34411594a5a30f0438e36f96d31a57..ed68f2bdc1ecd6242ab8a191545aa986103774e0 100644
--- a/media/formats/webm/webm_cluster_parser.cc
+++ b/media/formats/webm/webm_cluster_parser.cc
@@ -15,10 +15,20 @@
 #include "media/formats/webm/webm_crypto_helpers.h"
 #include "media/formats/webm/webm_webvtt_parser.h"
 
+// An arbitrarily-chosen number to estimate the duration of a buffer if none is
+// set and there is not enough information to get a better estimate.
+// TODO(wolenetz/acolwell): Parse audio codebook to determine missing audio
+// frame durations. See http://crbug.com/351166.
+static int kDefaultBufferDurationInMs = 50;

[acolwell GONE FROM CHROMIUM, 2014/03/26 23:32:04]

nit: If we are assuming this will be audio, then I think 23ms would be better.
This is equivalent to 1024 samples @ 44100, which is a common frame size for the
bitrates we care about.

[wolenetz, 2014/03/27 18:01:48]

Done. I've now differentiated audio vs video default buffer durations.

+
 namespace media {
 
 WebMClusterParser::WebMClusterParser(
-    int64 timecode_scale, int audio_track_num, int video_track_num,
+    int64 timecode_scale,
+    int audio_track_num,
+    base::TimeDelta audio_default_duration,
+    int video_track_num,
+    base::TimeDelta video_default_duration,
     const WebMTracksParser::TextTracks& text_tracks,
     const std::set<int64>& ignored_tracks,
     const std::string& audio_encryption_key_id,
@@ -38,13 +48,14 @@ WebMClusterParser::WebMClusterParser(
       cluster_timecode_(-1),
       cluster_start_time_(kNoTimestamp()),
       cluster_ended_(false),
-      audio_(audio_track_num, false),
-      video_(video_track_num, true),
+      audio_(audio_track_num, false, audio_default_duration),
+      video_(video_track_num, true, video_default_duration),
       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)));
+    text_track_map_.insert(std::make_pair(
+        it->first, Track(it->first, false, kNoTimestamp())));
   }
 }
 
@@ -56,8 +67,8 @@ void WebMClusterParser::Reset() {
   cluster_start_time_ = kNoTimestamp();
   cluster_ended_ = false;
   parser_.Reset();
-  audio_.Reset();
-  video_.Reset();
+  audio_.Flush();

[acolwell GONE FROM CHROMIUM, 2014/03/26 23:32:04]

nit: I think Reset() should be "clear everything" to be consistent w/ this
method's name. We should rename the Reset() you have to ClearBufferQueues() or
something like that so that it is clearer that not everything is cleared.

[wolenetz, 2014/03/27 18:01:48]

Done.

+  video_.Flush();
   ResetTextTracks();
 }
 
@@ -99,6 +110,18 @@ int WebMClusterParser::Parse(const uint8* buf, int size) {
   return result;
 }
 
+const WebMClusterParser::BufferQueue& WebMClusterParser::GetAudioBuffers() {
+  if (cluster_ended_)
+    audio_.ApplyDurationDefaultOrEstimateIfNeeded();
+  return audio_.buffers();
+}
+
+const WebMClusterParser::BufferQueue& WebMClusterParser::GetVideoBuffers() {
+  if (cluster_ended_)
+    video_.ApplyDurationDefaultOrEstimateIfNeeded();
+  return video_.buffers();
+}
+
 const WebMClusterParser::TextBufferQueueMap&
 WebMClusterParser::GetTextBuffers() {
   // Translate our |text_track_map_| into |text_buffers_map_|, inserting rows in
@@ -107,6 +130,9 @@ WebMClusterParser::GetTextBuffers() {
   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.ApplyDurationDefaultOrEstimateIfNeeded() here.
     const BufferQueue& text_buffers = itr->second.buffers();
     if (!text_buffers.empty())
       text_buffers_map_.insert(std::make_pair(itr->first, text_buffers));
@@ -390,9 +416,14 @@ bool WebMClusterParser::OnBlock(bool is_simple_block, int track_num,
   return track->AddBuffer(buffer);
 }
 
-WebMClusterParser::Track::Track(int track_num, bool is_video)
+WebMClusterParser::Track::Track(int track_num, bool is_video,
+                                base::TimeDelta default_duration)
     : track_num_(track_num),
-      is_video_(is_video) {
+      is_video_(is_video),
+      default_duration_(default_duration),
+      estimated_next_frame_duration_(kNoTimestamp()) {
+  DCHECK(default_duration_ == kNoTimestamp() ||
+         default_duration_ > base::TimeDelta());
 }
 
 WebMClusterParser::Track::~Track() {}
@@ -405,14 +436,88 @@ bool WebMClusterParser::Track::AddBuffer(
            << " kf " << buffer->IsKeyframe()
            << " size " << buffer->data_size();
 
+  if (last_added_buffer_missing_duration_) {
+    base::TimeDelta derived_duration =
+        buffer->timestamp() - last_added_buffer_missing_duration_->timestamp();
+    if (derived_duration < base::TimeDelta()) {
+      DVLOG(2) << "AddBuffer() : Invalid derived duration: "
+               << derived_duration.InSecondsF();
+      return false;
+    }
+
+    last_added_buffer_missing_duration_->set_duration(derived_duration);
+
+    DVLOG(2) << "AddBuffer() : applied derived duration to held-back buffer : "
+             << " ts "
+             << last_added_buffer_missing_duration_->timestamp().InSecondsF()
+             << " dur "
+             << last_added_buffer_missing_duration_->duration().InSecondsF()
+             << " kf " << last_added_buffer_missing_duration_->IsKeyframe()
+             << " size " << last_added_buffer_missing_duration_->data_size();
+    estimated_next_frame_duration_ = std::max(derived_duration,
+                                              estimated_next_frame_duration_);
+    buffers_.push_back(last_added_buffer_missing_duration_);
+    last_added_buffer_missing_duration_ = NULL;
+  }
+
+  if (buffer->duration() == kNoTimestamp()) {
+    last_added_buffer_missing_duration_ = buffer;
+    DVLOG(2) << "AddBuffer() : holding back buffer that is missing duration";
+    return true;
+  }
+
+  DCHECK(!last_added_buffer_missing_duration_);
+  if (buffer->duration() < base::TimeDelta()) {

[acolwell GONE FROM CHROMIUM, 2014/03/26 23:32:04]

nit: Looks like we could avoid some duplicate code if we had a QueueBuffer()
method that did the duration checks, push_back(), and
estimated_next_frame_duration_ updates.

[wolenetz, 2014/03/27 18:01:48]

Done.

+    DVLOG(2) << "AddBuffer() : Invalid negative duration: "
+             << buffer->duration().InSecondsF();
+    return false;
+  }
+
+  estimated_next_frame_duration_ = std::max(buffer->duration(),
+                                            estimated_next_frame_duration_);
   buffers_.push_back(buffer);
   return true;
 }
 
+void WebMClusterParser::Track::ApplyDurationDefaultOrEstimateIfNeeded() {
+  if (!last_added_buffer_missing_duration_)
+    return;
+
+  if (default_duration_ != kNoTimestamp()) {
+    DVLOG(3) << __FUNCTION__ << " : using TrackEntry DefaultDuration";
+    last_added_buffer_missing_duration_->set_duration(default_duration_);
+  } else if (estimated_next_frame_duration_ != kNoTimestamp()) {
+    DVLOG(3) << __FUNCTION__ << " : using estimated duration";
+    last_added_buffer_missing_duration_->set_duration(
+        estimated_next_frame_duration_);
+  } else {
+    DVLOG(3) << __FUNCTION__ << " : using hardcoded default duration";
+    last_added_buffer_missing_duration_->set_duration(
+        base::TimeDelta::FromMilliseconds(kDefaultBufferDurationInMs));
+  }
+
+  DVLOG(2) << "ApplyDurationDefaultOrEstimateIfNeeded() : new dur : "
+           << " ts "
+           << last_added_buffer_missing_duration_->timestamp().InSecondsF()
+           << " dur "
+           << last_added_buffer_missing_duration_->duration().InSecondsF()
+           << " kf " << last_added_buffer_missing_duration_->IsKeyframe()
+           << " size " << last_added_buffer_missing_duration_->data_size();
+  buffers_.push_back(last_added_buffer_missing_duration_);
+  last_added_buffer_missing_duration_ = NULL;
+}
+
 void WebMClusterParser::Track::Reset() {
+  // 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::Flush() {
+  Reset();
+  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.