media: Support better decoder switching

media/filters/decoder_selector.cc

Index: media/filters/decoder_selector.cc
diff --git a/media/filters/decoder_selector.cc b/media/filters/decoder_selector.cc
index 925ea499b17c4a3a130e14805df4c24834b4b10d..66186d92bbd01141a5927f61d4e8554dffb71b59 100644
--- a/media/filters/decoder_selector.cc
+++ b/media/filters/decoder_selector.cc
@@ -43,10 +43,10 @@ static bool HasValidStreamConfig(DemuxerStream* stream) {
 template <DemuxerStream::Type StreamType>
 DecoderSelector<StreamType>::DecoderSelector(
     const scoped_refptr<base::SingleThreadTaskRunner>& task_runner,
-    ScopedVector<Decoder> decoders,
+    CreateDecodersCB create_decoders_cb,
     MediaLog* media_log)
     : task_runner_(task_runner),
-      decoders_(std::move(decoders)),
+      create_decoders_cb_(std::move(create_decoders_cb)),
       media_log_(media_log),
       input_stream_(nullptr),
       weak_ptr_factory_(this) {}
@@ -68,18 +68,17 @@ void DecoderSelector<StreamType>::SelectDecoder(
     StreamTraits* traits,
     DemuxerStream* stream,
     CdmContext* cdm_context,
+    const std::string& blacklisted_decoder,
     const SelectDecoderCB& select_decoder_cb,
     const typename Decoder::OutputCB& output_cb,
     const base::Closure& waiting_for_decryption_key_cb) {
-  DVLOG(2) << __func__;
+  DVLOG(2) << __func__ << ": cdm_context=" << cdm_context
+           << ", blacklisted_decoder=" << blacklisted_decoder;
   DCHECK(task_runner_->BelongsToCurrentThread());
   DCHECK(traits);
   DCHECK(stream);
   DCHECK(select_decoder_cb_.is_null());
 
-  cdm_context_ = cdm_context;
-  waiting_for_decryption_key_cb_ = waiting_for_decryption_key_cb;
-
   // Make sure |select_decoder_cb| runs on a different execution stack.
   select_decoder_cb_ = BindToCurrentLoop(select_decoder_cb);
 
@@ -91,11 +90,20 @@ void DecoderSelector<StreamType>::SelectDecoder(
 
   traits_ = traits;
   input_stream_ = stream;
+  cdm_context_ = cdm_context;
+  blacklisted_decoder_ = blacklisted_decoder;
   output_cb_ = output_cb;
+  waiting_for_decryption_key_cb_ = waiting_for_decryption_key_cb;
+
+  decoders_ = create_decoders_cb_.Run();
+  config_ = StreamTraits::GetDecoderConfig(input_stream_);
 
   // When there is a CDM attached, always try the decrypting decoder or
   // demuxer-stream first.
-  if (cdm_context_) {
+  if (config_.is_encrypted()) {
+    DCHECK(cdm_context_);
+// TODO(xhwang): This if-defined doesn't make a lot of sense. It should be
+// replaced by some better checks.
 #if !defined(DISABLE_FFMPEG_VIDEO_DECODERS)
     InitializeDecryptingDecoder();
 #else
@@ -104,11 +112,6 @@ void DecoderSelector<StreamType>::SelectDecoder(
     return;
   }
 
-  config_ = StreamTraits::GetDecoderConfig(input_stream_);
-
-  // If the input stream is encrypted, CdmContext must be non-null.
-  DCHECK(!config_.is_encrypted());
-
   InitializeDecoder();
 }
 
@@ -116,9 +119,16 @@ void DecoderSelector<StreamType>::SelectDecoder(
 template <DemuxerStream::Type StreamType>
 void DecoderSelector<StreamType>::InitializeDecryptingDecoder() {
   DVLOG(2) << __func__;
+
   decoder_.reset(new typename StreamTraits::DecryptingDecoderType(
       task_runner_, media_log_, waiting_for_decryption_key_cb_));
 
+  if (decoder_->GetDisplayName() == blacklisted_decoder_) {
+    DVLOG(1) << __func__ << ": Decryping decoder is blacklisted.";

[watk, 2017/05/04 21:35:33]

decrypting

[xhwang, 2017/05/04 23:53:13]

Done.

+    DecryptingDecoderInitDone(false);
+    return;
+  }
+
   traits_->InitializeDecoder(
       decoder_.get(), StreamTraits::GetDecoderConfig(input_stream_),
       input_stream_->liveness() == DemuxerStream::LIVENESS_LIVE, cdm_context_,
@@ -134,6 +144,7 @@ void DecoderSelector<StreamType>::DecryptingDecoderInitDone(bool success) {
 
   if (success) {
     DVLOG(1) << __func__ << ": " << decoder_->GetDisplayName() << " selected.";
+    decoders_.clear();
     base::ResetAndReturn(&select_decoder_cb_)
         .Run(std::move(decoder_), std::unique_ptr<DecryptingDemuxerStream>());
     return;
@@ -177,11 +188,7 @@ void DecoderSelector<StreamType>::DecryptingDemuxerStreamInitDone(
     DCHECK(!config_.is_encrypted());
   } else {
     decrypted_stream_.reset();
-    config_ = StreamTraits::GetDecoderConfig(input_stream_);
-
-    // Prefer decrypting decoder by using an encrypted config.
-    if (!config_.is_encrypted())
-      config_.SetIsEncrypted(true);
+    DCHECK(config_.is_encrypted());
   }
 
   InitializeDecoder();
@@ -193,14 +200,24 @@ void DecoderSelector<StreamType>::InitializeDecoder() {
   DCHECK(task_runner_->BelongsToCurrentThread());
   DCHECK(!decoder_);
 
-  if (decoders_.empty()) {
+  // Select the next non-blacklisted decoder.
+  while (!decoders_.empty()) {
+    std::unique_ptr<Decoder> decoder(decoders_.front());
+    decoders_.weak_erase(decoders_.begin());
+    // When |decrypted_stream_| is selected, the |config_| has changed so ignore
+    // the blacklist.
+    if (decrypted_stream_ ||
+        decoder->GetDisplayName() != blacklisted_decoder_) {
+      decoder_ = std::move(decoder);
+      break;
+    }
+  }
+
+  if (!decoder_) {
     ReturnNullDecoder();
     return;
   }
 
-  decoder_.reset(decoders_.front());
-  decoders_.weak_erase(decoders_.begin());
-
   traits_->InitializeDecoder(
       decoder_.get(), config_,
       input_stream_->liveness() == DemuxerStream::LIVENESS_LIVE, cdm_context_,
@@ -224,6 +241,7 @@ void DecoderSelector<StreamType>::DecoderInitDone(bool success) {
            << " selected. DecryptingDemuxerStream "
            << (decrypted_stream_ ? "also" : "not") << " selected.";
 
+  decoders_.clear();
   base::ResetAndReturn(&select_decoder_cb_)
       .Run(std::move(decoder_), std::move(decrypted_stream_));
 }
@@ -232,6 +250,7 @@ template <DemuxerStream::Type StreamType>
 void DecoderSelector<StreamType>::ReturnNullDecoder() {
   DVLOG(1) << __func__ << ": No decoder selected.";
   DCHECK(task_runner_->BelongsToCurrentThread());
+  decoders_.clear();

[watk, 2017/05/04 21:35:33]

Potentially, instead of saving decoders_ and making sure we clear it in every
case, we could thread the decoder vector through the InitializeDone callbacks
(remaining_decoders or something).

Actually, not worth doing if you make the change where we use a factory that
creates one decoder at a time.

[xhwang, 2017/05/04 23:53:13]

Acknowledged.

   base::ResetAndReturn(&select_decoder_cb_)
       .Run(std::unique_ptr<Decoder>(),
            std::unique_ptr<DecryptingDemuxerStream>());