| Message ID | 20181109090534.27330-1-linjie.fu@intel.com |
|---|---|
| State | New |
| Headers | show |
> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf > Of Linjie Fu > Sent: Friday, November 9, 2018 5:06 PM > To: ffmpeg-devel@ffmpeg.org > Cc: Fu, Linjie <linjie.fu@intel.com> > Subject: [FFmpeg-devel] [PATCH v3] fftools/ffmpeg: add an option to forbid > the fallback to software path when hardware init fails > > Currently ff_get_format will go through all usable choices if the chosen > format was not supported. It will fallback to software path if the hardware > init fails. > > Provided an option "-require_hwaccel 1" in user-code to detect > frame->format and hwaccel_get_buffer in get_buffer. If hardware init fails, > returns an error. Would better if add commit message it is to track ticket #7519 > Signed-off-by: Linjie Fu <linjie.fu@intel.com> > --- > [v2] detect hardware init failures in get_buffer and modify in user-code [v3] > changed the option name, add error message > > fftools/ffmpeg.c | 4 ++++ > fftools/ffmpeg.h | 3 +++ > fftools/ffmpeg_opt.c | 4 ++++ > 3 files changed, 11 insertions(+) > > diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c index > da4259a9a8..113ab6312a 100644 > --- a/fftools/ffmpeg.c > +++ b/fftools/ffmpeg.c > @@ -2890,6 +2890,10 @@ static int get_buffer(AVCodecContext *s, > AVFrame *frame, int flags) > > if (ist->hwaccel_get_buffer && frame->format == > ist->hwaccel_pix_fmt) > return ist->hwaccel_get_buffer(s, frame, flags); > + else if (ist->require_hwaccel) { > + av_log(s, AV_LOG_ERROR, "Hardware acceleration is required > and will not fallback to try software path.\n"); > + return AVERROR(EINVAL); > + } > > return avcodec_default_get_buffer2(s, frame, flags); } diff --git > a/fftools/ffmpeg.h b/fftools/ffmpeg.h index eb1eaf6363..a5c85daa67 > 100644 > --- a/fftools/ffmpeg.h > +++ b/fftools/ffmpeg.h > @@ -133,6 +133,8 @@ typedef struct OptionsContext { > int nb_hwaccel_output_formats; > SpecifierOpt *autorotate; > int nb_autorotate; > + SpecifierOpt *require_hwaccel; > + int nb_require_hwaccel; > > /* output options */ > StreamMap *stream_maps; > @@ -365,6 +367,7 @@ typedef struct InputStream { > enum AVHWDeviceType hwaccel_device_type; > char *hwaccel_device; > enum AVPixelFormat hwaccel_output_format; > + int require_hwaccel; > > /* hwaccel context */ > void *hwaccel_ctx; > diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c index > d4851a2cd8..6890bb7fcf 100644 > --- a/fftools/ffmpeg_opt.c > +++ b/fftools/ffmpeg_opt.c > @@ -730,6 +730,7 @@ static void add_input_streams(OptionsContext *o, > AVFormatContext *ic) > ist->autorotate = 1; > MATCH_PER_STREAM_OPT(autorotate, i, ist->autorotate, ic, st); > > + MATCH_PER_STREAM_OPT(require_hwaccel, i, > ist->require_hwaccel, > + ic, st); > MATCH_PER_STREAM_OPT(codec_tags, str, codec_tag, ic, st); > if (codec_tag) { > uint32_t tag = strtol(codec_tag, &next, 0); @@ -3600,6 > +3601,9 @@ const OptionDef options[] = { > { "autorotate", HAS_ARG | OPT_BOOL | OPT_SPEC | > OPT_EXPERT | OPT_INPUT, > { .off = OFFSET(autorotate) }, > "automatically insert correct rotate filters" }, > + { "require_hwaccel", HAS_ARG | OPT_BOOL | OPT_SPEC | > + OPT_EXPERT | OPT_INPUT, > { .off = OFFSET(require_hwaccel)}, > + "forbid the fallback to default software path when hardware init > + fails"}, I would like to see a name like "force_hwaccel". HWACEEL is already required with the option "hwaccel" though it is not to force HW path.
On 09/11/18 09:05, Linjie Fu wrote: > Currently ff_get_format will go through all usable choices if the chosen > format was not supported. It will fallback to software path if the hardware > init fails. > > Provided an option "-require_hwaccel 1" in user-code to detect frame->format and > hwaccel_get_buffer in get_buffer. If hardware init fails, returns an error. > > Signed-off-by: Linjie Fu <linjie.fu@intel.com> > --- > [v2] detect hardware init failures in get_buffer and modify in user-code > [v3] changed the option name, add error message > > fftools/ffmpeg.c | 4 ++++ > fftools/ffmpeg.h | 3 +++ > fftools/ffmpeg_opt.c | 4 ++++ > 3 files changed, 11 insertions(+) > > diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c > index da4259a9a8..113ab6312a 100644 > --- a/fftools/ffmpeg.c > +++ b/fftools/ffmpeg.c > @@ -2890,6 +2890,10 @@ static int get_buffer(AVCodecContext *s, AVFrame *frame, int flags) > > if (ist->hwaccel_get_buffer && frame->format == ist->hwaccel_pix_fmt) > return ist->hwaccel_get_buffer(s, frame, flags); > + else if (ist->require_hwaccel) { > + av_log(s, AV_LOG_ERROR, "Hardware acceleration is required and will not fallback to try software path.\n"); > + return AVERROR(EINVAL); > + } Um, doesn't this just break every hardware case except QSV? The hwaccel_get_buffer function is only used for QSV, and will be unset for any other cases. (In fact, I don't think it does anything useful there anyway - maybe it can be removed completely, I'll have a look.) I think the right place to make this decision (to force a hardware format) is in get_format(), rather than after when the decoder is already configured - this avoids cases where the format you actually wanted was available but you didn't pick it and then fail later. I had a patch lying around for another purpose which gives you some support for this, just sent if you'd like to have a look. > > return avcodec_default_get_buffer2(s, frame, flags); > } > diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h > index eb1eaf6363..a5c85daa67 100644 > --- a/fftools/ffmpeg.h > +++ b/fftools/ffmpeg.h > @@ -133,6 +133,8 @@ typedef struct OptionsContext { > int nb_hwaccel_output_formats; > SpecifierOpt *autorotate; > int nb_autorotate; > + SpecifierOpt *require_hwaccel; > + int nb_require_hwaccel; > > /* output options */ > StreamMap *stream_maps; > @@ -365,6 +367,7 @@ typedef struct InputStream { > enum AVHWDeviceType hwaccel_device_type; > char *hwaccel_device; > enum AVPixelFormat hwaccel_output_format; > + int require_hwaccel; > > /* hwaccel context */ > void *hwaccel_ctx; > diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c > index d4851a2cd8..6890bb7fcf 100644 > --- a/fftools/ffmpeg_opt.c > +++ b/fftools/ffmpeg_opt.c > @@ -730,6 +730,7 @@ static void add_input_streams(OptionsContext *o, AVFormatContext *ic) > ist->autorotate = 1; > MATCH_PER_STREAM_OPT(autorotate, i, ist->autorotate, ic, st); > > + MATCH_PER_STREAM_OPT(require_hwaccel, i, ist->require_hwaccel, ic, st); > MATCH_PER_STREAM_OPT(codec_tags, str, codec_tag, ic, st); > if (codec_tag) { > uint32_t tag = strtol(codec_tag, &next, 0); > @@ -3600,6 +3601,9 @@ const OptionDef options[] = { > { "autorotate", HAS_ARG | OPT_BOOL | OPT_SPEC | > OPT_EXPERT | OPT_INPUT, { .off = OFFSET(autorotate) }, > "automatically insert correct rotate filters" }, > + { "require_hwaccel", HAS_ARG | OPT_BOOL | OPT_SPEC | > + OPT_EXPERT | OPT_INPUT, { .off = OFFSET(require_hwaccel)}, > + "forbid the fallback to default software path when hardware init fails"}, > > /* audio options */ > { "aframes", OPT_AUDIO | HAS_ARG | OPT_PERFILE | OPT_OUTPUT, { .func_arg = opt_audio_frames }, >
> -----Original Message----- > From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf > Of Mark Thompson > Sent: Sunday, November 11, 2018 23:09 > To: ffmpeg-devel@ffmpeg.org > Subject: Re: [FFmpeg-devel] [PATCH v3] fftools/ffmpeg: add an option to > forbid the fallback to software path when hardware init fails > > On 09/11/18 09:05, Linjie Fu wrote: > > Currently ff_get_format will go through all usable choices if the chosen > > format was not supported. It will fallback to software path if the hardware > > init fails. > > > > Provided an option "-require_hwaccel 1" in user-code to detect frame- > >format and > > hwaccel_get_buffer in get_buffer. If hardware init fails, returns an error. > > > > Signed-off-by: Linjie Fu <linjie.fu@intel.com> > > --- > > [v2] detect hardware init failures in get_buffer and modify in user-code > > [v3] changed the option name, add error message > > > > fftools/ffmpeg.c | 4 ++++ > > fftools/ffmpeg.h | 3 +++ > > fftools/ffmpeg_opt.c | 4 ++++ > > 3 files changed, 11 insertions(+) > > > > diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c > > index da4259a9a8..113ab6312a 100644 > > --- a/fftools/ffmpeg.c > > +++ b/fftools/ffmpeg.c > > @@ -2890,6 +2890,10 @@ static int get_buffer(AVCodecContext *s, > AVFrame *frame, int flags) > > > > if (ist->hwaccel_get_buffer && frame->format == ist->hwaccel_pix_fmt) > > return ist->hwaccel_get_buffer(s, frame, flags); > > + else if (ist->require_hwaccel) { > > + av_log(s, AV_LOG_ERROR, "Hardware acceleration is required and will > not fallback to try software path.\n"); > > + return AVERROR(EINVAL); > > + } > > Um, doesn't this just break every hardware case except QSV? The > hwaccel_get_buffer function is only used for QSV, and will be unset for any > other cases. (In fact, I don't think it does anything useful there anyway - > maybe it can be removed completely, I'll have a look.) > > I think the right place to make this decision (to force a hardware format) is in > get_format(), rather than after when the decoder is already configured - this > avoids cases where the format you actually wanted was available but you > didn't pick it and then fail later. I had a patch lying around for another > purpose which gives you some support for this, just sent if you'd like to have > a look. > Thanks for the comments and hints.
diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c index da4259a9a8..113ab6312a 100644 --- a/fftools/ffmpeg.c +++ b/fftools/ffmpeg.c @@ -2890,6 +2890,10 @@ static int get_buffer(AVCodecContext *s, AVFrame *frame, int flags) if (ist->hwaccel_get_buffer && frame->format == ist->hwaccel_pix_fmt) return ist->hwaccel_get_buffer(s, frame, flags); + else if (ist->require_hwaccel) { + av_log(s, AV_LOG_ERROR, "Hardware acceleration is required and will not fallback to try software path.\n"); + return AVERROR(EINVAL); + } return avcodec_default_get_buffer2(s, frame, flags); } diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h index eb1eaf6363..a5c85daa67 100644 --- a/fftools/ffmpeg.h +++ b/fftools/ffmpeg.h @@ -133,6 +133,8 @@ typedef struct OptionsContext { int nb_hwaccel_output_formats; SpecifierOpt *autorotate; int nb_autorotate; + SpecifierOpt *require_hwaccel; + int nb_require_hwaccel; /* output options */ StreamMap *stream_maps; @@ -365,6 +367,7 @@ typedef struct InputStream { enum AVHWDeviceType hwaccel_device_type; char *hwaccel_device; enum AVPixelFormat hwaccel_output_format; + int require_hwaccel; /* hwaccel context */ void *hwaccel_ctx; diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c index d4851a2cd8..6890bb7fcf 100644 --- a/fftools/ffmpeg_opt.c +++ b/fftools/ffmpeg_opt.c @@ -730,6 +730,7 @@ static void add_input_streams(OptionsContext *o, AVFormatContext *ic) ist->autorotate = 1; MATCH_PER_STREAM_OPT(autorotate, i, ist->autorotate, ic, st); + MATCH_PER_STREAM_OPT(require_hwaccel, i, ist->require_hwaccel, ic, st); MATCH_PER_STREAM_OPT(codec_tags, str, codec_tag, ic, st); if (codec_tag) { uint32_t tag = strtol(codec_tag, &next, 0); @@ -3600,6 +3601,9 @@ const OptionDef options[] = { { "autorotate", HAS_ARG | OPT_BOOL | OPT_SPEC | OPT_EXPERT | OPT_INPUT, { .off = OFFSET(autorotate) }, "automatically insert correct rotate filters" }, + { "require_hwaccel", HAS_ARG | OPT_BOOL | OPT_SPEC | + OPT_EXPERT | OPT_INPUT, { .off = OFFSET(require_hwaccel)}, + "forbid the fallback to default software path when hardware init fails"}, /* audio options */ { "aframes", OPT_AUDIO | HAS_ARG | OPT_PERFILE | OPT_OUTPUT, { .func_arg = opt_audio_frames },
Currently ff_get_format will go through all usable choices if the chosen format was not supported. It will fallback to software path if the hardware init fails. Provided an option "-require_hwaccel 1" in user-code to detect frame->format and hwaccel_get_buffer in get_buffer. If hardware init fails, returns an error. Signed-off-by: Linjie Fu <linjie.fu@intel.com> --- [v2] detect hardware init failures in get_buffer and modify in user-code [v3] changed the option name, add error message fftools/ffmpeg.c | 4 ++++ fftools/ffmpeg.h | 3 +++ fftools/ffmpeg_opt.c | 4 ++++ 3 files changed, 11 insertions(+)