Commit 7cb3f403 authored by Vi Nguyen's avatar Vi Nguyen Committed by Commit Bot

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/+/1974761Reviewed-by: default avatarFrank Liberato <liberato@chromium.org>
Reviewed-by: default avatarTed Meyer <tmathmeyer@chromium.org>
Commit-Queue: Vi Nguyen <ving@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#730012}
parent 8e1eba22
......@@ -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.
use_fp16_ = true;
want_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()) {
use_fp16_ = true;
want_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(!use_fp16_);
DCHECK(!want_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;
if (!checker.CheckOutputFormatSupport(DXGI_FORMAT_R16G16B16A16_FLOAT))
use_fp16_ = false;
support_fp16_ =
checker.CheckOutputFormatSupport(DXGI_FORMAT_R16G16B16A16_FLOAT);
// Enable multithreaded mode on the device. This ensures that accesses to
// context are synchronized across threads. We have multiple threads
......@@ -1388,18 +1388,19 @@ bool DXVAVideoDecodeAccelerator::InitDecoder(VideoCodecProfile profile) {
}
if (!gl::GLSurfaceEGL::IsPixelFormatFloatSupported())
use_fp16_ = false;
want_fp16_ = false;
EGLDisplay egl_display = gl::GLSurfaceEGL::GetHardwareDisplay();
while (true) {
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_) {
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()) {
config_attribs.push_back(EGL_COLOR_COMPONENT_TYPE_EXT);
config_attribs.push_back(EGL_COLOR_COMPONENT_TYPE_FLOAT_EXT);
}
......@@ -1419,7 +1420,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 == (use_fp16_ ? 16 : 8)) {
if (red_bits == (ShouldUseFp16() ? 16 : 8)) {
egl_config_ = configs[i];
break;
}
......@@ -1427,9 +1428,9 @@ bool DXVAVideoDecodeAccelerator::InitDecoder(VideoCodecProfile profile) {
}
if (!num_configs) {
if (use_fp16_) {
// Try again, but without use_fp16_
use_fp16_ = false;
if (ShouldUseFp16()) {
// Try again, but without want_fp16_
want_fp16_ = false;
continue;
}
return false;
......@@ -1439,7 +1440,7 @@ bool DXVAVideoDecodeAccelerator::InitDecoder(VideoCodecProfile profile) {
break;
}
if (use_fp16_) {
if (ShouldUseFp16()) {
// TODO(hubbe): Share/copy P010/P016 textures.
DisableSharedTextureSupport();
support_copy_nv12_textures_ = false;
......@@ -2760,7 +2761,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_ && !use_fp16_) ||
if ((!use_color_info_ && !ShouldUseFp16()) ||
GetPictureBufferMechanism() == PictureBufferMechanism::COPY_TO_NV12 ||
GetPictureBufferMechanism() ==
PictureBufferMechanism::DELAYED_COPY_TO_NV12) {
......@@ -2811,7 +2812,8 @@ bool DXVAVideoDecodeAccelerator::InitializeID3D11VideoProcessor(
return true;
}
if (use_fp16_ && config_.target_color_space.IsHDR() && color_space.IsHDR()) {
if (ShouldUseFp16() && 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();
......@@ -2984,7 +2986,7 @@ void DXVAVideoDecodeAccelerator::DisableSharedTextureSupport() {
DXVAVideoDecodeAccelerator::PictureBufferMechanism
DXVAVideoDecodeAccelerator::GetPictureBufferMechanism() const {
if (use_fp16_)
if (ShouldUseFp16())
return PictureBufferMechanism::COPY_TO_RGB;
if (support_share_nv12_textures_)
return PictureBufferMechanism::BIND;
......@@ -3011,4 +3013,8 @@ ID3D11Device* DXVAVideoDecodeAccelerator::D3D11Device() const {
return ShouldUseANGLEDevice() ? angle_device_.Get() : d3d11_device_.Get();
}
bool DXVAVideoDecodeAccelerator::ShouldUseFp16() const {
return want_fp16_ && support_fp16_;
}
} // namespace media
......@@ -397,6 +397,8 @@ class MEDIA_GPU_EXPORT DXVAVideoDecodeAccelerator
bool ShouldUseANGLEDevice() const;
ID3D11Device* D3D11Device() const;
bool ShouldUseFp16() const;
// To expose client callbacks from VideoDecodeAccelerator.
VideoDecodeAccelerator::Client* client_;
......@@ -556,8 +558,11 @@ class MEDIA_GPU_EXPORT DXVAVideoDecodeAccelerator
// Supports copying NV12 textures on the main thread to use in ANGLE.
bool support_delayed_copy_nv12_textures_;
// Copy video to FP16 scRGB textures.
bool use_fp16_ = false;
// Supports copying video to FP16 scRGB textures.
bool support_fp16_ = false;
// Copy video to FP16 scRGB textures if supported.
bool want_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