media/gpu/vaapiVEA: Copy video frame to surface before VaapiVED::Enco…

…de()

VaapiWrapper::UploadVideoFrameSurface() writes the VideoFrame content
to VASurface. It is executed VaapiVideoEncoderDelegate::Encode() if
VideoFrame is shared memory based one. If VideoFrame is
GpuMemoryBuffer based one, the video frame is imported to VASurface
before calling VaapiVED::Encode().
This CL moves the place of performing UploadVideoFrameSurface() to
VaapiVEA. This can have the consistency the input surface content is
ready before VaapiVED::Encode(). It also enables removing a few
variables in VaapiVED class.

Bug: b:229358029
Test: video_encode_accelerator_tests
Change-Id: Ic69163d19b86795f29946558ccd5e27207e5718d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3590336
Reviewed-by: Miguel Casas-Sanchez <mcasas@chromium.org>
Commit-Queue: Hirokazu Honda <hiroh@chromium.org>
Cr-Commit-Position: refs/heads/main@{#998250}

media/gpu/vaapi/h264_vaapi_video_encoder_delegate.cc

@@ -227,8 +227,6 @@ bool H264VaapiVideoEncoderDelegate::Initialize(
     }
   }
 
-  native_input_mode_ = ave_config.native_input_mode;
-
   visible_size_ = config.input_visible_size;
   // For 4:2:0, the pixel sizes have to be even.
   if ((visible_size_.width() % 2 != 0) || (visible_size_.height() % 2 != 0)) {

media/gpu/vaapi/h264_vaapi_video_encoder_delegate_unittest.cc

@@ -23,7 +23,6 @@ namespace {
 
 VaapiVideoEncoderDelegate::Config kDefaultVEADelegateConfig{
     .max_num_ref_frames = 4,
-    .native_input_mode = false,
     .bitrate_control =
         VaapiVideoEncoderDelegate::BitrateControl::kConstantBitrate,
 };
@@ -164,12 +163,6 @@ class H264VaapiVideoEncoderDelegateTest
 
 std::unique_ptr<VaapiVideoEncoderDelegate::EncodeJob>
 H264VaapiVideoEncoderDelegateTest::CreateEncodeJob(bool keyframe) {
-  auto input_frame = VideoFrame::CreateFrame(
-      kDefaultVEAConfig.input_format, kDefaultVEAConfig.input_visible_size,
-      gfx::Rect(kDefaultVEAConfig.input_visible_size),
-      kDefaultVEAConfig.input_visible_size, base::TimeDelta());
-  LOG_ASSERT(input_frame) << " Failed to create VideoFrame";
-
   auto va_surface = base::MakeRefCounted<VASurface>(
       next_surface_id_++, kDefaultVEAConfig.input_visible_size,
       VA_RT_FORMAT_YUV420, base::DoNothing());
@@ -180,9 +173,11 @@ H264VaapiVideoEncoderDelegateTest::CreateEncodeJob(bool keyframe) {
       kDummyVABufferID, VAEncCodedBufferType,
       kDefaultVEAConfig.input_visible_size.GetArea());
 
+  // TODO(b/229358029): Set a valid timestamp and check the timestamp in
+  // metadata.
+  constexpr base::TimeDelta timestamp;
   return std::make_unique<VaapiVideoEncoderDelegate::EncodeJob>(
-      input_frame, keyframe, next_surface_id_++,
-      kDefaultVEAConfig.input_visible_size, picture,
+      keyframe, timestamp, next_surface_id_++, picture,
       std::move(scoped_va_buffer));
 }
 

media/gpu/vaapi/vaapi_video_encode_accelerator.cc

@@ -358,8 +358,7 @@ void VaapiVideoEncodeAccelerator::InitializeTask(const Config& config) {
       },
       base::Unretained(this));
 
-  VaapiVideoEncoderDelegate::Config ave_config{.native_input_mode =
-                                                   native_input_mode_};
+  VaapiVideoEncoderDelegate::Config ave_config{};
   switch (output_codec_) {
     case VideoCodec::kH264:
       if (!IsConfiguredForTesting()) {
@@ -671,6 +670,12 @@ bool VaapiVideoEncodeAccelerator::CreateSurfacesForShmemEncoding(
     return false;
   }
 
+  if (!vaapi_wrapper_->UploadVideoFrameToSurface(frame, (*input_surface)->id(),
+                                                 (*input_surface)->size())) {
+    NOTIFY_ERROR(kPlatformFailureError, "Failed to upload frame");
+    return false;
+  }
+
   *reconstructed_surface = CreateEncodeSurface(encode_size);
   return !!*reconstructed_surface;
 }
@@ -795,12 +800,11 @@ scoped_refptr<VASurface> VaapiVideoEncodeAccelerator::ExecuteBlitSurface(
 
 std::unique_ptr<VaapiVideoEncoderDelegate::EncodeJob>
 VaapiVideoEncodeAccelerator::CreateEncodeJob(
-    scoped_refptr<VideoFrame> frame,
     bool force_keyframe,
+    base::TimeDelta frame_timestamp,
     const VASurface& input_surface,
     scoped_refptr<VASurface> reconstructed_surface) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(encoder_sequence_checker_);
-  DCHECK(frame);
   DCHECK_NE(input_surface.id(), VA_INVALID_ID);
   DCHECK(!input_surface.size().IsEmpty());
   DCHECK(reconstructed_surface);
@@ -832,8 +836,8 @@ VaapiVideoEncodeAccelerator::CreateEncodeJob(
       return nullptr;
   }
 
-  return std::make_unique<EncodeJob>(frame, force_keyframe, input_surface.id(),
-                                     input_surface.size(), std::move(picture),
+  return std::make_unique<EncodeJob>(force_keyframe, frame_timestamp,
+                                     input_surface.id(), std::move(picture),
                                      std::move(coded_buffer));
 }
 
@@ -892,7 +896,7 @@ void VaapiVideoEncodeAccelerator::EncodePendingInputs() {
       TRACE_EVENT0("media,gpu", "VAVEA::FromCreateEncodeJobToReturn");
       const bool force_key =
           (spatial_idx == 0 ? input_frame->force_keyframe : false);
-      job = CreateEncodeJob(input_frame->frame, force_key,
+      job = CreateEncodeJob(force_key, input_frame->frame->timestamp(),
                             *input_surfaces[spatial_idx],
                             std::move(reconstructed_surfaces[spatial_idx]));
       if (!job)

media/gpu/vaapi/vaapi_video_encode_accelerator.h

@@ -178,8 +178,8 @@ class MEDIA_GPU_EXPORT VaapiVideoEncodeAccelerator
   // are available, and if so, claims them by associating them with
   // a EncodeJob, and returns the newly-created job, nullptr otherwise.
   std::unique_ptr<EncodeJob> CreateEncodeJob(
-      scoped_refptr<VideoFrame> frame,
       bool force_keyframe,
+      base::TimeDelta frame_timestamp,
       const VASurface& input_surface,
       scoped_refptr<VASurface> reconstructed_surface);
 

media/gpu/vaapi/vaapi_video_encode_accelerator_unittest.cc

@@ -97,13 +97,11 @@ bool IsSVCSupported(const VideoEncodeAccelerator::Config& config) {
          config.output_profile == VP9PROFILE_PROFILE0;
 }
 
-MATCHER_P3(MatchesVaapiVideoEncoderDelegateConfig,
+MATCHER_P2(MatchesVaapiVideoEncoderDelegateConfig,
            max_ref_frames,
-           native_input_mode,
            bitrate_control,
            "") {
   return arg.max_num_ref_frames == max_ref_frames &&
-         arg.native_input_mode == native_input_mode &&
          arg.bitrate_control == bitrate_control;
 }
 
@@ -222,9 +220,6 @@ class MockVP9VaapiVideoEncoderDelegate : public VP9VaapiVideoEncoderDelegate {
   bool UpdateRates(const VideoBitrateAllocation&, uint32_t) override {
     return false;
   }
-  void set_native_input_mode(bool native_input_mode) {
-    native_input_mode_ = native_input_mode;
-  }
 };
 }  // namespace
 
@@ -311,10 +306,6 @@ class VaapiVideoEncodeAcceleratorTest
     ::testing::InSequence s;
     constexpr auto kBitrateControl = VaapiVideoEncoderDelegate::BitrateControl::
         kConstantQuantizationParameter;
-    const bool native_input_mode =
-        config.storage_type.value_or(
-            VideoEncodeAccelerator::Config::StorageType::kShmem) ==
-        VideoEncodeAccelerator::Config::StorageType::kGpuMemoryBuffer;
     const size_t num_spatial_layers = config.spatial_layers.size();
     // Scaling is needed only for non highest spatial layer, so here the vpp
     // number is |num_spatial_layers - 1|.
@@ -334,14 +325,8 @@ class VaapiVideoEncodeAcceleratorTest
 
     EXPECT_CALL(*mock_encoder_,
                 Initialize(_, MatchesVaapiVideoEncoderDelegateConfig(
-                                  kMaxNumOfRefFrames, native_input_mode,
-                                  kBitrateControl)))
-        .WillOnce(WithArgs<1>(
-            [mock_encoder = mock_encoder_](
-                const VaapiVideoEncoderDelegate::Config& ave_config) {
-              mock_encoder->set_native_input_mode(ave_config.native_input_mode);
-              return true;
-            }));
+                                  kMaxNumOfRefFrames, kBitrateControl)))
+        .WillOnce(Return(true));
     EXPECT_CALL(*mock_vaapi_wrapper_, CreateContext(kDefaultEncodeSize))
         .WillOnce(Return(true));
     EXPECT_CALL(client_, RequireBitstreamBuffers(_, kDefaultEncodeSize, _))
@@ -377,6 +362,11 @@ class VaapiVideoEncodeAcceleratorTest
               return va_surfaces;
             }));
 
