media/gpu/DmabufVideoFramePool: Request new video frame via RequestFrames().

When we allocate frames with new layout, we always need to know the
maximum number of frames. This CL combines SetMaxNumFrames() and
NegotiateFrameFormat() to RequestFrames() to simplify the interface.

BUG=b:136716638
TEST=media_unittests --gtest_filter=PlatformVideoFramePoolTest*
TEST=video_decode_accelerator_tests on Kevin and Eve

Change-Id: I7c194a9107319f1a91da961140548216d2240e8d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1861437
Commit-Queue: Chih-Yu Huang <akahuang@chromium.org>
Reviewed-by: Hirokazu Honda <hiroh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#711656}

media/gpu/chromeos/dmabuf_video_frame_pool.h

@@ -38,17 +38,14 @@ class MEDIA_GPU_EXPORT DmabufVideoFramePool {
   virtual void set_parent_task_runner(
       scoped_refptr<base::SequencedTaskRunner> parent_task_runner);
 
-  // Sets the maximum number of frames which can be allocated.
-  // Used to prevent client from draining all memory.
-  virtual void SetMaxNumFrames(size_t max_num_frames) = 0;
-
-  // Sets the parameters of allocating frames and returns a valid
-  // VideoFrameLayout that VideoFrame will be created by GetFrame() has.
-  // This method must be called before GetFrame() is called.
-  virtual base::Optional<VideoFrameLayout> NegotiateFrameFormat(
+  // Sets the parameters of allocating frames and the maximum number of frames
+  // which can be allocated. Returns a valid VideoFrameLayout that VideoFrame
+  // will be created by GetFrame().
+  virtual base::Optional<VideoFrameLayout> RequestFrames(
       const VideoFrameLayout& layout,
       const gfx::Rect& visible_rect,
-      const gfx::Size& natural_size) = 0;
+      const gfx::Size& natural_size,
+      size_t max_num_frames) = 0;
 
   // Returns a frame from the pool with the parameters assigned by
   // SetFrameFormat() and zero timestamp. Returns nullptr if the pool is

media/gpu/chromeos/platform_video_frame_pool.cc

@@ -62,7 +62,7 @@ scoped_refptr<VideoFrame> PlatformVideoFramePool::GetFrame() {
   base::AutoLock auto_lock(lock_);
 
   if (!frame_layout_) {
-    VLOGF(1) << "Please call NegotiateFrameFormat() first.";
+    VLOGF(1) << "Please call RequestFrames() first.";
     return nullptr;
   }
 
@@ -104,23 +104,18 @@ scoped_refptr<VideoFrame> PlatformVideoFramePool::GetFrame() {
   return wrapped_frame;
 }
 
-void PlatformVideoFramePool::SetMaxNumFrames(size_t max_num_frames) {
-  DVLOGF(4);
-  base::AutoLock auto_lock(lock_);
-
-  max_num_frames_ = max_num_frames;
-
-  if (frame_available_cb_ && !IsExhausted_Locked())
-    std::move(frame_available_cb_).Run();
-}
-
-base::Optional<VideoFrameLayout> PlatformVideoFramePool::NegotiateFrameFormat(
+base::Optional<VideoFrameLayout> PlatformVideoFramePool::RequestFrames(
     const VideoFrameLayout& layout,
     const gfx::Rect& visible_rect,
-    const gfx::Size& natural_size) {
+    const gfx::Size& natural_size,
+    size_t max_num_frames) {
   DVLOGF(4);
   base::AutoLock auto_lock(lock_);
 
+  visible_rect_ = visible_rect;
+  natural_size_ = natural_size;
+  max_num_frames_ = max_num_frames;
+
   // If the frame layout changed we need to allocate new frames so we will clear
   // the pool here. If only the visible or natural size changed we don't need to
   // allocate new frames, but will just update the properties of wrapped frames
@@ -131,20 +126,22 @@ base::Optional<VideoFrameLayout> PlatformVideoFramePool::NegotiateFrameFormat(
     free_frames_.clear();
   }
 
-  visible_rect_ = visible_rect;
-  natural_size_ = natural_size;
-
   // Create a temporary frame in order to know VideoFrameLayout that VideoFrame
   // that will be allocated in GetFrame() has.
   auto frame = create_frame_cb_.Run(gpu_memory_buffer_factory_, layout.format(),
-                                    layout.coded_size(), visible_rect,
+                                    layout.coded_size(), visible_rect_,
                                     natural_size_, base::TimeDelta());
   if (!frame) {
     VLOGF(1) << "Failed to create video frame";
     return base::nullopt;
   }
-  frame_layout_ = base::make_optional<VideoFrameLayout>(frame->layout());
 
+  // The pool might become available because of |max_num_frames_| increased.
+  // Notify the client if so.
+  if (frame_available_cb_ && !IsExhausted_Locked())
+    std::move(frame_available_cb_).Run();
+
+  frame_layout_ = base::make_optional<VideoFrameLayout>(frame->layout());
   return frame_layout_;
 }
 

media/gpu/chromeos/platform_video_frame_pool.h

@@ -43,11 +43,11 @@ class MEDIA_GPU_EXPORT PlatformVideoFramePool : public DmabufVideoFramePool {
   ~PlatformVideoFramePool() override;
 
   // VideoFramePoolBase Implementation.
-  void SetMaxNumFrames(size_t max_num_frames) override;
-  base::Optional<VideoFrameLayout> NegotiateFrameFormat(
+  base::Optional<VideoFrameLayout> RequestFrames(
       const VideoFrameLayout& layout,
       const gfx::Rect& visible_rect,
-      const gfx::Size& natural_size) override;
+      const gfx::Size& natural_size,
+      size_t max_num_frames) override;
   scoped_refptr<VideoFrame> GetFrame() override;
   bool IsExhausted() override;
   void NotifyWhenFrameAvailable(base::OnceClosure cb) override;

media/gpu/chromeos/platform_video_frame_pool_unittest.cc

@@ -60,17 +60,18 @@ class PlatformVideoFramePoolTest
     pool_.reset(new PlatformVideoFramePool(
         base::BindRepeating(&CreateDmabufVideoFrame)));
     pool_->set_parent_task_runner(base::ThreadTaskRunnerHandle::Get());
-    pool_->SetMaxNumFrames(10);
   }
 
-  void SetFrameFormat(VideoPixelFormat format) {
-    gfx::Size coded_size(320, 240);
-    visible_rect_.set_size(coded_size);
-    natural_size_ = coded_size;
-    layout_ = VideoFrameLayout::Create(format, coded_size);
+  void RequestFrames(VideoPixelFormat format) {
+    constexpr gfx::Size kCodedSize(320, 240);
+    constexpr size_t kNumFrames = 10;
+
+    visible_rect_.set_size(kCodedSize);
+    natural_size_ = kCodedSize;
+    layout_ = VideoFrameLayout::Create(format, kCodedSize);
     DCHECK(layout_);
 
-    pool_->NegotiateFrameFormat(*layout_, visible_rect_, natural_size_);
+    pool_->RequestFrames(*layout_, visible_rect_, natural_size_, kNumFrames);
   }
 
   scoped_refptr<VideoFrame> GetFrame(int timestamp_ms) {
@@ -107,7 +108,7 @@ INSTANTIATE_TEST_SUITE_P(,
                                          PIXEL_FORMAT_ARGB));
 
 TEST_F(PlatformVideoFramePoolTest, SingleFrameReuse) {
-  SetFrameFormat(PIXEL_FORMAT_I420);
+  RequestFrames(PIXEL_FORMAT_I420);
   scoped_refptr<VideoFrame> frame = GetFrame(10);
   DmabufId id = DmabufVideoFramePool::GetDmabufId(*frame);
 
@@ -121,7 +122,7 @@ TEST_F(PlatformVideoFramePoolTest, SingleFrameReuse) {
 }
 
 TEST_F(PlatformVideoFramePoolTest, MultipleFrameReuse) {
-  SetFrameFormat(PIXEL_FORMAT_I420);
+  RequestFrames(PIXEL_FORMAT_I420);
   scoped_refptr<VideoFrame> frame1 = GetFrame(10);
   scoped_refptr<VideoFrame> frame2 = GetFrame(20);
   DmabufId id1 = DmabufVideoFramePool::GetDmabufId(*frame1);
@@ -144,7 +145,7 @@ TEST_F(PlatformVideoFramePoolTest, MultipleFrameReuse) {
 }
 
 TEST_F(PlatformVideoFramePoolTest, FormatChange) {
-  SetFrameFormat(PIXEL_FORMAT_I420);
+  RequestFrames(PIXEL_FORMAT_I420);
   scoped_refptr<VideoFrame> frame_a = GetFrame(10);
   scoped_refptr<VideoFrame> frame_b = GetFrame(10);
 
@@ -158,13 +159,13 @@ TEST_F(PlatformVideoFramePoolTest, FormatChange) {
 
   // Verify that requesting a frame with a different format causes the pool
   // to get drained.
-  SetFrameFormat(PIXEL_FORMAT_I420A);
+  RequestFrames(PIXEL_FORMAT_I420A);
   scoped_refptr<VideoFrame> new_frame = GetFrame(10);
   CheckPoolSize(0u);
 }
 
 TEST_F(PlatformVideoFramePoolTest, UnwrapVideoFrame) {
-  SetFrameFormat(PIXEL_FORMAT_I420);
+  RequestFrames(PIXEL_FORMAT_I420);
   scoped_refptr<VideoFrame> frame_1 = GetFrame(10);
   scoped_refptr<VideoFrame> frame_2 = VideoFrame::WrapVideoFrame(
       frame_1, frame_1->format(), frame_1->visible_rect(),

media/gpu/v4l2/v4l2_slice_video_decoder.cc

@@ -331,7 +331,8 @@ V4L2SliceVideoDecoder::UpdateVideoFramePoolFormat(
   }
 
   gfx::Size natural_size = GetNaturalSize(visible_rect, pixel_aspect_ratio_);
-  return frame_pool_->NegotiateFrameFormat(*layout, visible_rect, natural_size);
+  return frame_pool_->RequestFrames(*layout, visible_rect, natural_size,
+                                    num_output_frames_);
 }
 
 void V4L2SliceVideoDecoder::Reset(base::OnceClosure closure) {
@@ -437,6 +438,8 @@ bool V4L2SliceVideoDecoder::ChangeResolution(gfx::Size pic_size,
   DCHECK_EQ(input_queue_->QueuedBuffersCount(), 0u);
   DCHECK_EQ(output_queue_->QueuedBuffersCount(), 0u);
 
+  num_output_frames_ = num_output_frames;
+
   if (!StopStreamV4L2Queue())
     return false;
 
@@ -473,12 +476,11 @@ bool V4L2SliceVideoDecoder::ChangeResolution(gfx::Size pic_size,
     SetState(State::kError);
     return false;
   }
-  if (output_queue_->AllocatedBuffersCount() != num_output_frames) {
+  if (output_queue_->AllocatedBuffersCount() != num_output_frames_) {
     VLOGF(1) << "Could not allocate requested number of output buffers.";
     SetState(State::kError);
     return false;
   }
-  frame_pool_->SetMaxNumFrames(num_output_frames);
 
   if (!StartStreamV4L2Queue()) {
     SetState(State::kError);

media/gpu/v4l2/v4l2_slice_video_decoder.h

@@ -160,6 +160,10 @@ class MEDIA_GPU_EXPORT V4L2SliceVideoDecoder
 
   // Parameters for generating output VideoFrame.
   base::Optional<VideoFrameLayout> frame_layout_;
+  // Number of output frames requested to |frame_pool_|.
+  // The default value is only used at the first time of
+  // DmabufVideoFramePool::RequestFrames() during Initialize().
+  size_t num_output_frames_ = 1;
   // Ratio of natural_size to visible_rect of the output frame.
   double pixel_aspect_ratio_ = 0.0;
 

media/gpu/vaapi/vaapi_video_decoder.cc

@@ -412,8 +412,8 @@ void VaapiVideoDecoder::ChangeFrameResolutionTask() {
   CHECK(format);
   frame_layout_ = VideoFrameLayout::Create(*format, pic_size);
   DCHECK(frame_layout_);
-  frame_pool_->NegotiateFrameFormat(*frame_layout_, visible_rect, natural_size);
-  frame_pool_->SetMaxNumFrames(decoder_->GetRequiredNumOfPictures());
+  frame_pool_->RequestFrames(*frame_layout_, visible_rect, natural_size,
+                             decoder_->GetRequiredNumOfPictures());
 
   // All pending decode operations will be completed before triggering a
   // resolution change, so we can safely destroy the context here.