Skip to content

Commit

Permalink
media: add flag to disable WebGPU SharedImage
Browse files Browse the repository at this point in the history
Flex supports devices with legacy AMD GPUs that do not support the
ANDROID_native_fence_sync EGL extension, required for synchronizing
WebGPU on OzoneImageBacking with other write streams. This change
prevents MailboxVideoFrameConverter from committing to webgpu zero
copy support, which signals to OzoneImageBackingFactory that WebGPU
will not be used on the SharedImage. As a result, WebGPU will not work
on the device. However, it was not working anyways and since this is a
legacy device, that support will probably not be added. I do not
outright disable WebGPU thru gpu/config/webgpu_blocklist.h as
there isn't any code in Ozone|MailboxVideoFrameConverter which
checks if WebGPU is enabled, so this wouldn't do anything to fix
the graphics regression. More Flex devices will be added to this
flag once testing is done.

BUG=b:293613437
TEST= twofold
1. locally downloaded h264 plays with hardware acceleration by vaapi

2.gn gen out/Default --args='target_os="chromeos" use_goma=true use_vaapi=true'
autoninja -C out/Default media_unittests
out/Default/media_unittests --gtest_filter=Mailbox*

Change-Id: I70bc71eee31660425db62ce84c432ee4688e158b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4997682
Reviewed-by: Joe Mason <joenotcharles@google.com>
Reviewed-by: Vasiliy Telezhnikov <vasilyt@chromium.org>
Reviewed-by: Andres Calderon Jaramillo <andrescj@chromium.org>
Auto-Submit: Andre Braga <andrebraga@google.com>
Commit-Queue: Andre Braga <andrebraga@google.com>
Cr-Commit-Position: refs/heads/main@{#1224549}
  • Loading branch information
andre-braga authored and Chromium LUCI CQ committed Nov 14, 2023
1 parent b70fc50 commit 61afc85
Show file tree
Hide file tree
Showing 10 changed files with 56 additions and 1 deletion.
1 change: 1 addition & 0 deletions gpu/command_buffer/common/shared_image_capabilities.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ struct GPU_EXPORT SharedImageCapabilities {
bool supports_luminance_shared_images = false;
bool supports_r16_shared_images = false;
bool disable_r8_shared_images = false;
bool disable_webgpu_shared_images = false;

bool shared_image_d3d = false;
bool shared_image_swap_chain = false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -877,6 +877,8 @@ gpu::SharedImageCapabilities SharedImageFactory::MakeCapabilities() {
is_angle_metal || is_skia_graphite;
shared_image_caps.disable_r8_shared_images =
workarounds_.r8_egl_images_broken;
shared_image_caps.disable_webgpu_shared_images =
workarounds_.disable_webgpu_shared_images;

#if BUILDFLAG(IS_WIN)
shared_image_caps.shared_image_d3d =
Expand Down
12 changes: 12 additions & 0 deletions gpu/config/gpu_driver_bug_list.json
Original file line number Diff line number Diff line change
Expand Up @@ -3828,6 +3828,18 @@
"disable_accelerated_vp8_encode",
"disable_accelerated_h264_encode"
]
},
{
"id": 423,
"description": "Legacy AMD GPUs can't synchronize multiple write streams on SharedImages [b/293613437]",
"os": {
"type": "chromeos"
},
"vendor_id": "0x1002",
"device_id": ["0x9802"],
"features": [
"disable_webgpu_shared_images"
]
}
]
}
1 change: 1 addition & 0 deletions gpu/config/gpu_workaround_list.txt
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ disable_vp_auto_hdr
disable_vp_scaling
disable_vp_super_resolution
disable_webgl_rgb_multisampling_usage
disable_webgpu_shared_images
disallow_large_instanced_draw
dont_delete_source_texture_for_egl_image
dont_initialize_uninitialized_locals
Expand Down
1 change: 1 addition & 0 deletions gpu/ipc/common/shared_image_capabilities.mojom
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ struct SharedImageCapabilities {
bool supports_luminance_shared_images;
bool supports_r16_shared_images;
bool disable_r8_shared_images;
bool disable_webgpu_shared_images;

bool shared_image_d3d;
bool shared_image_swap_chain;
Expand Down
1 change: 1 addition & 0 deletions gpu/ipc/common/shared_image_capabilities_mojom_traits.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ bool StructTraits<gpu::mojom::SharedImageCapabilitiesDataView,
data.supports_luminance_shared_images();
out->supports_r16_shared_images = data.supports_r16_shared_images();
out->disable_r8_shared_images = data.disable_r8_shared_images();
out->disable_webgpu_shared_images = data.disable_webgpu_shared_images();

out->shared_image_d3d = data.shared_image_d3d();
out->shared_image_swap_chain = data.shared_image_swap_chain();
Expand Down
5 changes: 5 additions & 0 deletions gpu/ipc/common/shared_image_capabilities_mojom_traits.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,11 @@ struct GPU_EXPORT StructTraits<gpu::mojom::SharedImageCapabilitiesDataView,
return input.disable_r8_shared_images;
}

static bool disable_webgpu_shared_images(
const gpu::SharedImageCapabilities& input) {
return input.disable_webgpu_shared_images;
}

