Skip to content

GitLab

T
tangled
Commit b81d4a51 authored by Corentin Wallez's avatar Corentin Wallez Committed by Commit Bot
Browse files
Options

Revert "Fix issue where DXGI_FORMAT_R16G16B16A16_FLOAT gets used even when unsupported by hardware" 

This reverts commit 7cb3f403.

Reason for revert: Make TabCaptureApiPixelTest.EndToEndThroughWebRTC fail consistently on Win10 FYI x64 Release (Intel HD 630)

Original change's description:
> Fix issue where DXGI_FORMAT_R16G16B16A16_FLOAT gets used even when unsupported by hardware
> 
> Issue backtrace:
> - video_device_->CreateVideoProcessorOutputView(...) in
>   DXVAVideoDecodeAccelerator::CopyTextureOnDecoderThread() fails because
>   DXGI_FORMAT_R16G16B16A16_FLOAT is not supported by client's hardware.
> - in DXVAVideoDecodeAccelerator::Initialize, use_fp16_ = true due to
>   "Issue 592074: Add support for different VP9 profiles".
> - DXGI_FORMAT_R16G16B16A16_FLOAT is in fact not supported, and
>   CheckOutputFormatSupport(DXGI_FORMAT_R16G16B16A16_FLOAT)
>   DXVAVideoDecodeAccelerator::CreateDX11DevManager() reflects that,
>   and use_fp16_ = false.
> 
> Proposed fix:
> - Make use_fp16_ an enum to reflect the fact that its non-binary states.
> - Because intent to use fp16 is expressed in DXVAVideoDecodeAccelerator::Initialize,
>   which can be called before or after support for fp16 is determined, let use_fp16_ be
>   set to kUsing for all states except kUnsupported. kUsing can be reversed later
>   when hardware support is queried.
> 
> Bug: 1032438
> Change-Id: I113f26b126fdc1e0131a2362cbf19629f268f3eb
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1974761
> Reviewed-by: Frank Liberato <liberato@chromium.org>
> Reviewed-by: Ted Meyer <tmathmeyer@chromium.org>
> Commit-Queue: Vi Nguyen <ving@microsoft.com>
> Cr-Commit-Position: refs/heads/master@{#730012}

TBR=sandersd@chromium.org,liberato@chromium.org,tmathmeyer@chromium.org,ving@microsoft.com

Change-Id: I5071f5b78391ba4479d04132402f728799821ffa
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1032438
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1995167Reviewed-by: default avatarCorentin Wallez <cwallez@chromium.org>
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#730140}
parent 26a0a748
Hide whitespace changes
Inline Side-by-side
Showing with 23 additions and 34 deletions
media/gpu/windows/dxva_video_decode_accelerator_win.cc
View file @ b81d4a51
......@@ -574,13 +574,13 @@ bool DXVAVideoDecodeAccelerator::Initialize(const Config& config,
config.profile == VP9PROFILE_PROFILE3 ||
config.profile == H264PROFILE_HIGH10PROFILE) {
// Input file has more than 8 bits per channel.
want_fp16_ = true;
use_fp16_ = true;
}
// Unfortunately, the profile is currently unreliable for
// VP9 (https://crbug.com/592074) so also try to use fp16 if HDR is on.
if (config.target_color_space.IsHDR()) {
want_fp16_ = true;
use_fp16_ = true;
}
// Not all versions of Windows 7 and later include Media Foundation DLLs.
......@@ -823,7 +823,7 @@ bool DXVAVideoDecodeAccelerator::CreateDX11DevManager() {
RETURN_ON_FAILURE(angle_device_.Get(), "Failed to get d3d11 device", false);
using_angle_device_ = true;
DCHECK(!want_fp16_);
DCHECK(!use_fp16_);
angle_device_->GetImmediateContext(&d3d11_device_context_);
hr = angle_device_.As(&video_device_);
......@@ -892,8 +892,8 @@ bool DXVAVideoDecodeAccelerator::CreateDX11DevManager() {
if (!checker.CheckOutputFormatSupport(DXGI_FORMAT_NV12))
support_copy_nv12_textures_ = false;
support_fp16_ =
checker.CheckOutputFormatSupport(DXGI_FORMAT_R16G16B16A16_FLOAT);
if (!checker.CheckOutputFormatSupport(DXGI_FORMAT_R16G16B16A16_FLOAT))
use_fp16_ = false;
// Enable multithreaded mode on the device. This ensures that accesses to
// context are synchronized across threads. We have multiple threads
......@@ -1388,19 +1388,18 @@ bool DXVAVideoDecodeAccelerator::InitDecoder(VideoCodecProfile profile) {
}
if (!gl::GLSurfaceEGL::IsPixelFormatFloatSupported())
want_fp16_ = false;
use_fp16_ = false;
EGLDisplay egl_display = gl::GLSurfaceEGL::GetHardwareDisplay();
while (true) {
std::vector<EGLint> config_attribs = {
EGL_BUFFER_SIZE, 32,
EGL_RED_SIZE, ShouldUseFp16() ? 16 : 8,
EGL_GREEN_SIZE, ShouldUseFp16() ? 16 : 8,
EGL_BLUE_SIZE, ShouldUseFp16() ? 16 : 8,
EGL_SURFACE_TYPE, EGL_PBUFFER_BIT,
EGL_ALPHA_SIZE, 0};
if (ShouldUseFp16()) {
std::vector<EGLint> config_attribs = {EGL_BUFFER_SIZE, 32,
EGL_RED_SIZE, use_fp16_ ? 16 : 8,
EGL_GREEN_SIZE, use_fp16_ ? 16 : 8,
EGL_BLUE_SIZE, use_fp16_ ? 16 : 8,
EGL_SURFACE_TYPE, EGL_PBUFFER_BIT,
EGL_ALPHA_SIZE, 0};
if (use_fp16_) {
config_attribs.push_back(EGL_COLOR_COMPONENT_TYPE_EXT);
config_attribs.push_back(EGL_COLOR_COMPONENT_TYPE_FLOAT_EXT);
}
......@@ -1420,7 +1419,7 @@ bool DXVAVideoDecodeAccelerator::InitDecoder(VideoCodecProfile profile) {
eglGetConfigAttrib(egl_display, configs[i], EGL_RED_SIZE, &red_bits);
// Try to pick a configuration with the right number of bits rather
// than one that just has enough bits.
if (red_bits == (ShouldUseFp16() ? 16 : 8)) {
if (red_bits == (use_fp16_ ? 16 : 8)) {
egl_config_ = configs[i];
break;
}
......@@ -1428,9 +1427,9 @@ bool DXVAVideoDecodeAccelerator::InitDecoder(VideoCodecProfile profile) {
}
if (!num_configs) {
if (ShouldUseFp16()) {
// Try again, but without want_fp16_
want_fp16_ = false;
if (use_fp16_) {
// Try again, but without use_fp16_
use_fp16_ = false;
continue;
}
return false;
......@@ -1440,7 +1439,7 @@ bool DXVAVideoDecodeAccelerator::InitDecoder(VideoCodecProfile profile) {
break;
}
if (ShouldUseFp16()) {
if (use_fp16_) {
// TODO(hubbe): Share/copy P010/P016 textures.
DisableSharedTextureSupport();
support_copy_nv12_textures_ = false;
......@@ -2761,7 +2760,7 @@ bool DXVAVideoDecodeAccelerator::InitializeID3D11VideoProcessor(
// If we're copying textures or just not using color space information, set
// the same color space on input and output.
if ((!use_color_info_ && !ShouldUseFp16()) ||
if ((!use_color_info_ && !use_fp16_) ||
GetPictureBufferMechanism() == PictureBufferMechanism::COPY_TO_NV12 ||
GetPictureBufferMechanism() ==
PictureBufferMechanism::DELAYED_COPY_TO_NV12) {
......@@ -2812,8 +2811,7 @@ bool DXVAVideoDecodeAccelerator::InitializeID3D11VideoProcessor(
return true;
}
if (ShouldUseFp16() && config_.target_color_space.IsHDR() &&
color_space.IsHDR()) {
if (use_fp16_ && config_.target_color_space.IsHDR() && color_space.IsHDR()) {
// Note, we only use the SCRGBLinear output color space when the input is
// PQ, because nvidia drivers will not convert G22 to G10 for some reason.
dx11_converter_output_color_space_ = gfx::ColorSpace::CreateSCRGBLinear();
......@@ -2986,7 +2984,7 @@ void DXVAVideoDecodeAccelerator::DisableSharedTextureSupport() {
DXVAVideoDecodeAccelerator::PictureBufferMechanism
DXVAVideoDecodeAccelerator::GetPictureBufferMechanism() const {
if (ShouldUseFp16())
if (use_fp16_)
return PictureBufferMechanism::COPY_TO_RGB;
if (support_share_nv12_textures_)
return PictureBufferMechanism::BIND;
......@@ -3013,8 +3011,4 @@ ID3D11Device* DXVAVideoDecodeAccelerator::D3D11Device() const {
return ShouldUseANGLEDevice() ? angle_device_.Get() : d3d11_device_.Get();
}
bool DXVAVideoDecodeAccelerator::ShouldUseFp16() const {
return want_fp16_ && support_fp16_;
}
} // namespace media
media/gpu/windows/dxva_video_decode_accelerator_win.h
View file @ b81d4a51
......@@ -397,8 +397,6 @@ class MEDIA_GPU_EXPORT DXVAVideoDecodeAccelerator
bool ShouldUseANGLEDevice() const;
ID3D11Device* D3D11Device() const;
bool ShouldUseFp16() const;
// To expose client callbacks from VideoDecodeAccelerator.
VideoDecodeAccelerator::Client* client_;
......@@ -558,11 +556,8 @@ class MEDIA_GPU_EXPORT DXVAVideoDecodeAccelerator
// Supports copying NV12 textures on the main thread to use in ANGLE.
bool support_delayed_copy_nv12_textures_;
// Supports copying video to FP16 scRGB textures.
bool support_fp16_ = false;
// Copy video to FP16 scRGB textures if supported.
bool want_fp16_ = false;
// Copy video to FP16 scRGB textures.
bool use_fp16_ = false;
// When converting YUV to RGB, make sure we tell the blitter about the input
// color space so that it can convert it correctly.
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment