Commit 72c52cf1 authored by Hirokazu Honda's avatar Hirokazu Honda Committed by Chromium LUCI CQ

media/gpu/vaapiWrapper: Enforce supported resolutions in CreateContext()

VaapiWrapper reports the supported min and max resolutions by
VaapiWrapper::GetSupportedDec/EncodeProfiles(). They might be
different from the min and max resolutions supported by a
hardware and driver for some reason. For instance, we set
the minimum encoder resolution to 321x241 and the webrtc encoder
enforces the resolution per kWebRtcUseMinMaxVEADimensions
(e.g. crbug.com/1008491).
Clients of VaapiWrapper were expected to respect the supported
resolutions, but could still request values outside of that
range, thus using hardware codecs unintentionally. This CL
enforces the supported range in VaapiWrapper::CreateContext().
This also introduces a feature,
VaapiEnforceVideoMinMaxResolution, to disable this check for
testing.

Bug: b:171041334
Test: video.Play.vp9_hw on atlas and cyan
Test: video_decode_accelerator_tests on atlas and cyan
Test: video_encode_accelerator_tests on atlas and cyan
Test: appr.tc/?vsc=h264&vrc=h264&debug=loopback on atlas and cyan
Change-Id: Ide589ee4d1456553e400a271cb8b3383b472b4d4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2549161
Commit-Queue: Hirokazu Honda <hiroh@chromium.org>
Auto-Submit: Hirokazu Honda <hiroh@chromium.org>
Reviewed-by: default avatarAndres Calderon Jaramillo <andrescj@chromium.org>
Reviewed-by: default avatarMiguel Casas <mcasas@chromium.org>
Reviewed-by: default avatarDale Curtis <dalecurtis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#832609}
parent 5012d99b
...@@ -436,6 +436,11 @@ const base::Feature kVaapiAV1Decoder{"VaapiAV1Decoder", ...@@ -436,6 +436,11 @@ const base::Feature kVaapiAV1Decoder{"VaapiAV1Decoder",
const base::Feature kVaapiLowPowerEncoderGen9x{ const base::Feature kVaapiLowPowerEncoderGen9x{
"VaapiLowPowerEncoderGen9x", base::FEATURE_DISABLED_BY_DEFAULT}; "VaapiLowPowerEncoderGen9x", base::FEATURE_DISABLED_BY_DEFAULT};
// Deny specific (likely small) resolutions for VA-API hardware decode and
// encode acceleration.
const base::Feature kVaapiEnforceVideoMinMaxResolution{
"VaapiEnforceVideoMinMaxResolution", base::FEATURE_ENABLED_BY_DEFAULT};
// Enable VA-API hardware encode acceleration for VP8. // Enable VA-API hardware encode acceleration for VP8.
const base::Feature kVaapiVP8Encoder{"VaapiVP8Encoder", const base::Feature kVaapiVP8Encoder{"VaapiVP8Encoder",
base::FEATURE_ENABLED_BY_DEFAULT}; base::FEATURE_ENABLED_BY_DEFAULT};
......
...@@ -173,6 +173,7 @@ MEDIA_EXPORT extern const base::Feature kUseR16Texture; ...@@ -173,6 +173,7 @@ MEDIA_EXPORT extern const base::Feature kUseR16Texture;
MEDIA_EXPORT extern const base::Feature kUseSodaForLiveCaption; MEDIA_EXPORT extern const base::Feature kUseSodaForLiveCaption;
MEDIA_EXPORT extern const base::Feature kVaapiAV1Decoder; MEDIA_EXPORT extern const base::Feature kVaapiAV1Decoder;
MEDIA_EXPORT extern const base::Feature kVaapiLowPowerEncoderGen9x; MEDIA_EXPORT extern const base::Feature kVaapiLowPowerEncoderGen9x;
MEDIA_EXPORT extern const base::Feature kVaapiEnforceVideoMinMaxResolution;
MEDIA_EXPORT extern const base::Feature kVaapiVP8Encoder; MEDIA_EXPORT extern const base::Feature kVaapiVP8Encoder;
MEDIA_EXPORT extern const base::Feature kVaapiVP9Encoder; MEDIA_EXPORT extern const base::Feature kVaapiVP9Encoder;
MEDIA_EXPORT extern const base::Feature kVideoBlitColorAccuracy; MEDIA_EXPORT extern const base::Feature kVideoBlitColorAccuracy;
......
...@@ -45,6 +45,11 @@ const std::vector<base::Feature> kDisabledFeaturesForVideoEncoderTest = { ...@@ -45,6 +45,11 @@ const std::vector<base::Feature> kDisabledFeaturesForVideoEncoderTest = {
// with this feature flag. We disable the feature to use VpxVideoDecoder to // with this feature flag. We disable the feature to use VpxVideoDecoder to
// decode any vp8 stream in BitstreamValidator. // decode any vp8 stream in BitstreamValidator.
kFFmpegDecodeOpaqueVP8, kFFmpegDecodeOpaqueVP8,
#if BUILDFLAG(USE_VAAPI)
// Disable this feature so that the encoder test can test a resolution
// which is denied for the sake of performance. See crbug.com/1008491.
kVaapiEnforceVideoMinMaxResolution,
#endif
}; };
uint32_t GetDefaultTargetBitrate(const gfx::Size& resolution, uint32_t GetDefaultTargetBitrate(const gfx::Size& resolution,
......
...@@ -56,7 +56,15 @@ VideoPlayerTestEnvironment::VideoPlayerTestEnvironment( ...@@ -56,7 +56,15 @@ VideoPlayerTestEnvironment::VideoPlayerTestEnvironment(
media::kVaapiAV1Decoder, media::kVaapiAV1Decoder,
#endif #endif
}, },
/*disabled_featureas=*/{}), /*disabled_features=*/
{
#if BUILDFLAG(USE_VAAPI)
// Disable this feature so that the decoder test can test a
// resolution which is denied for the sake of performance. See
// b/171041334.
kVaapiEnforceVideoMinMaxResolution,
#endif
}),
video_(std::move(video)), video_(std::move(video)),
enable_validator_(enable_validator), enable_validator_(enable_validator),
implementation_(implementation), implementation_(implementation),
......
...@@ -1099,6 +1099,26 @@ bool VASupportedProfiles::FillProfileInfo_Locked( ...@@ -1099,6 +1099,26 @@ bool VASupportedProfiles::FillProfileInfo_Locked(
return false; return false;
} }
if (base::FeatureList::IsEnabled(kVaapiEnforceVideoMinMaxResolution) &&
va_profile != VAProfileJPEGBaseline) {
// Deny unreasonably small resolutions (e.g. 0x0) for VA-API hardware video
// decode and encode acceleration.
profile_info->min_resolution.SetToMax(gfx::Size(16, 16));
if (entrypoint == VAEntrypointEncSliceLP ||
entrypoint == VAEntrypointEncSlice) {
// Using VA-API for accelerated encoding frames smaller than a certain
// size is less efficient than using a software encoder.
constexpr gfx::Size kMinEncodeResolution(320 + 1, 240 + 1);
if (!gfx::Rect(profile_info->min_resolution)
.Contains(gfx::Rect(kMinEncodeResolution))) {
profile_info->min_resolution.SetToMax(kMinEncodeResolution);
DVLOG(2) << "Setting the minimum supported encoding resolution to "
<< profile_info->min_resolution.ToString() << " for "
<< vaProfileStr(va_profile);
}
}
}
// Create a new configuration to find the supported RT formats. We don't pass // Create a new configuration to find the supported RT formats. We don't pass
// required attributes here because we want the driver to tell us all the // required attributes here because we want the driver to tell us all the
// supported RT formats. // supported RT formats.
...@@ -1383,10 +1403,7 @@ VaapiWrapper::GetSupportedEncodeProfiles() { ...@@ -1383,10 +1403,7 @@ VaapiWrapper::GetSupportedEncodeProfiles() {
VideoEncodeAccelerator::SupportedProfile profile; VideoEncodeAccelerator::SupportedProfile profile;
profile.profile = media_profile; profile.profile = media_profile;
// Using VA-API for accelerated encoding frames smaller than a certain profile.min_resolution = profile_info->min_resolution;
// size is less efficient than using a software encoder.
const gfx::Size kMinEncodeResolution = gfx::Size(320 + 1, 240 + 1);
profile.min_resolution = kMinEncodeResolution;
profile.max_resolution = profile_info->max_resolution; profile.max_resolution = profile_info->max_resolution;
// Maximum framerate of encoded profile. This value is an arbitrary // Maximum framerate of encoded profile. This value is an arbitrary
// limit and not taken from HW documentation. // limit and not taken from HW documentation.
...@@ -1426,7 +1443,7 @@ VaapiWrapper::GetSupportedDecodeProfiles( ...@@ -1426,7 +1443,7 @@ VaapiWrapper::GetSupportedDecodeProfiles(
VideoDecodeAccelerator::SupportedProfile profile; VideoDecodeAccelerator::SupportedProfile profile;
profile.profile = media_profile; profile.profile = media_profile;
profile.max_resolution = profile_info->max_resolution; profile.max_resolution = profile_info->max_resolution;
profile.min_resolution.SetSize(16, 16); profile.min_resolution = profile_info->min_resolution;
profiles.push_back(profile); profiles.push_back(profile);
} }
return profiles; return profiles;
...@@ -1853,6 +1870,25 @@ bool VaapiWrapper::CreateContext(const gfx::Size& size) { ...@@ -1853,6 +1870,25 @@ bool VaapiWrapper::CreateContext(const gfx::Size& size) {
// vpp, just passing 0x0. // vpp, just passing 0x0.
const int flag = mode_ != kVideoProcess ? VA_PROGRESSIVE : 0x0; const int flag = mode_ != kVideoProcess ? VA_PROGRESSIVE : 0x0;
const gfx::Size picture_size = mode_ != kVideoProcess ? size : gfx::Size(); const gfx::Size picture_size = mode_ != kVideoProcess ? size : gfx::Size();
if (mode_ != kVideoProcess) {
const VASupportedProfiles::ProfileInfo* profile_info =
VASupportedProfiles::Get().IsProfileSupported(mode_, va_profile_,
va_entrypoint_);
DCHECK(profile_info);
const bool is_picture_within_bounds =
gfx::Rect(picture_size)
.Contains(gfx::Rect(profile_info->min_resolution)) &&
gfx::Rect(profile_info->max_resolution)
.Contains(gfx::Rect(picture_size));
if (!is_picture_within_bounds) {
VLOG(2) << "Requested resolution=" << picture_size.ToString()
<< " is not within bounds ["
<< profile_info->min_resolution.ToString() << ", "
<< profile_info->max_resolution.ToString() << "]";
return false;
}
}
VAStatus va_res = vaCreateContext( VAStatus va_res = vaCreateContext(
va_display_, va_config_id_, picture_size.width(), picture_size.height(), va_display_, va_config_id_, picture_size.width(), picture_size.height(),
flag, empty_va_surfaces_ids_pointer, empty_va_surfaces_ids_size, flag, empty_va_surfaces_ids_pointer, empty_va_surfaces_ids_size,
......
...@@ -238,7 +238,10 @@ std::unique_ptr<base::test::ScopedFeatureList> CreateScopedFeatureList() { ...@@ -238,7 +238,10 @@ std::unique_ptr<base::test::ScopedFeatureList> CreateScopedFeatureList() {
media::kVaapiLowPowerEncoderGen9x, media::kVaapiLowPowerEncoderGen9x,
// TODO(crbug.com/811912): remove once enabled by default. // TODO(crbug.com/811912): remove once enabled by default.
media::kVaapiVP9Encoder}; media::kVaapiVP9Encoder};
scoped_feature_list->InitWithFeatures(enabled_features, {}); std::vector<base::Feature> disabled_features = {
media::kVaapiEnforceVideoMinMaxResolution,
};
scoped_feature_list->InitWithFeatures(enabled_features, disabled_features);
return scoped_feature_list; return scoped_feature_list;
#else #else
return nullptr; return nullptr;
......
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