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