diff mbox series

[FFmpeg-devel,2/3] lavfi/vf_scale: dump the exact swscale_options to passed to libswscale

Message ID 20210807101507.83161-2-fulinjie@zju.edu.cn
State New
Headers show
Series [FFmpeg-devel,1/3] Revert "fftools/ffmpeg_filter: fix the flags parsing for scaler" | expand

Checks

Context Check Description
andriy/x86_make_warn warning New warnings during build
andriy/x86_make success Make finished
andriy/x86_make_fate success Make fate finished
andriy/PPC64_make success Make finished
andriy/PPC64_make_fate success Make fate finished

Commit Message

Linjie Fu Aug. 7, 2021, 10:15 a.m. UTC
From: Linjie Fu <linjie.justin.fu@gmail.com>

Printed verbose log doesn't match the sws_flags specified in the cmdline
for simple filter graph.

  ffmpeg .. -sws_flags bicubic ..
 [auto_scaler_0] w:iw h:ih flags:'' interl:0
 [auto_scaler_0] w:310 h:449 fmt:yuva420p sar:0/1 -> w:310 h:449 fmt:yuv420p sar:0/1 flags:0x0

Filter complex doesn't have this issue as mentioned in 12e7e1d03, the
auto-inserted scaler accepts sws_flags in filtergraph complex which
overrides the 'flags' option for vf_scale and dump it as a verbose log:

  ffmpeg ..  -filter_complex "sws_flags=bicubic;format=nv12" ..
 [auto_scaler_0] w:iw h:ih flags:'bicubic' interl:0
 [auto_scaler_0] w:310 h:449 fmt:yuva420p sar:0/1 -> w:310 h:449 fmt:nv12 sar:0/1 flags:0x2

To catch the difference, dump the exact sws_flags which is passed to
libswscale.

 [auto_scaler_0] swscale_options:'sws_flags=bicubic'

Signed-off-by: Linjie Fu <linjie.justin.fu@gmail.com>
---
 libavfilter/vf_scale.c | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

