Delete GpuMemoryBufferDecoderWrapper, too onerous to continue.

Due to spurts of outputs, the decoder wrapper approach is too
onerous to be viable. We would need to teach it information about
VideoRendererImpl's queue size, which means it's just easier to
let VideoRendererImpl do this instead.

This work was superseded by preparation support in DecoderStream.

BUG=801245
TEST=passes bots

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

media/filters/BUILD.gn

@@ -43,8 +43,6 @@ source_set("filters") {
     "frame_buffer_pool.h",
     "frame_processor.cc",
     "frame_processor.h",
-    "gpu_memory_buffer_decoder_wrapper.cc",
-    "gpu_memory_buffer_decoder_wrapper.h",
     "gpu_video_decoder.cc",
     "gpu_video_decoder.h",
     "jpeg_parser.cc",
@@ -269,7 +267,6 @@ source_set("unit_tests") {
     "file_data_source_unittest.cc",
     "frame_buffer_pool_unittest.cc",
     "frame_processor_unittest.cc",
-    "gpu_memory_buffer_decoder_wrapper_unittest.cc",
     "ivf_parser_unittest.cc",
     "jpeg_parser_unittest.cc",
     "memory_data_source_unittest.cc",

media/filters/gpu_memory_buffer_decoder_wrapper.cc

-// Copyright 2018 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/gpu_memory_buffer_decoder_wrapper.h"
-
-#include "base/bind.h"
-#include "base/callback.h"
-#include "base/callback_helpers.h"
-#include "media/base/decoder_buffer.h"
-#include "media/video/gpu_memory_buffer_video_frame_pool.h"
-
-namespace media {
-
-GpuMemoryBufferDecoderWrapper::PendingDecode::PendingDecode(
-    scoped_refptr<DecoderBuffer> buffer,
-    DecodeCB decode_cb)
-    : buffer(std::move(buffer)), decode_cb(std::move(decode_cb)) {}
-
-GpuMemoryBufferDecoderWrapper::PendingDecode::PendingDecode(
-    PendingDecode&& pending_decode) = default;
-
-GpuMemoryBufferDecoderWrapper::PendingDecode::~PendingDecode() {
-  if (decode_cb)
-    decode_cb.Run(DecodeStatus::ABORTED);
-}
-
-GpuMemoryBufferDecoderWrapper::GpuMemoryBufferDecoderWrapper(
-    std::unique_ptr<GpuMemoryBufferVideoFramePool> gmb_pool,
-    std::unique_ptr<VideoDecoder> decoder)
-    : gmb_pool_(std::move(gmb_pool)),
-      decoder_(std::move(decoder)),
-      weak_factory_(this),
-      copy_factory_(this) {
-  DCHECK(gmb_pool_);
-  DCHECK(decoder_);
-  DETACH_FROM_THREAD(thread_checker_);
-}
-
-GpuMemoryBufferDecoderWrapper::~GpuMemoryBufferDecoderWrapper() {
-  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
-}
-
-std::string GpuMemoryBufferDecoderWrapper::GetDisplayName() const {
-  // This call is expected to be static and safe to call from any thread.
-  return decoder_->GetDisplayName();
-}
-
-void GpuMemoryBufferDecoderWrapper::Initialize(
-    const VideoDecoderConfig& config,
-    bool low_delay,
-    CdmContext* cdm_context,
-    const InitCB& init_cb,
-    const OutputCB& output_cb,
-    const WaitingForDecryptionKeyCB& waiting_for_decryption_key_cb) {
-  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
-  DCHECK_EQ(pending_copies_, 0u);
-  DCHECK(pending_decodes_.empty());
-  DCHECK(!eos_status_);
-
-  // Follow the standard Initialize() path, but replace the OutputCB provided
-  // with our own which handles coping to the GpuMemoryBuffer.
-  decoder_->Initialize(
-      config, low_delay, cdm_context, init_cb,
-      base::BindRepeating(&GpuMemoryBufferDecoderWrapper::OnOutputReady,
-                          weak_factory_.GetWeakPtr(), output_cb),
-      waiting_for_decryption_key_cb);
-}
-
-void GpuMemoryBufferDecoderWrapper::Decode(
-    const scoped_refptr<DecoderBuffer>& buffer,
-    const DecodeCB& decode_cb) {
-  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
-  DCHECK(!eos_status_);
-
-  // It's now okay to start copying outputs again if Reset() disabled them.
-  abort_copies_ = false;
-
-  // Don't start too many decodes if copies are taking too long...
-  pending_decodes_.emplace_back(buffer, decode_cb);
-  MaybeStartNextDecode();
-}
-
-void GpuMemoryBufferDecoderWrapper::Reset(const base::Closure& reset_cb) {
-  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
-
-  // Abort any in-flight copies and stop any new ones from being started until
-  // the next Decode() call (which will not occur until Reset() completes).
-  gmb_pool_->Abort();
-  abort_copies_ = true;
-  pending_copies_ = 0u;
-  copy_factory_.InvalidateWeakPtrs();
-  eos_status_.reset();
-
-  // Abort any outstanding decodes. The order here is important so that we
-  // return them in the scheduled order.
-  active_decodes_.clear();
-  pending_decodes_.clear();
-
-  decoder_->Reset(reset_cb);
-}
-
-int GpuMemoryBufferDecoderWrapper::GetMaxDecodeRequests() const {
-  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
-  return decoder_->GetMaxDecodeRequests();
-}
-
-void GpuMemoryBufferDecoderWrapper::MaybeStartNextDecode() {
-  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
-
-  if (pending_decodes_.empty())
-    return;
-
-  // Never allow more decodes than the decoder specifies. Callers are required
-  // to guarantee that they never exceed GetMaxDecodeRequests().
-  const size_t max_decodes = GetMaxDecodeRequests();
-  DCHECK_LE(active_decodes_.size(), max_decodes);
-  if (active_decodes_.size() == max_decodes)
-    return;
-
-  // Defer decoding if the number of pending copies exceeds the maximum number
-  // of decodes; this ensures that if the decoder is fast but copies are slow,
-  // we don't accumulate massive amounts of uncopied frames.
-  //
-  // E.g., when GetMaxDecodeRequests() == 1, this allows another decode if there
-  // is at most 1 copy outstanding. When GMDR() == 2, this allows another decode
-  // if there are at most 2 copies outstanding.
-  if (pending_copies_ > max_decodes)
-    return;
-
-  active_decodes_.emplace_back(std::move(pending_decodes_.front()));
-  pending_decodes_.pop_front();
-
-  decoder_->Decode(
-      active_decodes_.back().buffer,
-      base::BindRepeating(&GpuMemoryBufferDecoderWrapper::OnDecodeComplete,
-                          weak_factory_.GetWeakPtr()));
-}
-
-void GpuMemoryBufferDecoderWrapper::OnDecodeComplete(DecodeStatus status) {
-  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
-  DCHECK(!eos_status_);
-
-  // If Reset() was called after Decode() but before this callback, we should
-  // do nothing. Our DecodeCB already returned ABORTED during Reset().
-  if (abort_copies_) {
-    DCHECK_EQ(pending_copies_, 0u);
-    DCHECK(pending_decodes_.empty());
-    DCHECK(active_decodes_.empty());
-    return;
-  }
-
-  DCHECK(!active_decodes_.empty());
-
-  // If this isn't an end of stream, return the decode status and try to
-  // schedule another decode.
-  if (!active_decodes_.front().buffer->end_of_stream()) {
-    // Move the PendingDecode operation into a stack variable and start the next
-    // decode before invoking the DecodeCB since that call may invoke other
-    // operations on this class.
-    PendingDecode pd(std::move(active_decodes_.front()));
-    active_decodes_.pop_front();
-    MaybeStartNextDecode();
-
-    base::ResetAndReturn(&pd.decode_cb).Run(status);
-    return;
-  }
-
-  // We must now be handling an end of stream.
-  DCHECK(pending_decodes_.empty());
-  DCHECK_EQ(active_decodes_.size(), 1u);
-
-  // If all copies have finished we can return the status immediately. The
-  // VideoDecoder API guarantees that this function won't be called until
-  // _after_ all outputs have been sent to OnOutputReady().
-  if (!pending_copies_) {
-    // Move the PendingDecode operation into a stack variable since the call to
-    // the DecodeCB may invoke other operations on this class.
-    PendingDecode pd(std::move(active_decodes_.front()));
-    active_decodes_.clear();
-    base::ResetAndReturn(&pd.decode_cb).Run(status);
-    return;
-  }
-
-  // Normal case, we have copies to wait for before issuing the DecodeCB. We
-  // must ensure the callback is not fired until copies complete.
-  eos_status_ = status;
-}
-
-void GpuMemoryBufferDecoderWrapper::OnOutputReady(
-    const OutputCB& output_cb,
-    const scoped_refptr<VideoFrame>& frame) {
-  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
-
-  // If Reset() has been called, we shouldn't waste cycles on useless copies.
-  if (abort_copies_)
-    return;
-
-  ++pending_copies_;
-  gmb_pool_->MaybeCreateHardwareFrame(
-      frame, base::BindOnce(&GpuMemoryBufferDecoderWrapper::OnFrameCopied,
-                            copy_factory_.GetWeakPtr(), output_cb));
-}
-
-void GpuMemoryBufferDecoderWrapper::OnFrameCopied(
-    const OutputCB& output_cb,
-    const scoped_refptr<VideoFrame>& frame) {
-  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
-  DCHECK_GT(pending_copies_, 0u);
-  --pending_copies_;
-
-  output_cb.Run(frame);
-
-  // If this is an end of stream, we've finished all pending copies and should
-  // now notify the caller. Note: We must check |eos_status_| here because if
-  // the DecodeCB hasn't returned, it's not safe to destroy the DecoderBuffer
-  // it was given (since they are passed by const&...).
-  if (!pending_copies_ && eos_status_) {
-    DCHECK_EQ(active_decodes_.size(), 1u);
-    DCHECK(active_decodes_.front().buffer->end_of_stream());
-
-    // Move the PendingDecode operation and |eos_status_| values into stack
-    // variables since the call to the DecodeCB may invoke other operations on
-    // this class.
-    PendingDecode pd(std::move(active_decodes_.front()));
-    active_decodes_.clear();
-    DecodeStatus status = *eos_status_;
-    eos_status_.reset();
-
-    base::ResetAndReturn(&pd.decode_cb).Run(status);
-    return;
-  }
-
-  MaybeStartNextDecode();
-}
-
-}  // namespace media

media/filters/gpu_memory_buffer_decoder_wrapper.h

-// Copyright 2018 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_FILTERS_GPU_MEMORY_BUFFER_DECODER_WRAPPER_H_
-#define MEDIA_FILTERS_GPU_MEMORY_BUFFER_DECODER_WRAPPER_H_
-
-#include <memory>
-
-#include "base/callback_forward.h"
-#include "base/containers/circular_deque.h"
-#include "base/macros.h"
-#include "base/memory/weak_ptr.h"
-#include "base/optional.h"
-#include "base/threading/thread_checker.h"
-#include "media/base/media_export.h"
-#include "media/base/video_decoder.h"
-
-namespace media {
-class GpuMemoryBufferVideoFramePool;
-
-// Wrapper for VideoDecoder implementations that copies the frames returned by
-// |output_cb| into GpuMemoryBuffers.
-//
-// Subsequent pipeline stages (e.g., VideoFrameStream, VideoRendererImpl) may
-// each have a private cache. Some time may pass before frames move from one
-// cache to the next depending on current buffering levels. If the last stage is
-// to copy frames to GpuMemoryBuffers, we waste the idle time between caches.
-//
-// As such, it's most efficient to copy frames immediately as they come out of
-// the decoder instead of later to ensure they are copied expediently.
-class MEDIA_EXPORT GpuMemoryBufferDecoderWrapper : public VideoDecoder {
- public:
-  GpuMemoryBufferDecoderWrapper(
-      std::unique_ptr<GpuMemoryBufferVideoFramePool> gmb_pool,
-      std::unique_ptr<VideoDecoder> decoder);
-  ~GpuMemoryBufferDecoderWrapper() override;
-
-  // VideoDecoder implementation.
-  std::string GetDisplayName() const override;  // Returns |decoder_| name.
-  void Initialize(
-      const VideoDecoderConfig& config,
-      bool low_delay,
-      CdmContext* cdm_context,
-      const InitCB& init_cb,
-      const OutputCB& output_cb,
-      const WaitingForDecryptionKeyCB& waiting_for_decryption_key_cb) override;
-  void Decode(const scoped_refptr<DecoderBuffer>& buffer,
-              const DecodeCB& decode_cb) override;
-  void Reset(const base::Closure& reset_cb) override;
-  int GetMaxDecodeRequests() const override;  // Returns |decoder_| value.
-
- private:
-  void MaybeStartNextDecode();
-  void OnDecodeComplete(DecodeStatus status);
-  void OnOutputReady(const OutputCB& output_cb,
-                     const scoped_refptr<VideoFrame>& frame);
-  void OnFrameCopied(const OutputCB& output_cb,
-                     const scoped_refptr<VideoFrame>& frame);
-
-  THREAD_CHECKER(thread_checker_);
-
-  // Pool of GpuMemoryBuffers and resources used to create hardware frames.
-  std::unique_ptr<GpuMemoryBufferVideoFramePool> gmb_pool_;
-
-  // The decoder which will be wrapped.
-  std::unique_ptr<VideoDecoder> decoder_;
-
-  // Set upon Reset(), cleared by Decode(). Allows us to abort copies for better
-  // seeking performance.
-  bool abort_copies_ = false;
-
-  // Count of outstanding copies; used to moderate resource usage and trigger
-  // the final end of stream notification once all copies are complete. Cleared
-  // by Reset(), set by OnOutputReady() when a frame is received.
-  uint32_t pending_copies_ = 0u;
-
-  // Cached status of the last OnDecodeComplete call. Used by OnFrameCopied when
-  // all pending copies have been completed.
-  base::Optional<DecodeStatus> eos_status_;
-
-  // Ensures that we don't exceed GetMaxDecodeRequests() in copies and decodes
-  // outstanding. Avoids supersaturating |decoder_| if copies slow down. The
-  // |pending_decodes_| deque is unscheduled Decode() calls, while the
-  // |active_decodes_| deque is scheduled but unreturned Decode() calls.
-  struct PendingDecode {
-    PendingDecode(scoped_refptr<DecoderBuffer> buffer, DecodeCB decode_cb);
-    PendingDecode(PendingDecode&& pending_decode);
-    ~PendingDecode();
-    scoped_refptr<DecoderBuffer> buffer;
-    DecodeCB decode_cb;
-  };
-  base::circular_deque<PendingDecode> pending_decodes_;
-  base::circular_deque<PendingDecode> active_decodes_;
-
-  base::WeakPtrFactory<GpuMemoryBufferDecoderWrapper> weak_factory_;
-
-  // Separate WeakPtr factory which is used to abort copies after a Reset() so
-  // they don't mess up the |pending_copies_| count.
-  base::WeakPtrFactory<GpuMemoryBufferDecoderWrapper> copy_factory_;
-
-  DISALLOW_COPY_AND_ASSIGN(GpuMemoryBufferDecoderWrapper);
-};
-
-}  // namespace media
-
-#endif  // MEDIA_FILTERS_GPU_MEMORY_BUFFER_DECODER_WRAPPER_H_

media/renderers/default_renderer_factory.cc

@@ -15,7 +15,6 @@
 #include "media/base/decoder_factory.h"
 #include "media/base/media_log.h"
 #include "media/base/media_switches.h"
-#include "media/filters/gpu_memory_buffer_decoder_wrapper.h"
 #include "media/filters/gpu_video_decoder.h"
 #include "media/media_buildflags.h"
 #include "media/renderers/audio_renderer_impl.h"
@@ -48,32 +47,6 @@
 
 namespace media {
 
-// TODO(dalecurtis): Remove this flag once committed to using either the
-// GpuMemoryBufferDecoderWrapper or VideoRendererImpl + DecoderStream prepare.
-// http://crbug.com/801245
-#define USE_VIDEO_RENDERER_GMB
-
-static std::unique_ptr<VideoDecoder> MaybeUseGpuMemoryBufferWrapper(
-    GpuVideoAcceleratorFactories* gpu_factories,
-    scoped_refptr<base::SingleThreadTaskRunner> media_task_runner,
-    scoped_refptr<base::TaskRunner> worker_task_runner,
-    std::unique_ptr<VideoDecoder> decoder) {
-#if defined(USE_VIDEO_RENDERER_GMB)
-  return decoder;
-#else
-  if (!gpu_factories ||
-      !gpu_factories->ShouldUseGpuMemoryBuffersForVideoFrames()) {
-    return decoder;
-  }
-
-  return std::make_unique<GpuMemoryBufferDecoderWrapper>(
-      std::make_unique<GpuMemoryBufferVideoFramePool>(
-          std::move(media_task_runner), std::move(worker_task_runner),
-          gpu_factories),
-      std::move(decoder));
-#endif
-}
-
 DefaultRendererFactory::DefaultRendererFactory(
     MediaLog* media_log,
     DecoderFactory* decoder_factory,
@@ -111,7 +84,6 @@ DefaultRendererFactory::CreateAudioDecoders(
 std::vector<std::unique_ptr<VideoDecoder>>
 DefaultRendererFactory::CreateVideoDecoders(
     const scoped_refptr<base::SingleThreadTaskRunner>& media_task_runner,
-    const scoped_refptr<base::TaskRunner>& worker_task_runner,
     const RequestOverlayInfoCB& request_overlay_info_cb,
     const gfx::ColorSpace& target_color_space,
     GpuVideoAcceleratorFactories* gpu_factories) {
@@ -122,9 +94,8 @@ DefaultRendererFactory::CreateVideoDecoders(
   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_)));
+  video_decoders.push_back(
+      std::make_unique<DecryptingVideoDecoder>(media_task_runner, media_log_));
 #endif
 
   // Prefer an external decoder since one will only exist if it is hardware
@@ -133,7 +104,7 @@ DefaultRendererFactory::CreateVideoDecoders(
     // |gpu_factories_| requires that its entry points be called on its
     // |GetTaskRunner()|.  Since |pipeline_| will own decoders created from the
     // factories, require that their message loops are identical.
-    DCHECK(gpu_factories->GetTaskRunner() == media_task_runner.get());
+    DCHECK_EQ(gpu_factories->GetTaskRunner(), media_task_runner);
 
     if (decoder_factory_) {
       decoder_factory_->CreateVideoDecoders(media_task_runner, gpu_factories,
@@ -150,22 +121,16 @@ DefaultRendererFactory::CreateVideoDecoders(
   }
 
 #if BUILDFLAG(ENABLE_LIBVPX)
-  video_decoders.push_back(MaybeUseGpuMemoryBufferWrapper(
-      gpu_factories, media_task_runner, worker_task_runner,
-      std::make_unique<OffloadingVpxVideoDecoder>()));
+  video_decoders.push_back(std::make_unique<OffloadingVpxVideoDecoder>());
 #endif
 
 #if BUILDFLAG(ENABLE_AV1_DECODER)
   if (base::FeatureList::IsEnabled(kAv1Decoder))
-    video_decoders.push_back(MaybeUseGpuMemoryBufferWrapper(
-        gpu_factories, media_task_runner, worker_task_runner,
-        std::make_unique<AomVideoDecoder>(media_log_)));
+    video_decoders.push_back(std::make_unique<AomVideoDecoder>(media_log_));
 #endif
 
 #if BUILDFLAG(ENABLE_FFMPEG_VIDEO_DECODERS)
-  video_decoders.push_back(MaybeUseGpuMemoryBufferWrapper(
-      gpu_factories, media_task_runner, worker_task_runner,
-      std::make_unique<FFmpegVideoDecoder>(media_log_)));
+  video_decoders.push_back(std::make_unique<FFmpegVideoDecoder>(media_log_));
 #endif
 
   return video_decoders;
@@ -197,14 +162,12 @@ std::unique_ptr<Renderer> DefaultRendererFactory::CreateRenderer(
     gpu_factories = get_gpu_factories_cb_.Run();
 
   std::unique_ptr<GpuMemoryBufferVideoFramePool> gmb_pool;
-#if defined(USE_VIDEO_RENDERER_GMB)
   if (gpu_factories &&
       gpu_factories->ShouldUseGpuMemoryBuffersForVideoFrames()) {
     gmb_pool = std::make_unique<GpuMemoryBufferVideoFramePool>(
         std::move(media_task_runner), std::move(worker_task_runner),
         gpu_factories);
   }
-#endif
 
   std::unique_ptr<VideoRenderer> video_renderer(new VideoRendererImpl(
       media_task_runner, video_renderer_sink,
@@ -215,7 +178,7 @@ std::unique_ptr<Renderer> DefaultRendererFactory::CreateRenderer(
       // RendererFactory is owned by WMPI and gets called after WMPI destructor
       // finishes.
       base::Bind(&DefaultRendererFactory::CreateVideoDecoders,
-                 base::Unretained(this), media_task_runner, worker_task_runner,
+                 base::Unretained(this), media_task_runner,
                  request_overlay_info_cb, target_color_space, gpu_factories),
       true, media_log_, std::move(gmb_pool)));
 

media/renderers/default_renderer_factory.h

@@ -51,7 +51,6 @@ class MEDIA_EXPORT DefaultRendererFactory : public RendererFactory {
       const scoped_refptr<base::SingleThreadTaskRunner>& media_task_runner);
   std::vector<std::unique_ptr<VideoDecoder>> CreateVideoDecoders(
       const scoped_refptr<base::SingleThreadTaskRunner>& media_task_runner,
-      const scoped_refptr<base::TaskRunner>& worker_task_runner,
       const RequestOverlayInfoCB& request_overlay_info_cb,
       const gfx::ColorSpace& target_color_space,
       GpuVideoAcceleratorFactories* gpu_factories);