Update WebMClusterParser to compute durations for all StreamParserBuffers.

media/webm/webm_cluster_parser.cc

Index: media/webm/webm_cluster_parser.cc
diff --git a/media/webm/webm_cluster_parser.cc b/media/webm/webm_cluster_parser.cc
index ed1e94eafdbf403004a19d327c47314fff88c23c..59b3a9d2e8a12d699b8ef0279537ac932c8a431f 100644
--- a/media/webm/webm_cluster_parser.cc
+++ b/media/webm/webm_cluster_parser.cc
@@ -41,7 +41,9 @@ 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();
@@ -166,6 +168,7 @@ bool WebMClusterParser::OnBinary(int id, const uint8* data, int size) {
   block_data_size_ = size;
   return true;
 }
+
 bool WebMClusterParser::OnBlock(int track_num, int timecode,
                                 int  block_duration,
                                 int flags,
@@ -203,14 +206,17 @@ bool WebMClusterParser::OnBlock(int track_num, int timecode,
 
   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;
@@ -230,6 +236,27 @@ bool WebMClusterParser::OnBlock(int track_num, int timecode,
     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;
 }