media/gpu/chromeos: Specify DmabufVideoFramePool format by Fourcc.

Originally DmabufVideoFramePool allocates frames by VideoFrameLayout,
which mainly contains VideoPixelFormat and frame size.
VideoPixelFormat is a enum that lists well-known pixel formats.
However, we might need buffers with proprietary format during
hardware-accelerated video decoding.

To support that, we should use fourcc, which could also list
proprietary formats, to indicate pixel format. This CL changes
DmabufVideoFramePool to allocate frames by Fourcc instead of
VideoPixelFormat.

Bug: 1004727
Test: Run video_decode_accelerator_tests on Kevin and Eve
Test: media_unittests --gtest_filter=FourccTest.*
Test: media_unittests --gtest_filter=PlatformVideoFramePoolTest.*

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

media/gpu/chromeos/dmabuf_video_frame_pool.h

@@ -9,7 +9,8 @@
 #include "base/optional.h"
 #include "base/sequenced_task_runner.h"
 #include "media/base/video_frame.h"
-#include "media/base/video_frame_layout.h"
+#include "media/gpu/chromeos/fourcc.h"
+#include "media/gpu/chromeos/gpu_buffer_layout.h"
 #include "media/gpu/media_gpu_export.h"
 #include "ui/gfx/geometry/rect.h"
 #include "ui/gfx/geometry/size.h"
@@ -39,10 +40,11 @@ class MEDIA_GPU_EXPORT DmabufVideoFramePool {
       scoped_refptr<base::SequencedTaskRunner> parent_task_runner);
 
   // Sets the parameters of allocating frames and the maximum number of frames
-  // which can be allocated. Returns a valid VideoFrameLayout that VideoFrame
+  // which can be allocated. Returns a valid GpuBufferLayout if VideoFrame
   // will be created by GetFrame().
