Add config change handling to SourceBufferStream & ChunkDemuxer

media/filters/source_buffer_stream.cc

Index: media/filters/source_buffer_stream.cc
diff --git a/media/filters/source_buffer_stream.cc b/media/filters/source_buffer_stream.cc
index 2b1911a93d37f52b5e7d1b350879257a8d07c3ca..71b5c13f953407339cfae950cd598d5c51ac6482 100644
--- a/media/filters/source_buffer_stream.cc
+++ b/media/filters/source_buffer_stream.cc
@@ -90,6 +90,10 @@ class SourceBufferRange {
   bool GetNextBuffer(scoped_refptr<StreamParserBuffer>* out_buffer);
   bool HasNextBuffer() const;
 
+  // Returns the config ID for the buffer that will be returned by
+  // GetNextBuffer().
+  int GetNextConfigID() const;

[xhwang, 2012/07/12 18:09:12]

s/ID/Id?

[acolwell GONE FROM CHROMIUM, 2012/07/12 23:07:58]

Done.

+
   // Returns true if the range knows the position of the next buffer it should
   // return, i.e. it has been Seek()ed. This does not necessarily mean that it
   // has the next buffer yet.
@@ -223,7 +227,11 @@ static int kDefaultBufferDurationInMs = 125;
 namespace media {
 
 SourceBufferStream::SourceBufferStream(const AudioDecoderConfig& audio_config)
-    : seek_pending_(false),
+    : current_config_index_(0),
+      append_config_index_(0),
+      audio_configs_(1),
+      video_configs_(0),
+      seek_pending_(false),
       seek_buffer_timestamp_(kNoTimestamp()),
       selected_range_(NULL),
       end_of_stream_(false),
@@ -232,11 +240,16 @@ SourceBufferStream::SourceBufferStream(const AudioDecoderConfig& audio_config)
       new_media_segment_(false),
       last_buffer_timestamp_(kNoTimestamp()),
       max_interbuffer_distance_(kNoTimestamp()) {
-  audio_config_.CopyFrom(audio_config);
+  audio_configs_[0] = new AudioDecoderConfig();
+  audio_configs_[0]->CopyFrom(audio_config);
 }
 
 SourceBufferStream::SourceBufferStream(const VideoDecoderConfig& video_config)
-    : seek_pending_(false),
+    : current_config_index_(0),
+      append_config_index_(0),
+      audio_configs_(0),
+      video_configs_(1),
+      seek_pending_(false),
       seek_buffer_timestamp_(kNoTimestamp()),
       selected_range_(NULL),
       end_of_stream_(false),
@@ -245,7 +258,8 @@ SourceBufferStream::SourceBufferStream(const VideoDecoderConfig& video_config)
       new_media_segment_(false),
       last_buffer_timestamp_(kNoTimestamp()),
       max_interbuffer_distance_(kNoTimestamp()) {
-  video_config_.CopyFrom(video_config);
+  video_configs_[0] = new VideoDecoderConfig();
+  video_configs_[0]->CopyFrom(video_config);
 }
 
 SourceBufferStream::~SourceBufferStream() {
@@ -253,6 +267,14 @@ SourceBufferStream::~SourceBufferStream() {
     delete ranges_.front();
     ranges_.pop_front();
   }
+
+  for (size_t i = 0; i < audio_configs_.size(); ++i)
+    delete audio_configs_[i];
+  audio_configs_.clear();
+
+  for (size_t i = 0; i < video_configs_.size(); ++i)
+    delete video_configs_[i];
+  video_configs_.clear();

[xhwang, 2012/07/12 18:09:12]

Use STLDeleteElements here and above?

[acolwell GONE FROM CHROMIUM, 2012/07/12 23:07:58]

Done.

 }
 
 void SourceBufferStream::OnNewMediaSegment(
@@ -284,6 +306,7 @@ bool SourceBufferStream::Append(
   }
 
   UpdateMaxInterbufferDistance(buffers);
+  SetConfigIDs(buffers);
 
   // Save a snapshot of the |selected_range_| state before range modifications
   // are made.
@@ -395,6 +418,13 @@ void SourceBufferStream::UpdateMaxInterbufferDistance(
   }
 }
 
+void SourceBufferStream::SetConfigIDs(const BufferQueue& buffers) {
+  for (BufferQueue::const_iterator itr = buffers.begin();
+       itr != buffers.end(); ++itr) {
+    (*itr)->SetConfigID(append_config_index_);

[xhwang, 2012/07/12 18:09:12]

The comment in .h file uses set_config_id, which sounds right. Change
StreamParserBuffer::SetConfigID to set_config_id since it's a setter?

[acolwell GONE FROM CHROMIUM, 2012/07/12 23:07:58]

I just fixed the comment to reflect the SetConfigId() form. I originally had the
lower case form, but I used MixedCase to match the getter/setters in
StreamParserBuffer & DecoderBuffer.

+  }
+}
+
 void SourceBufferStream::InsertIntoExistingRange(
     const RangeList::iterator& range_for_new_buffers_itr,
     const BufferQueue& new_buffers,
@@ -606,21 +636,30 @@ bool SourceBufferStream::IsSeekPending() const {
   return seek_pending_;
 }
 
-bool SourceBufferStream::GetNextBuffer(
+SourceBufferStream::Status SourceBufferStream::GetNextBuffer(
     scoped_refptr<StreamParserBuffer>* out_buffer) {
   if (!track_buffer_.empty()) {
+    if (track_buffer_.front()->GetConfigID() != current_config_index_)
+      return kConfigChange;
+
     *out_buffer = track_buffer_.front();
     track_buffer_.pop_front();
-    return true;
+    return kSuccess;
   }
 
   if (end_of_stream_ && (!selected_range_ ||
                          !selected_range_->HasNextBuffer())) {
     *out_buffer = StreamParserBuffer::CreateEOSBuffer();
-    return true;
+    return kSuccess;
   }
 
-  return selected_range_ && selected_range_->GetNextBuffer(out_buffer);
+  if (!selected_range_ || !selected_range_->HasNextBuffer())
+    return kNeedBuffer;
+
+  if (selected_range_->GetNextConfigID() != current_config_index_)
+    return kConfigChange;
+
+  return selected_range_->GetNextBuffer(out_buffer) ? kSuccess : kNeedBuffer;

[vrk (LEFT CHROMIUM), 2012/07/12 19:37:55]

At this point, shouldn't selected_range_->GetNextBuffer(out_buffer) always
return true?

[acolwell GONE FROM CHROMIUM, 2012/07/12 23:07:58]

I wasn't sure. The guard in SourceBufferRange::GetNextBuffer() has an added
check for waiting_for_keyframe_, but SourceBufferRange::HasNextBuffer() doesn't
check for this. I wasn't sure if there was a situation were HasNextBuffer()
could return true and waiting_for_keyframe_ was true.

 }
 
 base::TimeDelta SourceBufferStream::GetNextBufferTimestamp() {
@@ -693,6 +732,99 @@ base::TimeDelta SourceBufferStream::GetMaxInterbufferDistance() const {
   return max_interbuffer_distance_;
 }
 
+bool SourceBufferStream::UpdateAudioConfig(const AudioDecoderConfig& config) {
+  DCHECK(!audio_configs_.empty());

[vrk (LEFT CHROMIUM), 2012/07/12 19:37:55]

DCHECK(video_configs_.empty())?

[acolwell GONE FROM CHROMIUM, 2012/07/12 23:07:58]

Done.

+
+  if (audio_configs_[0]->codec() != config.codec()) {
+    DVLOG(1) << "UpdateAudioConfig() : Codec changes not allowed.";
+    return false;
+  }
+
+  if (audio_configs_[0]->samples_per_second() != config.samples_per_second()) {
+    DVLOG(1) << "UpdateAudioConfig() : Sample rate changes not allowed.";
+    return false;
+  }
+
+  if (audio_configs_[0]->channel_layout() != config.channel_layout()) {
+    DVLOG(1) << "UpdateAudioConfig() : Channel layout changes not allowed.";
+    return false;
+  }
+
+  if (audio_configs_[0]->bits_per_channel() != config.bits_per_channel()) {
+    DVLOG(1) << "UpdateAudioConfig() : Bits per channel changes not allowed.";
+    return false;
+  }
+
+  // Check to see if the new config matches an existing one.
+  for (size_t i = 0; i < audio_configs_.size(); ++i) {
+    const AudioDecoderConfig& existing_config = *audio_configs_[i];
+    if ((existing_config.codec() == config.codec()) &&
+        (existing_config.bits_per_channel() == config.bits_per_channel()) &&
+        (existing_config.channel_layout() == config.channel_layout()) &&
+        (existing_config.samples_per_second() == config.samples_per_second()) &&
+        (existing_config.extra_data_size() == config.extra_data_size()) &&
+        (!existing_config.extra_data() ||
+         !memcmp(existing_config.extra_data(), config.extra_data(),
+                 existing_config.extra_data_size()))) {

[xhwang, 2012/07/12 18:09:12]

How about having helper functions for this and below (793)? e.g.
VideoDecoderConfig::Matches(const VideoDecoderConfig&)

[acolwell GONE FROM CHROMIUM, 2012/07/12 23:07:58]

Done.

+      append_config_index_ = i;
+      return true;
+    }
+  }
+
+  // No matches found so lets add this one to the list.

[vrk (LEFT CHROMIUM), 2012/07/12 19:37:55]

nit: "let's"

[acolwell GONE FROM CHROMIUM, 2012/07/12 23:07:58]

Done.

+  append_config_index_ = audio_configs_.size();
+  audio_configs_.resize(audio_configs_.size() + 1);
+  audio_configs_[append_config_index_] = new AudioDecoderConfig();
+  audio_configs_[append_config_index_]->CopyFrom(config);
+  return true;
+}
+
+bool SourceBufferStream::UpdateVideoConfig(const VideoDecoderConfig& config) {
+  DCHECK(!video_configs_.empty());

[vrk (LEFT CHROMIUM), 2012/07/12 19:37:55]

DCHECK(audio_configs_.empty())?

[acolwell GONE FROM CHROMIUM, 2012/07/12 23:07:58]

Done.

+
+  if (video_configs_[0]->codec() != config.codec()) {
+    DVLOG(1) << "UpdateVideoConfig() : Codec changes not allowed.";
+    return false;
+  }
+
+  // Check to see if the new config matches an existing one.
+  for (size_t i = 0; i < video_configs_.size(); ++i) {
+    const VideoDecoderConfig& existing_config = *video_configs_[i];
+    if ((existing_config.codec() == config.codec()) &&
+        (existing_config.format() == config.format()) &&
+        (existing_config.profile() == config.profile()) &&
+        (existing_config.coded_size() == config.coded_size()) &&
+        (existing_config.visible_rect() == config.visible_rect()) &&
+        (existing_config.natural_size() == config.natural_size()) &&
+        (existing_config.extra_data_size() == config.extra_data_size()) &&
+        (!existing_config.extra_data() ||
+         !memcmp(existing_config.extra_data(), config.extra_data(),
+                 existing_config.extra_data_size()))) {
+      append_config_index_ = i;
+      return true;
+    }
+  }
+
+  // No matches found so lets add this one to the list.

[vrk (LEFT CHROMIUM), 2012/07/12 19:37:55]

nit: "let's"

[acolwell GONE FROM CHROMIUM, 2012/07/12 23:07:58]

Done.

+  append_config_index_ = video_configs_.size();
+  video_configs_.resize(video_configs_.size() + 1);
+  video_configs_[append_config_index_] = new VideoDecoderConfig();
+  video_configs_[append_config_index_]->CopyFrom(config);
+  return true;
+}
+
+void SourceBufferStream::UpdateCurrentConfigIndex() {
+  if (!track_buffer_.empty()) {
+    current_config_index_ = track_buffer_.front()->GetConfigID();
+    return;
+  }
+
+  if (!selected_range_ || !selected_range_->HasNextBuffer())
+    return;
+
+  current_config_index_ = selected_range_->GetNextConfigID();
+}
+
 SourceBufferRange::SourceBufferRange(
     const BufferQueue& new_buffers, base::TimeDelta media_segment_start_time,
     const InterbufferDistanceCB& interbuffer_distance_cb)
@@ -888,6 +1020,12 @@ bool SourceBufferRange::HasNextBuffer() const {
          next_buffer_index_ < static_cast<int>(buffers_.size());

[vrk (LEFT CHROMIUM), 2012/07/12 19:37:55]

nit: indentation + add "&& !waiting_for_keyframe_" just to be safe.

[acolwell GONE FROM CHROMIUM, 2012/07/12 23:07:58]

Done. Changed GetNextBuffer() to use this now that the conditions match.

 }
 
+int SourceBufferRange::GetNextConfigID() const {
+  DCHECK(HasNextBuffer());
+  return buffers_.at(next_buffer_index_)->GetConfigID();
+}
+
+
 base::TimeDelta SourceBufferRange::GetNextTimestamp() const {
   DCHECK(!buffers_.empty());
   DCHECK(HasNextBufferPosition());