On 12/04/2020 18:19, Fu, Linjie wrote:
>> From: ffmpeg-devel <ffmpeg-devel-boun...@ffmpeg.org> On Behalf Of
>> Mark Thompson
>> Sent: Sunday, April 12, 2020 21:00
>> To: ffmpeg-devel@ffmpeg.org
>> Subject: [FFmpeg-devel] [PATCH] vaapi_decode: Improve logging around
>> codec/profile selection
>>
>> ---
>> On 12/04/2020 13:14, Mark Thompson wrote:
>>> ... This does rather suggest that the error messages in that file should be
>> clearer, though - it would be nice if it could distinguish between "this
>> codec
>> isn't supported by libavcodec at all", "this codec might work but hasn't
>> built
>> into this libavcodec" and "this codec is supported by libavcodec but not by
>> your hardware".
>>
>> Something like this?
>>
>>
>> libavcodec/vaapi_decode.c | 39 +++++++++++++++++++++++++++++++----
>> ----
>> 1 file changed, 31 insertions(+), 8 deletions(-)
>>
>> diff --git a/libavcodec/vaapi_decode.c b/libavcodec/vaapi_decode.c
>> index 54a0ecb47a..a191850e36 100644
>> --- a/libavcodec/vaapi_decode.c
>> +++ b/libavcodec/vaapi_decode.c
>> @@ -429,6 +429,7 @@ static int
>> vaapi_decode_make_config(AVCodecContext *avctx,
>> const AVCodecDescriptor *codec_desc;
>> VAProfile *profile_list = NULL, matched_va_profile, va_profile;
>> int profile_count, exact_match, matched_ff_profile, codec_profile;
>> + int found_codec, found_profile;
>>
>> AVHWDeviceContext *device = (AVHWDeviceContext*)device_ref-
>>> data;
>> AVVAAPIDeviceContext *hwctx = device->hwctx;
>> @@ -457,15 +458,19 @@ static int
>> vaapi_decode_make_config(AVCodecContext *avctx,
>> }
>>
>> matched_va_profile = VAProfileNone;
>> + found_codec = found_profile = 0;
>> exact_match = 0;
>>
>> for (i = 0; i < FF_ARRAY_ELEMS(vaapi_profile_map); i++) {
>> int profile_match = 0;
>> if (avctx->codec_id != vaapi_profile_map[i].codec_id)
>> continue;
>> + found_codec = 1;
>> if (avctx->profile == vaapi_profile_map[i].codec_profile ||
>> - vaapi_profile_map[i].codec_profile == FF_PROFILE_UNKNOWN)
>> + vaapi_profile_map[i].codec_profile == FF_PROFILE_UNKNOWN) {
>> profile_match = 1;
>> + found_profile = 1;
>> + }
>>
>> va_profile = vaapi_profile_map[i].profile_parser ?
>> vaapi_profile_map[i].profile_parser(avctx) :
>> @@ -487,24 +492,42 @@ static int
>> vaapi_decode_make_config(AVCodecContext *avctx,
>> }
>> av_freep(&profile_list);
>>
>> - if (matched_va_profile == VAProfileNone) {
>> - av_log(avctx, AV_LOG_ERROR, "No support for codec %s "
>> - "profile %d.\n", codec_desc->name, avctx->profile);
>> + if (!found_codec) {
>> + av_log(avctx, AV_LOG_ERROR, "This libavcodec build does not "
>> + "support VAAPI decoding of codec %s.\n",
>> + codec_desc->name);
>> + err = AVERROR(ENOSYS);
>> + goto fail;
>> + }
>> + if (!found_profile && !(avctx->hwaccel_flags &
>> + AV_HWACCEL_FLAG_ALLOW_PROFILE_MISMATCH)) {
>> + // We allow this case with profile-mismatch enabled to support
>> + // things like trying to decode H.264 extended profile.
>> + av_log(avctx, AV_LOG_ERROR, "This libavcodec build does not "
>> + "support VAAPI decoding of codec %s profile %d.\n",
>> + codec_desc->name, avctx->profile);
>> err = AVERROR(ENOSYS);
>> goto fail;
>> }
>> + if (matched_va_profile == VAProfileNone) {
>> + av_log(avctx, AV_LOG_ERROR, "This VAAPI driver does not "
>> + "support decoding of codec %s.\n",
>> + codec_desc->name);
>> + err = AVERROR(EINVAL);
>> + goto fail;
>> + }
>> if (!exact_match) {
>> if (avctx->hwaccel_flags &
>> AV_HWACCEL_FLAG_ALLOW_PROFILE_MISMATCH) {
>> - av_log(avctx, AV_LOG_VERBOSE, "Codec %s profile %d not "
>> - "supported for hardware decode.\n",
>> + av_log(avctx, AV_LOG_WARNING, "This VAAPI driver does not "
>> + "support decoding of codec %s profile %d.\n",
>> codec_desc->name, avctx->profile);
>> av_log(avctx, AV_LOG_WARNING, "Using possibly-"
>> "incompatible profile %d instead.\n",
>> matched_ff_profile);
>> } else {
>> - av_log(avctx, AV_LOG_VERBOSE, "Codec %s profile %d not "
>> - "supported for hardware decode.\n",
>> + av_log(avctx, AV_LOG_ERROR, "This VAAPI driver does not "
>> + "support decoding of codec %s profile %d.\n",
>> codec_desc->name, avctx->profile);
>> err = AVERROR(EINVAL);
>> goto fail;
>> --
> Generally makes sense, however there is one concern if I got it correctly:
>
> If a codec is not supported by VAAPI in current libavcodec build,
> ff_get_format()
> would not select VAAPI as the HW acceleration.
> Instead, it would fallback to the native software decoding path, and won't
> trigger
> the (!found_codec) logic.
>
> ./configure --enable-vaapi --disable-hwaccel=hevc_vaapi
>
> $ ffmpeg -v debug -hwaccel vaapi -i
> ./hevc_rext_decode/Main_422_10_A_RExt_Sony_2.bin -f null -
>
> Log:
> [hevc @ 0x5592f7b0fec0] Format yuv422p10le chosen by get_format().
>
> VAAPI error return would not be triggered.
Yeah, it's not covering that case because of how get_format() works - the
caller can't choose the VAAPI format, so we never get here. If the caller
really wanted VAAPI then they need to deal with it externally (for ffmpeg, you
want something like the proposed -decode_format option:
<https://lists.ffmpeg.org/pipermail/ffmpeg-devel/2018-November/236205.html>).
The case where the "no codec" message is triggered is when you can pick it (the
hwaccel is enabled) but the build still doesn't have the codec in the table
(for example if it were built against older libva headers).
- Mark
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
