Fix WebM parser not to wait for a next frame when DefaultDuration is specified.

Per Matroska spec difference between frames timestamps must be used as
block duration when neither Duration nor DefaultDuration is specified.
Problem was that WebMClusterParser was waiting for the next frame
even when DefaultDuration is specified.

BUG=338529

Review URL: https://codereview.chromium.org/230173002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@263097 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 24 insertions, 21 deletions

diff --git a/media/formats/webm/webm_cluster_parser.cc b/media/formats/webm/webm_cluster_parser.cc
index 6e2424b..5530815 100644
--- a/media/formats/webm/webm_cluster_parser.cc
+++ b/media/formats/webm/webm_cluster_parser.cc
@@ -113,13 +113,13 @@ int WebMClusterParser::Parse(const uint8* buf, int size) {
 
 const WebMClusterParser::BufferQueue& WebMClusterParser::GetAudioBuffers() {
   if (cluster_ended_)
-    audio_.ApplyDurationDefaultOrEstimateIfNeeded();
+    audio_.ApplyDurationEstimateIfNeeded();
   return audio_.buffers();
 }
 
 const WebMClusterParser::BufferQueue& WebMClusterParser::GetVideoBuffers() {
   if (cluster_ended_)
-    video_.ApplyDurationDefaultOrEstimateIfNeeded();
+    video_.ApplyDurationEstimateIfNeeded();
   return video_.buffers();
 }
 
@@ -133,7 +133,7 @@ WebMClusterParser::GetTextBuffers() {
        ++itr) {
     // Per OnBlock(), all text buffers should already have valid durations, so
     // there is no need to call
-    // itr->second.ApplyDurationDefaultOrEstimateIfNeeded() here.
+    // 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));
@@ -407,6 +407,9 @@ bool WebMClusterParser::OnBlock(bool is_simple_block, int track_num,
   if (block_duration >= 0) {
     buffer->set_duration(base::TimeDelta::FromMicroseconds(
         block_duration * timecode_multiplier_));
+  } else {
+    DCHECK_NE(buffer_type, DemuxerStream::TEXT);
+    buffer->set_duration(track->default_duration());
   }
 
   if (discard_padding != 0) {
@@ -465,14 +468,13 @@ bool WebMClusterParser::Track::AddBuffer(
   return QueueBuffer(buffer);
 }
 
-void WebMClusterParser::Track::ApplyDurationDefaultOrEstimateIfNeeded() {
+void WebMClusterParser::Track::ApplyDurationEstimateIfNeeded() {
   if (!last_added_buffer_missing_duration_)
     return;
 
-  last_added_buffer_missing_duration_->set_duration(
-      GetDurationDefaultOrEstimate());
+  last_added_buffer_missing_duration_->set_duration(GetDurationEstimate());
 
-  DVLOG(2) << "ApplyDurationDefaultOrEstimateIfNeeded() : new dur : "
+  DVLOG(2) << "ApplyDurationEstimateIfNeeded() : new dur : "
            << " ts "
            << last_added_buffer_missing_duration_->timestamp().InSecondsF()
            << " dur "
@@ -536,13 +538,10 @@ bool WebMClusterParser::Track::QueueBuffer(
   return true;
 }
 
-base::TimeDelta WebMClusterParser::Track::GetDurationDefaultOrEstimate() {
-  base::TimeDelta duration = default_duration_;
+base::TimeDelta WebMClusterParser::Track::GetDurationEstimate() {
+  base::TimeDelta duration = estimated_next_frame_duration_;
   if (duration != kNoTimestamp()) {
-    DVLOG(3) << __FUNCTION__ << " : using TrackEntry DefaultDuration";
-  } else if (estimated_next_frame_duration_ != kNoTimestamp()) {
     DVLOG(3) << __FUNCTION__ << " : using estimated duration";
-    duration = estimated_next_frame_duration_;
   } else {
     DVLOG(3) << __FUNCTION__ << " : using hardcoded default duration";
     if (is_video_) {
diff --git a/media/formats/webm/webm_cluster_parser.h b/media/formats/webm/webm_cluster_parser.h
index 749b6bd..5657e90 100644
--- a/media/formats/webm/webm_cluster_parser.h
+++ b/media/formats/webm/webm_cluster_parser.h
@@ -43,13 +43,12 @@ class MEDIA_EXPORT WebMClusterParser : public WebMParserClient {
     // otherwise adds |buffer| to |buffers_|.
     bool AddBuffer(const scoped_refptr<StreamParserBuffer>& buffer);
 
-    // If |last_added_buffer_missing_duration_| is set, updates its duration
-    // to be the first non-kNoTimestamp() value of |default_duration_|,
-    // |estimated_next_frame_duration_|, or an arbitrary default, then adds it
-    // to |buffers_| and unsets |last_added_buffer_missing_duration_|. (This
-    // method helps stream parser emit all buffers in a media segment before
-    // signaling end of segment.)
-    void ApplyDurationDefaultOrEstimateIfNeeded();
+    // If |last_added_buffer_missing_duration_| is set, updates its duration to
+    // be non-kNoTimestamp() value of |estimated_next_frame_duration_| or an
+    // arbitrary default, then adds it to |buffers_| and unsets
+    // |last_added_buffer_missing_duration_|. (This method helps stream parser
+    // emit all buffers in a media segment before signaling end of segment.)
+    void ApplyDurationEstimateIfNeeded();
 
     // Clears all buffer state, except a possibly held-aside buffer that is
     // missing duration.
@@ -65,6 +64,8 @@ class MEDIA_EXPORT WebMClusterParser : public WebMParserClient {
     // |size| indicates the number of bytes in |data|.
     bool IsKeyframe(const uint8* data, int size) const;
 
+    base::TimeDelta default_duration() const { return default_duration_; }
+
    private:
     // Helper that sanity-checks |buffer| duration, updates
     // |estimated_next_frame_duration_|, and adds |buffer| to |buffers_|.
@@ -73,8 +74,8 @@ class MEDIA_EXPORT WebMClusterParser : public WebMParserClient {
     bool QueueBuffer(const scoped_refptr<StreamParserBuffer>& buffer);
 
     // Helper that calculates the buffer duration to use in
-    // ApplyDurationDefaultOrEstimateIfNeeded().
-    base::TimeDelta GetDurationDefaultOrEstimate();
+    // ApplyDurationEstimateIfNeeded().
+    base::TimeDelta GetDurationEstimate();
 
     int track_num_;
     std::deque<scoped_refptr<StreamParserBuffer> > buffers_;
@@ -86,6 +87,9 @@ class MEDIA_EXPORT WebMClusterParser : public WebMParserClient {
     // If kNoTimestamp(), then a default value will be used. This estimate is
     // the maximum duration seen or derived so far for this track, and is valid
     // only if |default_duration_| is kNoTimestamp().
+    //
+    // TODO(wolenetz): Add unittests for duration estimation and default
+    // duration handling. http://crbug.com/361786 .
     base::TimeDelta estimated_next_frame_duration_;
   };