Encrypted Media: Support config change in DecryptingDemuxerStream.

media/filters/decrypting_demuxer_stream.cc

Index: media/filters/decrypting_demuxer_stream.cc
diff --git a/media/filters/decrypting_demuxer_stream.cc b/media/filters/decrypting_demuxer_stream.cc
index 53e921c2f4c5d09264342b6d9341b399ffbb2426..5fb9607419fb28bb283b7358dfd91b317815f59f 100644
--- a/media/filters/decrypting_demuxer_stream.cc
+++ b/media/filters/decrypting_demuxer_stream.cc
@@ -22,6 +22,16 @@ namespace media {
 #define BIND_TO_LOOP(function) \
     media::BindToLoop(message_loop_, base::Bind(function, this))
 
+static bool IsStreamValidAndEncrypted(
+    const scoped_refptr<DemuxerStream>& stream) {
+  return ((stream->type() == DemuxerStream::AUDIO &&
+           stream->audio_decoder_config().IsValidConfig() &&
+           stream->audio_decoder_config().is_encrypted()) ||
+          (stream->type() == DemuxerStream::VIDEO &&
+           stream->video_decoder_config().IsValidConfig() &&
+           stream->video_decoder_config().is_encrypted()));
+}
+
 DecryptingDemuxerStream::DecryptingDemuxerStream(
     const scoped_refptr<base::MessageLoopProxy>& message_loop,
     const SetDecryptorReadyCB& set_decryptor_ready_cb)
@@ -122,22 +132,16 @@ void DecryptingDemuxerStream::DoInitialize(
   DCHECK(message_loop_->BelongsToCurrentThread());
   DCHECK_EQ(state_, kUninitialized) << state_;
 
-  // Only valid potentially encrypted audio or video stream is accepted.
-  if (!((stream->type() == AUDIO &&
-         stream->audio_decoder_config().IsValidConfig() &&
-         stream->audio_decoder_config().is_encrypted()) ||
-        (stream->type() == VIDEO &&
-         stream->video_decoder_config().IsValidConfig() &&
-         stream->video_decoder_config().is_encrypted()))) {
-    status_cb.Run(DEMUXER_ERROR_NO_SUPPORTED_STREAMS);
-    return;
-  }
+  // Decoder selector makes sure the stream is valid and potentially encrypted.
+  DCHECK(IsStreamValidAndEncrypted(stream));

[ddorwin, 2013/01/03 17:48:29]

WDYT about moving this inside SetDecoderConfig() now that they are always called
together?

[xhwang, 2013/01/03 18:16:04]

Done.

 
   DCHECK(!demuxer_stream_);
   demuxer_stream_ = stream;
   stream_type_ = stream->type();
   init_cb_ = status_cb;
 
+  SetDecoderConfig();
+
   state_ = kDecryptorRequested;
   set_decryptor_ready_cb_.Run(
       BIND_TO_LOOP(&DecryptingDemuxerStream::SetDecryptor));
@@ -153,44 +157,6 @@ void DecryptingDemuxerStream::SetDecryptor(Decryptor* decryptor) {
   set_decryptor_ready_cb_.Reset();
   decryptor_ = decryptor;
 
-  switch (stream_type_) {
-    case AUDIO: {
-      const AudioDecoderConfig& input_audio_config =
-          demuxer_stream_->audio_decoder_config();
-      audio_config_.reset(new AudioDecoderConfig());
-      audio_config_->Initialize(input_audio_config.codec(),
-                                input_audio_config.bits_per_channel(),
-                                input_audio_config.channel_layout(),
-                                input_audio_config.samples_per_second(),
-                                input_audio_config.extra_data(),
-                                input_audio_config.extra_data_size(),
-                                false,  // Output audio is not encrypted.
-                                false);
-      break;
-    }
-
-    case VIDEO: {
-      const VideoDecoderConfig& input_video_config =
-          demuxer_stream_->video_decoder_config();
-      video_config_.reset(new VideoDecoderConfig());
-      video_config_->Initialize(input_video_config.codec(),
-                                input_video_config.profile(),
-                                input_video_config.format(),
-                                input_video_config.coded_size(),
-                                input_video_config.visible_rect(),
-                                input_video_config.natural_size(),
-                                input_video_config.extra_data(),
-                                input_video_config.extra_data_size(),
-                                false,  // Output video is not encrypted.
-                                false);
-      break;
-    }
-
-    default:
-      NOTREACHED();
-      return;
-  }
-
   decryptor_->RegisterNewKeyCB(
       GetDecryptorStreamType(),
       BIND_TO_LOOP(&DecryptingDemuxerStream::OnKeyAdded));
@@ -235,9 +201,15 @@ void DecryptingDemuxerStream::DoDecryptBuffer(
 
   if (status == kConfigChanged) {
     DVLOG(2) << "DoDecryptBuffer() - kConfigChanged.";
+    DCHECK_EQ(demuxer_stream_->type(), stream_type_);
+    // The demuxer should make sure the stream is still valid and potentially
+    // encrypted.
+    DCHECK(IsStreamValidAndEncrypted(demuxer_stream_));
+
+    // Update the decoder config with the new config.

[ddorwin, 2013/01/03 17:48:29]

This comment doesn't really add anything that the function name doesn't provide.
I would suggest something similar to your response to my comment: "Update the
decoder config, which the decoder will use when it is notified of
kConfigChanged."

[xhwang, 2013/01/03 18:16:04]

Done.

+    SetDecoderConfig();

[ddorwin, 2013/01/03 17:48:29]

SetDecoderConfigFromDemuxerStream? Should Set be Update?
I know this is long, but it's kind of odd to have a Set function that has no
parameters.
Alternatively, you could pass demuxer_stream_ as a parameter. That leaves the
function generic and makes it clear at the call site what value is being used.
It also makes it similar to the IsStreamValidAndEncrypted() calls that precede
it.

[xhwang, 2013/01/03 18:16:04]

Done.

     state_ = kIdle;
-    // TODO(xhwang): Support kConfigChanged!
-    base::ResetAndReturn(&read_cb_).Run(kAborted, NULL);
+    base::ResetAndReturn(&read_cb_).Run(kConfigChanged, NULL);
     return;
   }
 
@@ -348,4 +320,44 @@ Decryptor::StreamType DecryptingDemuxerStream::GetDecryptorStreamType() const {
   return stream_type_ == AUDIO ? Decryptor::kAudio : Decryptor::kVideo;
 }
 
+void DecryptingDemuxerStream::SetDecoderConfig() {
+  switch (stream_type_) {
+    case AUDIO: {
+      const AudioDecoderConfig& input_audio_config =
+          demuxer_stream_->audio_decoder_config();
+      audio_config_.reset(new AudioDecoderConfig());
+      audio_config_->Initialize(input_audio_config.codec(),
+                                input_audio_config.bits_per_channel(),
+                                input_audio_config.channel_layout(),
+                                input_audio_config.samples_per_second(),
+                                input_audio_config.extra_data(),
+                                input_audio_config.extra_data_size(),
+                                false,  // Output audio is not encrypted.
+                                false);
+      break;
+    }
+
+    case VIDEO: {
+      const VideoDecoderConfig& input_video_config =
+          demuxer_stream_->video_decoder_config();
+      video_config_.reset(new VideoDecoderConfig());
+      video_config_->Initialize(input_video_config.codec(),
+                                input_video_config.profile(),
+                                input_video_config.format(),
+                                input_video_config.coded_size(),
+                                input_video_config.visible_rect(),
+                                input_video_config.natural_size(),
+                                input_video_config.extra_data(),
+                                input_video_config.extra_data_size(),
+                                false,  // Output video is not encrypted.
+                                false);
+      break;
+    }
+
+    default:
+      NOTREACHED();
+      return;
+  }
+}
+
 }  // namespace media