static bool shared_image_d3d(const gpu::SharedImageCapabilities& input) {
return input.shared_image_d3d;
}
Expand Down
26 changes: 25 additions & 1 deletion media/gpu/chromeos/mailbox_video_frame_converter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,14 @@
#include "components/viz/common/resources/shared_image_format.h"
#include "gpu/command_buffer/common/shared_image_usage.h"
#include "gpu/command_buffer/service/scheduler.h"
#include "gpu/command_buffer/service/shared_image/shared_image_factory.h"
#include "gpu/ipc/service/gpu_channel.h"
#include "media/base/format_utils.h"
#include "media/base/media_switches.h"
#include "media/base/video_util.h"
#include "media/gpu/chromeos/platform_video_frame_utils.h"
#include "media/gpu/macros.h"
#include "third_party/abseil-cpp/absl/types/optional.h"
#include "ui/gfx/gpu_memory_buffer.h"
#include "ui/gl/gl_bindings.h"

Expand Down Expand Up @@ -97,6 +99,19 @@ class GpuDelegateImpl : public MailboxVideoFrameConverter::GpuDelegate {
return !!gpu_channel_;
}

absl::optional<gpu::SharedImageCapabilities> GetCapabilities() override {
DCHECK(gpu_task_runner_->BelongsToCurrentThread());
if (!gpu_channel_) {
return absl::nullopt;
}

gpu::SharedImageStub* shared_image_stub = gpu_channel_->shared_image_stub();
DCHECK(shared_image_stub);
gpu::SharedImageFactory* factory = shared_image_stub->factory();
CHECK(factory);
return factory->MakeCapabilities();
}

gpu::SharedImageStub::SharedImageDestructionCallback CreateSharedImage(
const gpu::Mailbox& mailbox,
gfx::GpuMemoryBufferHandle handle,
Expand Down Expand Up @@ -540,11 +555,20 @@ bool MailboxVideoFrameConverter::GenerateSharedImageOnGPUThread(
const gfx::Size shared_image_size =
GetRectSizeFromOrigin(destination_visible_rect);

const absl::optional<gpu::SharedImageCapabilities> shared_image_caps =
gpu_delegate_->GetCapabilities();

if (!shared_image_caps.has_value()) {
OnError(FROM_HERE, "Can't get the SharedImageCapabilities");
return false;
}

// The allocated SharedImages should be usable for the (Display) compositor
// and, potentially, for overlays (Scanout).
uint32_t shared_image_usage =
gpu::SHARED_IMAGE_USAGE_DISPLAY_READ | gpu::SHARED_IMAGE_USAGE_SCANOUT;
if (video_frame->metadata().is_webgpu_compatible) {
if (video_frame->metadata().is_webgpu_compatible &&
!shared_image_caps->disable_webgpu_shared_images) {
shared_image_usage |= gpu::SHARED_IMAGE_USAGE_WEBGPU;
}

Expand Down
3 changes: 3 additions & 0 deletions media/gpu/chromeos/mailbox_video_frame_converter.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,12 @@
#include "base/memory/scoped_refptr.h"
#include "base/memory/weak_ptr.h"
#include "gpu/command_buffer/common/mailbox.h"
#include "gpu/command_buffer/common/shared_image_capabilities.h"
#include "gpu/ipc/common/surface_handle.h"
#include "gpu/ipc/service/shared_image_stub.h"
#include "media/base/video_frame.h"
#include "media/gpu/media_gpu_export.h"
#include "third_party/abseil-cpp/absl/types/optional.h"
#include "third_party/skia/include/core/SkAlphaType.h"
#include "third_party/skia/include/gpu/GrTypes.h"
#include "ui/gfx/buffer_types.h"
Expand Down Expand Up @@ -54,6 +56,7 @@ class MEDIA_GPU_EXPORT MailboxVideoFrameConverter {
virtual ~GpuDelegate() = default;

virtual bool Initialize() = 0;
virtual absl::optional<gpu::SharedImageCapabilities> GetCapabilities() = 0;
virtual gpu::SharedImageStub::SharedImageDestructionCallback
CreateSharedImage(const gpu::Mailbox& mailbox,
gfx::GpuMemoryBufferHandle handle,
Expand Down
5 changes: 5 additions & 0 deletions media/gpu/chromeos/mailbox_video_frame_converter_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,14 @@
#include "base/task/thread_pool.h"
#include "base/test/mock_callback.h"
#include "base/test/task_environment.h"
#include "gpu/command_buffer/common/shared_image_capabilities.h"
#include "gpu/command_buffer/common/sync_token.h"
#include "media/base/media_switches.h"
#include "media/base/simple_sync_token_client.h"
#include "media/video/fake_gpu_memory_buffer.h"
#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "third_party/abseil-cpp/absl/types/optional.h"
#include "ui/gfx/gpu_fence_handle.h"
#include "ui/gfx/gpu_memory_buffer.h"

Expand All @@ -38,6 +40,7 @@ namespace {
class MockGpuDelegate : public MailboxVideoFrameConverter::GpuDelegate {
public:
MOCK_METHOD0(Initialize, bool());
MOCK_METHOD0(GetCapabilities, absl::optional<gpu::SharedImageCapabilities>());
MOCK_METHOD9(CreateSharedImage,
gpu::SharedImageStub::SharedImageDestructionCallback(
const gpu::Mailbox& mailbox,
Expand Down Expand Up @@ -248,6 +251,8 @@ TEST_F(MailboxVideoFrameConverterWithUnwrappedFramesTest,
.WillOnce(SaveArg<0>(&converted_frames[i]));
}

EXPECT_CALL(*mock_gpu_delegate_, GetCapabilities())
.WillRepeatedly(Return(gpu::SharedImageCapabilities()));
// Note: after this, the MailboxVideoFrameConverter should have full
// ownership of the *|gmb_frame|.
converter_->ConvertFrame(std::move(gmb_frame));
Expand Down

0 comments on commit 61afc85

Please sign in to comment.