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

Mark Thompson sw at jkqxz.net
Sun May 3 18:05:28 EEST 2020


On 29/04/2020 06:34, Fu, Linjie wrote:
>> 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?

I didn't include a log line here because it would be printed by /every/ encoder created which didn't touch one of the other cases.

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

Huh, yeah, that's exactly the same problem but with the device creation hidden in the ad-hoc libmfx setup code rather than in a filter graph.

Added a note to that effect and applied.

Thanks,

- Mark


More information about the ffmpeg-devel mailing list