Skip to content

Commit

Permalink
[m111] media: Destroy D3D11 resources before stub destruction
Browse files Browse the repository at this point in the history
If the CommandBufferStub gets destroyed before the D3D11VideoDecoder,
DefaultTexture2DWrapper::GpuResources destructor will fail to make the
context current and set the context lost flag on its shared images which
prevents the destruction of the associated GL textures.

This CL fixes that by hooking up the WillDestroyStub callback so that
GpuResources can destroy its shared images before the stub is destroyed.
This is intended as a speculative fix for crbug.com/1410436.

(cherry picked from commit f80d846)

Bug: 1410436
Change-Id: Ic369f67fb87d466ec634cf9c92d31fef6d508bca
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4201186
Auto-Submit: Sunny Sachanandani <sunnyps@chromium.org>
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Commit-Queue: Sunny Sachanandani <sunnyps@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1099034}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4217996
Commit-Queue: Dale Curtis <dalecurtis@chromium.org>
Cr-Commit-Position: refs/branch-heads/5563@{#119}
Cr-Branched-From: 3ac59a6-refs/heads/main@{#1097615}
  • Loading branch information
sunnyps authored and Chromium LUCI CQ committed Feb 2, 2023
1 parent 86201b5 commit f3b2e13
Show file tree
Hide file tree
Showing 6 changed files with 29 additions and 17 deletions.
13 changes: 7 additions & 6 deletions media/gpu/command_buffer_helper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -218,9 +218,9 @@ class CommandBufferHelperImpl
}

public:
void SetWillDestroyStubCB(WillDestroyStubCB will_destroy_stub_cb) override {
DCHECK(!will_destroy_stub_cb_);
will_destroy_stub_cb_ = std::move(will_destroy_stub_cb);
void AddWillDestroyStubCB(WillDestroyStubCB callback) override {
DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
will_destroy_stub_callbacks_.push_back(std::move(callback));
}

bool IsPassthrough() const override {
Expand Down Expand Up @@ -303,8 +303,9 @@ class CommandBufferHelperImpl
// sure that we're around a bit longer.
scoped_refptr<CommandBufferHelper> thiz(this);

if (will_destroy_stub_cb_)
std::move(will_destroy_stub_cb_).Run(have_context);
for (auto& callback : will_destroy_stub_callbacks_) {
std::move(callback).Run(have_context);
}

DestroyStub();
}
Expand Down Expand Up @@ -337,7 +338,7 @@ class CommandBufferHelperImpl
#endif
std::map<GLuint, std::unique_ptr<gpu::gles2::AbstractTexture>> textures_;

WillDestroyStubCB will_destroy_stub_cb_;
std::vector<WillDestroyStubCB> will_destroy_stub_callbacks_;

MemoryTrackerImpl memory_tracker_;
gpu::MemoryTypeTracker memory_type_tracker_;
Expand Down
4 changes: 2 additions & 2 deletions media/gpu/command_buffer_helper.h
Original file line number Diff line number Diff line change
Expand Up @@ -137,9 +137,9 @@ class MEDIA_GPU_EXPORT CommandBufferHelper
gl::GLImage* image) = 0;
#endif

// Set the callback to be called when our stub is destroyed. This callback
// Add a callback to be called when our stub is destroyed. This callback
// may not change the current context.
virtual void SetWillDestroyStubCB(WillDestroyStubCB will_destroy_stub_cb) = 0;
virtual void AddWillDestroyStubCB(WillDestroyStubCB callback) = 0;

// Is the backing command buffer passthrough (versus validating).
virtual bool IsPassthrough() const = 0;
Expand Down
11 changes: 5 additions & 6 deletions media/gpu/test/fake_command_buffer_helper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,9 @@ void FakeCommandBufferHelper::StubLost() {
DCHECK(task_runner_->BelongsToCurrentThread());
// Keep a reference to |this| in case the destruction cb drops the last one.
scoped_refptr<CommandBufferHelper> thiz(this);
if (will_destroy_stub_cb_)
std::move(will_destroy_stub_cb_).Run(!is_context_lost_);
for (auto& callback : will_destroy_stub_callbacks_) {
std::move(callback).Run(!is_context_lost_);
}
has_stub_ = false;
is_context_lost_ = true;
is_context_current_ = false;
Expand Down Expand Up @@ -173,10 +174,8 @@ gpu::Mailbox FakeCommandBufferHelper::CreateLegacyMailbox(GLuint service_id) {
return gpu::Mailbox::GenerateLegacyMailboxForTesting();
}

void FakeCommandBufferHelper::SetWillDestroyStubCB(
WillDestroyStubCB will_destroy_stub_cb) {
DCHECK(!will_destroy_stub_cb_);
will_destroy_stub_cb_ = std::move(will_destroy_stub_cb);
void FakeCommandBufferHelper::AddWillDestroyStubCB(WillDestroyStubCB callback) {
will_destroy_stub_callbacks_.push_back(std::move(callback));
}

bool FakeCommandBufferHelper::IsPassthrough() const {
Expand Down
4 changes: 2 additions & 2 deletions media/gpu/test/fake_command_buffer_helper.h
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ class FakeCommandBufferHelper : public CommandBufferHelper {
bool BindClientManagedImage(GLuint service_id, gl::GLImage* image) override;
#endif
gpu::Mailbox CreateLegacyMailbox(GLuint service_id) override;
void SetWillDestroyStubCB(WillDestroyStubCB will_destroy_stub_cb) override;
void AddWillDestroyStubCB(WillDestroyStubCB callback) override;
bool IsPassthrough() const override;
bool SupportsTextureRectangle() const override;
#endif
Expand All @@ -88,7 +88,7 @@ class FakeCommandBufferHelper : public CommandBufferHelper {
std::set<GLuint> service_ids_;
std::map<gpu::SyncToken, base::OnceClosure> waits_;

WillDestroyStubCB will_destroy_stub_cb_;
std::vector<WillDestroyStubCB> will_destroy_stub_callbacks_;
};

} // namespace media
Expand Down
10 changes: 9 additions & 1 deletion media/gpu/windows/d3d11_texture_wrapper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,9 @@ DefaultTexture2DWrapper::GpuResources::GpuResources(
return;
}

helper_->AddWillDestroyStubCB(
base::BindOnce(&GpuResources::Destroy, weak_factory_.GetWeakPtr()));

// Usage flags to allow the display compositor to draw from it, video to
// decode, and allow webgl/canvas access.
constexpr uint32_t usage =
Expand Down Expand Up @@ -270,7 +273,12 @@ DefaultTexture2DWrapper::GpuResources::GpuResources(

DefaultTexture2DWrapper::GpuResources::~GpuResources() {
// Destroy shared images with a current context, otherwise mark context lost.
if (!helper_ || !helper_->MakeContextCurrent()) {
const bool have_context = helper_ && helper_->MakeContextCurrent();
Destroy(have_context);
}

void DefaultTexture2DWrapper::GpuResources::Destroy(bool have_context) {
if (!have_context) {
for (auto& shared_image_rep : shared_images_) {
shared_image_rep->OnContextLost();
}
Expand Down
4 changes: 4 additions & 0 deletions media/gpu/windows/d3d11_texture_wrapper.h
Original file line number Diff line number Diff line change
Expand Up @@ -121,10 +121,14 @@ class MEDIA_GPU_EXPORT DefaultTexture2DWrapper : public Texture2DWrapper {
~GpuResources();

private:
void Destroy(bool have_context);

scoped_refptr<CommandBufferHelper> helper_;

std::vector<std::unique_ptr<gpu::SharedImageRepresentationFactoryRef>>
shared_images_;

base::WeakPtrFactory<GpuResources> weak_factory_{this};
};

// Receive an error from |gpu_resources_| and store it in |received_error_|.
Expand Down

0 comments on commit f3b2e13

Please sign in to comment.