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

2018-11-12 Thread Fu, Linjie
> -Original Message-
> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@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 
> > ---
> > [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.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


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

2018-11-11 Thread Mark Thompson
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 
> ---
> [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 {
>  intnb_hwaccel_output_formats;
>  SpecifierOpt *autorotate;
>  intnb_autorotate;
> +SpecifierOpt *require_hwaccel;
> +intnb_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 },
> 
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


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

2018-11-09 Thread Li, Zhong
> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf
> Of Linjie Fu
> Sent: Friday, November 9, 2018 5:06 PM
> To: ffmpeg-devel@ffmpeg.org
> Cc: Fu, Linjie 
> 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 
> ---
> [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 {
>  intnb_hwaccel_output_formats;
>  SpecifierOpt *autorotate;
>  intnb_autorotate;
> +SpecifierOpt *require_hwaccel;
> +intnb_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.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


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

2018-11-09 Thread Linjie Fu
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 
---
[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 {
 intnb_hwaccel_output_formats;
 SpecifierOpt *autorotate;
 intnb_autorotate;
+SpecifierOpt *require_hwaccel;
+intnb_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 },
-- 
2.17.1

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel