Move GpuMemoryBuffer upload stage earlier in pipeline.

This introduces GpuMemoryBufferDecoderWrapper which is used to wrap
software VideoDecoder implementations. The wrapper copies frames as
they are returned from the decoder, but before they are delivered to
the renderer.

This is an improvement over the current copy done by VideoRendererImpl
because the VideoFrameStream (which sits between the renderer and the
decoder) may hold undelivered frames until a subsequent Read(). At
which point the frame still needs to be copied.

Total reduction in read times is ~12.6% (VP9) to ~30.1% (H264) after
4000 frames when tested using local caches of:
https://www.youtube.com/watch?v=1La4QzGeaaQ (4K60 VP9)
https://www.youtube.com/watch?v=dEaSepKi9SQ (4K60 H264)

The real benefit of this approach will come later when we allow
parallel decodes for offloaded decoders; since these decoders may
accumulate more frames in the decoder stream without this approach.

This also allows us to remove a bit of code and specialized tests
from the VideoRendererImpl in favor of more compartmentalized tests
just for the wrapper.

The downside to this approach is each decoder has it's own pool,
but this only amounts to ~240 bytes of memory when no frames
have been created.

Perf bots may detect a memory increase from this change, but only
as an artifact of improved throughput and timing changes.

BUG=801245
TEST=new unittests, manual tests.

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: Iac5655c9da3e850818f9de83cb2993358fcbd716
Reviewed-on: https://chromium-review.googlesource.com/947383
Commit-Queue: Dale Curtis <dalecurtis@chromium.org>
Reviewed-by: Daniele Castagna <dcastagna@chromium.org>
Cr-Commit-Position: refs/heads/master@{#541284}

media/base/mock_filters.h

@@ -201,7 +201,7 @@ class MockVideoDecoder : public VideoDecoder {
   MOCK_METHOD2(Decode, void(const scoped_refptr<DecoderBuffer>& buffer,
                             const DecodeCB&));
   MOCK_METHOD1(Reset, void(const base::Closure&));
-  MOCK_CONST_METHOD0(HasAlpha, bool());
+  MOCK_CONST_METHOD0(GetMaxDecodeRequests, int());
   MOCK_CONST_METHOD0(CanReadWithoutStalling, bool());
 
  private:

media/filters/BUILD.gn

@@ -43,6 +43,8 @@ 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",
@@ -267,6 +269,7 @@ 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 "media/base/decoder_buffer.h"
+#include "media/video/gpu_memory_buffer_video_frame_pool.h"
+
+namespace media {
+
+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) {
+  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
+  DCHECK(eos_decode_cb_.is_null());
+  DCHECK_EQ(pending_copies_, 0u);
+
+  // 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));
+}
+
+void GpuMemoryBufferDecoderWrapper::Decode(
+    const scoped_refptr<DecoderBuffer>& buffer,
+    const DecodeCB& decode_cb) {
+  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
+  DCHECK(eos_decode_cb_.is_null());
+
+  // It's now okay to start copying outputs again if Reset() disabled them.
+  abort_copies_ = false;
+
+  // End of stream is a special case where we need to intercept the DecodeCB and
+  // wait until all copies are done before triggering it.
+  if (buffer->end_of_stream()) {
+    decoder_->Decode(buffer, base::BindRepeating(
+                                 &GpuMemoryBufferDecoderWrapper::OnDecodedEOS,
+                                 weak_factory_.GetWeakPtr(), decode_cb));
+    return;
+  }
+
+  decoder_->Decode(buffer, decode_cb);
+}
+
+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).
+  abort_copies_ = true;
+  pending_copies_ = 0u;
+  copy_factory_.InvalidateWeakPtrs();
+
+  // In case OnDecodedEOS() was called, but we have pending copies, we need to
+  // issue the |eos_decode_cb_| here since we're aborting any subsequent copies
+  // or OnOutputReady calls.
+  //
+  // Technically the status here should be ABORTED, but it doesn't matter and
+  // avoids adding yet another state variable to this already completed process.
+  if (!eos_decode_cb_.is_null())
+    std::move(eos_decode_cb_).Run();
+
+  decoder_->Reset(reset_cb);
+}
+
+int GpuMemoryBufferDecoderWrapper::GetMaxDecodeRequests() const {
+  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
+  return decoder_->GetMaxDecodeRequests();
+}
+
+void GpuMemoryBufferDecoderWrapper::OnDecodedEOS(const DecodeCB& decode_cb,
+                                                 DecodeStatus status) {
+  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
+  DCHECK(eos_decode_cb_.is_null());
+
+  // If Reset() was called after Decode() but before this callback, we should
+  // return ABORTED for the decode callback. Any pending outputs or copies will
+  // have been aborted by Reset().
+  if (abort_copies_) {
+    DCHECK_EQ(pending_copies_, 0u);
+    decode_cb.Run(DecodeStatus::ABORTED);
+    return;
+  }
+
+  // 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_) {
+    decode_cb.Run(status);
+    return;
+  }
+
+  // Normal case, we have copies to wait for before issuing the DecodeCB. We
+  // must ensure this callback is not fired until copies complete.
+  //
+  // It's crucial we set |eos_decode_cb_| only after signaled by the underlying
+  // decoder and not during Decode() itself, otherwise it's possible that
+  // |pending_copies_| will reach zero for previously started copies and notify
+  // the end of stream before we actually vend all frames.
+  eos_decode_cb_ = base::BindOnce(decode_cb, status);
+}
+
+void GpuMemoryBufferDecoderWrapper::OnOutputReady(
+    const OutputCB& output_cb,
+    const scoped_refptr<VideoFrame>& frame) {
+  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
+
+  // This should not be set until after all outputs have been queued.
+  DCHECK(eos_decode_cb_.is_null());
+
+  // 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);
+
+  // We've finished all pending copies and should now notify the caller.
+  if (!pending_copies_ && !eos_decode_cb_.is_null())
+    std::move(eos_decode_cb_).Run();
+}
+
+}  // 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/macros.h"
+#include "base/memory/weak_ptr.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) 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 OnDecodedEOS(const DecodeCB& decode_cb, 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;
+
+  // These two variables are used to track and trigger the final end of stream
+  // notification once all copies are complete. Cleared by Reset(), set by
+  // Decode() when an end of stream buffer is received.
+  uint32_t pending_copies_ = 0u;
+  base::OnceClosure eos_decode_cb_;
+
+  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,11 +15,13 @@
 #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_features.h"
 #include "media/renderers/audio_renderer_impl.h"
 #include "media/renderers/renderer_impl.h"
 #include "media/renderers/video_renderer_impl.h"
+#include "media/video/gpu_memory_buffer_video_frame_pool.h"
 #include "media/video/gpu_video_accelerator_factories.h"
 #include "third_party/libaom/av1_features.h"
 
@@ -41,6 +43,23 @@
 
 namespace media {
 
+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 (!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));
+}
+
 DefaultRendererFactory::DefaultRendererFactory(
     MediaLog* media_log,
     DecoderFactory* decoder_factory,
@@ -73,6 +92,7 @@ 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) {
@@ -105,16 +125,22 @@ DefaultRendererFactory::CreateVideoDecoders(
   }
 
 #if BUILDFLAG(ENABLE_LIBVPX)
-  video_decoders.push_back(std::make_unique<OffloadingVpxVideoDecoder>());
+  video_decoders.push_back(MaybeUseGpuMemoryBufferWrapper(
+      gpu_factories, media_task_runner, worker_task_runner,
+      std::make_unique<OffloadingVpxVideoDecoder>()));
 #endif
 
 #if BUILDFLAG(ENABLE_AV1_DECODER)
   if (base::FeatureList::IsEnabled(kAv1Decoder))
-    video_decoders.push_back(std::make_unique<AomVideoDecoder>(media_log_));
+    video_decoders.push_back(MaybeUseGpuMemoryBufferWrapper(
+        gpu_factories, media_task_runner, worker_task_runner,
+        std::make_unique<AomVideoDecoder>(media_log_)));
 #endif
 
 #if BUILDFLAG(ENABLE_FFMPEG_VIDEO_DECODERS)
-  video_decoders.push_back(std::make_unique<FFmpegVideoDecoder>(media_log_));
+  video_decoders.push_back(MaybeUseGpuMemoryBufferWrapper(
+      gpu_factories, media_task_runner, worker_task_runner,
+      std::make_unique<FFmpegVideoDecoder>(media_log_)));
 #endif
 
   return video_decoders;
@@ -146,7 +172,7 @@ std::unique_ptr<Renderer> DefaultRendererFactory::CreateRenderer(
     gpu_factories = get_gpu_factories_cb_.Run();
 
   std::unique_ptr<VideoRenderer> video_renderer(new VideoRendererImpl(
-      media_task_runner, worker_task_runner, video_renderer_sink,
+      media_task_runner, video_renderer_sink,
       // Unretained is safe here, because the RendererFactory is guaranteed to
       // outlive the RendererImpl. The RendererImpl is destroyed when WMPI
       // destructor calls pipeline_controller_.Stop() -> PipelineImpl::Stop() ->
@@ -154,9 +180,9 @@ 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,
+                 base::Unretained(this), media_task_runner, worker_task_runner,
                  request_overlay_info_cb, target_color_space, gpu_factories),
-      true, gpu_factories, media_log_));
+      true, media_log_));
 
   return std::make_unique<RendererImpl>(
       media_task_runner, std::move(audio_renderer), std::move(video_renderer));

