On Sat, 21 Jan 2017 22:16:15 +0000
Mark Thompson <s...@jkqxz.net> wrote:
> On 18/01/17 08:01, wm4 wrote:
> > On Tue, 17 Jan 2017 22:31:48 +0000
> > Mark Thompson <s...@jkqxz.net> wrote:
> >
> >> Previously it was only created if the decode hwaccel was requested;
> >> this makes it unconditionally so we can use it without the hwaccel.
> >> ---
> >> ffmpeg.h | 4 +---
> >> ffmpeg_opt.c | 13 ++++++++++++-
> >> ffmpeg_qsv.c | 14 ++++++--------
> >> 3 files changed, 19 insertions(+), 12 deletions(-)
> >>
> >> diff --git a/ffmpeg.h b/ffmpeg.h
> >> index 75bf50e..7362c2d 100644
> >> --- a/ffmpeg.h
> >> +++ b/ffmpeg.h
> >> @@ -604,9 +604,6 @@ extern const OptionDef options[];
> >> extern const HWAccel hwaccels[];
> >> extern int hwaccel_lax_profile_check;
> >> extern AVBufferRef *hw_device_ctx;
> >> -#if CONFIG_QSV
> >> -extern char *qsv_device;
> >> -#endif
> >>
> >>
> >> void term_init(void);
> >> @@ -641,6 +638,7 @@ int vdpau_init(AVCodecContext *s);
> >> int dxva2_init(AVCodecContext *s);
> >> int vda_init(AVCodecContext *s);
> >> int videotoolbox_init(AVCodecContext *s);
> >> +int qsv_device_init(const char *device);
> >> int qsv_init(AVCodecContext *s);
> >> int qsv_transcode_init(OutputStream *ost);
> >> int vaapi_decode_init(AVCodecContext *avctx);
> >> diff --git a/ffmpeg_opt.c b/ffmpeg_opt.c
> >> index a1c02fc..49397be 100644
> >> --- a/ffmpeg_opt.c
> >> +++ b/ffmpeg_opt.c
> >> @@ -462,6 +462,17 @@ static int opt_vaapi_device(void *optctx, const char
> >> *opt, const char *arg)
> >> }
> >> #endif
> >>
> >> +#if CONFIG_QSV
> >> +static int opt_qsv_device(void *optctx, const char *opt, const char *arg)
> >> +{
> >> + int err;
> >> + err = qsv_device_init(arg);
> >> + if (err < 0)
> >> + exit_program(1);
> >> + return 0;
> >> +}
> >> +#endif
> >> +
> >> /**
> >> * Parse a metadata specifier passed as 'arg' parameter.
> >> * @param arg metadata string to parse
> >> @@ -3694,7 +3705,7 @@ const OptionDef options[] = {
> >> #endif
> >>
> >> #if CONFIG_QSV
> >> - { "qsv_device", HAS_ARG | OPT_STRING | OPT_EXPERT, { &qsv_device },
> >> + { "qsv_device", HAS_ARG | OPT_EXPERT, { .func_arg = opt_qsv_device },
> >> "set QSV hardware device (DirectX adapter index, DRM path or X11
> >> display name)", "device"},
> >> #endif
> >>
> >> diff --git a/ffmpeg_qsv.c b/ffmpeg_qsv.c
> >> index 86824b6..9356f05 100644
> >> --- a/ffmpeg_qsv.c
> >> +++ b/ffmpeg_qsv.c
> >> @@ -28,8 +28,6 @@
> >>
> >> #include "ffmpeg.h"
> >>
> >> -char *qsv_device = NULL;
> >> -
> >> static int qsv_get_buffer(AVCodecContext *s, AVFrame *frame, int flags)
> >> {
> >> InputStream *ist = s->opaque;
> >> @@ -43,19 +41,19 @@ static void qsv_uninit(AVCodecContext *s)
> >> av_buffer_unref(&ist->hw_frames_ctx);
> >> }
> >>
> >> -static int qsv_device_init(InputStream *ist)
> >> +int qsv_device_init(const char *device)
> >> {
> >> int err;
> >> AVDictionary *dict = NULL;
> >>
> >> - if (qsv_device) {
> >> - err = av_dict_set(&dict, "child_device", qsv_device, 0);
> >> + if (device) {
> >> + err = av_dict_set(&dict, "child_device", device, 0);
> >> if (err < 0)
> >> return err;
> >> }
> >>
> >> err = av_hwdevice_ctx_create(&hw_device_ctx, AV_HWDEVICE_TYPE_QSV,
> >> - ist->hwaccel_device, dict, 0);
> >> + device, dict, 0);
> >> if (err < 0) {
> >> av_log(NULL, AV_LOG_ERROR, "Error creating a QSV device\n");
> >> goto err_out;
> >> @@ -76,7 +74,7 @@ int qsv_init(AVCodecContext *s)
> >> int ret;
> >>
> >> if (!hw_device_ctx) {
> >> - ret = qsv_device_init(ist);
> >> + ret = qsv_device_init(ist->hwaccel_device);
> >> if (ret < 0)
> >> return ret;
> >> }
> >> @@ -148,7 +146,7 @@ int qsv_transcode_init(OutputStream *ost)
> >> av_log(NULL, AV_LOG_VERBOSE, "Setting up QSV transcoding\n");
> >>
> >> if (!hw_device_ctx) {
> >> - err = qsv_device_init(ist);
> >> + err = qsv_device_init(ist->hwaccel_device);
> >> if (err < 0)
> >> goto fail;
> >> }
> >
> > Like the vaapi case, this leaks a device context each time the option
> > is used after the first time.
>
> Right, I can fix that. (For VAAPI too.)
>
> > Also, shouldn't this maybe only set the hwcontext type, and ffmpeg.c
> > should call av_hwdevice_ctx_create() in a common code path later?
>
> Is there any case where the ordering matters?
>
> (It does have to be created unconditionally once you have the option, because
> it needs to be set for filters to work. The skipped scale_qsv and subsequent
> deinterlace_qsv filters can be merged to work with hwupload after this.)
Probably would be a separate change at a much later time anyway.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