+    EXPECT_CALL(
+        *mock_vaapi_wrapper_,
+        UploadVideoFrameToSurface(_, kInputSurfaceId, kDefaultEncodeSize))
+        .WillOnce(Return(true));
+
     constexpr VASurfaceID kEncodeSurfaceId = 1234;
     EXPECT_CALL(*mock_vaapi_wrapper_,
                 CreateScopedVASurfaces(
@@ -420,10 +410,6 @@ class VaapiVideoEncodeAcceleratorTest
           }
           return true;
         }));
-    EXPECT_CALL(
-        *mock_vaapi_wrapper_,
-        UploadVideoFrameToSurface(_, kInputSurfaceId, kDefaultEncodeSize))
-        .WillOnce(Return(true));
     EXPECT_CALL(*mock_vaapi_wrapper_,
                 ExecuteAndDestroyPendingBuffers(kInputSurfaceId))
         .WillOnce(Return(true));

media/gpu/vaapi/vaapi_video_encoder_delegate.cc

@@ -19,28 +19,26 @@
 namespace media {
 
 VaapiVideoEncoderDelegate::EncodeJob::EncodeJob(
-    scoped_refptr<VideoFrame> input_frame,
     bool keyframe,
+    base::TimeDelta timestamp,
     VASurfaceID input_surface_id,
-    const gfx::Size& input_surface_size,
     scoped_refptr<CodecPicture> picture,
     std::unique_ptr<ScopedVABuffer> coded_buffer)
-    : input_frame_(input_frame),
-      keyframe_(keyframe),
+    : keyframe_(keyframe),
+      timestamp_(timestamp),
       input_surface_id_(input_surface_id),
-      input_surface_size_(input_surface_size),
       picture_(std::move(picture)),
       coded_buffer_(std::move(coded_buffer)) {
   DCHECK(picture_);
   DCHECK(coded_buffer_);
 }
 
-VaapiVideoEncoderDelegate::EncodeJob::EncodeJob(
-    scoped_refptr<VideoFrame> input_frame,
-    bool keyframe)
-    : input_frame_(input_frame),
-      keyframe_(keyframe),
-      input_surface_id_(VA_INVALID_ID) {}
+VaapiVideoEncoderDelegate::EncodeJob::EncodeJob(bool keyframe,
+                                                base::TimeDelta timestamp,
+                                                VASurfaceID input_surface_id)
+    : keyframe_(keyframe),
+      timestamp_(timestamp),
+      input_surface_id_(input_surface_id) {}
 
 VaapiVideoEncoderDelegate::EncodeJob::~EncodeJob() = default;
 
@@ -51,12 +49,7 @@ VaapiVideoEncoderDelegate::EncodeJob::CreateEncodeResult(
 }
 
 base::TimeDelta VaapiVideoEncoderDelegate::EncodeJob::timestamp() const {
-  return input_frame_->timestamp();
-}
-
-const scoped_refptr<VideoFrame>&
-VaapiVideoEncoderDelegate::EncodeJob::input_frame() const {
-  return input_frame_;
+  return timestamp_;
 }
 
 VABufferID VaapiVideoEncoderDelegate::EncodeJob::coded_buffer_id() const {
@@ -67,11 +60,6 @@ VASurfaceID VaapiVideoEncoderDelegate::EncodeJob::input_surface_id() const {
   return input_surface_id_;
 }
 
-const gfx::Size& VaapiVideoEncoderDelegate::EncodeJob::input_surface_size()
-    const {
-  return input_surface_size_;
-}
-
 const scoped_refptr<CodecPicture>&
 VaapiVideoEncoderDelegate::EncodeJob::picture() const {
   return picture_;
@@ -128,15 +116,8 @@ bool VaapiVideoEncoderDelegate::Encode(EncodeJob& encode_job) {
     return false;
   }
 
-  const VASurfaceID va_surface_id = encode_job.input_surface_id();
-  if (!native_input_mode_ && !vaapi_wrapper_->UploadVideoFrameToSurface(
-                                 *encode_job.input_frame(), va_surface_id,
-                                 encode_job.input_surface_size())) {
-    VLOGF(1) << "Failed to upload frame";
-    return false;
-  }
-
-  if (!vaapi_wrapper_->ExecuteAndDestroyPendingBuffers(va_surface_id)) {
+  if (!vaapi_wrapper_->ExecuteAndDestroyPendingBuffers(
+          encode_job.input_surface_id())) {
     VLOGF(1) << "Failed to execute encode";
     return false;
   }

media/gpu/vaapi/vaapi_video_encoder_delegate.h

@@ -52,14 +52,12 @@ class VaapiVideoEncoderDelegate {
   };
 
   struct Config {
-    // Maxium number of reference frames.
+    // Maximum number of reference frames.
     // For H.264 encoding, the value represents the maximum number of reference
     // frames for both the reference picture list 0 (bottom 16 bits) and the
     // reference picture list 1 (top 16 bits).
     size_t max_num_ref_frames;
 
-    bool native_input_mode = false;
-
     BitrateControl bitrate_control = BitrateControl::kConstantBitrate;
   };
 
@@ -91,12 +89,13 @@ class VaapiVideoEncoderDelegate {
     // Creates an EncodeJob to encode |input_frame|, which will be executed by
     // calling ExecuteSetupCallbacks() in VaapiVideoEncoderDelegate::Encode().
     // If |keyframe| is true, requests this job to produce a keyframe.
-    EncodeJob(scoped_refptr<VideoFrame> input_frame, bool keyframe);
+    EncodeJob(bool keyframe,
+              base::TimeDelta timestamp,
+              VASurfaceID input_surface_id);
     // Constructor for VA-API.
-    EncodeJob(scoped_refptr<VideoFrame> input_frame,
-              bool keyframe,
+    EncodeJob(bool keyframe,
+              base::TimeDelta timestamp,
               VASurfaceID input_surface_id,
-              const gfx::Size& input_surface_size,
               scoped_refptr<CodecPicture> picture,
               std::unique_ptr<ScopedVABuffer> coded_buffer);
 
@@ -121,25 +120,20 @@ class VaapiVideoEncoderDelegate {
 
     base::TimeDelta timestamp() const;
 
-    const scoped_refptr<VideoFrame>& input_frame() const;
-
     // VA-API specific methods.
     VABufferID coded_buffer_id() const;
     VASurfaceID input_surface_id() const;
-    const gfx::Size& input_surface_size() const;
     const scoped_refptr<CodecPicture>& picture() const;
 
    private:
-    // Input VideoFrame to be encoded.
-    const scoped_refptr<VideoFrame> input_frame_;
-
     // True if this job is to produce a keyframe.
     bool keyframe_;
+    // |timestamp_| to be added to the produced encoded chunk.
+    const base::TimeDelta timestamp_;
 
     // VA-API specific members.
     // Input surface ID and size for video frame data or scaled data.
     const VASurfaceID input_surface_id_;
-    const gfx::Size input_surface_size_;
     const scoped_refptr<CodecPicture> picture_;
     // Buffer that will contain the output bitstream data for this frame.
     std::unique_ptr<ScopedVABuffer> coded_buffer_;
@@ -192,8 +186,6 @@ class VaapiVideoEncoderDelegate {
 
   base::RepeatingClosure error_cb_;
 
-  bool native_input_mode_ = false;
-
   SEQUENCE_CHECKER(sequence_checker_);
 
  private:

media/gpu/vaapi/vp8_vaapi_video_encoder_delegate.cc

@@ -310,8 +310,6 @@ bool VP8VaapiVideoEncoderDelegate::Initialize(
     }
   }
 
-  native_input_mode_ = ave_config.native_input_mode;
-
   visible_size_ = config.input_visible_size;
   coded_size_ = gfx::Size(base::bits::AlignUp(visible_size_.width(), 16),
                           base::bits::AlignUp(visible_size_.height(), 16));

media/gpu/vaapi/vp9_vaapi_video_encoder_delegate.cc

@@ -193,8 +193,6 @@ bool VP9VaapiVideoEncoderDelegate::Initialize(
     return false;
   }
 
-  native_input_mode_ = ave_config.native_input_mode;
-
   visible_size_ = config.input_visible_size;
   coded_size_ = gfx::Size(base::bits::AlignUp(visible_size_.width(), 16),
                           base::bits::AlignUp(visible_size_.height(), 16));

media/gpu/vaapi/vp9_vaapi_video_encoder_delegate_unittest.cc

@@ -50,7 +50,6 @@ constexpr uint8_t kTemporalLayerPattern[][4] = {
 
 VaapiVideoEncoderDelegate::Config kDefaultVaapiVideoEncoderDelegateConfig{
     .max_num_ref_frames = kDefaultMaxNumRefFrames,
-    .native_input_mode = false,
     .bitrate_control = VaapiVideoEncoderDelegate::BitrateControl::
         kConstantQuantizationParameter};
 
@@ -324,21 +323,16 @@ VP9VaapiVideoEncoderDelegateTest::CreateEncodeJob(
     bool keyframe,
     const scoped_refptr<VASurface>& va_surface,
     const scoped_refptr<VP9Picture>& picture) {
-  auto input_frame = VideoFrame::CreateFrame(
-      kDefaultVideoEncodeAcceleratorConfig.input_format,
-      kDefaultVideoEncodeAcceleratorConfig.input_visible_size,
-      gfx::Rect(kDefaultVideoEncodeAcceleratorConfig.input_visible_size),
-      kDefaultVideoEncodeAcceleratorConfig.input_visible_size,
-      base::TimeDelta());
-  LOG_ASSERT(input_frame) << " Failed to create VideoFrame";
-
   constexpr VABufferID kDummyVABufferID = 12;
   auto scoped_va_buffer = ScopedVABuffer::CreateForTesting(
       kDummyVABufferID, VAEncCodedBufferType,
       kDefaultVideoEncodeAcceleratorConfig.input_visible_size.GetArea());
 
+  // TODO(b/229358029): Set a valid timestamp and check the timestamp in
+  // metadata.
+  constexpr base::TimeDelta timestamp;
   return std::make_unique<VaapiVideoEncoderDelegate::EncodeJob>(
-      input_frame, keyframe, va_surface->id(), va_surface->size(), picture,
+      keyframe, timestamp, va_surface->id(), picture,
       std::move(scoped_va_buffer));
 }