media/renderers/default_renderer_factory.h

@@ -51,6 +51,7 @@ 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);

media/renderers/video_renderer_impl.cc

@@ -25,8 +25,6 @@
 #include "media/base/pipeline_status.h"
 #include "media/base/renderer_client.h"
 #include "media/base/video_frame.h"
-#include "media/video/gpu_memory_buffer_video_frame_pool.h"
-#include "media/video/gpu_video_accelerator_factories.h"
 
 namespace media {
 
@@ -108,25 +106,20 @@ bool ShouldUseLowDelayMode(DemuxerStream* stream) {
 
 VideoRendererImpl::VideoRendererImpl(
     const scoped_refptr<base::SingleThreadTaskRunner>& media_task_runner,
-    const scoped_refptr<base::TaskRunner>& worker_task_runner,
     VideoRendererSink* sink,
     const CreateVideoDecodersCB& create_video_decoders_cb,
     bool drop_frames,
-    GpuVideoAcceleratorFactories* gpu_factories,
     MediaLog* media_log)
     : task_runner_(media_task_runner),
       sink_(sink),
       sink_started_(false),
       client_(nullptr),
-      gpu_memory_buffer_pool_(nullptr),
       media_log_(media_log),
       low_delay_(false),
       received_end_of_stream_(false),
       rendered_end_of_stream_(false),
       state_(kUninitialized),
       create_video_decoders_cb_(create_video_decoders_cb),
-      gpu_factories_(gpu_factories),
-      worker_task_runner_(worker_task_runner),
       pending_read_(false),
       drop_frames_(drop_frames),
       buffering_state_(BUFFERING_HAVE_NOTHING),
@@ -238,17 +231,6 @@ void VideoRendererImpl::Initialize(
   video_frame_stream_->set_config_change_observer(base::Bind(
       &VideoRendererImpl::OnConfigChange, weak_factory_.GetWeakPtr()));
 
-  // Always re-initialize or reset the |gpu_memory_buffer_pool_| in case we are
-  // switching between video tracks with incompatible video formats (e.g. 8-bit
-  // H.264 to 10-bit H264 or vice versa).
-  if (gpu_factories_ &&
-      gpu_factories_->ShouldUseGpuMemoryBuffersForVideoFrames()) {
-    gpu_memory_buffer_pool_.reset(new GpuMemoryBufferVideoFramePool(
-        task_runner_, worker_task_runner_, gpu_factories_));
-  } else {
-    gpu_memory_buffer_pool_.reset();
-  }
-
   low_delay_ = ShouldUseLowDelayMode(stream);
 
   UMA_HISTOGRAM_BOOLEAN("Media.VideoRenderer.LowDelay", low_delay_);
@@ -399,11 +381,6 @@ void VideoRendererImpl::SetTickClockForTesting(base::TickClock* tick_clock) {
   tick_clock_ = tick_clock;
 }
 
-void VideoRendererImpl::SetGpuMemoryBufferVideoForTesting(
-    std::unique_ptr<GpuMemoryBufferVideoFramePool> gpu_memory_buffer_pool) {
-  gpu_memory_buffer_pool_.swap(gpu_memory_buffer_pool);
-}
-
 void VideoRendererImpl::OnTimeProgressing() {
   DCHECK(task_runner_->BelongsToCurrentThread());
 
@@ -469,22 +446,6 @@ void VideoRendererImpl::OnTimeStopped() {
   }
 }
 
-void VideoRendererImpl::FrameReadyForCopyingToGpuMemoryBuffers(
-    base::TimeTicks read_time,
-    VideoFrameStream::Status status,
-    const scoped_refptr<VideoFrame>& frame) {
-  if (status != VideoFrameStream::OK || IsBeforeStartTime(frame->timestamp())) {
-    VideoRendererImpl::FrameReady(read_time, status, frame);
-    return;
-  }
-
-  DCHECK(frame);
-  gpu_memory_buffer_pool_->MaybeCreateHardwareFrame(
-      frame,
-      base::Bind(&VideoRendererImpl::FrameReady,
-                 frame_callback_weak_factory_.GetWeakPtr(), read_time, status));
-}
-
 void VideoRendererImpl::FrameReady(base::TimeTicks read_time,
                                    VideoFrameStream::Status status,
                                    const scoped_refptr<VideoFrame>& frame) {
@@ -687,17 +648,9 @@ void VideoRendererImpl::AttemptRead_Locked() {
   switch (state_) {
     case kPlaying:
       pending_read_ = true;
-      if (gpu_memory_buffer_pool_) {
-        video_frame_stream_->Read(base::Bind(
-            &VideoRendererImpl::FrameReadyForCopyingToGpuMemoryBuffers,
-            frame_callback_weak_factory_.GetWeakPtr(),
-            tick_clock_->NowTicks()));
-      } else {
-        video_frame_stream_->Read(
-            base::Bind(&VideoRendererImpl::FrameReady,
-                       frame_callback_weak_factory_.GetWeakPtr(),
-                       tick_clock_->NowTicks()));
-      }
+      video_frame_stream_->Read(base::BindRepeating(
+          &VideoRendererImpl::FrameReady,
+          frame_callback_weak_factory_.GetWeakPtr(), tick_clock_->NowTicks()));
       return;
     case kUninitialized:
     case kInitializing:

media/renderers/video_renderer_impl.h

@@ -28,13 +28,11 @@
 #include "media/filters/decoder_stream.h"
 #include "media/filters/video_renderer_algorithm.h"
 #include "media/renderers/default_renderer_factory.h"
-#include "media/video/gpu_memory_buffer_video_frame_pool.h"
-#include "media/video/gpu_video_accelerator_factories.h"
 
 namespace base {
 class SingleThreadTaskRunner;
 class TickClock;
-}
+}  // namespace base
 
 namespace media {
 
@@ -54,11 +52,9 @@ class MEDIA_EXPORT VideoRendererImpl
   // Setting |drop_frames_| to true causes the renderer to drop expired frames.
   VideoRendererImpl(
       const scoped_refptr<base::SingleThreadTaskRunner>& media_task_runner,
-      const scoped_refptr<base::TaskRunner>& worker_task_runner,
       VideoRendererSink* sink,
       const CreateVideoDecodersCB& create_video_decoders_cb,
       bool drop_frames,
-      GpuVideoAcceleratorFactories* gpu_factories,
       MediaLog* media_log);
   ~VideoRendererImpl() override;
 
@@ -74,8 +70,6 @@ class MEDIA_EXPORT VideoRendererImpl
   void OnTimeStopped() override;
 
   void SetTickClockForTesting(base::TickClock* tick_clock);
-  void SetGpuMemoryBufferVideoForTesting(
-      std::unique_ptr<GpuMemoryBufferVideoFramePool> gpu_memory_buffer_pool);
   size_t frames_queued_for_testing() const {
     return algorithm_->frames_queued();
   }
@@ -110,15 +104,6 @@ class MEDIA_EXPORT VideoRendererImpl
   // RenderClient of the new config.
   void OnConfigChange(const VideoDecoderConfig& config);
 
-  // Callback for |video_frame_stream_| to deliver decoded video frames and
-  // report video decoding status. If a frame is available the planes will be
-  // copied asynchronously and FrameReady will be called once finished copying.
-  // |read_time| is the time at which this read was started.
-  void FrameReadyForCopyingToGpuMemoryBuffers(
-      base::TimeTicks read_time,
-      VideoFrameStream::Status status,
-      const scoped_refptr<VideoFrame>& frame);
-
   // Callback for |video_frame_stream_| to deliver decoded video frames and
   // report video decoding status. |read_time| is the time at which this read
   // was started.
@@ -227,11 +212,6 @@ class MEDIA_EXPORT VideoRendererImpl
   // Provides video frames to VideoRendererImpl.
   std::unique_ptr<VideoFrameStream> video_frame_stream_;
 
-  // Pool of GpuMemoryBuffers and resources used to create hardware frames.
-  // Ensure this is destructed after |algorithm_| for optimal memory release
-  // when a frames are still held by the compositor.
-  std::unique_ptr<GpuMemoryBufferVideoFramePool> gpu_memory_buffer_pool_;
-
   MediaLog* media_log_;
 
   // Flag indicating low-delay mode.
@@ -261,20 +241,12 @@ class MEDIA_EXPORT VideoRendererImpl
   //           |                            |
   //           |                            | Flush()
   //           `---------> kPlaying --------'
-  enum State {
-    kUninitialized,
-    kInitializing,
-    kFlushing,
-    kFlushed,
-    kPlaying
-  };
+  enum State { kUninitialized, kInitializing, kFlushing, kFlushed, kPlaying };
   State state_;
 
   // TODO(servolk): Consider using DecoderFactory here instead of the
   // CreateVideoDecodersCB.
   CreateVideoDecodersCB create_video_decoders_cb_;
-  GpuVideoAcceleratorFactories* gpu_factories_;
-  scoped_refptr<base::TaskRunner> worker_task_runner_;
 
   // Keep track of the outstanding read on the VideoFrameStream. Flushing can
   // only complete once the read has completed.
@@ -299,8 +271,8 @@ class MEDIA_EXPORT VideoRendererImpl
 
   // Algorithm for selecting which frame to render; manages frames and all
   // timing related information. Ensure this is destructed before
-  // |gpu_memory_buffer_pool_| for optimal memory release when a frames are
-  // still held by the compositor.
+  // |video_frame_stream_| for optimal memory release when a frames are still
+  // held by the compositor.
   std::unique_ptr<VideoRendererAlgorithm> algorithm_;
 
   // Indicates that Render() was called with |background_rendering| set to true,

media/renderers/video_renderer_impl_unittest.cc

@@ -36,7 +36,6 @@
 #include "media/base/video_frame.h"
 #include "media/base/wall_clock_time_source.h"
 #include "media/renderers/video_renderer_impl.h"
-#include "media/video/mock_gpu_memory_buffer_video_frame_pool.h"
 #include "testing/gmock_mutant.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -71,6 +70,7 @@ class VideoRendererImplTest : public testing::Test {
         .WillByDefault(Invoke(this, &VideoRendererImplTest::DecodeRequested));
     ON_CALL(*decoder_, Reset(_))
         .WillByDefault(Invoke(this, &VideoRendererImplTest::FlushRequested));
+    ON_CALL(*decoder_, GetMaxDecodeRequests()).WillByDefault(Return(1));
     return decoders;
   }
 
@@ -88,13 +88,10 @@ class VideoRendererImplTest : public testing::Test {
     // written to test it, so enable it always.
     scoped_feature_list_.InitAndEnableFeature(kComplexityBasedVideoBuffering);
     renderer_.reset(new VideoRendererImpl(
-        message_loop_.task_runner(), message_loop_.task_runner().get(),
-        null_video_sink_.get(),
+        message_loop_.task_runner(), null_video_sink_.get(),
         base::Bind(&VideoRendererImplTest::CreateVideoDecodersForTest,
                    base::Unretained(this)),
-        true,
-        nullptr,  // gpu_factories
-        &media_log_));
+        true, &media_log_));
     renderer_->SetTickClockForTesting(&tick_clock_);
     null_video_sink_->set_tick_clock_for_testing(&tick_clock_);
     time_source_.set_tick_clock_for_testing(&tick_clock_);
@@ -1468,48 +1465,4 @@ TEST_F(VideoRendererImplTest, OpacityChange) {
   Destroy();
 }
 
-class VideoRendererImplAsyncAddFrameReadyTest : public VideoRendererImplTest {
- public:
-  void InitializeWithMockGpuMemoryBufferVideoFramePool() {
-    VideoRendererImplTest::Initialize();
-    std::unique_ptr<GpuMemoryBufferVideoFramePool> gpu_memory_buffer_pool(
-        new MockGpuMemoryBufferVideoFramePool(&frame_ready_cbs_));
-    renderer_->SetGpuMemoryBufferVideoForTesting(
-        std::move(gpu_memory_buffer_pool));
-  }
-
- protected:
-  std::vector<base::OnceClosure> frame_ready_cbs_;
-};
-
-TEST_F(VideoRendererImplAsyncAddFrameReadyTest, InitializeAndStartPlayingFrom) {
-  InitializeWithMockGpuMemoryBufferVideoFramePool();
-  QueueFrames("0 10 20 30");
-  EXPECT_CALL(mock_cb_, FrameReceived(HasTimestampMatcher(0)));
-  EXPECT_CALL(mock_cb_, OnBufferingStateChange(BUFFERING_HAVE_ENOUGH));
-  EXPECT_CALL(mock_cb_, OnStatisticsUpdate(_)).Times(AnyNumber());
-  EXPECT_CALL(mock_cb_, OnVideoNaturalSizeChange(_)).Times(1);
-  EXPECT_CALL(mock_cb_, OnVideoOpacityChange(_)).Times(1);
-  StartPlayingFrom(0);
-  ASSERT_EQ(1u, frame_ready_cbs_.size());
-
-  uint32_t frame_ready_index = 0;
-  while (frame_ready_index < frame_ready_cbs_.size()) {
-    std::move(frame_ready_cbs_[frame_ready_index++]).Run();
-    base::RunLoop().RunUntilIdle();
-  }
-  Destroy();
-}
-
-TEST_F(VideoRendererImplAsyncAddFrameReadyTest, WeakFactoryDiscardsOneFrame) {
-  InitializeWithMockGpuMemoryBufferVideoFramePool();
-  QueueFrames("0 10 20 30");
-  StartPlayingFrom(0);
-  Flush();
-  ASSERT_EQ(1u, frame_ready_cbs_.size());
-  // This frame will be discarded.
-  std::move(frame_ready_cbs_.front()).Run();
-  Destroy();
-}
-
 }  // namespace media

media/test/pipeline_integration_test_base.cc

@@ -459,12 +459,10 @@ std::unique_ptr<Renderer> PipelineIntegrationTestBase::CreateRenderer(
 
   // Disable frame dropping if hashing is enabled.
   std::unique_ptr<VideoRenderer> video_renderer(new VideoRendererImpl(
-      scoped_task_environment_.GetMainThreadTaskRunner(),
-      scoped_task_environment_.GetMainThreadTaskRunner().get(),
-      video_sink_.get(),
+      scoped_task_environment_.GetMainThreadTaskRunner(), video_sink_.get(),
       base::Bind(&CreateVideoDecodersForTest, &media_log_,
                  prepend_video_decoders_cb),
-      false, nullptr, &media_log_));
+      false, &media_log_));
 
   if (!clockless_playback_) {
     audio_sink_ =