[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