Implement predicate-based decoder selection

This CL revamps the way a VideoDecoder or AudioDecoder is selected to decode a given VideoDecoderConfig or AudioDecoderConfig, respectively. The goal is to first filter and rank available decoders based on their high-level attributes ("IsPlatformDecoder", "SupportsDecryption"), rather than attempting to initialize them in the order they're submitted. The result is that an initial decoder is selected more efficiently, and the decoding system can gracefully transition between platform and non-platform decoders if the config changes mid-stream. The full design doc is available at: go/predicate-based-decoder-selection Bug: 684792 Change-Id: I8660e7315881b2bea44bf52afb4505f979a43728 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2247331 Commit-Queue: Will Cassella <[email protected]> Reviewed-by: Xiaohan Wang <[email protected]> Reviewed-by: Dale Curtis <[email protected]> Reviewed-by: Brian White <[email protected]> Cr-Commit-Position: refs/heads/master@{#784292}
hadissulejmani · Jul 1, 2020 · 36a8065 · 36a8065
1 parent 0d3e446
commit 36a8065
Show file tree

Hide file tree

Showing 36 changed files with 943 additions and 196 deletions.
diff --git a/media/base/BUILD.gn b/media/base/BUILD.gn
@@ -127,6 +127,8 @@ jumbo_source_set("base") {
     "data_source.h",
     "decode_status.cc",
     "decode_status.h",
+    "decoder.cc",
+    "decoder.h",
     "decoder_buffer.cc",
     "decoder_buffer.h",
     "decoder_buffer_queue.cc",

diff --git a/media/base/audio_decoder.cc b/media/base/audio_decoder.cc
@@ -12,10 +12,6 @@ AudioDecoder::AudioDecoder() = default;
 
 AudioDecoder::~AudioDecoder() = default;
 
-bool AudioDecoder::IsPlatformDecoder() const {
-  return false;
-}
-
 bool AudioDecoder::NeedsBitstreamConversion() const {
   return false;
 }

diff --git a/media/base/audio_decoder.h b/media/base/audio_decoder.h
@@ -13,6 +13,7 @@
 #include "media/base/audio_decoder_config.h"
 #include "media/base/channel_layout.h"
 #include "media/base/decode_status.h"
+#include "media/base/decoder.h"
 #include "media/base/decoder_buffer.h"
 #include "media/base/media_export.h"
 #include "media/base/pipeline_status.h"
@@ -24,9 +25,9 @@ namespace media {
 class AudioBuffer;
 class CdmContext;
 
-class MEDIA_EXPORT AudioDecoder {
+class MEDIA_EXPORT AudioDecoder : public Decoder {
  public:
-  // Callback for VideoDecoder initialization.
+  // Callback for AudioDecoder initialization.
   using InitCB = base::OnceCallback<void(Status)>;
 
   // Callback for AudioDecoder to return a decoded frame whenever it becomes
@@ -43,20 +44,7 @@ class MEDIA_EXPORT AudioDecoder {
   // Note: Since this is a destructor, |this| will be destroyed after this call.
   // Make sure the callbacks fired from this call doesn't post any task that
   // depends on |this|.
-  virtual ~AudioDecoder();
-
-  // Returns the name of the decoder for logging and decoder selection purposes.
-  // This name should be available immediately after construction (e.g. before
-  // Initialize() is called). It should also be stable in the sense that the
-  // name does not change across multiple constructions.
-  // TODO(xhwang): Rename this method since the name is not only for display.
-  virtual std::string GetDisplayName() const = 0;
-
-  // Returns true if the implementation is expected to be implemented by the
-  // platform. The value should be available immediately after construction and
-  // should not change within the lifetime of a decoder instance. The value is
-  // used only for logging.
-  virtual bool IsPlatformDecoder() const;
+  ~AudioDecoder() override;
 
   // Initializes an AudioDecoder with |config|, executing the |init_cb| upon
   // completion.

diff --git a/media/base/decoder.cc b/media/base/decoder.cc
@@ -0,0 +1,21 @@
+// Copyright 2020 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/base/decoder.h"
+
+namespace media {
+
+Decoder::Decoder() = default;
+
+Decoder::~Decoder() = default;
+
+bool Decoder::IsPlatformDecoder() const {
+  return false;
+}
+
+bool Decoder::SupportsDecryption() const {
+  return false;
+}
+
+}  // namespace media
diff --git a/media/base/decoder.h b/media/base/decoder.h
@@ -0,0 +1,44 @@
+// Copyright 2020 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.
+
+#ifndef MEDIA_BASE_DECODER_H_
+#define MEDIA_BASE_DECODER_H_
+
+#include <string>
+
+#include "base/macros.h"
+#include "media/base/media_export.h"
+
+namespace media {
+
+class MEDIA_EXPORT Decoder {
+ public:
+  virtual ~Decoder();
+
+  // Returns true if the implementation is expected to be implemented by the
+  // platform. The value should be available immediately after construction and
+  // should not change within the lifetime of a decoder instance.
+  virtual bool IsPlatformDecoder() const;
+
+  // Returns true if the implementation supports decoding configs with
+  // encryption.
+  // TODO(crbug.com/1099488): Sometimes it's not possible to give a definitive
+  // yes or no answer unless more context is given. While this doesn't pose any
+  // problems, it does allow incompatible decoders to pass the filtering step in
+  // |DecoderSelector| potentially slowing down the selection process.
+  virtual bool SupportsDecryption() const;
+
+  // Returns the name of the decoder for logging and decoder selection purposes.
+  // This name should be available immediately after construction, and should
+  // also be stable in the sense that the name does not change across multiple
+  // constructions.
+  virtual std::string GetDisplayName() const = 0;
+
+ protected:
+  Decoder();
+};
+
+}  // namespace media
+
+#endif  // MEDIA_BASE_DECODER_H_
diff --git a/media/base/fake_demuxer_stream.cc b/media/base/fake_demuxer_stream.cc
@@ -31,22 +31,36 @@ namespace media {
 
 const int kStartTimestampMs = 0;
 const int kDurationMs = 30;
-const int kStartWidth = 320;
-const int kStartHeight = 240;
-const int kWidthDelta = 4;
-const int kHeightDelta = 3;
+const int kDefaultStartWidth = 320;
+const int kDefaultStartHeight = 240;
+const int kDefaultWidthDelta = 4;
+const int kDefaultHeightDelta = 3;
 const uint8_t kKeyId[] = {0x00, 0x01, 0x02, 0x03};
 const uint8_t kIv[] = {0x20, 0x21, 0x22, 0x23, 0x24, 0x25, 0x26, 0x27,
                        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00};
 
 FakeDemuxerStream::FakeDemuxerStream(int num_configs,
                                      int num_buffers_in_one_config,
                                      bool is_encrypted)
+    : FakeDemuxerStream(
+          num_configs,
+          num_buffers_in_one_config,
+          is_encrypted,
+          gfx::Size(kDefaultStartWidth, kDefaultStartHeight),
+          gfx::Vector2dF(kDefaultWidthDelta, kDefaultHeightDelta)) {}
+
+FakeDemuxerStream::FakeDemuxerStream(int num_configs,
+                                     int num_buffers_in_one_config,
+                                     bool is_encrypted,
+                                     gfx::Size start_coded_size,
+                                     gfx::Vector2dF coded_size_delta)
     : task_runner_(base::ThreadTaskRunnerHandle::Get()),
       num_configs_(num_configs),
       num_buffers_in_one_config_(num_buffers_in_one_config),
       config_changes_(num_configs > 1),
       is_encrypted_(is_encrypted),
+      start_coded_size_(start_coded_size),
+      coded_size_delta_(coded_size_delta),
       read_to_hold_(-1) {
   DCHECK_GT(num_configs, 0);
   DCHECK_GT(num_buffers_in_one_config, 0);
@@ -63,7 +77,7 @@ void FakeDemuxerStream::Initialize() {
   num_buffers_returned_ = 0;
   current_timestamp_ = base::TimeDelta::FromMilliseconds(kStartTimestampMs);
   duration_ = base::TimeDelta::FromMilliseconds(kDurationMs);
-  next_coded_size_ = gfx::Size(kStartWidth, kStartHeight);
+  next_size_ = start_coded_size_;
   next_read_num_ = 0;
 }
 
@@ -160,14 +174,14 @@ void FakeDemuxerStream::SeekToEndOfStream() {
 }
 
 void FakeDemuxerStream::UpdateVideoDecoderConfig() {
-  const gfx::Rect kVisibleRect(kStartWidth, kStartHeight);
+  const gfx::Rect kVisibleRect(next_size_.width(), next_size_.height());
   video_decoder_config_.Initialize(
       kCodecVP8, VIDEO_CODEC_PROFILE_UNKNOWN,
       VideoDecoderConfig::AlphaMode::kIsOpaque, VideoColorSpace(),
-      kNoTransformation, next_coded_size_, kVisibleRect, next_coded_size_,
-      EmptyExtraData(),
+      kNoTransformation, next_size_, kVisibleRect, next_size_, EmptyExtraData(),
       is_encrypted_ ? EncryptionScheme::kCenc : EncryptionScheme::kUnencrypted);
-  next_coded_size_.Enlarge(kWidthDelta, kHeightDelta);
+  next_size_.set_width(next_size_.width() + coded_size_delta_.x());
+  next_size_.set_height(next_size_.height() + coded_size_delta_.y());
 }
 
 void FakeDemuxerStream::DoRead() {

diff --git a/media/base/fake_demuxer_stream.h b/media/base/fake_demuxer_stream.h
@@ -26,6 +26,19 @@ class FakeDemuxerStream : public DemuxerStream {
   FakeDemuxerStream(int num_configs,
                     int num_buffers_in_one_config,
                     bool is_encrypted);
+  // Constructs an object that outputs |num_configs| different configs in
+  // sequence with |num_frames_in_one_config| buffers for each config. The
+  // output buffers are encrypted if |is_encrypted| is true.
+  // The starting config |coded_size| is specified by the
+  // |start_coded_size| parameter, and each config change increases/decreases it
+  // by the |coded_size_delta| parameter.
+  // The returned config always has equal |coded_size| and |visible_rect|
+  // properties.
+  FakeDemuxerStream(int num_configs,
+                    int num_buffers_in_one_config,
+                    bool is_encrypted,
+                    gfx::Size start_coded_size,
+                    gfx::Vector2dF coded_size_delta);
   ~FakeDemuxerStream() override;
 
   // DemuxerStream implementation.
@@ -82,6 +95,8 @@ class FakeDemuxerStream : public DemuxerStream {
   const int num_buffers_in_one_config_;
   const bool config_changes_;
   const bool is_encrypted_;
+  const gfx::Size start_coded_size_;
+  const gfx::Vector2dF coded_size_delta_;
 
   int num_configs_left_;
 
@@ -93,7 +108,7 @@ class FakeDemuxerStream : public DemuxerStream {
   base::TimeDelta current_timestamp_;
   base::TimeDelta duration_;
 
-  gfx::Size next_coded_size_;
+  gfx::Size next_size_;
   VideoDecoderConfig video_decoder_config_;
 
   ReadCB read_cb_;

diff --git a/media/base/mock_filters.cc b/media/base/mock_filters.cc
@@ -72,22 +72,56 @@ void MockDemuxerStream::set_liveness(DemuxerStream::Liveness liveness) {
   liveness_ = liveness;
 }
 
-MockVideoDecoder::MockVideoDecoder(const std::string& decoder_name)
-    : decoder_name_(decoder_name) {
+MockVideoDecoder::MockVideoDecoder() : MockVideoDecoder("MockVideoDecoder") {}
+
+MockVideoDecoder::MockVideoDecoder(std::string decoder_name)
+    : MockVideoDecoder(false, false, std::move(decoder_name)) {}
+
+MockVideoDecoder::MockVideoDecoder(bool is_platform_decoder,
+                                   bool supports_decryption,
+                                   std::string decoder_name)
+    : is_platform_decoder_(is_platform_decoder),
+      supports_decryption_(supports_decryption),
+      decoder_name_(std::move(decoder_name)) {
   ON_CALL(*this, CanReadWithoutStalling()).WillByDefault(Return(true));
 }
 
 MockVideoDecoder::~MockVideoDecoder() = default;
 
+bool MockVideoDecoder::IsPlatformDecoder() const {
+  return is_platform_decoder_;
+}
+
+bool MockVideoDecoder::SupportsDecryption() const {
+  return supports_decryption_;
+}
+
 std::string MockVideoDecoder::GetDisplayName() const {
   return decoder_name_;
 }
 
-MockAudioDecoder::MockAudioDecoder(const std::string& decoder_name)
-    : decoder_name_(decoder_name) {}
+MockAudioDecoder::MockAudioDecoder() : MockAudioDecoder("MockAudioDecoder") {}
+
+MockAudioDecoder::MockAudioDecoder(std::string decoder_name)
+    : MockAudioDecoder(false, false, std::move(decoder_name)) {}
+
+MockAudioDecoder::MockAudioDecoder(bool is_platform_decoder,
+                                   bool supports_decryption,
+                                   std::string decoder_name)
+    : is_platform_decoder_(is_platform_decoder),
+      supports_decryption_(supports_decryption),
+      decoder_name_(decoder_name) {}
 
 MockAudioDecoder::~MockAudioDecoder() = default;
 
+bool MockAudioDecoder::IsPlatformDecoder() const {
+  return is_platform_decoder_;
+}
+
+bool MockAudioDecoder::SupportsDecryption() const {
+  return supports_decryption_;
+}
+
 std::string MockAudioDecoder::GetDisplayName() const {
   return decoder_name_;
 }

diff --git a/media/base/mock_filters.h b/media/base/mock_filters.h
@@ -216,12 +216,19 @@ class MockDemuxerStream : public DemuxerStream {
 
 class MockVideoDecoder : public VideoDecoder {
  public:
-  explicit MockVideoDecoder(
-      const std::string& decoder_name = "MockVideoDecoder");
+  MockVideoDecoder();
+  explicit MockVideoDecoder(std::string decoder_name);
+  MockVideoDecoder(bool is_platform_decoder,
+                   bool supports_decryption,
+                   std::string decoder_name);
   ~MockVideoDecoder() override;
 
-  // VideoDecoder implementation.
+  // Decoder implementation
+  bool IsPlatformDecoder() const override;
+  bool SupportsDecryption() const override;
   std::string GetDisplayName() const override;
+
+  // VideoDecoder implementation.
   void Initialize(const VideoDecoderConfig& config,
                   bool low_delay,
                   CdmContext* cdm_context,
@@ -248,18 +255,27 @@ class MockVideoDecoder : public VideoDecoder {
   MOCK_CONST_METHOD0(NeedsBitstreamConversion, bool());
 
  private:
-  std::string decoder_name_;
+  const bool is_platform_decoder_;
+  const bool supports_decryption_;
+  const std::string decoder_name_;
   DISALLOW_COPY_AND_ASSIGN(MockVideoDecoder);
 };
 
 class MockAudioDecoder : public AudioDecoder {
  public:
-  explicit MockAudioDecoder(
-      const std::string& decoder_name = "MockAudioDecoder");
+  MockAudioDecoder();
+  explicit MockAudioDecoder(std::string decoder_name);
+  explicit MockAudioDecoder(bool is_platform_decoder,
+                            bool supports_decryption,
+                            std::string decoder_name);
   ~MockAudioDecoder() override;
 
-  // AudioDecoder implementation.
+  // Decoder implementation
+  bool IsPlatformDecoder() const override;
+  bool SupportsDecryption() const override;
   std::string GetDisplayName() const override;
+
+  // AudioDecoder implementation.
   void Initialize(const AudioDecoderConfig& config,
                   CdmContext* cdm_context,
                   InitCB init_cb,
@@ -278,7 +294,9 @@ class MockAudioDecoder : public AudioDecoder {
   MOCK_METHOD1(Reset_, void(base::OnceClosure&));
 
  private:
-  std::string decoder_name_;
+  const bool is_platform_decoder_;
+  const bool supports_decryption_;
+  const std::string decoder_name_;
   DISALLOW_COPY_AND_ASSIGN(MockAudioDecoder);
 };