Enable offloading for Dav1dVideoDecoder.

This moves decoding of AV1 content via dav1d to a task pool instead of
sharing the media thread -- which was apparently leading to underflow
of the audio.

This probably only fixes pausing and audio drop outs, if a video frame
is taking more than 200ms (audio buffer size) to return, we've probably
exhausted the video frame buffer too. This would result in video
apprearing frozen for a moment, but would not trigger underflow since
we have a 3 second threshold for underflow on video.

Unlike the VP9 offloading, we offload for all resolutions of AV1
content, since we seem to be getting these issues even with 360p
content.

This removes an unnecessary BindToCurrentLoop for the OutputCB in
the non-offloaded case for clarity.

BUG=940686
TEST=all existing tests pass with offloading decoder.

Change-Id: I7426a0353249646600c12019f868aa058c40e456
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1574830
Commit-Queue: Dale Curtis <dalecurtis@chromium.org>
Auto-Submit: Dale Curtis <dalecurtis@chromium.org>
Reviewed-by: Chrome Cunningham <chcunningham@chromium.org>
Cr-Commit-Position: refs/heads/master@{#652635}

media/filters/dav1d_video_decoder.cc

@@ -113,13 +113,15 @@ struct ScopedDav1dPictureFree {
   }
 };
 
-Dav1dVideoDecoder::Dav1dVideoDecoder(MediaLog* media_log)
-    : media_log_(media_log) {
-  DETACH_FROM_THREAD(thread_checker_);
+Dav1dVideoDecoder::Dav1dVideoDecoder(MediaLog* media_log,
+                                     OffloadState offload_state)
+    : media_log_(media_log),
+      bind_callbacks_(offload_state == OffloadState::kNormal) {
+  DETACH_FROM_SEQUENCE(sequence_checker_);
 }
 
 Dav1dVideoDecoder::~Dav1dVideoDecoder() {
-  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   CloseDecoder();
 }
 
@@ -133,10 +135,10 @@ void Dav1dVideoDecoder::Initialize(const VideoDecoderConfig& config,
                                    const InitCB& init_cb,
                                    const OutputCB& output_cb,
                                    const WaitingCB& /* waiting_cb */) {
-  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   DCHECK(config.IsValidConfig());
 
-  InitCB bound_init_cb = BindToCurrentLoop(init_cb);
+  InitCB bound_init_cb = bind_callbacks_ ? BindToCurrentLoop(init_cb) : init_cb;
   if (config.is_encrypted() || config.codec() != kCodecAV1) {
     bound_init_cb.Run(false);
     return;
@@ -163,19 +165,20 @@ void Dav1dVideoDecoder::Initialize(const VideoDecoderConfig& config,
 
   config_ = config;
   state_ = DecoderState::kNormal;
-  output_cb_ = BindToCurrentLoop(output_cb);
+  output_cb_ = output_cb;
   bound_init_cb.Run(true);
 }
 
 void Dav1dVideoDecoder::Decode(scoped_refptr<DecoderBuffer> buffer,
                                const DecodeCB& decode_cb) {
-  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   DCHECK(buffer);
   DCHECK(decode_cb);
   DCHECK_NE(state_, DecoderState::kUninitialized)
       << "Called Decode() before successful Initialize()";
 
-  DecodeCB bound_decode_cb = BindToCurrentLoop(decode_cb);
+  DecodeCB bound_decode_cb =
+      bind_callbacks_ ? BindToCurrentLoop(decode_cb) : decode_cb;
 
   if (state_ == DecoderState::kError) {
     bound_decode_cb.Run(DecodeStatus::DECODE_ERROR);
@@ -193,14 +196,30 @@ void Dav1dVideoDecoder::Decode(scoped_refptr<DecoderBuffer> buffer,
 }
 
 void Dav1dVideoDecoder::Reset(const base::RepeatingClosure& reset_cb) {
-  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   state_ = DecoderState::kNormal;
   dav1d_flush(dav1d_decoder_);
-  base::SequencedTaskRunnerHandle::Get()->PostTask(FROM_HERE, reset_cb);
+
+  if (bind_callbacks_)
+    base::SequencedTaskRunnerHandle::Get()->PostTask(FROM_HERE, reset_cb);
+  else
+    reset_cb.Run();
+}
+
+void Dav1dVideoDecoder::Detach() {
+  // Even though we offload all resolutions of AV1, this may be called in a
+  // transition from clear to encrypted content. Which will subsequently fail
+  // Initialize() since encrypted content isn't supported by this decoder.
+
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  DCHECK(!bind_callbacks_);
+
+  CloseDecoder();
+  DETACH_FROM_SEQUENCE(sequence_checker_);
 }
 
 void Dav1dVideoDecoder::CloseDecoder() {
-  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   if (!dav1d_decoder_)
     return;
   dav1d_close(&dav1d_decoder_);
@@ -208,7 +227,7 @@ void Dav1dVideoDecoder::CloseDecoder() {
 }
 
 bool Dav1dVideoDecoder::DecodeBuffer(scoped_refptr<DecoderBuffer> buffer) {
-  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
   using ScopedPtrDav1dData = std::unique_ptr<Dav1dData, ScopedDav1dDataFree>;
   ScopedPtrDav1dData input_buffer;
@@ -296,7 +315,7 @@ bool Dav1dVideoDecoder::DecodeBuffer(scoped_refptr<DecoderBuffer> buffer) {
 
 scoped_refptr<VideoFrame> Dav1dVideoDecoder::CopyImageToVideoFrame(
     const Dav1dPicture* pic) {
-  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
   VideoPixelFormat pixel_format = Dav1dImgFmtToVideoPixelFormat(&pic->p);
   if (pixel_format == PIXEL_FORMAT_UNKNOWN)

media/filters/dav1d_video_decoder.h

@@ -14,6 +14,7 @@
 #include "media/base/video_decoder_config.h"
 #include "media/base/video_frame.h"
 #include "media/base/video_frame_pool.h"
+#include "media/filters/offloading_video_decoder.h"
 
 struct Dav1dContext;
 struct Dav1dPicture;
@@ -21,9 +22,10 @@ struct Dav1dPicture;
 namespace media {
 class MediaLog;
 
-class MEDIA_EXPORT Dav1dVideoDecoder : public VideoDecoder {
+class MEDIA_EXPORT Dav1dVideoDecoder : public OffloadableVideoDecoder {
  public:
-  explicit Dav1dVideoDecoder(MediaLog* media_log);
+  Dav1dVideoDecoder(MediaLog* media_log,
+                    OffloadState offload_state = OffloadState::kNormal);
   ~Dav1dVideoDecoder() override;
 
   // VideoDecoder implementation.
@@ -38,6 +40,9 @@ class MEDIA_EXPORT Dav1dVideoDecoder : public VideoDecoder {
               const DecodeCB& decode_cb) override;
   void Reset(const base::RepeatingClosure& reset_cb) override;
 
+  // OffloadableVideoDecoder implementation.
+  void Detach() override;
+
  private:
   enum class DecoderState {
     kUninitialized,
@@ -55,11 +60,15 @@ class MEDIA_EXPORT Dav1dVideoDecoder : public VideoDecoder {
 
   scoped_refptr<VideoFrame> CopyImageToVideoFrame(const Dav1dPicture* img);
 
-  THREAD_CHECKER(thread_checker_);
-
   // Used to report error messages to the client.
   MediaLog* const media_log_ = nullptr;
 
+  // Indicates if the decoder is being wrapped by OffloadVideoDecoder; controls
+  // whether callbacks are bound to the current loop on calls.
+  const bool bind_callbacks_;
+
+  SEQUENCE_CHECKER(sequence_checker_);
+
   // Current decoder state. Used to ensure methods are called as expected.
   DecoderState state_ = DecoderState::kUninitialized;
 
@@ -77,6 +86,19 @@ class MEDIA_EXPORT Dav1dVideoDecoder : public VideoDecoder {
   DISALLOW_COPY_AND_ASSIGN(Dav1dVideoDecoder);
 };
 
+// Helper class for creating a Dav1dVideoDecoder which will offload all AV1
+// content from the media thread.
+class OffloadingDav1dVideoDecoder : public OffloadingVideoDecoder {
+ public:
+  explicit OffloadingDav1dVideoDecoder(MediaLog* media_log)
+      : OffloadingVideoDecoder(
+            0,
+            std::vector<VideoCodec>(1, kCodecAV1),
+            std::make_unique<Dav1dVideoDecoder>(
+                media_log,
+                OffloadableVideoDecoder::OffloadState::kOffloaded)) {}
+};
+
 }  // namespace media
 
 #endif  // MEDIA_FILTERS_DAV1D_VIDEO_DECODER_H_

media/filters/offloading_video_decoder.h

@@ -27,6 +27,16 @@ class CancellationHelper;
 // and Reset() does not need to wait for |reset_cb| to return.
 class MEDIA_EXPORT OffloadableVideoDecoder : public VideoDecoder {
  public:
+  enum class OffloadState {
+    kOffloaded,  // Indicates the VideoDecoder is being used with
+                 // OffloadingVideoDecoder and that callbacks provided to
+                 // VideoDecoder methods should not be bound to the current
+                 // loop.
+
+    kNormal,  // Indicates the VideoDecoder is being used as a normal
+              // VideoDecoder, meaning callbacks should always be asynchronous.
+  };
+
   ~OffloadableVideoDecoder() override {}
 
   // Called by the OffloadingVideoDecoder when closing the decoder and switching

media/filters/vpx_video_decoder.cc

@@ -177,8 +177,6 @@ void VpxVideoDecoder::Decode(scoped_refptr<DecoderBuffer> buffer,
   if (video_frame) {
     video_frame->metadata()->SetBoolean(VideoFrameMetadata::POWER_EFFICIENT,
                                         false);
-    // Safe to call |output_cb_| here even if we're on the offload thread since
-    // it is only set once during Initialize() and never changed.
     output_cb_.Run(video_frame);
   }
 

media/filters/vpx_video_decoder.h

@@ -29,15 +29,6 @@ class FrameBufferPool;
 // [1] http://wiki.webmproject.org/alpha-channel
 class MEDIA_EXPORT VpxVideoDecoder : public OffloadableVideoDecoder {
  public:
-  enum class OffloadState {
-    kOffloaded,  // Indicates VpxVideoDecoder is being used with
-                 // OffloadVideoDecoder and that callbacks provided to
-                 // VideoDecoder methods should not be bound to the current
-                 // loop.
-
-    kNormal,  // Indicates VpxVideoDecoder is being used as a normal
-              // VideoDecoder, meaning callbacks should always be asynchronous.
-  };
   explicit VpxVideoDecoder(OffloadState offload_state = OffloadState::kNormal);
   ~VpxVideoDecoder() override;
 
@@ -123,11 +114,11 @@ class MEDIA_EXPORT VpxVideoDecoder : public OffloadableVideoDecoder {
 class OffloadingVpxVideoDecoder : public OffloadingVideoDecoder {
  public:
   OffloadingVpxVideoDecoder()
-      : OffloadingVideoDecoder(1024,
-                               std::vector<VideoCodec>(1, kCodecVP9),
-                               std::make_unique<VpxVideoDecoder>(
-                                   VpxVideoDecoder::OffloadState::kOffloaded)) {
-  }
+      : OffloadingVideoDecoder(
+            1024,
+            std::vector<VideoCodec>(1, kCodecVP9),
+            std::make_unique<VpxVideoDecoder>(
+                OffloadableVideoDecoder::OffloadState::kOffloaded)) {}
 };
 
 }  // namespace media

media/renderers/default_decoder_factory.cc

@@ -121,7 +121,8 @@ void DefaultDecoderFactory::CreateVideoDecoders(
 #endif
 
 #if BUILDFLAG(ENABLE_DAV1D_DECODER)
-  video_decoders->push_back(std::make_unique<Dav1dVideoDecoder>(media_log));
+  video_decoders->push_back(
+      std::make_unique<OffloadingDav1dVideoDecoder>(media_log));
 #elif BUILDFLAG(ENABLE_LIBAOM_DECODER)
   video_decoders->push_back(std::make_unique<AomVideoDecoder>(media_log));
 #endif

media/test/pipeline_integration_test_base.cc

@@ -74,7 +74,8 @@ static std::vector<std::unique_ptr<VideoDecoder>> CreateVideoDecodersForTest(
 #endif
 
 #if BUILDFLAG(ENABLE_DAV1D_DECODER)
-  video_decoders.push_back(std::make_unique<Dav1dVideoDecoder>(media_log));
+  video_decoders.push_back(
+      std::make_unique<OffloadingDav1dVideoDecoder>(media_log));
 #elif BUILDFLAG(ENABLE_LIBAOM_DECODER)
   video_decoders.push_back(std::make_unique<AomVideoDecoder>(media_log));
 #endif