Encrypted Media: Support config change in DecryptingDemuxerStream.

media/filters/decrypting_demuxer_stream.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/filters/decrypting_demuxer_stream.h"
 
 #include "base/bind.h"
 #include "base/callback_helpers.h"
 #include "base/location.h"
 #include "base/logging.h"
 #include "base/message_loop_proxy.h"
 #include "media/base/audio_decoder_config.h"
 #include "media/base/video_decoder_config.h"
 #include "media/base/bind_to_loop.h"
 #include "media/base/decoder_buffer.h"
 #include "media/base/decryptor.h"
 #include "media/base/demuxer_stream.h"
 #include "media/base/pipeline.h"
 
 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)
     : message_loop_(message_loop),
       state_(kUninitialized),
       stream_type_(UNKNOWN),
       set_decryptor_ready_cb_(set_decryptor_ready_cb),
       decryptor_(NULL),
       key_added_while_decrypt_pending_(false) {
 }
@@ skipping 80 matching lines @@
 
 DecryptingDemuxerStream::~DecryptingDemuxerStream() {}
 
 void DecryptingDemuxerStream::DoInitialize(
     const scoped_refptr<DemuxerStream>& stream,
     const PipelineStatusCB& status_cb) {
   DVLOG(2) << "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));
 }
 
 void DecryptingDemuxerStream::SetDecryptor(Decryptor* decryptor) {
   DVLOG(2) << "SetDecryptor()";
   DCHECK(message_loop_->BelongsToCurrentThread());
   DCHECK_EQ(state_, kDecryptorRequested) << state_;
   DCHECK(!init_cb_.is_null());
   DCHECK(!set_decryptor_ready_cb_.is_null());
 
   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));
 
   state_ = kIdle;
   base::ResetAndReturn(&init_cb_).Run(PIPELINE_OK);
 }
 
 void DecryptingDemuxerStream::DecryptBuffer(
     DemuxerStream::Status status,
@@ skipping 24 matching lines @@
 
   if (status == kAborted) {
     DVLOG(2) << "DoDecryptBuffer() - kAborted.";
     state_ = kIdle;
     base::ResetAndReturn(&read_cb_).Run(kAborted, NULL);
     return;
   }
 
   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;
   }
 
   if (buffer->IsEndOfStream()) {
     DVLOG(2) << "DoDecryptBuffer() - EOS buffer.";
     state_ = kIdle;
     base::ResetAndReturn(&read_cb_).Run(status, buffer);
     return;
   }
 
@@ skipping 90 matching lines @@
   DCHECK(read_cb_.is_null());
   state_ = kIdle;
   base::ResetAndReturn(&reset_cb_).Run();
 }
 
 Decryptor::StreamType DecryptingDemuxerStream::GetDecryptorStreamType() const {
   DCHECK(stream_type_ == AUDIO || stream_type_ == VIDEO);
   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