> From: ffmpeg-devel <ffmpeg-devel-boun...@ffmpeg.org> On Behalf Of > Mark Thompson > Sent: Wednesday, April 29, 2020 06:57 > To: FFmpeg development discussions and patches <ffmpeg- > de...@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...@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 _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".