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..a3123b26d427997a7cba95ab63d779b766f54969 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 IsStreamValidAndEncrytped(

[acolwell GONE FROM CHROMIUM, 2013/01/03 01:39:40]

nit: s/Encrytped/Encrypted/

[xhwang, 2013/01/03 17:13:54]

Done.

+    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,16 +132,8 @@ 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(IsStreamValidAndEncrytped(stream));
 
   DCHECK(!demuxer_stream_);
   demuxer_stream_ = stream;
@@ -153,43 +155,7 @@ 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;
-  }
+  SetDecoderConfig();

[ddorwin, 2013/01/03 01:45:36]

Why was/is this done here instead of in DoInitialize()?

[xhwang, 2013/01/03 17:13:54]

Good question. I don't see why not. Fixed.

 
   decryptor_->RegisterNewKeyCB(
       GetDecryptorStreamType(),
@@ -235,9 +201,16 @@ 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.
+    // TODO(xhwang): If needed, supported switching from encrypted stream to

[acolwell GONE FROM CHROMIUM, 2013/01/03 01:39:40]

nit: s/supported/support

[ddorwin, 2013/01/03 01:45:36]

Does this TODO belong in SourceBufferStream instead? It seems you have two close
locations that could change where you might need this comment in a couple
decryptor files. Or, we can just remove it (and maybe file a bug).

[xhwang, 2013/01/03 17:13:54]

Removed as I don't see any use case for that.

+    // clear stream.
+    DCHECK(IsStreamValidAndEncrytped(demuxer_stream_));
+
+    SetDecoderConfig();
     state_ = kIdle;
-    // TODO(xhwang): Support kConfigChanged!
-    base::ResetAndReturn(&read_cb_).Run(kAborted, NULL);
+    base::ResetAndReturn(&read_cb_).Run(kConfigChanged, NULL);

[ddorwin, 2013/01/03 01:45:36]

Where does the config change take place? Does the actual decoder pick up the
change by calling the accessor? Might be worth commenting what's expected to
happen to complete the process.

[xhwang, 2013/01/03 17:13:54]

Added comment. The decoder will get notified of kConfigChange, and it will call
xxx_decoder_config() to get the new config, which is updated on line 211.

     return;
   }
 
@@ -348,4 +321,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