Andreas Rheinhardt Aug. 7, 2021, 1:12 p.m. UTC | #1
Linjie Fu:
> From: Linjie Fu <linjie.justin.fu@gmail.com>
> 
> Printed verbose log doesn't match the sws_flags specified in the cmdline
> for simple filter graph.
> 
>   ffmpeg .. -sws_flags bicubic ..
>  [auto_scaler_0] w:iw h:ih flags:'' interl:0
>  [auto_scaler_0] w:310 h:449 fmt:yuva420p sar:0/1 -> w:310 h:449 fmt:yuv420p sar:0/1 flags:0x0
> 
> Filter complex doesn't have this issue as mentioned in 12e7e1d03, the
> auto-inserted scaler accepts sws_flags in filtergraph complex which
> overrides the 'flags' option for vf_scale and dump it as a verbose log:
> 
>   ffmpeg ..  -filter_complex "sws_flags=bicubic;format=nv12" ..
>  [auto_scaler_0] w:iw h:ih flags:'bicubic' interl:0
>  [auto_scaler_0] w:310 h:449 fmt:yuva420p sar:0/1 -> w:310 h:449 fmt:nv12 sar:0/1 flags:0x2
> 
> To catch the difference, dump the exact sws_flags which is passed to
> libswscale.
> 
>  [auto_scaler_0] swscale_options:'sws_flags=bicubic'
> 
> Signed-off-by: Linjie Fu <linjie.justin.fu@gmail.com>
> ---
>  libavfilter/vf_scale.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/libavfilter/vf_scale.c b/libavfilter/vf_scale.c
> index aa855b894a..f994217bdc 100644
> --- a/libavfilter/vf_scale.c
> +++ b/libavfilter/vf_scale.c
> @@ -552,11 +552,17 @@ static int config_props(AVFilterLink *outlink)
>                                 scale->out_range == AVCOL_RANGE_JPEG, 0);
>  
>              if (scale->opts) {
> +                char args[512];
> +                args[0] = 0;

This will lead to a declaration-after-statement warning.

>                  AVDictionaryEntry *e = NULL;
>                  while ((e = av_dict_get(scale->opts, "", e, AV_DICT_IGNORE_SUFFIX))) {
>                      if ((ret = av_opt_set(*s, e->key, e->value, 0)) < 0)
>                          return ret;
> +                    av_strlcatf(args, sizeof(args), "%s=%s:", e->key, e->value);
>                  }
> +                if (strlen(args))

I doubt strlen(args) == 0 can happen; anyway, checking for whether a
string is empty can be done easier: "if (args[0] != '\0')".

> +                    args[strlen(args)-1] = 0;
> +                av_log(ctx, AV_LOG_VERBOSE, "swscale_options:'%s'\n", (char *)av_x_if_null(args, ""));

The av_x_if_null() makes no sense at all, as args is never NULL: It is a
stack array.

>              }
>              /* Override YUV420P default settings to have the correct (MPEG-2) chroma positions
>               * MPEG-2 chroma positions are used by convention
>
Linjie Fu Aug. 8, 2021, 2:16 p.m. UTC | #2
On Sat, Aug 7, 2021 at 9:12 PM Andreas Rheinhardt <
andreas.rheinhardt@outlook.com> wrote:

> Linjie Fu:
> > From: Linjie Fu <linjie.justin.fu@gmail.com>
> >
> > Printed verbose log doesn't match the sws_flags specified in the cmdline
> > for simple filter graph.
> >
> >   ffmpeg .. -sws_flags bicubic ..
> >  [auto_scaler_0] w:iw h:ih flags:'' interl:0
> >  [auto_scaler_0] w:310 h:449 fmt:yuva420p sar:0/1 -> w:310 h:449
> fmt:yuv420p sar:0/1 flags:0x0
> >
> > Filter complex doesn't have this issue as mentioned in 12e7e1d03, the
> > auto-inserted scaler accepts sws_flags in filtergraph complex which
> > overrides the 'flags' option for vf_scale and dump it as a verbose log:
> >
> >   ffmpeg ..  -filter_complex "sws_flags=bicubic;format=nv12" ..
> >  [auto_scaler_0] w:iw h:ih flags:'bicubic' interl:0
> >  [auto_scaler_0] w:310 h:449 fmt:yuva420p sar:0/1 -> w:310 h:449
> fmt:nv12 sar:0/1 flags:0x2
> >
> > To catch the difference, dump the exact sws_flags which is passed to
> > libswscale.
> >
> >  [auto_scaler_0] swscale_options:'sws_flags=bicubic'
> >
> > Signed-off-by: Linjie Fu <linjie.justin.fu@gmail.com>
> > ---
> >  libavfilter/vf_scale.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/libavfilter/vf_scale.c b/libavfilter/vf_scale.c
> > index aa855b894a..f994217bdc 100644
> > --- a/libavfilter/vf_scale.c
> > +++ b/libavfilter/vf_scale.c
> > @@ -552,11 +552,17 @@ static int config_props(AVFilterLink *outlink)
> >                                 scale->out_range == AVCOL_RANGE_JPEG, 0);
> >
> >              if (scale->opts) {
> > +                char args[512];
> > +                args[0] = 0;
>
> This will lead to a declaration-after-statement warning.
>
> >                  AVDictionaryEntry *e = NULL;
> >                  while ((e = av_dict_get(scale->opts, "", e,
> AV_DICT_IGNORE_SUFFIX))) {
> >                      if ((ret = av_opt_set(*s, e->key, e->value, 0)) < 0)
> >                          return ret;
> > +                    av_strlcatf(args, sizeof(args), "%s=%s:", e->key,
> e->value);
> >                  }
> > +                if (strlen(args))
>
> I doubt strlen(args) == 0 can happen; anyway, checking for whether a
> string is empty can be done easier: "if (args[0] != '\0')".
>
> > +                    args[strlen(args)-1] = 0;
> > +                av_log(ctx, AV_LOG_VERBOSE, "swscale_options:'%s'\n",
> (char *)av_x_if_null(args, ""));
>
> The av_x_if_null() makes no sense at all, as args is never NULL: It is a
> stack array.


Modified as suggested, thx.

- linjie
diff mbox series

Patch

diff --git a/libavfilter/vf_scale.c b/libavfilter/vf_scale.c
index aa855b894a..f994217bdc 100644
--- a/libavfilter/vf_scale.c
+++ b/libavfilter/vf_scale.c
@@ -552,11 +552,17 @@  static int config_props(AVFilterLink *outlink)
                                scale->out_range == AVCOL_RANGE_JPEG, 0);
 
             if (scale->opts) {
+                char args[512];
+                args[0] = 0;
                 AVDictionaryEntry *e = NULL;
                 while ((e = av_dict_get(scale->opts, "", e, AV_DICT_IGNORE_SUFFIX))) {
                     if ((ret = av_opt_set(*s, e->key, e->value, 0)) < 0)
                         return ret;
+                    av_strlcatf(args, sizeof(args), "%s=%s:", e->key, e->value);
                 }
+                if (strlen(args))
+                    args[strlen(args)-1] = 0;
+                av_log(ctx, AV_LOG_VERBOSE, "swscale_options:'%s'\n", (char *)av_x_if_null(args, ""));
             }
             /* Override YUV420P default settings to have the correct (MPEG-2) chroma positions
              * MPEG-2 chroma positions are used by convention