Update WebMClusterParser to compute durations for all StreamParserBuffers.

media/webm/webm_cluster_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/webm/webm_cluster_parser.h"
 
 #include "base/logging.h"
 #include "media/base/data_buffer.h"
 #include "media/base/decrypt_config.h"
 #include "media/webm/webm_constants.h"
@@ skipping 23 matching lines @@
     video_encryption_key_id_.reset(new uint8[video_encryption_key_id_size]);
     memcpy(video_encryption_key_id_.get(), video_encryption_key_id,
            video_encryption_key_id_size);
   }
 }
 
 WebMClusterParser::~WebMClusterParser() {}
 
 void WebMClusterParser::Reset() {
   audio_buffers_.clear();
+  delayed_audio_buffer_ = NULL;
   video_buffers_.clear();
+  delayed_video_buffer_ = NULL;
   last_block_timecode_ = -1;
   cluster_timecode_ = -1;
   parser_.Reset();
 }
 
 int WebMClusterParser::Parse(const uint8* buf, int size) {
   audio_buffers_.clear();
   video_buffers_.clear();
 
   int result = parser_.Parse(buf, size);
@@ skipping 104 matching lines @@
   if (block_data_.get()) {
     DVLOG(1) << "More than 1 Block in a BlockGroup is not supported.";
     return false;
   }
 
   block_data_.reset(new uint8[size]);
   memcpy(block_data_.get(), data, size);
   block_data_size_ = size;
   return true;
 }
+
 bool WebMClusterParser::OnBlock(int track_num, int timecode,
                                 int  block_duration,
                                 int flags,
                                 const uint8* data, int size) {
   if (cluster_timecode_ == -1) {
     DVLOG(1) << "Got a block before cluster timecode.";
     return false;
   }
 
   if (timecode < 0) {
@@ skipping 17 matching lines @@
   scoped_refptr<StreamParserBuffer> buffer =
       StreamParserBuffer::CopyFrom(data, size, is_keyframe);
 
   if (track_num == video_track_num_ && video_encryption_key_id_.get()) {
     buffer->SetDecryptConfig(scoped_ptr<DecryptConfig>(new DecryptConfig(
         video_encryption_key_id_.get(), video_encryption_key_id_size_)));
   }
 
   buffer->SetTimestamp(timestamp);
   BufferQueue* queue = NULL;
+  scoped_refptr<StreamParserBuffer>* delayed_buffer;

[scherkus (not reviewing), 2012/05/17 02:45:46]

I'm finding this pointer-to-a-scoped_refptr<T> to be overly clever. I was
searching to figure out where delayed_audio_buffer_ was being set and it took me
a while to realize that it's on line 256 where we deref the pointer

do we need to have two separate delayed buffers? would we ever mistakenly queue
a video buffer into an audio queue? perhaps it's worth duplicating some code
here for the sake of clarity

would refactoring {duration, queue, delayed_buffer} into a struct/class and
having an audio instance and a video instance help here?

WDYT?

[acolwell GONE FROM CHROMIUM, 2012/05/17 18:28:51]

Excellent idea! I moved this all into a helper class and it is so much nicer. :)

   base::TimeDelta duration = kNoTimestamp();
 
   if (track_num == audio_track_num_) {
     duration = audio_default_duration_;
     queue = &audio_buffers_;
+    delayed_buffer = &delayed_audio_buffer_;
   } else if (track_num == video_track_num_) {
     duration = video_default_duration_;
     queue = &video_buffers_;
+    delayed_buffer = &delayed_video_buffer_;
   } else {
     DVLOG(1) << "Unexpected track number " << track_num;
     return false;
   }
 
   if (block_duration >= 0) {
     duration = base::TimeDelta::FromMicroseconds(
         block_duration * timecode_multiplier_);
   }
 
   buffer->SetDuration(duration);
 
   if (!queue->empty() &&
       buffer->GetTimestamp() == queue->back()->GetTimestamp()) {
     DVLOG(1) << "Got a block timecode that is not strictly monotonically "
              << "increasing for track " << track_num;
     return false;
   }
 
+  if (*delayed_buffer) {
+    // Update the duration of the delayed buffer and place it into the queue.
+    base::TimeDelta new_duration =
+        buffer->GetTimestamp() - (*delayed_buffer)->GetTimestamp();
+
+    if (new_duration <= base::TimeDelta())

[scherkus (not reviewing), 2012/05/17 02:45:46]

when would this happen? out of order timestamps?

[acolwell GONE FROM CHROMIUM, 2012/05/17 18:28:51]

Yes. It can happen if a muxer creates a cluster with out of order timestamps or
per stream timestamps that are not monotonically increasing.

+      return false;
+
+    (*delayed_buffer)->SetDuration(new_duration);
+    queue->push_back(*delayed_buffer);
+
+    *delayed_buffer = NULL;
+  }
+
+  // Place the buffer in delayed buffer slot if we don't know
+  // its duration.
+  if (buffer->GetDuration() == kNoTimestamp()) {
+    *delayed_buffer = buffer;
+    return true;
+  }
+
   queue->push_back(buffer);
   return true;
 }
 
 }  // namespace media