[FFmpeg-devel,v3] fftools/ffmpeg: add an option to forbid the fallback to software path when hardware init fails

Submitted by Linjie Fu on Nov. 9, 2018, 9:05 a.m.

Details

Message ID 20181109090534.27330-1-linjie.fu@intel.com
State New
Headers show

Commit Message

Linjie Fu Nov. 9, 2018, 9:05 a.m.
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(+)

Comments

Zhong Li Nov. 9, 2018, 10:29 a.m.
> 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.
Mark Thompson Nov. 11, 2018, 3:08 p.m.
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 },
>
Linjie Fu Nov. 13, 2018, 1:30 a.m.
> -----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.

Patch hide | download patch | download mbox

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 },