-  virtual base::Optional<VideoFrameLayout> RequestFrames(
-      const VideoFrameLayout& layout,
+  virtual base::Optional<GpuBufferLayout> RequestFrames(
+      const Fourcc& fourcc,
+      const gfx::Size& coded_size,
       const gfx::Rect& visible_rect,
       const gfx::Size& natural_size,
       size_t max_num_frames) = 0;

media/gpu/chromeos/platform_video_frame_pool.cc

@@ -9,6 +9,7 @@
 #include "base/logging.h"
 #include "base/optional.h"
 #include "base/task/post_task.h"
+#include "media/gpu/chromeos/gpu_buffer_layout.h"
 #include "media/gpu/linux/platform_video_frame_utils.h"
 #include "media/gpu/macros.h"
 
@@ -66,8 +67,8 @@ scoped_refptr<VideoFrame> PlatformVideoFramePool::GetFrame() {
     return nullptr;
   }
 
-  VideoPixelFormat format = frame_layout_->format();
-  const gfx::Size& coded_size = frame_layout_->coded_size();
+  VideoPixelFormat format = frame_layout_->fourcc().ToVideoPixelFormat();
+  const gfx::Size& coded_size = frame_layout_->size();
   if (free_frames_.empty()) {
     if (GetTotalNumFrames_Locked() >= max_num_frames_)
       return nullptr;
@@ -104,8 +105,9 @@ scoped_refptr<VideoFrame> PlatformVideoFramePool::GetFrame() {
   return wrapped_frame;
 }
 
-base::Optional<VideoFrameLayout> PlatformVideoFramePool::RequestFrames(
-    const VideoFrameLayout& layout,
+base::Optional<GpuBufferLayout> PlatformVideoFramePool::RequestFrames(
+    const Fourcc& fourcc,
+    const gfx::Size& coded_size,
     const gfx::Rect& visible_rect,
     const gfx::Size& natural_size,
     size_t max_num_frames) {
@@ -116,21 +118,28 @@ base::Optional<VideoFrameLayout> PlatformVideoFramePool::RequestFrames(
   natural_size_ = natural_size;
   max_num_frames_ = max_num_frames;
 
+  // Only support the Fourcc that could map to VideoPixelFormat.
+  VideoPixelFormat format = fourcc.ToVideoPixelFormat();
+  if (format == PIXEL_FORMAT_UNKNOWN) {
+    VLOGF(1) << "Unsupported fourcc: " << fourcc.ToString();
+    return base::nullopt;
+  }
+
   // 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
   // returned by GetFrame().
   // NOTE: It is assumed layout is determined by |format| and |coded_size|.
-  if (!IsSameLayout_Locked(layout)) {
+  if (!IsSameFormat_Locked(format, coded_size)) {
     DVLOGF(4) << "The video frame format is changed. Clearing the pool.";
     free_frames_.clear();
   }
 
   // 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_,
-                                    natural_size_, base::TimeDelta());
+  auto frame =
+      create_frame_cb_.Run(gpu_memory_buffer_factory_, format, coded_size,
+                           visible_rect_, natural_size_, base::TimeDelta());
   if (!frame) {
     VLOGF(1) << "Failed to create video frame";
     return base::nullopt;
@@ -141,7 +150,8 @@ base::Optional<VideoFrameLayout> PlatformVideoFramePool::RequestFrames(
   if (frame_available_cb_ && !IsExhausted_Locked())
     std::move(frame_available_cb_).Run();
 
-  frame_layout_ = base::make_optional<VideoFrameLayout>(frame->layout());
+  frame_layout_ = GpuBufferLayout::Create(fourcc, frame->coded_size(),
+                                          frame->layout().planes());
   return frame_layout_;
 }
 
@@ -204,7 +214,7 @@ void PlatformVideoFramePool::OnFrameReleased(
   DCHECK(it != frames_in_use_.end());
   frames_in_use_.erase(it);
 
-  if (IsSameLayout_Locked(origin_frame->layout())) {
+  if (IsSameFormat_Locked(origin_frame->format(), origin_frame->coded_size())) {
     InsertFreeFrame_Locked(std::move(origin_frame));
   }
 
@@ -229,13 +239,15 @@ size_t PlatformVideoFramePool::GetTotalNumFrames_Locked() const {
   return free_frames_.size() + frames_in_use_.size();
 }
 
-bool PlatformVideoFramePool::IsSameLayout_Locked(
-    const VideoFrameLayout& layout) const {
+bool PlatformVideoFramePool::IsSameFormat_Locked(
+    VideoPixelFormat format,
+    const gfx::Size& coded_size) const {
   DVLOGF(4);
   lock_.AssertAcquired();
 
-  return frame_layout_ && frame_layout_->format() == layout.format() &&
-         frame_layout_->coded_size() == layout.coded_size();
+  return frame_layout_ &&
+         frame_layout_->fourcc().ToVideoPixelFormat() == format &&
+         frame_layout_->size() == coded_size;
 }
 
 size_t PlatformVideoFramePool::GetPoolSizeForTesting() {

media/gpu/chromeos/platform_video_frame_pool.h

@@ -13,11 +13,12 @@
 #include "base/files/scoped_file.h"
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
+#include "base/optional.h"
 #include "base/sequenced_task_runner.h"
 #include "base/synchronization/lock.h"
 #include "base/thread_annotations.h"
 #include "media/base/video_frame.h"
-#include "media/base/video_frame_layout.h"
+#include "media/base/video_types.h"
 #include "media/gpu/chromeos/dmabuf_video_frame_pool.h"
 #include "media/gpu/media_gpu_export.h"
 
@@ -43,8 +44,8 @@ class MEDIA_GPU_EXPORT PlatformVideoFramePool : public DmabufVideoFramePool {
   ~PlatformVideoFramePool() override;
 
   // VideoFramePoolBase Implementation.
-  base::Optional<VideoFrameLayout> RequestFrames(
-      const VideoFrameLayout& layout,
+  base::Optional<GpuBufferLayout> RequestFrames(const Fourcc& fourcc,
+                                                const gfx::Size& coded_size,
                                                 const gfx::Rect& visible_rect,
                                                 const gfx::Size& natural_size,
                                                 size_t max_num_frames) override;
@@ -92,7 +93,8 @@ class MEDIA_GPU_EXPORT PlatformVideoFramePool : public DmabufVideoFramePool {
   void InsertFreeFrame_Locked(scoped_refptr<VideoFrame> frame)
       EXCLUSIVE_LOCKS_REQUIRED(lock_);
   size_t GetTotalNumFrames_Locked() const EXCLUSIVE_LOCKS_REQUIRED(lock_);
-  bool IsSameLayout_Locked(const VideoFrameLayout& layout) const
+  bool IsSameFormat_Locked(VideoPixelFormat format,
+                           const gfx::Size& coded_size) const
       EXCLUSIVE_LOCKS_REQUIRED(lock_);
   bool IsExhausted_Locked() EXCLUSIVE_LOCKS_REQUIRED(lock_);
 
@@ -109,9 +111,9 @@ class MEDIA_GPU_EXPORT PlatformVideoFramePool : public DmabufVideoFramePool {
   gpu::GpuMemoryBufferFactory* const gpu_memory_buffer_factory_ = nullptr;
 
   // The arguments of current frame. We allocate new frames only if a pixel
-  // format or coded size in |frame_layout_| is changed. When GetFrame() is
+  // format or size in |frame_layout_| is changed. When GetFrame() is
   // called, we update |visible_rect_| and |natural_size_| of wrapped frames.
-  base::Optional<VideoFrameLayout> frame_layout_ GUARDED_BY(lock_);
+  base::Optional<GpuBufferLayout> frame_layout_ GUARDED_BY(lock_);
   gfx::Rect visible_rect_ GUARDED_BY(lock_);
   gfx::Size natural_size_ GUARDED_BY(lock_);
 

media/gpu/chromeos/platform_video_frame_pool_unittest.cc

@@ -15,6 +15,7 @@
 #include "base/files/scoped_file.h"
 #include "base/test/task_environment.h"
 #include "base/threading/thread_task_runner_handle.h"
+#include "media/gpu/chromeos/fourcc.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace media {
@@ -62,24 +63,24 @@ class PlatformVideoFramePoolTest
     pool_->set_parent_task_runner(base::ThreadTaskRunnerHandle::Get());
   }
 
-  void RequestFrames(VideoPixelFormat format) {
+  void RequestFrames(const Fourcc& fourcc) {
     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_->RequestFrames(*layout_, visible_rect_, natural_size_, kNumFrames);
+    layout_ = pool_->RequestFrames(fourcc, kCodedSize, visible_rect_,
+                                   natural_size_, kNumFrames);
+    EXPECT_TRUE(layout_);
   }
 
   scoped_refptr<VideoFrame> GetFrame(int timestamp_ms) {
     scoped_refptr<VideoFrame> frame = pool_->GetFrame();
     frame->set_timestamp(base::TimeDelta::FromMilliseconds(timestamp_ms));
 
-    EXPECT_EQ(layout_->format(), frame->format());
-    EXPECT_EQ(layout_->coded_size(), frame->coded_size());
+    EXPECT_EQ(layout_->fourcc(), Fourcc::FromVideoPixelFormat(frame->format()));
+    EXPECT_EQ(layout_->size(), frame->coded_size());
     EXPECT_EQ(visible_rect_, frame->visible_rect());
     EXPECT_EQ(natural_size_, frame->natural_size());
 
@@ -96,7 +97,7 @@ class PlatformVideoFramePoolTest
                   std::default_delete<DmabufVideoFramePool>>
       pool_;
 
-  base::Optional<VideoFrameLayout> layout_;
+  base::Optional<GpuBufferLayout> layout_;
   gfx::Rect visible_rect_;
   gfx::Size natural_size_;
 };
@@ -108,7 +109,7 @@ INSTANTIATE_TEST_SUITE_P(,
                                          PIXEL_FORMAT_ARGB));
 
 TEST_F(PlatformVideoFramePoolTest, SingleFrameReuse) {
-  RequestFrames(PIXEL_FORMAT_I420);
+  RequestFrames(Fourcc(Fourcc::YV12));
   scoped_refptr<VideoFrame> frame = GetFrame(10);
   DmabufId id = DmabufVideoFramePool::GetDmabufId(*frame);
 
@@ -122,7 +123,7 @@ TEST_F(PlatformVideoFramePoolTest, SingleFrameReuse) {
 }
 
 TEST_F(PlatformVideoFramePoolTest, MultipleFrameReuse) {
-  RequestFrames(PIXEL_FORMAT_I420);
+  RequestFrames(Fourcc(Fourcc::YV12));
   scoped_refptr<VideoFrame> frame1 = GetFrame(10);
   scoped_refptr<VideoFrame> frame2 = GetFrame(20);
   DmabufId id1 = DmabufVideoFramePool::GetDmabufId(*frame1);
@@ -145,7 +146,7 @@ TEST_F(PlatformVideoFramePoolTest, MultipleFrameReuse) {
 }
 
 TEST_F(PlatformVideoFramePoolTest, FormatChange) {
-  RequestFrames(PIXEL_FORMAT_I420);
+  RequestFrames(Fourcc(Fourcc::YV12));
   scoped_refptr<VideoFrame> frame_a = GetFrame(10);
   scoped_refptr<VideoFrame> frame_b = GetFrame(10);
 
@@ -159,13 +160,13 @@ TEST_F(PlatformVideoFramePoolTest, FormatChange) {
 
   // Verify that requesting a frame with a different format causes the pool
   // to get drained.
-  RequestFrames(PIXEL_FORMAT_I420A);
+  RequestFrames(Fourcc(Fourcc::NV12));
   scoped_refptr<VideoFrame> new_frame = GetFrame(10);
   CheckPoolSize(0u);
 }
 
 TEST_F(PlatformVideoFramePoolTest, UnwrapVideoFrame) {
-  RequestFrames(PIXEL_FORMAT_I420);
+  RequestFrames(Fourcc(Fourcc::YV12));
   scoped_refptr<VideoFrame> frame_1 = GetFrame(10);
   scoped_refptr<VideoFrame> frame_2 = VideoFrame::WrapVideoFrame(
       frame_1, frame_1->format(), frame_1->visible_rect(),
@@ -178,4 +179,7 @@ TEST_F(PlatformVideoFramePoolTest, UnwrapVideoFrame) {
   EXPECT_FALSE(frame_1->IsSameDmaBufsAs(*frame_3));
 }
 
+// TODO(akahuang): Add a testcase to verify calling RequestFrames() only with
+// different |max_num_frames|.
+
 }  // namespace media

media/gpu/v4l2/v4l2_slice_video_decoder.cc

@@ -271,7 +271,7 @@ bool V4L2SliceVideoDecoder::SetCodedSizeOnInputQueue(
   return true;
 }
 
-base::Optional<VideoFrameLayout> V4L2SliceVideoDecoder::SetupOutputFormat(
+base::Optional<GpuBufferLayout> V4L2SliceVideoDecoder::SetupOutputFormat(
     const gfx::Size& size,
     const gfx::Rect& visible_rect) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(decoder_sequence_checker_);
@@ -294,17 +294,19 @@ base::Optional<VideoFrameLayout> V4L2SliceVideoDecoder::SetupOutputFormat(
                             format->fmt.pix_mp.height);
 
     // Make sure VFPool can allocate video frames with width and height.
-    auto frame_layout =
+    auto layout =
         UpdateVideoFramePoolFormat(format_fourcc, adjusted_size, visible_rect);
-    if (frame_layout) {
-      if (frame_layout->coded_size() != adjusted_size) {
-        VLOGF(1) << "The size adjusted by VFPool is different from one "
-                 << "adjusted by a video driver";
+    if (!layout)
       continue;
-      }
 
-      return frame_layout;
+    if (layout->size() != adjusted_size) {
+      VLOGF(1) << "The size adjusted by VFPool is different from one "
+               << "adjusted by a video driver. fourcc: " << format_fourcc
+               << ", (video driver v.s. VFPool) " << adjusted_size.ToString()
+               << " != " << layout->size().ToString();
+      continue;
     }
+    return layout;
   }
 
   // TODO(akahuang): Use ImageProcessor in this case.
@@ -314,25 +316,15 @@ base::Optional<VideoFrameLayout> V4L2SliceVideoDecoder::SetupOutputFormat(
   return base::nullopt;
 }
 
-base::Optional<VideoFrameLayout>
+base::Optional<GpuBufferLayout>
 V4L2SliceVideoDecoder::UpdateVideoFramePoolFormat(
     uint32_t output_format_fourcc,
     const gfx::Size& size,
     const gfx::Rect& visible_rect) {
-  VideoPixelFormat output_format =
-      Fourcc::FromV4L2PixFmt(output_format_fourcc).ToVideoPixelFormat();
-  if (output_format == PIXEL_FORMAT_UNKNOWN) {
-    return base::nullopt;
-  }
-  auto layout = VideoFrameLayout::Create(output_format, size);
-  if (!layout) {
-    VLOGF(1) << "Failed to create video frame layout.";
-    return base::nullopt;
-  }
-
   gfx::Size natural_size = GetNaturalSize(visible_rect, pixel_aspect_ratio_);
-  return frame_pool_->RequestFrames(*layout, visible_rect, natural_size,
-                                    num_output_frames_);
+  return frame_pool_->RequestFrames(
+      Fourcc::FromV4L2PixFmt(output_format_fourcc), size, visible_rect,
+      natural_size, num_output_frames_);
 }
 
 void V4L2SliceVideoDecoder::Reset(base::OnceClosure closure) {
@@ -454,14 +446,14 @@ bool V4L2SliceVideoDecoder::ChangeResolution(gfx::Size pic_size,
     return false;
   }
 
-  auto frame_layout = SetupOutputFormat(pic_size, visible_rect);
-  if (!frame_layout) {
+  auto layout = SetupOutputFormat(pic_size, visible_rect);
+  if (!layout) {
     VLOGF(1) << "No format is available with thew new resolution";
     SetState(State::kError);
     return false;
   }
 
-  auto coded_size = frame_layout->coded_size();
+  auto coded_size = layout->size();
   DCHECK_EQ(coded_size.width() % 16, 0);
   DCHECK_EQ(coded_size.height() % 16, 0);
   if (!gfx::Rect(coded_size).Contains(gfx::Rect(pic_size))) {

media/gpu/v4l2/v4l2_slice_video_decoder.h

@@ -25,6 +25,7 @@
 #include "base/time/time.h"
 #include "media/base/video_frame_layout.h"
 #include "media/base/video_types.h"
+#include "media/gpu/chromeos/gpu_buffer_layout.h"
 #include "media/gpu/chromeos/video_decoder_pipeline.h"
 #include "media/gpu/media_gpu_export.h"
 #include "media/gpu/v4l2/v4l2_device.h"
@@ -119,12 +120,12 @@ class MEDIA_GPU_EXPORT V4L2SliceVideoDecoder
   // in VideoFramePool. The returned VideoFrameLayout is one of VideoFrame that
   // VideoFramePool will allocate. Returns base::nullopt on failure of if there
   // is no format that satisfies the above conditions.
-  base::Optional<VideoFrameLayout> SetupOutputFormat(
+  base::Optional<GpuBufferLayout> SetupOutputFormat(
       const gfx::Size& size,
       const gfx::Rect& visible_rect);
   // Update the format of frames in |frame_pool_| with |output_format_fourcc|,
   // |size| and |visible_rect|.
-  base::Optional<VideoFrameLayout> UpdateVideoFramePoolFormat(
+  base::Optional<GpuBufferLayout> UpdateVideoFramePoolFormat(
       uint32_t output_format_fourcc,
       const gfx::Size& size,
       const gfx::Rect& visible_rect);

media/gpu/vaapi/vaapi_video_decoder.cc

@@ -410,9 +410,8 @@ void VaapiVideoDecoder::ChangeFrameResolutionTask() {
   const base::Optional<VideoPixelFormat> format =
       GfxBufferFormatToVideoPixelFormat(GetBufferFormat());
   CHECK(format);
-  frame_layout_ = VideoFrameLayout::Create(*format, pic_size);
-  DCHECK(frame_layout_);
-  frame_pool_->RequestFrames(*frame_layout_, visible_rect, natural_size,
+  frame_pool_->RequestFrames(Fourcc::FromVideoPixelFormat(*format), pic_size,
+                             visible_rect, natural_size,
                              decoder_->GetRequiredNumOfPictures());
 
   // All pending decode operations will be completed before triggering a

media/gpu/vaapi/vaapi_video_decoder.h

@@ -140,8 +140,6 @@ class VaapiVideoDecoder : public VideoDecoderPipeline::DecoderInterface,
   // The video stream's profile.
   VideoCodecProfile profile_ = VIDEO_CODEC_PROFILE_UNKNOWN;
 
-  // Output frame properties.
-  base::Optional<VideoFrameLayout> frame_layout_;
   // Ratio of natural size to |visible_rect_| of the output frames.
   double pixel_aspect_ratio_ = 0.0;