media/gpu/vaapi: Handle profile change in VaapiVD/VDA

crrev.com/c/1923986 handles a profile change in V4L2SVD/VDA. This CL
handles a profile change in VaapiVD/VDA.

Since VA-API is initialized with a codec profile, it is necessary to
reinitialize VA-API with a new profile. VaapiWrapper manages several
VA-API resources. For example, surfaces will be destroyed after
the profile change. It will be easier to replace VaapiWrapper used in
AVD::Accelerator with a new VaapiWrapper than reinitialize the
existing VaapiWrapper properly.

Bug: 1022246, 1029397
Test: video.DecodeAccel(VD).h264_profile_change (crrev.com/c/1924217) on eve and grunt
Change-Id: Ic3ff0b231cf454ce61193ee9f2642050d588ef24
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1930368
Commit-Queue: Hirokazu Honda <hiroh@chromium.org>
Reviewed-by: Miguel Casas <mcasas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#729685}

media/gpu/vaapi/vaapi_video_decode_accelerator.cc

@@ -200,16 +200,22 @@ bool VaapiVideoDecodeAccelerator::Initialize(const Config& config,
   }
 
   if (profile >= H264PROFILE_MIN && profile <= H264PROFILE_MAX) {
-    decoder_.reset(new H264Decoder(
-        std::make_unique<H264VaapiVideoDecoderDelegate>(this, vaapi_wrapper_),
-        profile, config.container_color_space));
+    auto accelerator =
+        std::make_unique<H264VaapiVideoDecoderDelegate>(this, vaapi_wrapper_);
+    decoder_delegate_ = accelerator.get();
+    decoder_.reset(new H264Decoder(std::move(accelerator), profile,
+                                   config.container_color_space));
   } else if (profile >= VP8PROFILE_MIN && profile <= VP8PROFILE_MAX) {
-    decoder_.reset(new VP8Decoder(
-        std::make_unique<VP8VaapiVideoDecoderDelegate>(this, vaapi_wrapper_)));
+    auto accelerator =
+        std::make_unique<VP8VaapiVideoDecoderDelegate>(this, vaapi_wrapper_);
+    decoder_delegate_ = accelerator.get();
+    decoder_.reset(new VP8Decoder(std::move(accelerator)));
   } else if (profile >= VP9PROFILE_MIN && profile <= VP9PROFILE_MAX) {
-    decoder_.reset(new VP9Decoder(
-        std::make_unique<VP9VaapiVideoDecoderDelegate>(this, vaapi_wrapper_),
-        profile, config.container_color_space));
+    auto accelerator =
+        std::make_unique<VP9VaapiVideoDecoderDelegate>(this, vaapi_wrapper_);
+    decoder_delegate_ = accelerator.get();
+    decoder_.reset(new VP9Decoder(std::move(accelerator), profile,
+                                  config.container_color_space));
   } else {
     VLOGF(1) << "Unsupported profile " << GetProfileName(profile);
     return false;
@@ -445,20 +451,14 @@ void VaapiVideoDecodeAccelerator::DecodeTask() {
 
     switch (res) {
       case AcceleratedVideoDecoder::kConfigChange: {
-        gfx::Size new_size = decoder_->GetPicSize();
-        if (profile_ != decoder_->GetProfile() &&
-            requested_pic_size_ == new_size) {
-          // Profile only is changed.
-          // TODO(crbug.com/1022246): Handle profile change.
-          continue;
-        }
         VLOGF(2) << "Decoder requesting a new set of surfaces";
         task_runner_->PostTask(
             FROM_HERE,
             base::BindOnce(
                 &VaapiVideoDecodeAccelerator::InitiateSurfaceSetChange,
-                weak_this_, decoder_->GetRequiredNumOfPictures(), new_size,
-                decoder_->GetNumReferenceFrames(), decoder_->GetVisibleRect()));
+                weak_this_, decoder_->GetRequiredNumOfPictures(),
+                decoder_->GetPicSize(), decoder_->GetNumReferenceFrames(),
+                decoder_->GetVisibleRect()));
         // We'll get rescheduled once ProvidePictureBuffers() finishes.
         return;
       }
@@ -566,11 +566,25 @@ void VaapiVideoDecodeAccelerator::TryFinishSurfaceSetChange() {
 
   // All surfaces released, destroy them and dismiss all PictureBuffers.
   awaiting_va_surfaces_recycle_ = false;
-  if (buffer_allocation_mode_ != BufferAllocationMode::kNone) {
-    vaapi_wrapper_->DestroyContextAndSurfaces(std::vector<VASurfaceID>(
-        available_va_surfaces_.begin(), available_va_surfaces_.end()));
+
+  VideoCodecProfile new_profile = decoder_->GetProfile();
+  if (profile_ != new_profile) {
+    DCHECK(decoder_delegate_);
+    profile_ = new_profile;
+    auto new_vaapi_wrapper = VaapiWrapper::CreateForVideoCodec(
+        VaapiWrapper::kDecode, profile_, base::Bind(&ReportToUMA, VAAPI_ERROR));
+    RETURN_AND_NOTIFY_ON_FAILURE(new_vaapi_wrapper.get(),
+                                 "Failed creating VaapiWrapper",
+                                 INVALID_ARGUMENT, );
+    decoder_delegate_->set_vaapi_wrapper(new_vaapi_wrapper.get());
+    vaapi_wrapper_ = std::move(new_vaapi_wrapper);
   } else {
-    vaapi_wrapper_->DestroyContext();
+    if (buffer_allocation_mode_ != BufferAllocationMode::kNone) {
+      vaapi_wrapper_->DestroyContextAndSurfaces(std::vector<VASurfaceID>(
+          available_va_surfaces_.begin(), available_va_surfaces_.end()));
+    } else {
+      vaapi_wrapper_->DestroyContext();
+    }
   }
   available_va_surfaces_.clear();
 

media/gpu/vaapi/vaapi_video_decode_accelerator.h

@@ -44,6 +44,7 @@ class GLImage;
 namespace media {
 
 class AcceleratedVideoDecoder;
+class VaapiVideoDecoderDelegate;
 class VaapiPicture;
 
 // Class to provide video decode acceleration for Intel systems with hardware
@@ -246,6 +247,9 @@ class MEDIA_GPU_EXPORT VaapiVideoDecodeAccelerator
   scoped_refptr<VaapiWrapper> vaapi_wrapper_;
   // Only used on |decoder_thread_task_runner_|.
   std::unique_ptr<AcceleratedVideoDecoder> decoder_;
+  // TODO(crbug.com/1022246): Instead of having the raw pointer here, getting
+  // the pointer from AcceleratedVideoDecoder.
+  VaapiVideoDecoderDelegate* decoder_delegate_ = nullptr;
 
   // Filled in during Initialize().
   BufferAllocationMode buffer_allocation_mode_;

media/gpu/vaapi/vaapi_video_decode_accelerator_unittest.cc

@@ -228,16 +228,16 @@ class VaapiVideoDecodeAcceleratorTest : public TestWithParam<TestParams>,
     EXPECT_CALL(*mock_decoder_, Decode())
         .WillOnce(Return(AcceleratedVideoDecoder::kConfigChange));
 
-    EXPECT_CALL(*mock_decoder_, GetPicSize()).WillOnce(Return(picture_size));
-    EXPECT_CALL(*mock_decoder_, GetProfile())
-        .WillOnce(Return(GetParam().video_codec));
     EXPECT_CALL(*mock_decoder_, GetRequiredNumOfPictures())
         .WillOnce(Return(num_pictures));
+    EXPECT_CALL(*mock_decoder_, GetPicSize()).WillOnce(Return(picture_size));
     const size_t kNumReferenceFrames = num_pictures / 2;
     EXPECT_CALL(*mock_decoder_, GetNumReferenceFrames())
         .WillOnce(Return(kNumReferenceFrames));
     EXPECT_CALL(*mock_decoder_, GetVisibleRect())
         .WillOnce(Return(gfx::Rect(picture_size)));
+    EXPECT_CALL(*mock_decoder_, GetProfile())
+        .WillOnce(Return(GetParam().video_codec));
     if (vda_.buffer_allocation_mode_ !=
         VaapiVideoDecodeAccelerator::BufferAllocationMode::kNone) {
       EXPECT_CALL(*mock_vaapi_wrapper_, DestroyContextAndSurfaces(_));

media/gpu/vaapi/vaapi_video_decoder.cc

@@ -124,6 +124,7 @@ void VaapiVideoDecoder::Initialize(const VideoDecoderConfig& config,
 
     decoder_ = nullptr;
     vaapi_wrapper_ = nullptr;
+    decoder_delegate_ = nullptr;
     SetState(State::kUninitialized);
   }
 
@@ -236,16 +237,6 @@ void VaapiVideoDecoder::HandleDecodeTask() {
       }
       break;
     case AcceleratedVideoDecoder::kConfigChange:
-      if (profile_ != decoder_->GetProfile()) {
-        DVLOGF(3) << "Profile is changed: " << profile_ << " -> "
-                  << decoder_->GetProfile();
-      }
-      if (pic_size_ == decoder_->GetPicSize()) {
-        // Profile only is changed.
-        // TODO(crbug.com/1022246): Handle profile change.
-        SetState(State::kError);
-        break;
-      }
       // A new set of output buffers is requested. We either didn't have any
       // output buffers yet or encountered a resolution change.
       // After the pipeline flushes all frames, OnPipelineFlushed() will be
@@ -401,7 +392,6 @@ void VaapiVideoDecoder::OnPipelineFlushed() {
   const gfx::Rect visible_rect = decoder_->GetVisibleRect();
   gfx::Size natural_size = GetNaturalSize(visible_rect, pixel_aspect_ratio_);
   pic_size_ = decoder_->GetPicSize();
-  profile_ = decoder_->GetProfile();
   const base::Optional<VideoPixelFormat> format =
       GfxBufferFormatToVideoPixelFormat(GetBufferFormat());
   CHECK(format);
@@ -413,7 +403,22 @@ void VaapiVideoDecoder::OnPipelineFlushed() {
 
   // All pending decode operations will be completed before triggering a
   // resolution change, so we can safely destroy the context here.
-  vaapi_wrapper_->DestroyContext();
+  if (profile_ != decoder_->GetProfile()) {
+    // When a profile is changed, we need to re-initialize VaapiWrapper.
+    profile_ = decoder_->GetProfile();
+    auto new_vaapi_wrapper = VaapiWrapper::CreateForVideoCodec(
+        VaapiWrapper::kDecode, profile_, base::DoNothing());
+    if (!new_vaapi_wrapper.get()) {
+      DLOG(WARNING) << "Failed creating VaapiWrapper";
+      SetState(State::kError);
+      return;
+    }
+    decoder_delegate_->set_vaapi_wrapper(new_vaapi_wrapper.get());
+    vaapi_wrapper_ = std::move(new_vaapi_wrapper);
+  } else {
+    vaapi_wrapper_->DestroyContext();
+  }
+
   vaapi_wrapper_->CreateContext(pic_size_);
 
   // If we reset during resolution change, then there is no decode tasks. In
@@ -526,16 +531,25 @@ bool VaapiVideoDecoder::CreateAcceleratedVideoDecoder() {
   pic_size_ = gfx::Size();
 
   if (profile_ >= H264PROFILE_MIN && profile_ <= H264PROFILE_MAX) {
-    decoder_.reset(new H264Decoder(
-        std::make_unique<H264VaapiVideoDecoderDelegate>(this, vaapi_wrapper_),
-        profile_, color_space_));
+    auto accelerator =
+        std::make_unique<H264VaapiVideoDecoderDelegate>(this, vaapi_wrapper_);
+    decoder_delegate_ = accelerator.get();
+
+    decoder_.reset(
+        new H264Decoder(std::move(accelerator), profile_, color_space_));
   } else if (profile_ >= VP8PROFILE_MIN && profile_ <= VP8PROFILE_MAX) {
-    decoder_.reset(new VP8Decoder(
-        std::make_unique<VP8VaapiVideoDecoderDelegate>(this, vaapi_wrapper_)));
+    auto accelerator =
+        std::make_unique<VP8VaapiVideoDecoderDelegate>(this, vaapi_wrapper_);
+    decoder_delegate_ = accelerator.get();
+
+    decoder_.reset(new VP8Decoder(std::move(accelerator)));
   } else if (profile_ >= VP9PROFILE_MIN && profile_ <= VP9PROFILE_MAX) {
-    decoder_.reset(new VP9Decoder(
-        std::make_unique<VP9VaapiVideoDecoderDelegate>(this, vaapi_wrapper_),
-        profile_, color_space_));
+    auto accelerator =
+        std::make_unique<VP9VaapiVideoDecoderDelegate>(this, vaapi_wrapper_);
+    decoder_delegate_ = accelerator.get();
+
+    decoder_.reset(
+        new VP9Decoder(std::move(accelerator), profile_, color_space_));
   } else {
     VLOGF(1) << "Unsupported profile " << GetProfileName(profile_);
     return false;

media/gpu/vaapi/vaapi_video_decoder.h

@@ -33,6 +33,7 @@
 namespace media {
 
 class AcceleratedVideoDecoder;
+class VaapiVideoDecoderDelegate;
 class DmabufVideoFramePool;
 class VaapiWrapper;
 class VideoFrame;
@@ -170,6 +171,9 @@ class VaapiVideoDecoder : public DecoderInterface,
   // Platform and codec specific video decoder.
   std::unique_ptr<AcceleratedVideoDecoder> decoder_;
   scoped_refptr<VaapiWrapper> vaapi_wrapper_;
+  // TODO(crbug.com/1022246): Instead of having the raw pointer here, getting
+  // the pointer from AcceleratedVideoDecoder.
+  VaapiVideoDecoderDelegate* decoder_delegate_ = nullptr;
 
   SEQUENCE_CHECKER(decoder_sequence_checker_);
 

media/gpu/vaapi/vaapi_video_decoder_delegate.cc

@@ -24,4 +24,10 @@ VaapiVideoDecoderDelegate::~VaapiVideoDecoderDelegate() {
   // DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 }
 
+void VaapiVideoDecoderDelegate::set_vaapi_wrapper(
+    scoped_refptr<VaapiWrapper> vaapi_wrapper) {
+  DETACH_FROM_SEQUENCE(sequence_checker_);
+  vaapi_wrapper_ = std::move(vaapi_wrapper);
+}
+
 }  // namespace media

media/gpu/vaapi/vaapi_video_decoder_delegate.h

@@ -16,12 +16,16 @@ class VaapiWrapper;
 class VASurface;
 
 // The common part of each AcceleratedVideoDecoder's Accelerator for VA-API.
+// This class allows clients to reset VaapiWrapper in case of a profile change.
+// DecodeSurfaceHandler must stay alive for the lifetime of this class.
 class VaapiVideoDecoderDelegate {
  public:
   VaapiVideoDecoderDelegate(DecodeSurfaceHandler<VASurface>* const vaapi_dec,
                             scoped_refptr<VaapiWrapper> vaapi_wrapper);
   virtual ~VaapiVideoDecoderDelegate();
 
+  void set_vaapi_wrapper(scoped_refptr<VaapiWrapper> vaapi_wrapper);
+
   VaapiVideoDecoderDelegate(const VaapiVideoDecoderDelegate&) = delete;
   VaapiVideoDecoderDelegate& operator=(const VaapiVideoDecoderDelegate&) =
       delete;
@@ -29,7 +33,7 @@ class VaapiVideoDecoderDelegate {
  protected:
   // Both owned by caller.
   DecodeSurfaceHandler<VASurface>* const vaapi_dec_;
-  const scoped_refptr<VaapiWrapper> vaapi_wrapper_;
+  scoped_refptr<VaapiWrapper> vaapi_wrapper_;
 
   SEQUENCE_CHECKER(sequence_checker_);
 };