Move encrypted decoder setup outside of DecoderSelector.

This removes the hardcoded setup of the decrypting audio
and video decoders out of the DecoderSelector and into the
standard decoder list.

This functions similarly to the old behavior, but instead of
knowing except for the following differences:
- All decoders are checked for encrypted support first now.
- If no decoder succeeds in initialization with an encrypted
configuration, we'll try creating a DecryptingDemuxerStream,
if this succeeds we'll check the decoder list again, if it
fails, we'll abort as before.

Mainly this implies that falling back to decrypt-only will
take slightly longer than before. This also adds back support
for EME to use GpuMemoryBuffers.

BUG=801245
TEST=updated lots of unittests...

Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: I578809aeebaa59d81f65a5be2f2f1f76713d0f68
Reviewed-on: https://chromium-review.googlesource.com/952207Reviewed-by: Xiaohan Wang <xhwang@chromium.org>
Commit-Queue: Dale Curtis <dalecurtis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#542323}

media/filters/decoder_selector.cc

@@ -20,11 +20,6 @@
 #include "media/filters/decoder_stream_traits.h"
 #include "media/filters/decrypting_demuxer_stream.h"
 
-#if !defined(OS_ANDROID)
-#include "media/filters/decrypting_audio_decoder.h"
-#include "media/filters/decrypting_video_decoder.h"
-#endif
-
 namespace media {
 
 static bool HasValidStreamConfig(DemuxerStream* stream) {
@@ -48,7 +43,6 @@ DecoderSelector<StreamType>::DecoderSelector(
     : task_runner_(task_runner),
       create_decoders_cb_(std::move(create_decoders_cb)),
       media_log_(media_log),
-      input_stream_(nullptr),
       weak_ptr_factory_(this) {}
 
 template <DemuxerStream::Type StreamType>
@@ -98,99 +92,6 @@ void DecoderSelector<StreamType>::SelectDecoder(
   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 (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(OS_ANDROID)
-    InitializeDecryptingDecoder();
-#else
-    InitializeDecryptingDemuxerStream();
-#endif
-    return;
-  }
-
-  InitializeDecoder();
-}
-
-#if !defined(OS_ANDROID)
-template <DemuxerStream::Type StreamType>
-void DecoderSelector<StreamType>::InitializeDecryptingDecoder() {
-  DVLOG(2) << __func__;
-
-  decoder_.reset(new typename StreamTraits::DecryptingDecoderType(task_runner_,
-                                                                  media_log_));
-
-  if (decoder_->GetDisplayName() == blacklisted_decoder_) {
-    DVLOG(1) << __func__ << ": Decrypting decoder is blacklisted.";
-    DecryptingDecoderInitDone(false);
-    return;
-  }
-
-  traits_->InitializeDecoder(
-      decoder_.get(), StreamTraits::GetDecoderConfig(input_stream_),
-      input_stream_->liveness() == DemuxerStream::LIVENESS_LIVE, cdm_context_,
-      base::Bind(&DecoderSelector<StreamType>::DecryptingDecoderInitDone,
-                 weak_ptr_factory_.GetWeakPtr()),
-      output_cb_, waiting_for_decryption_key_cb_);
-}
-
-template <DemuxerStream::Type StreamType>
-void DecoderSelector<StreamType>::DecryptingDecoderInitDone(bool success) {
-  DVLOG(2) << __func__ << ": success=" << success;
-  DCHECK(task_runner_->BelongsToCurrentThread());
-
-  if (success) {
-    DVLOG(1) << __func__ << ": " << decoder_->GetDisplayName() << " selected.";
-    decoders_.clear();
-    base::ResetAndReturn(&select_decoder_cb_)
-        .Run(std::move(decoder_), std::unique_ptr<DecryptingDemuxerStream>());
-    return;
-  }
-
-  decoder_.reset();
-
-  // When we get here decrypt-and-decode is not supported. Try to use
-  // DecryptingDemuxerStream to do decrypt-only.
-  InitializeDecryptingDemuxerStream();
-}
-#endif  // !defined(OS_ANDROID)
-
-template <DemuxerStream::Type StreamType>
-void DecoderSelector<StreamType>::InitializeDecryptingDemuxerStream() {
-  decrypted_stream_.reset(new DecryptingDemuxerStream(
-      task_runner_, media_log_, waiting_for_decryption_key_cb_));
-
-  decrypted_stream_->Initialize(
-      input_stream_, cdm_context_,
-      base::Bind(&DecoderSelector<StreamType>::DecryptingDemuxerStreamInitDone,
-                 weak_ptr_factory_.GetWeakPtr()));
-}
-
-template <DemuxerStream::Type StreamType>
-void DecoderSelector<StreamType>::DecryptingDemuxerStreamInitDone(
-    PipelineStatus status) {
-  DVLOG(2) << __func__
-           << ": status=" << MediaLog::PipelineStatusToString(status);
-  DCHECK(task_runner_->BelongsToCurrentThread());
-  DCHECK(cdm_context_);
-
-  // If DecryptingDemuxerStream initialization succeeded, we'll use it to do
-  // decryption and use a decoder to decode the clear stream. Otherwise, we'll
-  // try to see whether any decoder can decrypt-and-decode the encrypted stream
-  // directly. So in both cases, we'll initialize the decoders.
-
-  if (status == PIPELINE_OK) {
-    input_stream_ = decrypted_stream_.get();
-    config_ = StreamTraits::GetDecoderConfig(input_stream_);
-    DCHECK(!config_.is_encrypted());
-  } else {
-    decrypted_stream_.reset();
-    DCHECK(config_.is_encrypted());
-  }
-
   InitializeDecoder();
 }
 
@@ -214,6 +115,12 @@ void DecoderSelector<StreamType>::InitializeDecoder() {
   }
 
   if (!decoder_) {
+    // No decoder could handle encrypted content, try to do decrypt-only.
+    if (!tried_decrypting_demuxer_stream_ && config_.is_encrypted()) {
+      InitializeDecryptingDemuxerStream();
+      return;
+    }
+
     ReturnNullDecoder();
     return;
   }
@@ -256,6 +163,53 @@ void DecoderSelector<StreamType>::ReturnNullDecoder() {
            std::unique_ptr<DecryptingDemuxerStream>());
 }
 
+template <DemuxerStream::Type StreamType>
+void DecoderSelector<StreamType>::InitializeDecryptingDemuxerStream() {
+  decrypted_stream_.reset(new DecryptingDemuxerStream(
+      task_runner_, media_log_, waiting_for_decryption_key_cb_));
+
+  decrypted_stream_->Initialize(
+      input_stream_, cdm_context_,
+      base::Bind(&DecoderSelector<StreamType>::DecryptingDemuxerStreamInitDone,
+                 weak_ptr_factory_.GetWeakPtr()));
+}
+
+template <DemuxerStream::Type StreamType>
+void DecoderSelector<StreamType>::DecryptingDemuxerStreamInitDone(
+    PipelineStatus status) {
+  DVLOG(2) << __func__
+           << ": status=" << MediaLog::PipelineStatusToString(status);
+  DCHECK(task_runner_->BelongsToCurrentThread());
+  DCHECK(cdm_context_);
+
+  // If DecryptingDemuxerStream initialization failed, we've already tried every
+  // possible decoder, so we can just ReturnNullDecoder() here.
+  if (status != PIPELINE_OK) {
+    DCHECK(decoders_.empty());
+    DCHECK(config_.is_encrypted());
+    decrypted_stream_.reset();
+    ReturnNullDecoder();
+    return;
+  }
+
+  // If DecryptingDemuxerStream initialization succeeded, we'll use it to do
+  // decryption and use a decoder to decode the clear stream. Otherwise, we'll
+  // try to see whether any decoder can decrypt-and-decode the encrypted stream
+  // directly. So in both cases, we'll initialize the decoders.
+  input_stream_ = decrypted_stream_.get();
+  config_ = StreamTraits::GetDecoderConfig(input_stream_);
+  DCHECK(!config_.is_encrypted());
+
+  // If we're here we tried all the decoders w/ is_encrypted=true, try again
+  // now that the stream is being decrypted by the demuxer.
+  DCHECK(decoders_.empty());
+  DCHECK(!tried_decrypting_demuxer_stream_);
+  decoders_ = create_decoders_cb_.Run();
+  tried_decrypting_demuxer_stream_ = true;
+
+  InitializeDecoder();
+}
+
 // These forward declarations tell the compiler that we will use
 // DecoderSelector with these arguments, allowing us to keep these definitions
 // in our .cc without causing linker errors. This also means if anyone tries to

media/filters/decoder_selector.h

@@ -12,7 +12,6 @@
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/weak_ptr.h"
-#include "build/build_config.h"
 #include "media/base/demuxer_stream.h"
 #include "media/base/pipeline_status.h"
 #include "media/filters/decoder_stream_traits.h"
@@ -91,15 +90,11 @@ class MEDIA_EXPORT DecoderSelector {
                      const base::Closure& waiting_for_decryption_key_cb);
 
  private:
-#if !defined(OS_ANDROID)
-  void InitializeDecryptingDecoder();
-  void DecryptingDecoderInitDone(bool success);
-#endif
-  void InitializeDecryptingDemuxerStream();
-  void DecryptingDemuxerStreamInitDone(PipelineStatus status);
   void InitializeDecoder();
   void DecoderInitDone(bool success);
   void ReturnNullDecoder();
+  void InitializeDecryptingDemuxerStream();
+  void DecryptingDemuxerStreamInitDone(PipelineStatus status);
 
   scoped_refptr<base::SingleThreadTaskRunner> task_runner_;
   CreateDecodersCB create_decoders_cb_;
@@ -109,7 +104,7 @@ class MEDIA_EXPORT DecoderSelector {
 
   // Could be the |stream| passed in SelectDecoder, or |decrypted_stream_| when
   // a DecryptingDemuxerStream is selected.
-  DemuxerStream* input_stream_;
+  DemuxerStream* input_stream_ = nullptr;
 
   CdmContext* cdm_context_;
   std::string blacklisted_decoder_;
@@ -125,6 +120,9 @@ class MEDIA_EXPORT DecoderSelector {
   // Config of the |input_stream| used to initialize decoders.
   DecoderConfig config_;
 
+  // Indicates if we tried to initialize |decrypted_stream_|.
+  bool tried_decrypting_demuxer_stream_ = false;
+
   // NOTE: Weak pointers must be invalidated before all other member variables.
   base::WeakPtrFactory<DecoderSelector> weak_ptr_factory_;
 

media/filters/decoder_stream_traits.h

@@ -21,8 +21,6 @@ namespace media {
 class AudioBuffer;
 class AudioDecoderConfig;
 class CdmContext;
-class DecryptingAudioDecoder;
-class DecryptingVideoDecoder;
 class DemuxerStream;
 class VideoDecoderConfig;
 class VideoFrame;
@@ -38,7 +36,6 @@ class MEDIA_EXPORT DecoderStreamTraits<DemuxerStream::AUDIO> {
   using OutputType = AudioBuffer;
   using DecoderType = AudioDecoder;
   using DecoderConfigType = AudioDecoderConfig;
-  using DecryptingDecoderType = DecryptingAudioDecoder;
   using InitCB = AudioDecoder::InitCB;
   using OutputCB = AudioDecoder::OutputCB;
   using WaitingForDecryptionKeyCB = AudioDecoder::WaitingForDecryptionKeyCB;
@@ -79,7 +76,6 @@ class MEDIA_EXPORT DecoderStreamTraits<DemuxerStream::VIDEO> {
   using OutputType = VideoFrame;
   using DecoderType = VideoDecoder;
   using DecoderConfigType = VideoDecoderConfig;
-  using DecryptingDecoderType = DecryptingVideoDecoder;
   using InitCB = VideoDecoder::InitCB;
   using OutputCB = VideoDecoder::OutputCB;
   using WaitingForDecryptionKeyCB = VideoDecoder::WaitingForDecryptionKeyCB;

media/filters/decrypting_audio_decoder.cc

@@ -42,6 +42,7 @@ DecryptingAudioDecoder::DecryptingAudioDecoder(
       state_(kUninitialized),
       decryptor_(NULL),
       key_added_while_decode_pending_(false),
+      support_clear_content_(false),
       weak_factory_(this) {}
 
 std::string DecryptingAudioDecoder::GetDisplayName() const {
@@ -58,10 +59,25 @@ void DecryptingAudioDecoder::Initialize(
   DCHECK(task_runner_->BelongsToCurrentThread());
   DCHECK(decode_cb_.is_null());
   DCHECK(reset_cb_.is_null());
-  DCHECK(cdm_context);
 
-  weak_this_ = weak_factory_.GetWeakPtr();
   init_cb_ = BindToCurrentLoop(init_cb);
+  if (!cdm_context) {
+    // Once we have a CDM context, one should always be present.
+    DCHECK(!support_clear_content_);
+    base::ResetAndReturn(&init_cb_).Run(false);
+    return;
+  }
+
+  if (!config.is_encrypted() && !support_clear_content_) {
+    base::ResetAndReturn(&init_cb_).Run(false);
+    return;
+  }
+
+  // Once initialized with encryption support, the value is sticky, so we'll use
+  // the decryptor for clear content as well.
+  support_clear_content_ = true;
+
+  weak_this_ = weak_factory_.GetWeakPtr();
   output_cb_ = BindToCurrentLoop(output_cb);
 
   DCHECK(!waiting_for_decryption_key_cb.is_null());

media/filters/decrypting_audio_decoder.h

@@ -118,6 +118,10 @@ class MEDIA_EXPORT DecryptingAudioDecoder : public AudioDecoder {
 
   std::unique_ptr<AudioTimestampHelper> timestamp_helper_;
 
+  // Once Initialized() with encrypted content support, if the stream changes to
+  // clear content, we want to ensure this decoder remains used.
+  bool support_clear_content_;
+
   base::WeakPtr<DecryptingAudioDecoder> weak_this_;
   base::WeakPtrFactory<DecryptingAudioDecoder> weak_factory_;
 

media/filters/decrypting_video_decoder.cc

@@ -30,6 +30,7 @@ DecryptingVideoDecoder::DecryptingVideoDecoder(
       decryptor_(NULL),
       key_added_while_decode_pending_(false),
       trace_id_(0),
+      support_clear_content_(false),
       weak_factory_(this) {}
 
 std::string DecryptingVideoDecoder::GetDisplayName() const {
@@ -52,9 +53,24 @@ void DecryptingVideoDecoder::Initialize(
   DCHECK(decode_cb_.is_null());
   DCHECK(reset_cb_.is_null());
   DCHECK(config.IsValidConfig());
-  DCHECK(cdm_context);
 
   init_cb_ = BindToCurrentLoop(init_cb);
+  if (!cdm_context) {
+    // Once we have a CDM context, one should always be present.
+    DCHECK(!support_clear_content_);
+    base::ResetAndReturn(&init_cb_).Run(false);
+    return;
+  }
+
+  if (!config.is_encrypted() && !support_clear_content_) {
+    base::ResetAndReturn(&init_cb_).Run(false);
+    return;
+  }
+
+  // Once initialized with encryption support, the value is sticky, so we'll use
+  // the decryptor for clear content as well.
+  support_clear_content_ = true;
+
   output_cb_ = BindToCurrentLoop(output_cb);
   weak_this_ = weak_factory_.GetWeakPtr();
   config_ = config;

media/filters/decrypting_video_decoder.h

@@ -112,6 +112,10 @@ class MEDIA_EXPORT DecryptingVideoDecoder : public VideoDecoder {
   // matching DecryptCB call (in DoDeliverFrame()).
   uint32_t trace_id_;
 
+  // Once Initialized() with encrypted content support, if the stream changes to
+  // clear content, we want to ensure this decoder remains used.
+  bool support_clear_content_;
+
   base::WeakPtr<DecryptingVideoDecoder> weak_this_;
   base::WeakPtrFactory<DecryptingVideoDecoder> weak_factory_;
 

media/filters/fake_video_decoder.cc

@@ -80,7 +80,9 @@ void FakeVideoDecoder::Initialize(
 
   if (config.is_encrypted() && (!supports_encrypted_config_ || !cdm_context)) {
     DVLOG(1) << "Encrypted config not supported.";
-    fail_to_initialize_ = true;
+    state_ = STATE_NORMAL;
+    init_cb_.RunOrHold(false);
+    return;
   }
 
   if (fail_to_initialize_) {

media/filters/gpu_video_decoder.cc

@@ -187,9 +187,11 @@ void GpuVideoDecoder::Initialize(
 
   VideoDecodeAccelerator::Capabilities capabilities =
       factories_->GetVideoDecodeAcceleratorCapabilities();
-  if (config.is_encrypted() &&
-      !(capabilities.flags &
-        VideoDecodeAccelerator::Capabilities::SUPPORTS_ENCRYPTED_STREAMS)) {
+
+  const bool supports_encrypted_streams =
+      capabilities.flags &
+      VideoDecodeAccelerator::Capabilities::SUPPORTS_ENCRYPTED_STREAMS;
+  if (config.is_encrypted() && (!cdm_context || !supports_encrypted_streams)) {
     DVLOG(1) << "Encrypted stream not supported.";
     bound_init_cb.Run(false);
     return;

media/filters/video_frame_stream_unittest.cc

@@ -11,6 +11,7 @@
 #include "base/message_loop/message_loop.h"
 #include "base/run_loop.h"
 #include "base/strings/string_number_conversions.h"
+#include "build/build_config.h"
 #include "media/base/fake_demuxer_stream.h"
 #include "media/base/gmock_callback_support.h"
 #include "media/base/mock_filters.h"
@@ -21,6 +22,10 @@
 #include "media/filters/fake_video_decoder.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
+#if !defined(OS_ANDROID)
+#include "media/filters/decrypting_video_decoder.h"
+#endif
+
 using ::testing::_;
 using ::testing::AnyNumber;
 using ::testing::Assign;
@@ -141,6 +146,14 @@ class VideoFrameStreamTest
     // supports encrypted streams. Currently this is hard to test because we use
     // parameterized tests which need to pass in all combinations.
     std::vector<std::unique_ptr<VideoDecoder>> decoders;
+
+#if !defined(OS_ANDROID)
+    // Note this is _not_ inserted into |decoders_| below, so we don't need to
+    // adjust the indices used below to compensate.
+    decoders.push_back(std::make_unique<DecryptingVideoDecoder>(
+        message_loop_.task_runner(), &media_log_));
+#endif
+
     for (int i = 0; i < 3; ++i) {
       auto decoder = std::make_unique<FakeVideoDecoder>(
           GetDecoderName(i), GetParam().decoding_delay,
@@ -167,11 +180,13 @@ class VideoFrameStreamTest
     for (const auto& i : decoder_indices_to_hold_decode_)
       decoders_[i]->HoldDecode();
 
+    return decoders;
+  }
+
+  void ClearDecoderInitExpectations() {
     decoder_indices_to_fail_init_.clear();
     decoder_indices_to_hold_init_.clear();
     decoder_indices_to_hold_decode_.clear();
-
-    return decoders;
   }
 
   // On next decoder selection, fail initialization on decoders specified by
@@ -1187,6 +1202,7 @@ TEST_P(VideoFrameStreamTest, FallbackDecoder_SelectedOnInitThenDecodeErrors) {
   FailDecoderInitOnSelection({0});
   Initialize();
   ASSERT_EQ(GetDecoderName(1), decoder_->GetDisplayName());
+  ClearDecoderInitExpectations();
 
   decoder_->HoldDecode();
   ReadOneFrame();

media/renderers/default_renderer_factory.cc

@@ -25,6 +25,11 @@
 #include "media/video/gpu_video_accelerator_factories.h"
 #include "third_party/libaom/av1_features.h"
 
+#if !defined(OS_ANDROID)
+#include "media/filters/decrypting_audio_decoder.h"
+#include "media/filters/decrypting_video_decoder.h"
+#endif
+
 #if BUILDFLAG(ENABLE_AV1_DECODER)
 #include "media/filters/aom_video_decoder.h"
 #endif
@@ -76,6 +81,11 @@ DefaultRendererFactory::CreateAudioDecoders(
   // Create our audio decoders and renderer.
   std::vector<std::unique_ptr<AudioDecoder>> audio_decoders;
 
+#if !defined(OS_ANDROID)
+  audio_decoders.push_back(
+      std::make_unique<DecryptingAudioDecoder>(media_task_runner, media_log_));
+#endif
+
 #if BUILDFLAG(ENABLE_FFMPEG)
   audio_decoders.push_back(
       std::make_unique<FFmpegAudioDecoder>(media_task_runner, media_log_));
@@ -102,6 +112,12 @@ DefaultRendererFactory::CreateVideoDecoders(
   // Create our video decoders and renderer.
   std::vector<std::unique_ptr<VideoDecoder>> video_decoders;
 
+#if !defined(OS_ANDROID)
+  video_decoders.push_back(MaybeUseGpuMemoryBufferWrapper(
+      gpu_factories, media_task_runner, worker_task_runner,
+      std::make_unique<DecryptingVideoDecoder>(media_task_runner, media_log_)));
+#endif
+
   // Prefer an external decoder since one will only exist if it is hardware
   // accelerated.
   if (gpu_factories) {