[FFmpeg-devel] [PATCH] vaapi_decode: Improve logging around codec/profile selection

[Mark Thompson]

On 12/04/2020 18:19, Fu, Linjie wrote:
>> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of
>> Mark Thompson
>> Sent: Sunday, April 12, 2020 21:00
>> To: ffmpeg-devel at 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