From 28ec09a49dbebbda4db5c5c582c0eca4d6af3139 Mon Sep 17 00:00:00 2001
From: Eugene Zemtsov <eugene@chromium.org>
Date: Fri, 24 Apr 2026 18:26:26 -0700
Subject: [PATCH] media: Fix ScopedReadAccess lifetime in D3D12 and MF VEAs

The Windows hardware video encoders (D3D12 and Media Foundation)
prematurely destroyed gpu::VideoImageRepresentation::ScopedReadAccess
on the GPU main thread before the hardware encoder finished reading
the texture, allowing concurrent write access.

This change fixes the race condition by keeping the read access alive
for the duration of the encode. The VideoImageRepresentation
and ScopedReadAccess are encapsulated within a SharedImageReadAccess
struct. This struct internally utilizes a unique_ptr with a
base::OnTaskRunnerDeleter, guaranteeing that EndReadAccess() and the
representation are safely destroyed on the GPU main thread when the
background encoding thread finishes.

Bug: 503879873
Change-Id: Iff8412e0648b1c01105f6ee9cc39a1e9c7ce9996
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7787507
Reviewed-by: Qiu, Jianlin <jianlin.qiu@intel.com>
Commit-Queue: Eugene Zemtsov <eugene@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1620599}
---

diff --git a/media/base/win/mf_helpers.cc b/media/base/win/mf_helpers.cc
index 546874e..fe13e001 100644
--- a/media/base/win/mf_helpers.cc
+++ b/media/base/win/mf_helpers.cc
@@ -45,6 +45,20 @@
 
 namespace media {
 
+SharedImageReadLock::SharedImageReadLock(
+    std::unique_ptr<gpu::VideoImageRepresentation> representation,
+    std::unique_ptr<gpu::VideoImageRepresentation::ScopedReadAccess>
+        scoped_read_access)
+    : representation_(representation.release(),
+                      base::OnTaskRunnerDeleter(
+                          base::SequencedTaskRunner::GetCurrentDefault())),
+      scoped_read_access_(scoped_read_access.release(),
+                          base::OnTaskRunnerDeleter(
+                              base::SequencedTaskRunner::GetCurrentDefault())) {
+}
+
+SharedImageReadLock::~SharedImageReadLock() = default;
+
 using Microsoft::WRL::ComPtr;
 using Microsoft::WRL::MakeAndInitialize;
 
@@ -1047,12 +1061,13 @@
     ResourceAvailableCB sample_available_cb) {
   TRACE_EVENT0("media", "GenerateResourceOnSyncTokenReleased");
 
-#define RETURN_ON_FAILURE_WITH_CALLBACK(hr, message)                       \
-  if (FAILED(hr)) {                                                        \
-    LOG(ERROR) << message << ": " << logging::SystemErrorCodeToString(hr); \
-    std::move(sample_available_cb)                                         \
-        .Run(std::move(frame), nullptr, std::nullopt, std::nullopt, hr);   \
-    return;                                                                \
+#define RETURN_ON_FAILURE_WITH_CALLBACK(hr, message)                         \
+  if (FAILED(hr)) {                                                          \
+    LOG(ERROR) << message << ": " << logging::SystemErrorCodeToString(hr);   \
+    std::move(sample_available_cb)                                           \
+        .Run(std::move(frame), nullptr, std::nullopt, nullptr, std::nullopt, \
+             hr);                                                            \
+    return;                                                                  \
   }
 
   auto* shared_image_stub = command_buffer_helper->GetSharedImageStub();
@@ -1083,8 +1098,19 @@
   }
 
   auto scoped_read_access = image_representation->BeginScopedReadAccess();
+  if (!scoped_read_access) {
+    RETURN_ON_FAILURE_WITH_CALLBACK(E_FAIL, "Failed to begin read access");
+  }
+  ComPtr<SharedImageReadLock> si_lock =
+      Microsoft::WRL::Make<SharedImageReadLock>(std::move(image_representation),
+                                                std::move(scoped_read_access));
+  if (!si_lock) {
+    RETURN_ON_FAILURE_WITH_CALLBACK(E_OUTOFMEMORY,
+                                    "Failed to create SharedImageReadLock");
+  }
+
   gpu::D3D11TextureAndArrayIndex input_texture =
-      scoped_read_access->GetD3D11Texture();
+      si_lock->access()->GetD3D11Texture();
 
   D3D11_TEXTURE2D_DESC texture_desc;
   input_texture.texture->GetDesc(&texture_desc);
@@ -1104,8 +1130,8 @@
     RETURN_ON_FAILURE_WITH_CALLBACK(sample != nullptr ? S_OK : E_FAIL,
                                     "Failed to create MF sample");
     std::move(sample_available_cb)
-        .Run(std::move(frame), std::move(sample), std::nullopt, std::nullopt,
-             S_OK);
+        .Run(std::move(frame), std::move(sample), std::nullopt,
+             std::move(si_lock), std::nullopt, S_OK);
     return;
   }
 
@@ -1185,7 +1211,7 @@
 
   std::move(sample_available_cb)
       .Run(std::move(frame), nullptr, std::move(scoped_shared_handle),
-           input_texture_has_been_copied, S_OK);
+           std::move(si_lock), input_texture_has_been_copied, S_OK);
 #undef RETURN_ON_FAILURE_WITH_CALLBACK
 }
 
diff --git a/media/base/win/mf_helpers.h b/media/base/win/mf_helpers.h
index 2dbfac5..ea0e28e 100644
--- a/media/base/win/mf_helpers.h
+++ b/media/base/win/mf_helpers.h
@@ -9,10 +9,13 @@
 #include <mfidl.h>
 #include <stdint.h>
 #include <wrl/client.h>
+#include <wrl/implements.h>
 
 #include "base/functional/callback.h"
 #include "base/logging.h"
+#include "base/task/sequenced_task_runner.h"
 #include "base/time/time.h"
+#include "gpu/command_buffer/service/shared_image/shared_image_representation.h"
 #include "media/base/audio_decoder_config.h"
 #include "media/base/channel_layout.h"
 #include "media/base/decoder_buffer.h"
@@ -228,6 +231,46 @@
 
 class CommandBufferHelper;
 
+// Manages the lifetime of a VideoImageRepresentation and its ScopedReadAccess.
+// Since these objects are not thread-safe and must be destroyed on the GPU
+// main thread, this COM class encapsulates them and uses
+// base::OnTaskRunnerDeleter to ensure they are safely destroyed on the
+// original task runner, even if the SharedImageReadLock is passed to and
+// destroyed by a background OS thread (e.g., inside an IMFSample).
+// It also guarantees destruction order (ScopedReadAccess before
+// VideoImageRepresentation).
+class MEDIA_EXPORT SharedImageReadLock
+    : public Microsoft::WRL::RuntimeClass<
+          Microsoft::WRL::RuntimeClassFlags<Microsoft::WRL::ClassicCom>,
+          IUnknown> {
+ public:
+  // Custom GUID used to attach the SharedImageReadLock to an IMFSample.
+  // This is a randomly generated GUID and has no special meaning.
+  static constexpr GUID kSampleExtensionGUID = {
+      0xb30e9cc8,
+      0xd05a,
+      0x482a,
+      {0xba, 0x28, 0x0e, 0x21, 0xc0, 0xe3, 0x95, 0xa9}};
+
+  SharedImageReadLock(
+      std::unique_ptr<gpu::VideoImageRepresentation> representation,
+      std::unique_ptr<gpu::VideoImageRepresentation::ScopedReadAccess>
+          scoped_read_access);
+
+  gpu::VideoImageRepresentation::ScopedReadAccess* access() const {
+    return scoped_read_access_.get();
+  }
+
+ private:
+  ~SharedImageReadLock() override;
+
+  std::unique_ptr<gpu::VideoImageRepresentation, base::OnTaskRunnerDeleter>
+      representation_;
+  std::unique_ptr<gpu::VideoImageRepresentation::ScopedReadAccess,
+                  base::OnTaskRunnerDeleter>
+      scoped_read_access_;
+};
+
 // Parameters:
 //   frame: The original video frame.
 //   sample: The generated IMFSample, or nullptr if a shared handle is needed.
@@ -245,6 +288,7 @@
     scoped_refptr<VideoFrame> frame,
     Microsoft::WRL::ComPtr<IMFSample> sample,
     std::optional<base::win::ScopedHandle> texture_handle,
+    Microsoft::WRL::ComPtr<SharedImageReadLock> scoped_read_access,
     std::optional<bool> texture_has_been_copied,
     HRESULT hr)>
     ResourceAvailableCB;
diff --git a/media/gpu/windows/d3d12_video_encode_accelerator.cc b/media/gpu/windows/d3d12_video_encode_accelerator.cc
index c629c2a..ddba3ae 100644
--- a/media/gpu/windows/d3d12_video_encode_accelerator.cc
+++ b/media/gpu/windows/d3d12_video_encode_accelerator.cc
@@ -85,7 +85,8 @@
   if (FAILED(hr)) {                                                        \
     LOG(ERROR) << message << ": " << logging::SystemErrorCodeToString(hr); \
     std::move(frame_available_cb)                                          \
-        .Run(std::move(frame), base::win::ScopedHandle(), 0, hr);          \
+        .Run(std::move(frame), base::win::ScopedHandle(),                  \
+             Microsoft::WRL::ComPtr<SharedImageReadLock>(), 0, hr);        \
     return;                                                                \
   }
 
@@ -135,11 +136,14 @@