Skip to content

Commit

Permalink
Revert "media: Switch to NV12_SINGLE_GMB on Windows"
Browse files Browse the repository at this point in the history 
This reverts commit e776d4c.

Reason for revert: Break video playback on Windows for some contents.

Bug: b/264689730, 1408794
Change-Id: I6d98bcbd4fb173b01ada5832a2deeff18b5439da
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4181770
Reviewed-by: Sunny Sachanandani <sunnyps@chromium.org>
Reviewed-by: Ilya Nikolaevskiy <ilnik@chromium.org>
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Reviewed-by: Matthew Denton <mpdenton@chromium.org>
Commit-Queue: Xiaohan Wang <xhwang@chromium.org>
Auto-Submit: Xiaohan Wang <xhwang@chromium.org>
Cr-Commit-Position: refs/branch-heads/5481@{#600}
Cr-Branched-From: 130f3e4-refs/heads/main@{#1084008}
  • Loading branch information
@xhwang-chromium
xhwang-chromium committed Jan 24, 2023
1 parent 765be8b commit c5e8e76
Show file tree
Hide file tree
Showing 7 changed files with 45 additions and 50 deletions.
9 changes: 1 addition & 8 deletions content/renderer/media/gpu/gpu_video_accelerator_factories_impl.cc
View file
Expand Up @@ -485,15 +485,8 @@ GpuVideoAcceleratorFactoriesImpl::VideoFrameOutputFormat(
!capabilities.image_ycbcr_420v_disabled_for_video_frames) {
return media::GpuVideoAcceleratorFactories::OutputFormat::NV12_SINGLE_GMB;
}
if (capabilities.texture_rg) {
#if BUILDFLAG(IS_WIN)
// Windows supports binding single shmem GMB as separate shared images. We
// prefer single GMB because it makes dcomp overlay code simpler.
return media::GpuVideoAcceleratorFactories::OutputFormat::NV12_SINGLE_GMB;
#else
if (capabilities.texture_rg)
return media::GpuVideoAcceleratorFactories::OutputFormat::NV12_DUAL_GMB;
#endif
}
return media::GpuVideoAcceleratorFactories::OutputFormat::UNDEFINED;
}

Expand Down
6 changes: 2 additions & 4 deletions gpu/ipc/common/gpu_memory_buffer_support.cc
View file
Expand Up @@ -77,9 +77,6 @@ bool GpuMemoryBufferSupport::IsNativeGpuMemoryBufferConfigurationSupported(
format == gfx::BufferFormat::BGRX_8888 ||
format == gfx::BufferFormat::RGBX_8888 ||
format == gfx::BufferFormat::R_8 ||
format == gfx::BufferFormat::RG_88 ||
format == gfx::BufferFormat::R_16 ||
format == gfx::BufferFormat::RG_1616 ||
format == gfx::BufferFormat::RGBA_F16 ||
format == gfx::BufferFormat::BGRA_1010102 ||
format == gfx::BufferFormat::YUV_420_BIPLANAR ||
Expand Down Expand Up @@ -125,7 +122,8 @@ bool GpuMemoryBufferSupport::IsNativeGpuMemoryBufferConfigurationSupported(
case gfx::BufferUsage::GPU_READ:
case gfx::BufferUsage::SCANOUT:
return format == gfx::BufferFormat::RGBA_8888 ||
format == gfx::BufferFormat::RGBX_8888;
format == gfx::BufferFormat::RGBX_8888 ||
format == gfx::BufferFormat::YUV_420_BIPLANAR;
case gfx::BufferUsage::SCANOUT_CPU_READ_WRITE:
case gfx::BufferUsage::GPU_READ_CPU_READ_WRITE:
case gfx::BufferUsage::SCANOUT_VDA_WRITE:
Expand Down
12 changes: 1 addition & 11 deletions media/base/media_switches.cc
View file
Expand Up @@ -458,21 +458,11 @@ BASE_FEATURE(kMemoryPressureBasedSourceBufferGC,
"MemoryPressureBasedSourceBufferGC",
base::FEATURE_DISABLED_BY_DEFAULT);

// Enables binding software video NV12/P010 GMBs as separate shared images.
BASE_FEATURE(kMultiPlaneSoftwareVideoSharedImages,
"MultiPlaneSoftwareVideoSharedImages",
#if BUILDFLAG(IS_MAC) || BUILDFLAG(IS_WIN)
base::FEATURE_ENABLED_BY_DEFAULT
#else
base::FEATURE_DISABLED_BY_DEFAULT
#endif
);

// Enable binding multiple shared images to a single GpuMemoryBuffer for video
// frames created by video capture.
BASE_FEATURE(kMultiPlaneVideoCaptureSharedImages,
"MultiPlaneVideoCaptureSharedImages",
#if BUILDFLAG(IS_MAC) || BUILDFLAG(IS_WIN)
#if BUILDFLAG(IS_MAC)
base::FEATURE_ENABLED_BY_DEFAULT
#else
base::FEATURE_DISABLED_BY_DEFAULT
Expand Down
1 change: 0 additions & 1 deletion media/base/media_switches.h
View file
Expand Up @@ -221,7 +221,6 @@ MEDIA_EXPORT BASE_DECLARE_FEATURE(kMediaLearningSmoothnessExperiment);
MEDIA_EXPORT BASE_DECLARE_FEATURE(kMediaOptimizer);
MEDIA_EXPORT BASE_DECLARE_FEATURE(kMediaPowerExperiment);
MEDIA_EXPORT BASE_DECLARE_FEATURE(kMemoryPressureBasedSourceBufferGC);
MEDIA_EXPORT BASE_DECLARE_FEATURE(kMultiPlaneSoftwareVideoSharedImages);
MEDIA_EXPORT BASE_DECLARE_FEATURE(kMultiPlaneVideoCaptureSharedImages);
MEDIA_EXPORT BASE_DECLARE_FEATURE(kOpenscreenCastStreamingSession);
MEDIA_EXPORT BASE_DECLARE_FEATURE(kOverlayFullscreenVideo);
Expand Down
18 changes: 12 additions & 6 deletions media/video/gpu_memory_buffer_video_frame_pool.cc
View file
Expand Up @@ -55,6 +55,15 @@

namespace media {

BASE_FEATURE(kMultiPlaneSoftwareVideoSharedImages,
"MultiPlaneSoftwareVideoSharedImages",
#if BUILDFLAG(IS_MAC)
base::FEATURE_ENABLED_BY_DEFAULT
#else
base::FEATURE_DISABLED_BY_DEFAULT
#endif
);

bool GpuMemoryBufferVideoFramePool::MultiPlaneVideoSharedImagesEnabled() {
return base::FeatureList::IsEnabled(kMultiPlaneSoftwareVideoSharedImages);
}
Expand Down Expand Up @@ -1228,12 +1237,9 @@ scoped_refptr<VideoFrame> GpuMemoryBufferVideoFramePool::PoolImpl::

bool allow_overlay = false;
#if BUILDFLAG(IS_WIN)
// Windows direct composition path only supports NV12 video overlays. We use
// separate shared images for the planes for both single and dual NV12 GMBs.
// Windows direct composition path only supports dual GMB NV12 video overlays.
allow_overlay = (output_format_ ==
GpuVideoAcceleratorFactories::OutputFormat::NV12_DUAL_GMB) ||
(output_format_ ==
GpuVideoAcceleratorFactories::OutputFormat::NV12_SINGLE_GMB);
GpuVideoAcceleratorFactories::OutputFormat::NV12_DUAL_GMB);
#else
switch (output_format_) {
case GpuVideoAcceleratorFactories::OutputFormat::I420:
Expand All @@ -1244,7 +1250,7 @@ scoped_refptr<VideoFrame> GpuMemoryBufferVideoFramePool::PoolImpl::
allow_overlay = true;
break;
case GpuVideoAcceleratorFactories::OutputFormat::NV12_DUAL_GMB:
// Only used on configurations where we can't support overlays.
// Only used on Windows where we can't use single NV12 textures.
break;
case GpuVideoAcceleratorFactories::OutputFormat::XR30:
case GpuVideoAcceleratorFactories::OutputFormat::XB30:
Expand Down
28 changes: 14 additions & 14 deletions media/video/gpu_memory_buffer_video_frame_pool_unittest.cc
View file
Expand Up @@ -774,13 +774,13 @@ TEST_F(GpuMemoryBufferVideoFramePoolTest, CreateOneHardwareP010Frame) {

EXPECT_NE(software_frame.get(), frame.get());
EXPECT_EQ(PIXEL_FORMAT_P016LE, frame->format());
if (GpuMemoryBufferVideoFramePool::MultiPlaneVideoSharedImagesEnabled()) {
EXPECT_EQ(2u, frame->NumTextures());
EXPECT_EQ(2u, sii_->shared_image_count());
} else {
EXPECT_EQ(1u, frame->NumTextures());
EXPECT_EQ(1u, sii_->shared_image_count());
}
#if BUILDFLAG(IS_MAC)
EXPECT_EQ(2u, frame->NumTextures());
EXPECT_EQ(2u, sii_->shared_image_count());
#else
EXPECT_EQ(1u, frame->NumTextures());
EXPECT_EQ(1u, sii_->shared_image_count());
#endif
EXPECT_TRUE(frame->metadata().read_lock_fences_enabled);

EXPECT_EQ(1u, mock_gpu_factories_->created_memory_buffers().size());
Expand Down Expand Up @@ -814,13 +814,13 @@ TEST_F(GpuMemoryBufferVideoFramePoolTest,
gfx::IsOddHeightMultiPlanarBuffersAllowed()) {
EXPECT_NE(software_frame.get(), frame.get());
EXPECT_EQ(PIXEL_FORMAT_P016LE, frame->format());
if (GpuMemoryBufferVideoFramePool::MultiPlaneVideoSharedImagesEnabled()) {
EXPECT_EQ(2u, frame->NumTextures());
EXPECT_EQ(2u, sii_->shared_image_count());
} else {
EXPECT_EQ(1u, frame->NumTextures());
EXPECT_EQ(1u, sii_->shared_image_count());
}
#if BUILDFLAG(IS_MAC)
EXPECT_EQ(2u, frame->NumTextures());
EXPECT_EQ(2u, sii_->shared_image_count());
#else
EXPECT_EQ(1u, frame->NumTextures());
EXPECT_EQ(1u, sii_->shared_image_count());
#endif
EXPECT_TRUE(frame->metadata().read_lock_fences_enabled);

EXPECT_EQ(1u, mock_gpu_factories_->created_memory_buffers().size());
Expand Down
21 changes: 15 additions & 6 deletions third_party/blink/renderer/platform/video_capture/video_capture_impl.cc
View file
Expand Up @@ -527,8 +527,21 @@ bool VideoCaptureImpl::VideoFrameBufferPreparer::BindVideoFrameOnMediaThread(
usage |= gpu::SHARED_IMAGE_USAGE_MACOS_VIDEO_TOOLBOX;
#endif

if (base::FeatureList::IsEnabled(
media::kMultiPlaneVideoCaptureSharedImages)) {
unsigned texture_target =
buffer_context_->gpu_factories()->ImageTextureTarget(
gpu_memory_buffer_->GetFormat());

// TODO(sunnyps): Get rid of NV12_DUAL_GMB format and instead rely on enabled
// by default multi plane shared images on Windows.

const bool use_multiplane =
#if BUILDFLAG(IS_WIN)
output_format ==
media::GpuVideoAcceleratorFactories::OutputFormat::NV12_DUAL_GMB ||
#endif
base::FeatureList::IsEnabled(media::kMultiPlaneVideoCaptureSharedImages);

if (use_multiplane) {
planes.push_back(gfx::BufferPlane::Y);
planes.push_back(gfx::BufferPlane::UV);
} else {
Expand All @@ -550,10 +563,6 @@ bool VideoCaptureImpl::VideoFrameBufferPreparer::BindVideoFrameOnMediaThread(
}
}

const unsigned texture_target =
buffer_context_->gpu_factories()->ImageTextureTarget(
gpu_memory_buffer_->GetFormat());

const gpu::SyncToken sync_token = sii->GenVerifiedSyncToken();

gpu::MailboxHolder mailbox_holder_array[media::VideoFrame::kMaxPlanes];
Expand Down

0 comments on commit c5e8e76

Please sign in to comment.