[FFmpeg-devel] [PATCH] ffmpeg: Don't require a known device to pass a frames context to an encoder

[Fu, Linjie]

> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of
> Mark Thompson
> Sent: Wednesday, April 29, 2020 06:57
> To: FFmpeg development discussions and patches <ffmpeg-
> devel at ffmpeg.org>
> Subject: [FFmpeg-devel] [PATCH] ffmpeg: Don't require a known device to
> pass a frames context to an encoder
> 
> The previous version here did not handle passing a frames context when
> ffmpeg itself did not know about the device it came from (for example,
> because it was created by device derivation inside a filter graph), which
> would break encoders requiring that input.  Fix that by checking for HW
> frames and device context methods independently, and prefer to use a
> frames context method if possible.  At the same time, revert the encoding
> additions to the device matching function because the additional
> complexity was not relevant to decoding.
> ---
>  fftools/ffmpeg_hw.c | 75 +++++++++++++++++++++++++--------------------
>  1 file changed, 42 insertions(+), 33 deletions(-)
> diff --git a/fftools/ffmpeg_hw.c b/fftools/ffmpeg_hw.c
> index c5c8aa97ef..fc4a5d31d6 100644
> --- a/fftools/ffmpeg_hw.c
> +++ b/fftools/ffmpeg_hw.c
> @@ -19,6 +19,7 @@
>  #include <string.h>
> 
>  #include "libavutil/avstring.h"
> +#include "libavutil/pixdesc.h"
>  #include "libavfilter/buffersink.h"
> 
>  #include "ffmpeg.h"
> @@ -282,10 +283,7 @@ void hw_device_free_all(void)
>      nb_hw_devices = 0;
>  }
> 
> -static HWDevice *hw_device_match_by_codec(const AVCodec *codec,
> -                                          enum AVPixelFormat format,
> -                                          int possible_methods,
> -                                          int *matched_methods)
> +static HWDevice *hw_device_match_by_codec(const AVCodec *codec)
>  {
>      const AVCodecHWConfig *config;
>      HWDevice *dev;
> @@ -294,18 +292,11 @@ static HWDevice
> *hw_device_match_by_codec(const AVCodec *codec,
>          config = avcodec_get_hw_config(codec, i);
>          if (!config)
>              return NULL;
> -        if (format != AV_PIX_FMT_NONE &&
> -            config->pix_fmt != AV_PIX_FMT_NONE &&
> -            config->pix_fmt != format)
> -            continue;
> -        if (!(config->methods & possible_methods))
> +        if (!(config->methods &
> AV_CODEC_HW_CONFIG_METHOD_HW_DEVICE_CTX))
>              continue;
>          dev = hw_device_get_by_type(config->device_type);
> -        if (dev) {
> -            if (matched_methods)
> -                *matched_methods = config->methods & possible_methods;
> +        if (dev)
>              return dev;
> -        }
>      }
>  }
> 
> @@ -351,9 +342,7 @@ int hw_device_setup_for_decode(InputStream *ist)
>              if (!dev)
>                  err = hw_device_init_from_type(type, NULL, &dev);
>          } else {
> -            dev = hw_device_match_by_codec(ist->dec, AV_PIX_FMT_NONE,
> -                                           AV_CODEC_HW_CONFIG_METHOD_HW_DEVICE_CTX,
> -                                           NULL);
> +            dev = hw_device_match_by_codec(ist->dec);
>              if (!dev) {
>                  // No device for this codec, but not using generic hwaccel
>                  // and therefore may well not need one - ignore.
> @@ -429,37 +418,57 @@ int hw_device_setup_for_decode(InputStream
> *ist)
> 
>  int hw_device_setup_for_encode(OutputStream *ost)
>  {
> -    HWDevice *dev;
> -    AVBufferRef *frames_ref;
> -    int methods = AV_CODEC_HW_CONFIG_METHOD_HW_DEVICE_CTX;
> -    int matched_methods;
> +    const AVCodecHWConfig *config;
> +    HWDevice *dev = NULL;
> +    AVBufferRef *frames_ref = NULL;
> +    int i;
> 
>      if (ost->filter) {
>          frames_ref = av_buffersink_get_hw_frames_ctx(ost->filter->filter);
>          if (frames_ref &&
>              ((AVHWFramesContext*)frames_ref->data)->format ==
> -            ost->enc_ctx->pix_fmt)
> -            methods |= AV_CODEC_HW_CONFIG_METHOD_HW_FRAMES_CTX;
> +            ost->enc_ctx->pix_fmt) {
> +            // Matching format, will try to use hw_frames_ctx.
> +        } else {
> +            frames_ref = NULL;
> +        }
>      }
> 
> -    dev = hw_device_match_by_codec(ost->enc, ost->enc_ctx->pix_fmt,
> -                                   methods, &matched_methods);
> -    if (dev) {
> -        if (matched_methods &
> AV_CODEC_HW_CONFIG_METHOD_HW_DEVICE_CTX) {
> -            ost->enc_ctx->hw_device_ctx = av_buffer_ref(dev->device_ref);
> -            if (!ost->enc_ctx->hw_device_ctx)
> -                return AVERROR(ENOMEM);
> -        }
> -        if (matched_methods &
> AV_CODEC_HW_CONFIG_METHOD_HW_FRAMES_CTX) {
> +    for (i = 0;; i++) {
> +        config = avcodec_get_hw_config(ost->enc, i);
> +        if (!config)
> +            break;
> +
> +        if (frames_ref &&
> +            config->methods &
> AV_CODEC_HW_CONFIG_METHOD_HW_FRAMES_CTX &&
> +            (config->pix_fmt == AV_PIX_FMT_NONE ||
> +             config->pix_fmt == ost->enc_ctx->pix_fmt)) {
> +            av_log(ost->enc_ctx, AV_LOG_VERBOSE, "Using input "
> +                   "frames context (format %s) with %s encoder.\n",
> +                   av_get_pix_fmt_name(ost->enc_ctx->pix_fmt),
> +                   ost->enc->name);
>              ost->enc_ctx->hw_frames_ctx = av_buffer_ref(frames_ref);
>              if (!ost->enc_ctx->hw_frames_ctx)
>                  return AVERROR(ENOMEM);
> +            return 0;
>          }
> -        return 0;
> +
> +        if (!dev &&
> +            config->methods &
> AV_CODEC_HW_CONFIG_METHOD_HW_DEVICE_CTX)
> +            dev = hw_device_get_by_type(config->device_type);
> +    }
> +
> +    if (dev) {
> +        av_log(ost->enc_ctx, AV_LOG_VERBOSE, "Using device %s "
> +               "(type %s) with %s encoder.\n", dev->name,
> +               av_hwdevice_get_type_name(dev->type), ost->enc->name);
> +        ost->enc_ctx->hw_device_ctx = av_buffer_ref(dev->device_ref);
> +        if (!ost->enc_ctx->hw_device_ctx)
> +            return AVERROR(ENOMEM);
>      } else {
>          // No device required, or no device available.

I've got a question on these comments for long time which is not related to the patch
itself, isn't it a good idea to treat them as logs with a verbosity level of warning or DEBUG,
then user/developer may catch/seek this easier if something unexpected happens?

Another instance is:
https://patchwork.ffmpeg.org/project/ffmpeg/patch/20190815043242.20478-1-linjie.fu@intel.com/

> -        return 0;
>      }
> +    return 0;
>  }
> 
>  static int hwaccel_retrieve_data(AVCodecContext *avctx, AVFrame *input)
> --

Works for me for #8637, and also tested in device context methods with
-init_hw_device qsv=hw:/dev/dri/renderD128 -filter_hw_device hw

Hence would it be better to mention the ticket number as well?

- Linjie