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

[Fu, Linjie]

> 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.

- Linjie