diff mbox series

[FFmpeg-devel] fftools/ffmpeg_filter: add -autoscale to disable/enable the default scale

Message ID 1581783177-10335-1-git-send-email-linjie.fu@intel.com
State New
Headers show
Series [FFmpeg-devel] fftools/ffmpeg_filter: add -autoscale to disable/enable the default scale | expand

Checks

Context Check Description
andriy/ffmpeg-patchwork success Make fate finished

Commit Message

Fu, Linjie Feb. 15, 2020, 4:12 p.m. UTC 
Currently, ffmpeg inserts scale filter by default in the filter graph
to force the whole decoded stream to scale into the same size with the
first frame. It's not quite make sense in resolution changing cases if
user wants the rawvideo without any scale.

Using autoscale/noautoscale as an output option to indicate whether auto
inserting the scale filter in the filter graph:
    -noautoscale or -autoscale 0:
    disable the default auto scale filter inserting.

ffmpeg -y input.mp4 out1.yuv -noautoscale out2.yuv -autoscale 0 out3.yuv

Update docs.

Signed-off-by: U. Artie Eoff <ullysses.a.eoff@intel.com>
Signed-off-by: Linjie Fu <linjie.fu@intel.com>
---
 doc/ffmpeg.texi         | 16 ++++++++++++----
 fftools/ffmpeg.h        |  3 +++
 fftools/ffmpeg_filter.c |  2 +-
 fftools/ffmpeg_opt.c    |  5 +++++
 4 files changed, 21 insertions(+), 5 deletions(-)

Comments

Fu, Linjie Feb. 17, 2020, 3:43 p.m. UTC | #1 
> -----Original Message-----
> From: Fu, Linjie <linjie.fu@intel.com>
> Sent: Sunday, February 16, 2020 00:13
> To: ffmpeg-devel@ffmpeg.org
> Cc: Fu, Linjie <linjie.fu@intel.com>; Eoff, Ullysses A
> <ullysses.a.eoff@intel.com>
> Subject: [PATCH] fftools/ffmpeg_filter: add -autoscale to disable/enable the
> default scale
> 
> Currently, ffmpeg inserts scale filter by default in the filter graph
> to force the whole decoded stream to scale into the same size with the
> first frame. It's not quite make sense in resolution changing cases if
> user wants the rawvideo without any scale.
> 
> Using autoscale/noautoscale as an output option to indicate whether auto
> inserting the scale filter in the filter graph:
>     -noautoscale or -autoscale 0:
>     disable the default auto scale filter inserting.
> 
> ffmpeg -y input.mp4 out1.yuv -noautoscale out2.yuv -autoscale 0 out3.yuv
> 
> Update docs.
> 
> Signed-off-by: U. Artie Eoff <ullysses.a.eoff@intel.com>
> Signed-off-by: Linjie Fu <linjie.fu@intel.com>
> ---
>  doc/ffmpeg.texi         | 16 ++++++++++++----
>  fftools/ffmpeg.h        |  3 +++
>  fftools/ffmpeg_filter.c |  2 +-
>  fftools/ffmpeg_opt.c    |  5 +++++
>  4 files changed, 21 insertions(+), 5 deletions(-)
> 
> diff --git a/doc/ffmpeg.texi b/doc/ffmpeg.texi
> index 29753f0..aebafb3 100644
> --- a/doc/ffmpeg.texi
> +++ b/doc/ffmpeg.texi
> @@ -734,10 +734,6 @@ ffmpeg -dump_attachment:t "" -i INPUT
>  Technical note -- attachments are implemented as codec extradata, so this
>  option can actually be used to extract extradata from any stream, not just
>  attachments.
> -
> -@item -noautorotate
> -Disable automatically rotating video based on file metadata.
> -
>  @end table
> 
>  @section Video Options
> @@ -819,6 +815,18 @@ Create the filtergraph specified by @var{filtergraph}
> and use it to
>  filter the stream.
> 
>  This is an alias for @code{-filter:v}, see the @ref{filter_option,,-filter option}.
> +
> +@item -autorotate
> +Automatically rotate the video according to file metadata. Enabled by
> +default, use @option{-noautorotate} to disable it.
> +
> +@item -autoscale
> +Automatically scale the video according to the resolution of first frame.
> +Enabled by default, use @option{-noautoscale} to disable it. When
> autoscale is
> +disabled, all output frames of filter graph might not be in the same
> resolution
> +and may be inadequate for some encoder/muxer. Therefore, it is not
> recommended
> +to disable it unless you really know what you are doing.
> +Disable autoscale at your own risk.
>  @end table
> 
>  @section Advanced Video options
> diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
> index 7b6f802..8beba6c 100644
> --- a/fftools/ffmpeg.h
> +++ b/fftools/ffmpeg.h
> @@ -230,6 +230,8 @@ typedef struct OptionsContext {
>      int        nb_time_bases;
>      SpecifierOpt *enc_time_bases;
>      int        nb_enc_time_bases;
> +    SpecifierOpt *autoscale;
> +    int        nb_autoscale;
>  } OptionsContext;
> 
>  typedef struct InputFilter {
> @@ -479,6 +481,7 @@ typedef struct OutputStream {
>      int force_fps;
>      int top_field_first;
>      int rotate_overridden;
> +    int autoscale;
>      double rotate_override_value;
> 
>      AVRational frame_aspect_ratio;
> diff --git a/fftools/ffmpeg_filter.c b/fftools/ffmpeg_filter.c
> index 40cc4c1..46c8ea8 100644
> --- a/fftools/ffmpeg_filter.c
> +++ b/fftools/ffmpeg_filter.c
> @@ -469,7 +469,7 @@ static int configure_output_video_filter(FilterGraph
> *fg, OutputFilter *ofilter,
>      if (ret < 0)
>          return ret;
> 
> -    if (ofilter->width || ofilter->height) {
> +    if ((ofilter->width || ofilter->height) && ofilter->ost->autoscale) {
>          char args[255];
>          AVFilterContext *filter;
>          AVDictionaryEntry *e = NULL;
> diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
> index 12d4488..a6f4216 100644
> --- a/fftools/ffmpeg_opt.c
> +++ b/fftools/ffmpeg_opt.c
> @@ -1405,6 +1405,8 @@ static OutputStream
> *new_output_stream(OptionsContext *o, AVFormatContext *oc, e
>          ost->encoder_opts  = filter_codec_opts(o->g->codec_opts, ost->enc-
> >id, oc, st, ost->enc);
> 
>          MATCH_PER_STREAM_OPT(presets, str, preset, oc, st);
> +        ost->autoscale = 1;
> +        MATCH_PER_STREAM_OPT(autoscale, i, ost->autoscale, oc, st);
>          if (preset && (!(ret = get_preset_file_2(preset, ost->enc->name, &s))))
> {
>              do  {
>                  buf = get_line(s);
> @@ -3650,6 +3652,9 @@ const OptionDef options[] = {
>      { "autorotate",       HAS_ARG | OPT_BOOL | OPT_SPEC |
>                            OPT_EXPERT | OPT_INPUT,                                { .off =
> OFFSET(autorotate) },
>          "automatically insert correct rotate filters" },
> +    { "autoscale",        HAS_ARG | OPT_BOOL | OPT_SPEC |
> +                          OPT_EXPERT | OPT_OUTPUT,                               { .off =
> OFFSET(autoscale) },
> +        "automatically insert a scale filter at the end of the filter graph" },
> 
>      /* audio options */
>      { "aframes",        OPT_AUDIO | HAS_ARG  | OPT_PERFILE | OPT_OUTPUT,
> { .func_arg = opt_audio_frames },
> --
> 2.7.4

Hi,

After several reviews, ping for this patch.
Nicolas George Feb. 17, 2020, 4:44 p.m. UTC | #2 
Fu, Linjie (12020-02-17):
> After several reviews, ping for this patch.

I only had concerns with the length of the help, they were addressed. I
have no more remarks.

Regards,
Fu, Linjie Feb. 17, 2020, 5:27 p.m. UTC | #3 
> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> Gyan Doshi
> Sent: Tuesday, February 18, 2020 00:53
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH] fftools/ffmpeg_filter: add -autoscale
> to disable/enable the default scale
> 
> 
> 
> On 17-02-2020 09:13 pm, Fu, Linjie wrote:
> >> -----Original Message-----
> >> From: Fu, Linjie <linjie.fu@intel.com>
> >> Sent: Sunday, February 16, 2020 00:13
> >> To: ffmpeg-devel@ffmpeg.org
> >> Cc: Fu, Linjie <linjie.fu@intel.com>; Eoff, Ullysses A
> >> <ullysses.a.eoff@intel.com>
> >> Subject: [PATCH] fftools/ffmpeg_filter: add -autoscale to disable/enable
> the
> >> default scale
> >>
> >> Currently, ffmpeg inserts scale filter by default in the filter graph
> >> to force the whole decoded stream to scale into the same size with the
> >> first frame. It's not quite make sense in resolution changing cases if
> >> user wants the rawvideo without any scale.
> >>
> >> Using autoscale/noautoscale as an output option to indicate whether auto
> >> inserting the scale filter in the filter graph:
> >>      -noautoscale or -autoscale 0:
> >>      disable the default auto scale filter inserting.
> >>
> >> ffmpeg -y input.mp4 out1.yuv -noautoscale out2.yuv -autoscale 0
> out3.yuv
> >>
> >> Update docs.
> >>
> >> Signed-off-by: U. Artie Eoff <ullysses.a.eoff@intel.com>
> >> Signed-off-by: Linjie Fu <linjie.fu@intel.com>
> >> ---
> >>   doc/ffmpeg.texi         | 16 ++++++++++++----
> >>   fftools/ffmpeg.h        |  3 +++
> >>   fftools/ffmpeg_filter.c |  2 +-
> >>   fftools/ffmpeg_opt.c    |  5 +++++
> >>   4 files changed, 21 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/doc/ffmpeg.texi b/doc/ffmpeg.texi
> >> index 29753f0..aebafb3 100644
> >> --- a/doc/ffmpeg.texi
> >> +++ b/doc/ffmpeg.texi
> >> @@ -734,10 +734,6 @@ ffmpeg -dump_attachment:t "" -i INPUT
> >>   Technical note -- attachments are implemented as codec extradata, so
> this
> >>   option can actually be used to extract extradata from any stream, not
> just
> >>   attachments.
> >> -
> >> -@item -noautorotate
> >> -Disable automatically rotating video based on file metadata.
> >> -
> >>   @end table
> >>
> >>   @section Video Options
> >> @@ -819,6 +815,18 @@ Create the filtergraph specified by
> @var{filtergraph}
> >> and use it to
> >>   filter the stream.
> >>
> >>   This is an alias for @code{-filter:v}, see the @ref{filter_option,,-filter
> option}.
> >> +
> >> +@item -autorotate
> >> +Automatically rotate the video according to file metadata. Enabled by
> >> +default, use @option{-noautorotate} to disable it.
> >> +
> >> +@item -autoscale
> >> +Automatically scale the video according to the resolution of first frame.
> >> +Enabled by default, use @option{-noautoscale} to disable it. When
> >> autoscale is
> >> +disabled, all output frames of filter graph might not be in the same
> >> resolution
> >> +and may be inadequate for some encoder/muxer. Therefore, it is not
> >> recommended
> >> +to disable it unless you really know what you are doing.
> >> +Disable autoscale at your own risk.
> >>   @end table
> >>
> >>   @section Advanced Video options
> >> diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
> >> index 7b6f802..8beba6c 100644
> >> --- a/fftools/ffmpeg.h
> >> +++ b/fftools/ffmpeg.h
> >> @@ -230,6 +230,8 @@ typedef struct OptionsContext {
> >>       int        nb_time_bases;
> >>       SpecifierOpt *enc_time_bases;
> >>       int        nb_enc_time_bases;
> >> +    SpecifierOpt *autoscale;
> >> +    int        nb_autoscale;
> >>   } OptionsContext;
> >>
> >>   typedef struct InputFilter {
> >> @@ -479,6 +481,7 @@ typedef struct OutputStream {
> >>       int force_fps;
> >>       int top_field_first;
> >>       int rotate_overridden;
> >> +    int autoscale;
> >>       double rotate_override_value;
> >>
> >>       AVRational frame_aspect_ratio;
> >> diff --git a/fftools/ffmpeg_filter.c b/fftools/ffmpeg_filter.c
> >> index 40cc4c1..46c8ea8 100644
> >> --- a/fftools/ffmpeg_filter.c
> >> +++ b/fftools/ffmpeg_filter.c
> >> @@ -469,7 +469,7 @@ static int
> configure_output_video_filter(FilterGraph
> >> *fg, OutputFilter *ofilter,
> >>       if (ret < 0)
> >>           return ret;
> >>
> >> -    if (ofilter->width || ofilter->height) {
> >> +    if ((ofilter->width || ofilter->height) && ofilter->ost->autoscale) {
> >>           char args[255];
> >>           AVFilterContext *filter;
> >>           AVDictionaryEntry *e = NULL;
> >> diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
> >> index 12d4488..a6f4216 100644
> >> --- a/fftools/ffmpeg_opt.c
> >> +++ b/fftools/ffmpeg_opt.c
> >> @@ -1405,6 +1405,8 @@ static OutputStream
> >> *new_output_stream(OptionsContext *o, AVFormatContext *oc, e
> >>           ost->encoder_opts  = filter_codec_opts(o->g->codec_opts, ost-
> >enc-
> >>> id, oc, st, ost->enc);
> >>           MATCH_PER_STREAM_OPT(presets, str, preset, oc, st);
> >> +        ost->autoscale = 1;
> >> +        MATCH_PER_STREAM_OPT(autoscale, i, ost->autoscale, oc, st);
> >>           if (preset && (!(ret = get_preset_file_2(preset, ost->enc->name,
> &s))))
> >> {
> >>               do  {
> >>                   buf = get_line(s);
> >> @@ -3650,6 +3652,9 @@ const OptionDef options[] = {
> >>       { "autorotate",       HAS_ARG | OPT_BOOL | OPT_SPEC |
> >>                             OPT_EXPERT | OPT_INPUT,                                { .off =
> >> OFFSET(autorotate) },
> >>           "automatically insert correct rotate filters" },
> >> +    { "autoscale",        HAS_ARG | OPT_BOOL | OPT_SPEC |
> >> +                          OPT_EXPERT | OPT_OUTPUT,                               { .off =
> >> OFFSET(autoscale) },
> >> +        "automatically insert a scale filter at the end of the filter graph" },
> >>
> >>       /* audio options */
> >>       { "aframes",        OPT_AUDIO | HAS_ARG  | OPT_PERFILE |
> OPT_OUTPUT,
> >> { .func_arg = opt_audio_frames },
> >> --
> >> 2.7.4
> 
> What happens if user disables autoscale and the frame sizes do vary but
> the encoder doesn't support changing props?

That's the point. Currently they would fail without specific notifications for this usage.

One of the solutions is to declare AV_CODEC_CAP_VARIABLE_DIMENSIONS  capability for encoders [1],
and prompt an error information [2] to indicate " Dynamic resolution encode is not supported by this encoder."

The further work would be providing dynamic encoding support for some encoders [3].

So this patch is the first step.

[1] https://patchwork.ffmpeg.org/project/ffmpeg/patch/1564461924-4772-1-git-send-email-linjie.fu@intel.com/
[2] https://patchwork.ffmpeg.org/project/ffmpeg/patch/1564461944-4820-1-git-send-email-linjie.fu@intel.com/
[3] https://patchwork.ffmpeg.org/project/ffmpeg/patch/1565688544-9043-1-git-send-email-linjie.fu@intel.com/




> Gyan
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Gyan Doshi Feb. 17, 2020, 5:32 p.m. UTC | #4 
On 17-02-2020 10:57 pm, Fu, Linjie wrote:
>> -----Original Message-----
>> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
>> Gyan Doshi
>> Sent: Tuesday, February 18, 2020 00:53
>> To: ffmpeg-devel@ffmpeg.org
>> Subject: Re: [FFmpeg-devel] [PATCH] fftools/ffmpeg_filter: add -autoscale
>> to disable/enable the default scale
>>
>>
>>
>> On 17-02-2020 09:13 pm, Fu, Linjie wrote:
>>>> -----Original Message-----
>>>> From: Fu, Linjie <linjie.fu@intel.com>
>>>> Sent: Sunday, February 16, 2020 00:13
>>>> To: ffmpeg-devel@ffmpeg.org
>>>> Cc: Fu, Linjie <linjie.fu@intel.com>; Eoff, Ullysses A
>>>> <ullysses.a.eoff@intel.com>
>>>> Subject: [PATCH] fftools/ffmpeg_filter: add -autoscale to disable/enable
>> the
>>>> default scale
>>>>
>>>> Currently, ffmpeg inserts scale filter by default in the filter graph
>>>> to force the whole decoded stream to scale into the same size with the
>>>> first frame. It's not quite make sense in resolution changing cases if
>>>> user wants the rawvideo without any scale.
>>>>
>>>> Using autoscale/noautoscale as an output option to indicate whether auto
>>>> inserting the scale filter in the filter graph:
>>>>       -noautoscale or -autoscale 0:
>>>>       disable the default auto scale filter inserting.
>>>>
>>>> ffmpeg -y input.mp4 out1.yuv -noautoscale out2.yuv -autoscale 0
>> out3.yuv
>>>> Update docs.
>>>>
>>>> Signed-off-by: U. Artie Eoff <ullysses.a.eoff@intel.com>
>>>> Signed-off-by: Linjie Fu <linjie.fu@intel.com>
>>>> ---
>>>>    doc/ffmpeg.texi         | 16 ++++++++++++----
>>>>    fftools/ffmpeg.h        |  3 +++
>>>>    fftools/ffmpeg_filter.c |  2 +-
>>>>    fftools/ffmpeg_opt.c    |  5 +++++
>>>>    4 files changed, 21 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/doc/ffmpeg.texi b/doc/ffmpeg.texi
>>>> index 29753f0..aebafb3 100644
>>>> --- a/doc/ffmpeg.texi
>>>> +++ b/doc/ffmpeg.texi
>>>> @@ -734,10 +734,6 @@ ffmpeg -dump_attachment:t "" -i INPUT
>>>>    Technical note -- attachments are implemented as codec extradata, so
>> this
>>>>    option can actually be used to extract extradata from any stream, not
>> just
>>>>    attachments.
>>>> -
>>>> -@item -noautorotate
>>>> -Disable automatically rotating video based on file metadata.
>>>> -
>>>>    @end table
>>>>
>>>>    @section Video Options
>>>> @@ -819,6 +815,18 @@ Create the filtergraph specified by
>> @var{filtergraph}
>>>> and use it to
>>>>    filter the stream.
>>>>
>>>>    This is an alias for @code{-filter:v}, see the @ref{filter_option,,-filter
>> option}.
>>>> +
>>>> +@item -autorotate
>>>> +Automatically rotate the video according to file metadata. Enabled by
>>>> +default, use @option{-noautorotate} to disable it.
>>>> +
>>>> +@item -autoscale
>>>> +Automatically scale the video according to the resolution of first frame.
>>>> +Enabled by default, use @option{-noautoscale} to disable it. When
>>>> autoscale is
>>>> +disabled, all output frames of filter graph might not be in the same
>>>> resolution
>>>> +and may be inadequate for some encoder/muxer. Therefore, it is not
>>>> recommended
>>>> +to disable it unless you really know what you are doing.
>>>> +Disable autoscale at your own risk.
>>>>    @end table
>>>>
>>>>    @section Advanced Video options
>>>> diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
>>>> index 7b6f802..8beba6c 100644
>>>> --- a/fftools/ffmpeg.h
>>>> +++ b/fftools/ffmpeg.h
>>>> @@ -230,6 +230,8 @@ typedef struct OptionsContext {
>>>>        int        nb_time_bases;
>>>>        SpecifierOpt *enc_time_bases;
>>>>        int        nb_enc_time_bases;
>>>> +    SpecifierOpt *autoscale;
>>>> +    int        nb_autoscale;
>>>>    } OptionsContext;
>>>>
>>>>    typedef struct InputFilter {
>>>> @@ -479,6 +481,7 @@ typedef struct OutputStream {
>>>>        int force_fps;
>>>>        int top_field_first;
>>>>        int rotate_overridden;
>>>> +    int autoscale;
>>>>        double rotate_override_value;
>>>>
>>>>        AVRational frame_aspect_ratio;
>>>> diff --git a/fftools/ffmpeg_filter.c b/fftools/ffmpeg_filter.c
>>>> index 40cc4c1..46c8ea8 100644
>>>> --- a/fftools/ffmpeg_filter.c
>>>> +++ b/fftools/ffmpeg_filter.c
>>>> @@ -469,7 +469,7 @@ static int
>> configure_output_video_filter(FilterGraph
>>>> *fg, OutputFilter *ofilter,
>>>>        if (ret < 0)
>>>>            return ret;
>>>>
>>>> -    if (ofilter->width || ofilter->height) {
>>>> +    if ((ofilter->width || ofilter->height) && ofilter->ost->autoscale) {
>>>>            char args[255];
>>>>            AVFilterContext *filter;
>>>>            AVDictionaryEntry *e = NULL;
>>>> diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
>>>> index 12d4488..a6f4216 100644
>>>> --- a/fftools/ffmpeg_opt.c
>>>> +++ b/fftools/ffmpeg_opt.c
>>>> @@ -1405,6 +1405,8 @@ static OutputStream
>>>> *new_output_stream(OptionsContext *o, AVFormatContext *oc, e
>>>>            ost->encoder_opts  = filter_codec_opts(o->g->codec_opts, ost-
>>> enc-
>>>>> id, oc, st, ost->enc);
>>>>            MATCH_PER_STREAM_OPT(presets, str, preset, oc, st);
>>>> +        ost->autoscale = 1;
>>>> +        MATCH_PER_STREAM_OPT(autoscale, i, ost->autoscale, oc, st);
>>>>            if (preset && (!(ret = get_preset_file_2(preset, ost->enc->name,
>> &s))))
>>>> {
>>>>                do  {
>>>>                    buf = get_line(s);
>>>> @@ -3650,6 +3652,9 @@ const OptionDef options[] = {
>>>>        { "autorotate",       HAS_ARG | OPT_BOOL | OPT_SPEC |
>>>>                              OPT_EXPERT | OPT_INPUT,                                { .off =
>>>> OFFSET(autorotate) },
>>>>            "automatically insert correct rotate filters" },
>>>> +    { "autoscale",        HAS_ARG | OPT_BOOL | OPT_SPEC |
>>>> +                          OPT_EXPERT | OPT_OUTPUT,                               { .off =
>>>> OFFSET(autoscale) },
>>>> +        "automatically insert a scale filter at the end of the filter graph" },
>>>>
>>>>        /* audio options */
>>>>        { "aframes",        OPT_AUDIO | HAS_ARG  | OPT_PERFILE |
>> OPT_OUTPUT,
>>>> { .func_arg = opt_audio_frames },
>>>> --
>>>> 2.7.4
>> What happens if user disables autoscale and the frame sizes do vary but
>> the encoder doesn't support changing props?
> That's the point. Currently they would fail without specific notifications for this usage.
>
> One of the solutions is to declare AV_CODEC_CAP_VARIABLE_DIMENSIONS  capability for encoders [1],
> and prompt an error information [2] to indicate " Dynamic resolution encode is not supported by this encoder."
>
> The further work would be providing dynamic encoding support for some encoders [3].
>
> So this patch is the first step.

Is this patch intended to be applied first, before the codec flag is 
added and checked?

Gyan
Fu, Linjie Feb. 17, 2020, 5:37 p.m. UTC | #5 
> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> Gyan Doshi
> Sent: Tuesday, February 18, 2020 01:33
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH] fftools/ffmpeg_filter: add -autoscale
> to disable/enable the default scale
> 
> 
> 
> On 17-02-2020 10:57 pm, Fu, Linjie wrote:
> >> -----Original Message-----
> >> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> >> Gyan Doshi
> >> Sent: Tuesday, February 18, 2020 00:53
> >> To: ffmpeg-devel@ffmpeg.org
> >> Subject: Re: [FFmpeg-devel] [PATCH] fftools/ffmpeg_filter: add -
> autoscale
> >> to disable/enable the default scale
> >>
> >>
> >>
> >> On 17-02-2020 09:13 pm, Fu, Linjie wrote:
> >>>> -----Original Message-----
> >>>> From: Fu, Linjie <linjie.fu@intel.com>
> >>>> Sent: Sunday, February 16, 2020 00:13
> >>>> To: ffmpeg-devel@ffmpeg.org
> >>>> Cc: Fu, Linjie <linjie.fu@intel.com>; Eoff, Ullysses A
> >>>> <ullysses.a.eoff@intel.com>
> >>>> Subject: [PATCH] fftools/ffmpeg_filter: add -autoscale to
> disable/enable
> >> the
> >>>> default scale
> >>>>
> >>>> Currently, ffmpeg inserts scale filter by default in the filter graph
> >>>> to force the whole decoded stream to scale into the same size with the
> >>>> first frame. It's not quite make sense in resolution changing cases if
> >>>> user wants the rawvideo without any scale.
> >>>>
> >>>> Using autoscale/noautoscale as an output option to indicate whether
> auto
> >>>> inserting the scale filter in the filter graph:
> >>>>       -noautoscale or -autoscale 0:
> >>>>       disable the default auto scale filter inserting.
> >>>>
> >>>> ffmpeg -y input.mp4 out1.yuv -noautoscale out2.yuv -autoscale 0
> >> out3.yuv
> >>>> Update docs.
> >>>>
> >>>> Signed-off-by: U. Artie Eoff <ullysses.a.eoff@intel.com>
> >>>> Signed-off-by: Linjie Fu <linjie.fu@intel.com>
> >>>> ---
> >>>>    doc/ffmpeg.texi         | 16 ++++++++++++----
> >>>>    fftools/ffmpeg.h        |  3 +++
> >>>>    fftools/ffmpeg_filter.c |  2 +-
> >>>>    fftools/ffmpeg_opt.c    |  5 +++++
> >>>>    4 files changed, 21 insertions(+), 5 deletions(-)
> >>>>
> >>>> diff --git a/doc/ffmpeg.texi b/doc/ffmpeg.texi
> >>>> index 29753f0..aebafb3 100644
> >>>> --- a/doc/ffmpeg.texi
> >>>> +++ b/doc/ffmpeg.texi
> >>>> @@ -734,10 +734,6 @@ ffmpeg -dump_attachment:t "" -i INPUT
> >>>>    Technical note -- attachments are implemented as codec extradata,
> so
> >> this
> >>>>    option can actually be used to extract extradata from any stream, not
> >> just
> >>>>    attachments.
> >>>> -
> >>>> -@item -noautorotate
> >>>> -Disable automatically rotating video based on file metadata.
> >>>> -
> >>>>    @end table
> >>>>
> >>>>    @section Video Options
> >>>> @@ -819,6 +815,18 @@ Create the filtergraph specified by
> >> @var{filtergraph}
> >>>> and use it to
> >>>>    filter the stream.
> >>>>
> >>>>    This is an alias for @code{-filter:v}, see the @ref{filter_option,,-filter
> >> option}.
> >>>> +
> >>>> +@item -autorotate
> >>>> +Automatically rotate the video according to file metadata. Enabled by
> >>>> +default, use @option{-noautorotate} to disable it.
> >>>> +
> >>>> +@item -autoscale
> >>>> +Automatically scale the video according to the resolution of first frame.
> >>>> +Enabled by default, use @option{-noautoscale} to disable it. When
> >>>> autoscale is
> >>>> +disabled, all output frames of filter graph might not be in the same
> >>>> resolution
> >>>> +and may be inadequate for some encoder/muxer. Therefore, it is not
> >>>> recommended
> >>>> +to disable it unless you really know what you are doing.
> >>>> +Disable autoscale at your own risk.
> >>>>    @end table
> >>>>
> >>>>    @section Advanced Video options
> >>>> diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
> >>>> index 7b6f802..8beba6c 100644
> >>>> --- a/fftools/ffmpeg.h
> >>>> +++ b/fftools/ffmpeg.h
> >>>> @@ -230,6 +230,8 @@ typedef struct OptionsContext {
> >>>>        int        nb_time_bases;
> >>>>        SpecifierOpt *enc_time_bases;
> >>>>        int        nb_enc_time_bases;
> >>>> +    SpecifierOpt *autoscale;
> >>>> +    int        nb_autoscale;
> >>>>    } OptionsContext;
> >>>>
> >>>>    typedef struct InputFilter {
> >>>> @@ -479,6 +481,7 @@ typedef struct OutputStream {
> >>>>        int force_fps;
> >>>>        int top_field_first;
> >>>>        int rotate_overridden;
> >>>> +    int autoscale;
> >>>>        double rotate_override_value;
> >>>>
> >>>>        AVRational frame_aspect_ratio;
> >>>> diff --git a/fftools/ffmpeg_filter.c b/fftools/ffmpeg_filter.c
> >>>> index 40cc4c1..46c8ea8 100644
> >>>> --- a/fftools/ffmpeg_filter.c
> >>>> +++ b/fftools/ffmpeg_filter.c
> >>>> @@ -469,7 +469,7 @@ static int
> >> configure_output_video_filter(FilterGraph
> >>>> *fg, OutputFilter *ofilter,
> >>>>        if (ret < 0)
> >>>>            return ret;
> >>>>
> >>>> -    if (ofilter->width || ofilter->height) {
> >>>> +    if ((ofilter->width || ofilter->height) && ofilter->ost->autoscale) {
> >>>>            char args[255];
> >>>>            AVFilterContext *filter;
> >>>>            AVDictionaryEntry *e = NULL;
> >>>> diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
> >>>> index 12d4488..a6f4216 100644
> >>>> --- a/fftools/ffmpeg_opt.c
> >>>> +++ b/fftools/ffmpeg_opt.c
> >>>> @@ -1405,6 +1405,8 @@ static OutputStream
> >>>> *new_output_stream(OptionsContext *o, AVFormatContext *oc, e
> >>>>            ost->encoder_opts  = filter_codec_opts(o->g->codec_opts, ost-
> >>> enc-
> >>>>> id, oc, st, ost->enc);
> >>>>            MATCH_PER_STREAM_OPT(presets, str, preset, oc, st);
> >>>> +        ost->autoscale = 1;
> >>>> +        MATCH_PER_STREAM_OPT(autoscale, i, ost->autoscale, oc, st);
> >>>>            if (preset && (!(ret = get_preset_file_2(preset, ost->enc->name,
> >> &s))))
> >>>> {
> >>>>                do  {
> >>>>                    buf = get_line(s);
> >>>> @@ -3650,6 +3652,9 @@ const OptionDef options[] = {
> >>>>        { "autorotate",       HAS_ARG | OPT_BOOL | OPT_SPEC |
> >>>>                              OPT_EXPERT | OPT_INPUT,                                { .off =
> >>>> OFFSET(autorotate) },
> >>>>            "automatically insert correct rotate filters" },
> >>>> +    { "autoscale",        HAS_ARG | OPT_BOOL | OPT_SPEC |
> >>>> +                          OPT_EXPERT | OPT_OUTPUT,                               { .off =
> >>>> OFFSET(autoscale) },
> >>>> +        "automatically insert a scale filter at the end of the filter graph" },
> >>>>
> >>>>        /* audio options */
> >>>>        { "aframes",        OPT_AUDIO | HAS_ARG  | OPT_PERFILE |
> >> OPT_OUTPUT,
> >>>> { .func_arg = opt_audio_frames },
> >>>> --
> >>>> 2.7.4
> >> What happens if user disables autoscale and the frame sizes do vary but
> >> the encoder doesn't support changing props?
> > That's the point. Currently they would fail without specific notifications for
> this usage.
> >
> > One of the solutions is to declare
> AV_CODEC_CAP_VARIABLE_DIMENSIONS  capability for encoders [1],
> > and prompt an error information [2] to indicate " Dynamic resolution
> encode is not supported by this encoder."
> >
> > The further work would be providing dynamic encoding support for some
> encoders [3].
> >
> > So this patch is the first step.
> 
> Is this patch intended to be applied first, before the codec flag is
> added and checked?

Considering it won't impact the default behavior, IMHO yes it could be.
Gyan Doshi Feb. 17, 2020, 5:44 p.m. UTC | #6 
On 17-02-2020 11:07 pm, Fu, Linjie wrote:
>> -----Original Message-----
>> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
>> Gyan Doshi
>> Sent: Tuesday, February 18, 2020 01:33
>> To: ffmpeg-devel@ffmpeg.org
>> Subject: Re: [FFmpeg-devel] [PATCH] fftools/ffmpeg_filter: add -autoscale
>> to disable/enable the default scale
>>
>>
>>
>> On 17-02-2020 10:57 pm, Fu, Linjie wrote:
>>>> -----Original Message-----
>>>> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
>>>> Gyan Doshi
>>>> Sent: Tuesday, February 18, 2020 00:53
>>>> To: ffmpeg-devel@ffmpeg.org
>>>> Subject: Re: [FFmpeg-devel] [PATCH] fftools/ffmpeg_filter: add -
>> autoscale
>>>> to disable/enable the default scale
>>>>
>>>>
>>>>
>>>> On 17-02-2020 09:13 pm, Fu, Linjie wrote:
>>>>>> -----Original Message-----
>>>>>> From: Fu, Linjie <linjie.fu@intel.com>
>>>>>> Sent: Sunday, February 16, 2020 00:13
>>>>>> To: ffmpeg-devel@ffmpeg.org
>>>>>> Cc: Fu, Linjie <linjie.fu@intel.com>; Eoff, Ullysses A
>>>>>> <ullysses.a.eoff@intel.com>
>>>>>> Subject: [PATCH] fftools/ffmpeg_filter: add -autoscale to
>> disable/enable
>>>> the
>>>>>> default scale
>>>>>>
>>>>>> Currently, ffmpeg inserts scale filter by default in the filter graph
>>>>>> to force the whole decoded stream to scale into the same size with the
>>>>>> first frame. It's not quite make sense in resolution changing cases if
>>>>>> user wants the rawvideo without any scale.
>>>>>>
>>>>>> Using autoscale/noautoscale as an output option to indicate whether
>> auto
>>>>>> inserting the scale filter in the filter graph:
>>>>>>        -noautoscale or -autoscale 0:
>>>>>>        disable the default auto scale filter inserting.
>>>>>>
>>>>>> ffmpeg -y input.mp4 out1.yuv -noautoscale out2.yuv -autoscale 0
>>>> out3.yuv
>>>>>> Update docs.
>>>>>>
>>>>>> Signed-off-by: U. Artie Eoff <ullysses.a.eoff@intel.com>
>>>>>> Signed-off-by: Linjie Fu <linjie.fu@intel.com>
>>>>>> ---
>>>>>>     doc/ffmpeg.texi         | 16 ++++++++++++----
>>>>>>     fftools/ffmpeg.h        |  3 +++
>>>>>>     fftools/ffmpeg_filter.c |  2 +-
>>>>>>     fftools/ffmpeg_opt.c    |  5 +++++
>>>>>>     4 files changed, 21 insertions(+), 5 deletions(-)
>>>>>>
>>>>>> diff --git a/doc/ffmpeg.texi b/doc/ffmpeg.texi
>>>>>> index 29753f0..aebafb3 100644
>>>>>> --- a/doc/ffmpeg.texi
>>>>>> +++ b/doc/ffmpeg.texi
>>>>>> @@ -734,10 +734,6 @@ ffmpeg -dump_attachment:t "" -i INPUT
>>>>>>     Technical note -- attachments are implemented as codec extradata,
>> so
>>>> this
>>>>>>     option can actually be used to extract extradata from any stream, not
>>>> just
>>>>>>     attachments.
>>>>>> -
>>>>>> -@item -noautorotate
>>>>>> -Disable automatically rotating video based on file metadata.
>>>>>> -
>>>>>>     @end table
>>>>>>
>>>>>>     @section Video Options
>>>>>> @@ -819,6 +815,18 @@ Create the filtergraph specified by
>>>> @var{filtergraph}
>>>>>> and use it to
>>>>>>     filter the stream.
>>>>>>
>>>>>>     This is an alias for @code{-filter:v}, see the @ref{filter_option,,-filter
>>>> option}.
>>>>>> +
>>>>>> +@item -autorotate
>>>>>> +Automatically rotate the video according to file metadata. Enabled by
>>>>>> +default, use @option{-noautorotate} to disable it.
>>>>>> +
>>>>>> +@item -autoscale
>>>>>> +Automatically scale the video according to the resolution of first frame.
>>>>>> +Enabled by default, use @option{-noautoscale} to disable it. When
>>>>>> autoscale is
>>>>>> +disabled, all output frames of filter graph might not be in the same
>>>>>> resolution
>>>>>> +and may be inadequate for some encoder/muxer. Therefore, it is not
>>>>>> recommended
>>>>>> +to disable it unless you really know what you are doing.
>>>>>> +Disable autoscale at your own risk.
>>>>>>     @end table
>>>>>>
>>>>>>     @section Advanced Video options
>>>>>> diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
>>>>>> index 7b6f802..8beba6c 100644
>>>>>> --- a/fftools/ffmpeg.h
>>>>>> +++ b/fftools/ffmpeg.h
>>>>>> @@ -230,6 +230,8 @@ typedef struct OptionsContext {
>>>>>>         int        nb_time_bases;
>>>>>>         SpecifierOpt *enc_time_bases;
>>>>>>         int        nb_enc_time_bases;
>>>>>> +    SpecifierOpt *autoscale;
>>>>>> +    int        nb_autoscale;
>>>>>>     } OptionsContext;
>>>>>>
>>>>>>     typedef struct InputFilter {
>>>>>> @@ -479,6 +481,7 @@ typedef struct OutputStream {
>>>>>>         int force_fps;
>>>>>>         int top_field_first;
>>>>>>         int rotate_overridden;
>>>>>> +    int autoscale;
>>>>>>         double rotate_override_value;
>>>>>>
>>>>>>         AVRational frame_aspect_ratio;
>>>>>> diff --git a/fftools/ffmpeg_filter.c b/fftools/ffmpeg_filter.c
>>>>>> index 40cc4c1..46c8ea8 100644
>>>>>> --- a/fftools/ffmpeg_filter.c
>>>>>> +++ b/fftools/ffmpeg_filter.c
>>>>>> @@ -469,7 +469,7 @@ static int
>>>> configure_output_video_filter(FilterGraph
>>>>>> *fg, OutputFilter *ofilter,
>>>>>>         if (ret < 0)
>>>>>>             return ret;
>>>>>>
>>>>>> -    if (ofilter->width || ofilter->height) {
>>>>>> +    if ((ofilter->width || ofilter->height) && ofilter->ost->autoscale) {
>>>>>>             char args[255];
>>>>>>             AVFilterContext *filter;
>>>>>>             AVDictionaryEntry *e = NULL;
>>>>>> diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
>>>>>> index 12d4488..a6f4216 100644
>>>>>> --- a/fftools/ffmpeg_opt.c
>>>>>> +++ b/fftools/ffmpeg_opt.c
>>>>>> @@ -1405,6 +1405,8 @@ static OutputStream
>>>>>> *new_output_stream(OptionsContext *o, AVFormatContext *oc, e
>>>>>>             ost->encoder_opts  = filter_codec_opts(o->g->codec_opts, ost-
>>>>> enc-
>>>>>>> id, oc, st, ost->enc);
>>>>>>             MATCH_PER_STREAM_OPT(presets, str, preset, oc, st);
>>>>>> +        ost->autoscale = 1;
>>>>>> +        MATCH_PER_STREAM_OPT(autoscale, i, ost->autoscale, oc, st);
>>>>>>             if (preset && (!(ret = get_preset_file_2(preset, ost->enc->name,
>>>> &s))))
>>>>>> {
>>>>>>                 do  {
>>>>>>                     buf = get_line(s);
>>>>>> @@ -3650,6 +3652,9 @@ const OptionDef options[] = {
>>>>>>         { "autorotate",       HAS_ARG | OPT_BOOL | OPT_SPEC |
>>>>>>                               OPT_EXPERT | OPT_INPUT,                                { .off =
>>>>>> OFFSET(autorotate) },
>>>>>>             "automatically insert correct rotate filters" },
>>>>>> +    { "autoscale",        HAS_ARG | OPT_BOOL | OPT_SPEC |
>>>>>> +                          OPT_EXPERT | OPT_OUTPUT,                               { .off =
>>>>>> OFFSET(autoscale) },
>>>>>> +        "automatically insert a scale filter at the end of the filter graph" },
>>>>>>
>>>>>>         /* audio options */
>>>>>>         { "aframes",        OPT_AUDIO | HAS_ARG  | OPT_PERFILE |
>>>> OPT_OUTPUT,
>>>>>> { .func_arg = opt_audio_frames },
>>>>>> --
>>>>>> 2.7.4
>>>> What happens if user disables autoscale and the frame sizes do vary but
>>>> the encoder doesn't support changing props?
>>> That's the point. Currently they would fail without specific notifications for
>> this usage.
>>> One of the solutions is to declare
>> AV_CODEC_CAP_VARIABLE_DIMENSIONS  capability for encoders [1],
>>> and prompt an error information [2] to indicate " Dynamic resolution
>> encode is not supported by this encoder."
>>> The further work would be providing dynamic encoding support for some
>> encoders [3].
>>> So this patch is the first step.
>> Is this patch intended to be applied first, before the codec flag is
>> added and checked?
> Considering it won't impact the default behavior, IMHO yes it could be.

So, my question still stands. If it is applied tomorrow, and a user 
disables autoscale, and ends up feeding frames of varying sizes to, say, 
x264 or aom, what happens?

Gyan
Fu, Linjie Feb. 17, 2020, 5:59 p.m. UTC | #7 
> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> Gyan Doshi
> Sent: Tuesday, February 18, 2020 01:44
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH] fftools/ffmpeg_filter: add -autoscale
> to disable/enable the default scale
> 
> 
> 
> On 17-02-2020 11:07 pm, Fu, Linjie wrote:
> >> -----Original Message-----
> >> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> >> Gyan Doshi
> >> Sent: Tuesday, February 18, 2020 01:33
> >> To: ffmpeg-devel@ffmpeg.org
> >> Subject: Re: [FFmpeg-devel] [PATCH] fftools/ffmpeg_filter: add -
> autoscale
> >> to disable/enable the default scale
> >>
> >>
> >>
> >> On 17-02-2020 10:57 pm, Fu, Linjie wrote:
> >>>> -----Original Message-----
> >>>> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf
> Of
> >>>> Gyan Doshi
> >>>> Sent: Tuesday, February 18, 2020 00:53
> >>>> To: ffmpeg-devel@ffmpeg.org
> >>>> Subject: Re: [FFmpeg-devel] [PATCH] fftools/ffmpeg_filter: add -
> >> autoscale
> >>>> to disable/enable the default scale
> >>>>
> >>>>
> >>>>
> >>>> On 17-02-2020 09:13 pm, Fu, Linjie wrote:
> >>>>>> -----Original Message-----
> >>>>>> From: Fu, Linjie <linjie.fu@intel.com>
> >>>>>> Sent: Sunday, February 16, 2020 00:13
> >>>>>> To: ffmpeg-devel@ffmpeg.org
> >>>>>> Cc: Fu, Linjie <linjie.fu@intel.com>; Eoff, Ullysses A
> >>>>>> <ullysses.a.eoff@intel.com>
> >>>>>> Subject: [PATCH] fftools/ffmpeg_filter: add -autoscale to
> >> disable/enable
> >>>> the
> >>>>>> default scale
> >>>>>>
> >>>>>> Currently, ffmpeg inserts scale filter by default in the filter graph
> >>>>>> to force the whole decoded stream to scale into the same size with
> the
> >>>>>> first frame. It's not quite make sense in resolution changing cases if
> >>>>>> user wants the rawvideo without any scale.
> >>>>>>
> >>>>>> Using autoscale/noautoscale as an output option to indicate
> whether
> >> auto
> >>>>>> inserting the scale filter in the filter graph:
> >>>>>>        -noautoscale or -autoscale 0:
> >>>>>>        disable the default auto scale filter inserting.
> >>>>>>
> >>>>>> ffmpeg -y input.mp4 out1.yuv -noautoscale out2.yuv -autoscale 0
> >>>> out3.yuv
> >>>>>> Update docs.
> >>>>>>
> >>>>>> Signed-off-by: U. Artie Eoff <ullysses.a.eoff@intel.com>
> >>>>>> Signed-off-by: Linjie Fu <linjie.fu@intel.com>
> >>>>>> ---
> >>>>>>     doc/ffmpeg.texi         | 16 ++++++++++++----
> >>>>>>     fftools/ffmpeg.h        |  3 +++
> >>>>>>     fftools/ffmpeg_filter.c |  2 +-
> >>>>>>     fftools/ffmpeg_opt.c    |  5 +++++
> >>>>>>     4 files changed, 21 insertions(+), 5 deletions(-)
> >>>>>>
> >>>>>> diff --git a/doc/ffmpeg.texi b/doc/ffmpeg.texi
> >>>>>> index 29753f0..aebafb3 100644
> >>>>>> --- a/doc/ffmpeg.texi
> >>>>>> +++ b/doc/ffmpeg.texi
> >>>>>> @@ -734,10 +734,6 @@ ffmpeg -dump_attachment:t "" -i INPUT
> >>>>>>     Technical note -- attachments are implemented as codec extradata,
> >> so
> >>>> this
> >>>>>>     option can actually be used to extract extradata from any stream,
> not
> >>>> just
> >>>>>>     attachments.
> >>>>>> -
> >>>>>> -@item -noautorotate
> >>>>>> -Disable automatically rotating video based on file metadata.
> >>>>>> -
> >>>>>>     @end table
> >>>>>>
> >>>>>>     @section Video Options
> >>>>>> @@ -819,6 +815,18 @@ Create the filtergraph specified by
> >>>> @var{filtergraph}
> >>>>>> and use it to
> >>>>>>     filter the stream.
> >>>>>>
> >>>>>>     This is an alias for @code{-filter:v}, see the @ref{filter_option,,-
> filter
> >>>> option}.
> >>>>>> +
> >>>>>> +@item -autorotate
> >>>>>> +Automatically rotate the video according to file metadata. Enabled
> by
> >>>>>> +default, use @option{-noautorotate} to disable it.
> >>>>>> +
> >>>>>> +@item -autoscale
> >>>>>> +Automatically scale the video according to the resolution of first
> frame.
> >>>>>> +Enabled by default, use @option{-noautoscale} to disable it. When
> >>>>>> autoscale is
> >>>>>> +disabled, all output frames of filter graph might not be in the same
> >>>>>> resolution
> >>>>>> +and may be inadequate for some encoder/muxer. Therefore, it is
> not
> >>>>>> recommended
> >>>>>> +to disable it unless you really know what you are doing.
> >>>>>> +Disable autoscale at your own risk.
> >>>>>>     @end table
> >>>>>>
> >>>>>>     @section Advanced Video options
> >>>>>> diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
> >>>>>> index 7b6f802..8beba6c 100644
> >>>>>> --- a/fftools/ffmpeg.h
> >>>>>> +++ b/fftools/ffmpeg.h
> >>>>>> @@ -230,6 +230,8 @@ typedef struct OptionsContext {
> >>>>>>         int        nb_time_bases;
> >>>>>>         SpecifierOpt *enc_time_bases;
> >>>>>>         int        nb_enc_time_bases;
> >>>>>> +    SpecifierOpt *autoscale;
> >>>>>> +    int        nb_autoscale;
> >>>>>>     } OptionsContext;
> >>>>>>
> >>>>>>     typedef struct InputFilter {
> >>>>>> @@ -479,6 +481,7 @@ typedef struct OutputStream {
> >>>>>>         int force_fps;
> >>>>>>         int top_field_first;
> >>>>>>         int rotate_overridden;
> >>>>>> +    int autoscale;
> >>>>>>         double rotate_override_value;
> >>>>>>
> >>>>>>         AVRational frame_aspect_ratio;
> >>>>>> diff --git a/fftools/ffmpeg_filter.c b/fftools/ffmpeg_filter.c
> >>>>>> index 40cc4c1..46c8ea8 100644
> >>>>>> --- a/fftools/ffmpeg_filter.c
> >>>>>> +++ b/fftools/ffmpeg_filter.c
> >>>>>> @@ -469,7 +469,7 @@ static int
> >>>> configure_output_video_filter(FilterGraph
> >>>>>> *fg, OutputFilter *ofilter,
> >>>>>>         if (ret < 0)
> >>>>>>             return ret;
> >>>>>>
> >>>>>> -    if (ofilter->width || ofilter->height) {
> >>>>>> +    if ((ofilter->width || ofilter->height) && ofilter->ost->autoscale) {
> >>>>>>             char args[255];
> >>>>>>             AVFilterContext *filter;
> >>>>>>             AVDictionaryEntry *e = NULL;
> >>>>>> diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
> >>>>>> index 12d4488..a6f4216 100644
> >>>>>> --- a/fftools/ffmpeg_opt.c
> >>>>>> +++ b/fftools/ffmpeg_opt.c
> >>>>>> @@ -1405,6 +1405,8 @@ static OutputStream
> >>>>>> *new_output_stream(OptionsContext *o, AVFormatContext *oc, e
> >>>>>>             ost->encoder_opts  = filter_codec_opts(o->g->codec_opts,
> ost-
> >>>>> enc-
> >>>>>>> id, oc, st, ost->enc);
> >>>>>>             MATCH_PER_STREAM_OPT(presets, str, preset, oc, st);
> >>>>>> +        ost->autoscale = 1;
> >>>>>> +        MATCH_PER_STREAM_OPT(autoscale, i, ost->autoscale, oc, st);
> >>>>>>             if (preset && (!(ret = get_preset_file_2(preset, ost->enc-
> >name,
> >>>> &s))))
> >>>>>> {
> >>>>>>                 do  {
> >>>>>>                     buf = get_line(s);
> >>>>>> @@ -3650,6 +3652,9 @@ const OptionDef options[] = {
> >>>>>>         { "autorotate",       HAS_ARG | OPT_BOOL | OPT_SPEC |
> >>>>>>                               OPT_EXPERT | OPT_INPUT,                                { .off =
> >>>>>> OFFSET(autorotate) },
> >>>>>>             "automatically insert correct rotate filters" },
> >>>>>> +    { "autoscale",        HAS_ARG | OPT_BOOL | OPT_SPEC |
> >>>>>> +                          OPT_EXPERT | OPT_OUTPUT,                               { .off =
> >>>>>> OFFSET(autoscale) },
> >>>>>> +        "automatically insert a scale filter at the end of the filter
> graph" },
> >>>>>>
> >>>>>>         /* audio options */
> >>>>>>         { "aframes",        OPT_AUDIO | HAS_ARG  | OPT_PERFILE |
> >>>> OPT_OUTPUT,
> >>>>>> { .func_arg = opt_audio_frames },
> >>>>>> --
> >>>>>> 2.7.4
> >>>> What happens if user disables autoscale and the frame sizes do vary
> but
> >>>> the encoder doesn't support changing props?
> >>> That's the point. Currently they would fail without specific notifications
> for
> >> this usage.
> >>> One of the solutions is to declare
> >> AV_CODEC_CAP_VARIABLE_DIMENSIONS  capability for encoders [1],
> >>> and prompt an error information [2] to indicate " Dynamic resolution
> >> encode is not supported by this encoder."
> >>> The further work would be providing dynamic encoding support for
> some
> >> encoders [3].
> >>> So this patch is the first step.
> >> Is this patch intended to be applied first, before the codec flag is
> >> added and checked?
> > Considering it won't impact the default behavior, IMHO yes it could be.
> 
> So, my question still stands. If it is applied tomorrow, and a user
> disables autoscale, and ends up feeding frames of varying sizes to, say,
> x264 or aom, what happens?
> 

Failed without specific notifications for wrongly disabling autoscale.

Taking x264 as an example to transcode clips with resolution changing
from 352x288 to 240x196:

[libx264 @ 0x5559143d51c0] Input picture width (352) is greater than stride (256)
Video encoding failed
(This differs according to the encoder.)

BTW, it would be great if the patch set for codec flag and check could be applied at
the same time.
Gyan Doshi Feb. 17, 2020, 6:10 p.m. UTC | #8 
On 17-02-2020 11:29 pm, Fu, Linjie wrote:
>> -----Original Message-----
>> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
>> Gyan Doshi
>> Sent: Tuesday, February 18, 2020 01:44
>> To: ffmpeg-devel@ffmpeg.org
>> Subject: Re: [FFmpeg-devel] [PATCH] fftools/ffmpeg_filter: add -autoscale
>> to disable/enable the default scale
>>
>>
>>
>> On 17-02-2020 11:07 pm, Fu, Linjie wrote:
>>>> -----Original Message-----
>>>> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
>>>> Gyan Doshi
>>>> Sent: Tuesday, February 18, 2020 01:33
>>>> To: ffmpeg-devel@ffmpeg.org
>>>> Subject: Re: [FFmpeg-devel] [PATCH] fftools/ffmpeg_filter: add -
>> autoscale
>>>> to disable/enable the default scale
>>>>
>>>>
>>>>
>>>> On 17-02-2020 10:57 pm, Fu, Linjie wrote:
>>>>>> -----Original Message-----
>>>>>> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf
>> Of
>>>>>> Gyan Doshi
>>>>>> Sent: Tuesday, February 18, 2020 00:53
>>>>>> To: ffmpeg-devel@ffmpeg.org
>>>>>> Subject: Re: [FFmpeg-devel] [PATCH] fftools/ffmpeg_filter: add -
>>>> autoscale
>>>>>> to disable/enable the default scale
>>>>>>
>>>>>>
>>>>>>
>>>>>> On 17-02-2020 09:13 pm, Fu, Linjie wrote:
>>>>>>>> -----Original Message-----
>>>>>>>> From: Fu, Linjie <linjie.fu@intel.com>
>>>>>>>> Sent: Sunday, February 16, 2020 00:13
>>>>>>>> To: ffmpeg-devel@ffmpeg.org
>>>>>>>> Cc: Fu, Linjie <linjie.fu@intel.com>; Eoff, Ullysses A
>>>>>>>> <ullysses.a.eoff@intel.com>
>>>>>>>> Subject: [PATCH] fftools/ffmpeg_filter: add -autoscale to
>>>> disable/enable
>>>>>> the
>>>>>>>> default scale
>>>>>>>>
>>>>>>>> Currently, ffmpeg inserts scale filter by default in the filter graph
>>>>>>>> to force the whole decoded stream to scale into the same size with
>> the
>>>>>>>> first frame. It's not quite make sense in resolution changing cases if
>>>>>>>> user wants the rawvideo without any scale.
>>>>>>>>
>>>>>>>> Using autoscale/noautoscale as an output option to indicate
>> whether
>>>> auto
>>>>>>>> inserting the scale filter in the filter graph:
>>>>>>>>         -noautoscale or -autoscale 0:
>>>>>>>>         disable the default auto scale filter inserting.
>>>>>>>>
>>>>>>>> ffmpeg -y input.mp4 out1.yuv -noautoscale out2.yuv -autoscale 0
>>>>>> out3.yuv
>>>>>>>> Update docs.
>>>>>>>>
>>>>>>>> Signed-off-by: U. Artie Eoff <ullysses.a.eoff@intel.com>
>>>>>>>> Signed-off-by: Linjie Fu <linjie.fu@intel.com>
>>>>>>>> ---
>>>>>>>>      doc/ffmpeg.texi         | 16 ++++++++++++----
>>>>>>>>      fftools/ffmpeg.h        |  3 +++
>>>>>>>>      fftools/ffmpeg_filter.c |  2 +-
>>>>>>>>      fftools/ffmpeg_opt.c    |  5 +++++
>>>>>>>>      4 files changed, 21 insertions(+), 5 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/doc/ffmpeg.texi b/doc/ffmpeg.texi
>>>>>>>> index 29753f0..aebafb3 100644
>>>>>>>> --- a/doc/ffmpeg.texi
>>>>>>>> +++ b/doc/ffmpeg.texi
>>>>>>>> @@ -734,10 +734,6 @@ ffmpeg -dump_attachment:t "" -i INPUT
>>>>>>>>      Technical note -- attachments are implemented as codec extradata,
>>>> so
>>>>>> this
>>>>>>>>      option can actually be used to extract extradata from any stream,
>> not
>>>>>> just
>>>>>>>>      attachments.
>>>>>>>> -
>>>>>>>> -@item -noautorotate
>>>>>>>> -Disable automatically rotating video based on file metadata.
>>>>>>>> -
>>>>>>>>      @end table
>>>>>>>>
>>>>>>>>      @section Video Options
>>>>>>>> @@ -819,6 +815,18 @@ Create the filtergraph specified by
>>>>>> @var{filtergraph}
>>>>>>>> and use it to
>>>>>>>>      filter the stream.
>>>>>>>>
>>>>>>>>      This is an alias for @code{-filter:v}, see the @ref{filter_option,,-
>> filter
>>>>>> option}.
>>>>>>>> +
>>>>>>>> +@item -autorotate
>>>>>>>> +Automatically rotate the video according to file metadata. Enabled
>> by
>>>>>>>> +default, use @option{-noautorotate} to disable it.
>>>>>>>> +
>>>>>>>> +@item -autoscale
>>>>>>>> +Automatically scale the video according to the resolution of first
>> frame.
>>>>>>>> +Enabled by default, use @option{-noautoscale} to disable it. When
>>>>>>>> autoscale is
>>>>>>>> +disabled, all output frames of filter graph might not be in the same
>>>>>>>> resolution
>>>>>>>> +and may be inadequate for some encoder/muxer. Therefore, it is
>> not
>>>>>>>> recommended
>>>>>>>> +to disable it unless you really know what you are doing.
>>>>>>>> +Disable autoscale at your own risk.
>>>>>>>>      @end table
>>>>>>>>
>>>>>>>>      @section Advanced Video options
>>>>>>>> diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
>>>>>>>> index 7b6f802..8beba6c 100644
>>>>>>>> --- a/fftools/ffmpeg.h
>>>>>>>> +++ b/fftools/ffmpeg.h
>>>>>>>> @@ -230,6 +230,8 @@ typedef struct OptionsContext {
>>>>>>>>          int        nb_time_bases;
>>>>>>>>          SpecifierOpt *enc_time_bases;
>>>>>>>>          int        nb_enc_time_bases;
>>>>>>>> +    SpecifierOpt *autoscale;
>>>>>>>> +    int        nb_autoscale;
>>>>>>>>      } OptionsContext;
>>>>>>>>
>>>>>>>>      typedef struct InputFilter {
>>>>>>>> @@ -479,6 +481,7 @@ typedef struct OutputStream {
>>>>>>>>          int force_fps;
>>>>>>>>          int top_field_first;
>>>>>>>>          int rotate_overridden;
>>>>>>>> +    int autoscale;
>>>>>>>>          double rotate_override_value;
>>>>>>>>
>>>>>>>>          AVRational frame_aspect_ratio;
>>>>>>>> diff --git a/fftools/ffmpeg_filter.c b/fftools/ffmpeg_filter.c
>>>>>>>> index 40cc4c1..46c8ea8 100644
>>>>>>>> --- a/fftools/ffmpeg_filter.c
>>>>>>>> +++ b/fftools/ffmpeg_filter.c
>>>>>>>> @@ -469,7 +469,7 @@ static int
>>>>>> configure_output_video_filter(FilterGraph
>>>>>>>> *fg, OutputFilter *ofilter,
>>>>>>>>          if (ret < 0)
>>>>>>>>              return ret;
>>>>>>>>
>>>>>>>> -    if (ofilter->width || ofilter->height) {
>>>>>>>> +    if ((ofilter->width || ofilter->height) && ofilter->ost->autoscale) {
>>>>>>>>              char args[255];
>>>>>>>>              AVFilterContext *filter;
>>>>>>>>              AVDictionaryEntry *e = NULL;
>>>>>>>> diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
>>>>>>>> index 12d4488..a6f4216 100644
>>>>>>>> --- a/fftools/ffmpeg_opt.c
>>>>>>>> +++ b/fftools/ffmpeg_opt.c
>>>>>>>> @@ -1405,6 +1405,8 @@ static OutputStream
>>>>>>>> *new_output_stream(OptionsContext *o, AVFormatContext *oc, e
>>>>>>>>              ost->encoder_opts  = filter_codec_opts(o->g->codec_opts,
>> ost-
>>>>>>> enc-
>>>>>>>>> id, oc, st, ost->enc);
>>>>>>>>              MATCH_PER_STREAM_OPT(presets, str, preset, oc, st);
>>>>>>>> +        ost->autoscale = 1;
>>>>>>>> +        MATCH_PER_STREAM_OPT(autoscale, i, ost->autoscale, oc, st);
>>>>>>>>              if (preset && (!(ret = get_preset_file_2(preset, ost->enc-
>>> name,
>>>>>> &s))))
>>>>>>>> {
>>>>>>>>                  do  {
>>>>>>>>                      buf = get_line(s);
>>>>>>>> @@ -3650,6 +3652,9 @@ const OptionDef options[] = {
>>>>>>>>          { "autorotate",       HAS_ARG | OPT_BOOL | OPT_SPEC |
>>>>>>>>                                OPT_EXPERT | OPT_INPUT,                                { .off =
>>>>>>>> OFFSET(autorotate) },
>>>>>>>>              "automatically insert correct rotate filters" },
>>>>>>>> +    { "autoscale",        HAS_ARG | OPT_BOOL | OPT_SPEC |
>>>>>>>> +                          OPT_EXPERT | OPT_OUTPUT,                               { .off =
>>>>>>>> OFFSET(autoscale) },
>>>>>>>> +        "automatically insert a scale filter at the end of the filter
>> graph" },
>>>>>>>>          /* audio options */
>>>>>>>>          { "aframes",        OPT_AUDIO | HAS_ARG  | OPT_PERFILE |
>>>>>> OPT_OUTPUT,
>>>>>>>> { .func_arg = opt_audio_frames },
>>>>>>>> --
>>>>>>>> 2.7.4
>>>>>> What happens if user disables autoscale and the frame sizes do vary
>> but
>>>>>> the encoder doesn't support changing props?
>>>>> That's the point. Currently they would fail without specific notifications
>> for
>>>> this usage.
>>>>> One of the solutions is to declare
>>>> AV_CODEC_CAP_VARIABLE_DIMENSIONS  capability for encoders [1],
>>>>> and prompt an error information [2] to indicate " Dynamic resolution
>>>> encode is not supported by this encoder."
>>>>> The further work would be providing dynamic encoding support for
>> some
>>>> encoders [3].
>>>>> So this patch is the first step.
>>>> Is this patch intended to be applied first, before the codec flag is
>>>> added and checked?
>>> Considering it won't impact the default behavior, IMHO yes it could be.
>> So, my question still stands. If it is applied tomorrow, and a user
>> disables autoscale, and ends up feeding frames of varying sizes to, say,
>> x264 or aom, what happens?
>>
> Failed without specific notifications for wrongly disabling autoscale.
>
> Taking x264 as an example to transcode clips with resolution changing
> from 352x288 to 240x196:
>
> [libx264 @ 0x5559143d51c0] Input picture width (352) is greater than stride (256)
> Video encoding failed
> (This differs according to the encoder.)
>
> BTW, it would be great if the patch set for codec flag and check could be applied at
> the same time.

The flag should be added first, then this patch updated to check and 
abort if encoder doesn't have the capability.

Gyan
Nicolas George Feb. 17, 2020, 6:20 p.m. UTC | #9 
Gyan Doshi (12020-02-17):
> The flag should be added first, then this patch updated to check and abort
> if encoder doesn't have the capability.

Why? The described behavior seems quite fine. Aborting the encoding
process a little earlier would be fine, but it's hardly a critical
issue.

Regards,
Fu, Linjie Feb. 17, 2020, 6:26 p.m. UTC | #10 
> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> Gyan Doshi
> Sent: Tuesday, February 18, 2020 02:10
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH] fftools/ffmpeg_filter: add -autoscale
> to disable/enable the default scale
> 
> 
> 
> On 17-02-2020 11:29 pm, Fu, Linjie wrote:
> >> -----Original Message-----
> >> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> >> Gyan Doshi
> >> Sent: Tuesday, February 18, 2020 01:44
> >> To: ffmpeg-devel@ffmpeg.org
> >> Subject: Re: [FFmpeg-devel] [PATCH] fftools/ffmpeg_filter: add -
> autoscale
> >> to disable/enable the default scale
> >>
> >>
> >>
> >> On 17-02-2020 11:07 pm, Fu, Linjie wrote:
> >>>> -----Original Message-----
> >>>> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf
> Of
> >>>> Gyan Doshi
> >>>> Sent: Tuesday, February 18, 2020 01:33
> >>>> To: ffmpeg-devel@ffmpeg.org
> >>>> Subject: Re: [FFmpeg-devel] [PATCH] fftools/ffmpeg_filter: add -
> >> autoscale
> >>>> to disable/enable the default scale
> >>>>
> >>>>
> >>>>
> >>>> On 17-02-2020 10:57 pm, Fu, Linjie wrote:
> >>>>>> -----Original Message-----
> >>>>>> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On
> Behalf
> >> Of
> >>>>>> Gyan Doshi
> >>>>>> Sent: Tuesday, February 18, 2020 00:53
> >>>>>> To: ffmpeg-devel@ffmpeg.org
> >>>>>> Subject: Re: [FFmpeg-devel] [PATCH] fftools/ffmpeg_filter: add -
> >>>> autoscale
> >>>>>> to disable/enable the default scale
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>> On 17-02-2020 09:13 pm, Fu, Linjie wrote:
> >>>>>>>> -----Original Message-----
> >>>>>>>> From: Fu, Linjie <linjie.fu@intel.com>
> >>>>>>>> Sent: Sunday, February 16, 2020 00:13
> >>>>>>>> To: ffmpeg-devel@ffmpeg.org
> >>>>>>>> Cc: Fu, Linjie <linjie.fu@intel.com>; Eoff, Ullysses A
> >>>>>>>> <ullysses.a.eoff@intel.com>
> >>>>>>>> Subject: [PATCH] fftools/ffmpeg_filter: add -autoscale to
> >>>> disable/enable
> >>>>>> the
> >>>>>>>> default scale
> >>>>>>>>
> >>>>>>>> Currently, ffmpeg inserts scale filter by default in the filter graph
> >>>>>>>> to force the whole decoded stream to scale into the same size
> with
> >> the
> >>>>>>>> first frame. It's not quite make sense in resolution changing cases
> if
> >>>>>>>> user wants the rawvideo without any scale.
> >>>>>>>>
> >>>>>>>> Using autoscale/noautoscale as an output option to indicate
> >> whether
> >>>> auto
> >>>>>>>> inserting the scale filter in the filter graph:
> >>>>>>>>         -noautoscale or -autoscale 0:
> >>>>>>>>         disable the default auto scale filter inserting.
> >>>>>>>>
> >>>>>>>> ffmpeg -y input.mp4 out1.yuv -noautoscale out2.yuv -autoscale 0
> >>>>>> out3.yuv
> >>>>>>>> Update docs.
> >>>>>>>>
> >>>>>>>> Signed-off-by: U. Artie Eoff <ullysses.a.eoff@intel.com>
> >>>>>>>> Signed-off-by: Linjie Fu <linjie.fu@intel.com>
> >>>>>>>> ---
> >>>>>>>>      doc/ffmpeg.texi         | 16 ++++++++++++----
> >>>>>>>>      fftools/ffmpeg.h        |  3 +++
> >>>>>>>>      fftools/ffmpeg_filter.c |  2 +-
> >>>>>>>>      fftools/ffmpeg_opt.c    |  5 +++++
> >>>>>>>>      4 files changed, 21 insertions(+), 5 deletions(-)
> >>>>>>>>
> >>>>>>>> diff --git a/doc/ffmpeg.texi b/doc/ffmpeg.texi
> >>>>>>>> index 29753f0..aebafb3 100644
> >>>>>>>> --- a/doc/ffmpeg.texi
> >>>>>>>> +++ b/doc/ffmpeg.texi
> >>>>>>>> @@ -734,10 +734,6 @@ ffmpeg -dump_attachment:t "" -i INPUT
> >>>>>>>>      Technical note -- attachments are implemented as codec
> extradata,
> >>>> so
> >>>>>> this
> >>>>>>>>      option can actually be used to extract extradata from any
> stream,
> >> not
> >>>>>> just
> >>>>>>>>      attachments.
> >>>>>>>> -
> >>>>>>>> -@item -noautorotate
> >>>>>>>> -Disable automatically rotating video based on file metadata.
> >>>>>>>> -
> >>>>>>>>      @end table
> >>>>>>>>
> >>>>>>>>      @section Video Options
> >>>>>>>> @@ -819,6 +815,18 @@ Create the filtergraph specified by
> >>>>>> @var{filtergraph}
> >>>>>>>> and use it to
> >>>>>>>>      filter the stream.
> >>>>>>>>
> >>>>>>>>      This is an alias for @code{-filter:v}, see the @ref{filter_option,,-
> >> filter
> >>>>>> option}.
> >>>>>>>> +
> >>>>>>>> +@item -autorotate
> >>>>>>>> +Automatically rotate the video according to file metadata.
> Enabled
> >> by
> >>>>>>>> +default, use @option{-noautorotate} to disable it.
> >>>>>>>> +
> >>>>>>>> +@item -autoscale
> >>>>>>>> +Automatically scale the video according to the resolution of first
> >> frame.
> >>>>>>>> +Enabled by default, use @option{-noautoscale} to disable it.
> When
> >>>>>>>> autoscale is
> >>>>>>>> +disabled, all output frames of filter graph might not be in the
> same
> >>>>>>>> resolution
> >>>>>>>> +and may be inadequate for some encoder/muxer. Therefore, it
> is
> >> not
> >>>>>>>> recommended
> >>>>>>>> +to disable it unless you really know what you are doing.
> >>>>>>>> +Disable autoscale at your own risk.
> >>>>>>>>      @end table
> >>>>>>>>
> >>>>>>>>      @section Advanced Video options
> >>>>>>>> diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
> >>>>>>>> index 7b6f802..8beba6c 100644
> >>>>>>>> --- a/fftools/ffmpeg.h
> >>>>>>>> +++ b/fftools/ffmpeg.h
> >>>>>>>> @@ -230,6 +230,8 @@ typedef struct OptionsContext {
> >>>>>>>>          int        nb_time_bases;
> >>>>>>>>          SpecifierOpt *enc_time_bases;
> >>>>>>>>          int        nb_enc_time_bases;
> >>>>>>>> +    SpecifierOpt *autoscale;
> >>>>>>>> +    int        nb_autoscale;
> >>>>>>>>      } OptionsContext;
> >>>>>>>>
> >>>>>>>>      typedef struct InputFilter {
> >>>>>>>> @@ -479,6 +481,7 @@ typedef struct OutputStream {
> >>>>>>>>          int force_fps;
> >>>>>>>>          int top_field_first;
> >>>>>>>>          int rotate_overridden;
> >>>>>>>> +    int autoscale;
> >>>>>>>>          double rotate_override_value;
> >>>>>>>>
> >>>>>>>>          AVRational frame_aspect_ratio;
> >>>>>>>> diff --git a/fftools/ffmpeg_filter.c b/fftools/ffmpeg_filter.c
> >>>>>>>> index 40cc4c1..46c8ea8 100644
> >>>>>>>> --- a/fftools/ffmpeg_filter.c
> >>>>>>>> +++ b/fftools/ffmpeg_filter.c
> >>>>>>>> @@ -469,7 +469,7 @@ static int
> >>>>>> configure_output_video_filter(FilterGraph
> >>>>>>>> *fg, OutputFilter *ofilter,
> >>>>>>>>          if (ret < 0)
> >>>>>>>>              return ret;
> >>>>>>>>
> >>>>>>>> -    if (ofilter->width || ofilter->height) {
> >>>>>>>> +    if ((ofilter->width || ofilter->height) && ofilter->ost-
> >autoscale) {
> >>>>>>>>              char args[255];
> >>>>>>>>              AVFilterContext *filter;
> >>>>>>>>              AVDictionaryEntry *e = NULL;
> >>>>>>>> diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
> >>>>>>>> index 12d4488..a6f4216 100644
> >>>>>>>> --- a/fftools/ffmpeg_opt.c
> >>>>>>>> +++ b/fftools/ffmpeg_opt.c
> >>>>>>>> @@ -1405,6 +1405,8 @@ static OutputStream
> >>>>>>>> *new_output_stream(OptionsContext *o, AVFormatContext *oc,
> e
> >>>>>>>>              ost->encoder_opts  = filter_codec_opts(o->g->codec_opts,
> >> ost-
> >>>>>>> enc-
> >>>>>>>>> id, oc, st, ost->enc);
> >>>>>>>>              MATCH_PER_STREAM_OPT(presets, str, preset, oc, st);
> >>>>>>>> +        ost->autoscale = 1;
> >>>>>>>> +        MATCH_PER_STREAM_OPT(autoscale, i, ost->autoscale, oc,
> st);
> >>>>>>>>              if (preset && (!(ret = get_preset_file_2(preset, ost->enc-
> >>> name,
> >>>>>> &s))))
> >>>>>>>> {
> >>>>>>>>                  do  {
> >>>>>>>>                      buf = get_line(s);
> >>>>>>>> @@ -3650,6 +3652,9 @@ const OptionDef options[] = {
> >>>>>>>>          { "autorotate",       HAS_ARG | OPT_BOOL | OPT_SPEC |
> >>>>>>>>                                OPT_EXPERT | OPT_INPUT,                                { .off =
> >>>>>>>> OFFSET(autorotate) },
> >>>>>>>>              "automatically insert correct rotate filters" },
> >>>>>>>> +    { "autoscale",        HAS_ARG | OPT_BOOL | OPT_SPEC |
> >>>>>>>> +                          OPT_EXPERT | OPT_OUTPUT,                               { .off =
> >>>>>>>> OFFSET(autoscale) },
> >>>>>>>> +        "automatically insert a scale filter at the end of the filter
> >> graph" },
> >>>>>>>>          /* audio options */
> >>>>>>>>          { "aframes",        OPT_AUDIO | HAS_ARG  | OPT_PERFILE |
> >>>>>> OPT_OUTPUT,
> >>>>>>>> { .func_arg = opt_audio_frames },
> >>>>>>>> --
> >>>>>>>> 2.7.4
> >>>>>> What happens if user disables autoscale and the frame sizes do vary
> >> but
> >>>>>> the encoder doesn't support changing props?
> >>>>> That's the point. Currently they would fail without specific
> notifications
> >> for
> >>>> this usage.
> >>>>> One of the solutions is to declare
> >>>> AV_CODEC_CAP_VARIABLE_DIMENSIONS  capability for encoders [1],
> >>>>> and prompt an error information [2] to indicate " Dynamic resolution
> >>>> encode is not supported by this encoder."
> >>>>> The further work would be providing dynamic encoding support for
> >> some
> >>>> encoders [3].
> >>>>> So this patch is the first step.
> >>>> Is this patch intended to be applied first, before the codec flag is
> >>>> added and checked?
> >>> Considering it won't impact the default behavior, IMHO yes it could be.
> >> So, my question still stands. If it is applied tomorrow, and a user
> >> disables autoscale, and ends up feeding frames of varying sizes to, say,
> >> x264 or aom, what happens?
> >>
> > Failed without specific notifications for wrongly disabling autoscale.
> >
> > Taking x264 as an example to transcode clips with resolution changing
> > from 352x288 to 240x196:
> >
> > [libx264 @ 0x5559143d51c0] Input picture width (352) is greater than stride
> (256)
> > Video encoding failed
> > (This differs according to the encoder.)
> >
> > BTW, it would be great if the patch set for codec flag and check could be
> applied at
> > the same time.
> 
> The flag should be added first, then this patch updated to check and
> abort if encoder doesn't have the capability.
> 
Actually the patches for codec flags and capability check are pending for some time.
Please also help to review. (and apply if it's ok)
Fu, Linjie Feb. 17, 2020, 6:42 p.m. UTC | #11 
> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> Nicolas George
> Sent: Tuesday, February 18, 2020 00:44
> To: FFmpeg development discussions and patches <ffmpeg-
> devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH] fftools/ffmpeg_filter: add -autoscale
> to disable/enable the default scale
> 
> Fu, Linjie (12020-02-17):
> > After several reviews, ping for this patch.
> 
> I only had concerns with the length of the help, they were addressed. I
> have no more remarks.

Thanks for the review, hoping this could be applied.
Gyan Doshi Feb. 17, 2020, 6:45 p.m. UTC | #12 
On 17-02-2020 11:50 pm, Nicolas George wrote:
> Gyan Doshi (12020-02-17):
>> The flag should be added first, then this patch updated to check and abort
>> if encoder doesn't have the capability.
> Why? The described behavior seems quite fine. Aborting the encoding
> process a little earlier would be fine, but it's hardly a critical
> issue.
There are dozens of video encoders - can we be sure that no encoder will 
crash or read out of bounds if frame res is different than expected

Gyan
Nicolas George Feb. 17, 2020, 6:50 p.m. UTC | #13 
Gyan Doshi (12020-02-18):
> There are dozens of video encoders - can we be sure that no encoder will
> crash or read out of bounds if frame res is different than expected

If they do, that's a serious problem by itself, and a protection in the
command-line tool will only hide it.

Regards,
Gyan Doshi Feb. 17, 2020, 6:56 p.m. UTC | #14 
On 18-02-2020 12:20 am, Nicolas George wrote:
> Gyan Doshi (12020-02-18):
>> There are dozens of video encoders - can we be sure that no encoder will
>> crash or read out of bounds if frame res is different than expected
> If they do, that's a serious problem by itself, and a protection in the
> command-line tool will only hide it.

Protection in the command line tool will help steer the user towards 
choosing an accommodating encoder, which is possible once the flag and a 
check is added.

Gyan
Nicolas George Feb. 17, 2020, 7:01 p.m. UTC | #15 
Gyan Doshi (12020-02-18):
> Protection in the command line tool will help steer the user towards
> choosing an accommodating encoder, which is possible once the flag and a
> check is added.

True, but minor and should not block the patch.

Regards,
Fu, Linjie March 14, 2020, 3:18 a.m. UTC | #16 
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> Nicolas George
> Sent: Tuesday, February 18, 2020 03:02
> To: FFmpeg development discussions and patches <ffmpeg-
> devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH] fftools/ffmpeg_filter: add -autoscale
> to disable/enable the default scale
> 
> Gyan Doshi (12020-02-18):
> > Protection in the command line tool will help steer the user towards
> > choosing an accommodating encoder, which is possible once the flag and a
> > check is added.
> 
> True, but minor and should not block the patch.
> 

Ping, thanks.

- Linjie
Eoff, Ullysses A June 5, 2020, 4:16 p.m. UTC | #17 
> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Fu, Linjie
> Sent: Friday, March 13, 2020 8:18 PM
> To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH] fftools/ffmpeg_filter: add -autoscale to disable/enable the default scale
> 
> > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> > Nicolas George
> > Sent: Tuesday, February 18, 2020 03:02
> > To: FFmpeg development discussions and patches <ffmpeg-
> > devel@ffmpeg.org>
> > Subject: Re: [FFmpeg-devel] [PATCH] fftools/ffmpeg_filter: add -autoscale
> > to disable/enable the default scale
> >
> > Gyan Doshi (12020-02-18):
> > > Protection in the command line tool will help steer the user towards
> > > choosing an accommodating encoder, which is possible once the flag and a
> > > check is added.
> >
> > True, but minor and should not block the patch.
> >
> 
> Ping, thanks.
> 
> - Linjie

Are all of the reviews addressed for this patch?  Is it ready to be merged, yet?

Cheers,
U. Artie

> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Fu, Linjie June 7, 2020, 4:57 a.m. UTC | #18 
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> Eoff, Ullysses A
> Sent: Saturday, June 6, 2020 00:16
> To: FFmpeg development discussions and patches <ffmpeg-
> devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH] fftools/ffmpeg_filter: add -autoscale
> to disable/enable the default scale
> 
> > -----Original Message-----
> > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> Fu, Linjie
> > Sent: Friday, March 13, 2020 8:18 PM
> > To: FFmpeg development discussions and patches <ffmpeg-
> devel@ffmpeg.org>
> > Subject: Re: [FFmpeg-devel] [PATCH] fftools/ffmpeg_filter: add -autoscale
> to disable/enable the default scale
> >
> > > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> > > Nicolas George
> > > Sent: Tuesday, February 18, 2020 03:02
> > > To: FFmpeg development discussions and patches <ffmpeg-
> > > devel@ffmpeg.org>
> > > Subject: Re: [FFmpeg-devel] [PATCH] fftools/ffmpeg_filter: add -
> autoscale
> > > to disable/enable the default scale
> > >
> > > Gyan Doshi (12020-02-18):
> > > > Protection in the command line tool will help steer the user towards
> > > > choosing an accommodating encoder, which is possible once the flag
> and a
> > > > check is added.
> > >
> > > True, but minor and should not block the patch.
> > >
> >
> > Ping, thanks.
> >
> > - Linjie
> 
> Are all of the reviews addressed for this patch?  Is it ready to be merged, yet?
> 

Yes, and there are some discussions[1] previously about disabling autoscale, since
this may lead to undefined behavior in the pipeline.

And a possible solution is to define a capability[2] and add the capability check[3] to
restrict the usage and prevent the unexpected condition.


Please help to comment.

This choice is optional for user and many users are requesting for this.
Hence if no objections, I'd like to have this set step further.

Regards,
Linjie

[1] https://patchwork.ffmpeg.org/project/ffmpeg/patch/1564461924-4772-1-git-send-email-linjie.fu@intel.com/
[2] https://patchwork.ffmpeg.org/project/ffmpeg/patch/1582820025-22302-1-git-send-email-linjie.fu@intel.com/
[3] https://patchwork.ffmpeg.org/project/ffmpeg/patch/1582820043-22426-1-git-send-email-linjie.fu@intel.com/
Fu, Linjie June 8, 2020, 9:13 a.m. UTC | #19 
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Fu,
> Linjie
> Sent: Sunday, June 7, 2020 12:58
> To: FFmpeg development discussions and patches <ffmpeg-
> devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH] fftools/ffmpeg_filter: add -autoscale
> to disable/enable the default scale
> 
> > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> > Eoff, Ullysses A
> > Sent: Saturday, June 6, 2020 00:16
> > To: FFmpeg development discussions and patches <ffmpeg-
> > devel@ffmpeg.org>
> > Subject: Re: [FFmpeg-devel] [PATCH] fftools/ffmpeg_filter: add -autoscale
> > to disable/enable the default scale
> >
> > > -----Original Message-----
> > > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> > Fu, Linjie
> > > Sent: Friday, March 13, 2020 8:18 PM
> > > To: FFmpeg development discussions and patches <ffmpeg-
> > devel@ffmpeg.org>
> > > Subject: Re: [FFmpeg-devel] [PATCH] fftools/ffmpeg_filter: add -
> autoscale
> > to disable/enable the default scale
> > >
> > > > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf
> Of
> > > > Nicolas George
> > > > Sent: Tuesday, February 18, 2020 03:02
> > > > To: FFmpeg development discussions and patches <ffmpeg-
> > > > devel@ffmpeg.org>
> > > > Subject: Re: [FFmpeg-devel] [PATCH] fftools/ffmpeg_filter: add -
> > autoscale
> > > > to disable/enable the default scale
> > > >
> > > > Gyan Doshi (12020-02-18):
> > > > > Protection in the command line tool will help steer the user towards
> > > > > choosing an accommodating encoder, which is possible once the flag
> > and a
> > > > > check is added.
> > > >
> > > > True, but minor and should not block the patch.
> > > >
> > >
> > > Ping, thanks.
> > >
> > > - Linjie
> >
> > Are all of the reviews addressed for this patch?  Is it ready to be merged,
> yet?
> >
> 
> Yes, and there are some discussions[1] previously about disabling autoscale,
> since
> this may lead to undefined behavior in the pipeline.
> 
> And a possible solution is to define a capability[2] and add the capability
> check[3] to
> restrict the usage and prevent the unexpected condition.
> 
> Please help to comment.
> 
> This choice is optional for user and many users are requesting for this.
> Hence if no objections, I'd like to have this set step further.
> 
> [1] https://patchwork.ffmpeg.org/project/ffmpeg/patch/1564461924-4772-
> 1-git-send-email-linjie.fu@intel.com/
> [2] https://patchwork.ffmpeg.org/project/ffmpeg/patch/1582820025-22302-
> 1-git-send-email-linjie.fu@intel.com/
> [3] https://patchwork.ffmpeg.org/project/ffmpeg/patch/1582820043-22426-
> 1-git-send-email-linjie.fu@intel.com/

Rebased and updated the patch set, prefer to apply soon if no objections.
https://patchwork.ffmpeg.org/project/ffmpeg/list/?series=1434

Regards,
Linjie
diff mbox series

Patch

diff --git a/doc/ffmpeg.texi b/doc/ffmpeg.texi
index 29753f0..aebafb3 100644
--- a/doc/ffmpeg.texi
+++ b/doc/ffmpeg.texi
@@ -734,10 +734,6 @@  ffmpeg -dump_attachment:t "" -i INPUT
 Technical note -- attachments are implemented as codec extradata, so this
 option can actually be used to extract extradata from any stream, not just
 attachments.
-
-@item -noautorotate
-Disable automatically rotating video based on file metadata.
-
 @end table
 
 @section Video Options
@@ -819,6 +815,18 @@  Create the filtergraph specified by @var{filtergraph} and use it to
 filter the stream.
 
 This is an alias for @code{-filter:v}, see the @ref{filter_option,,-filter option}.
+
+@item -autorotate
+Automatically rotate the video according to file metadata. Enabled by
+default, use @option{-noautorotate} to disable it.
+
+@item -autoscale
+Automatically scale the video according to the resolution of first frame.
+Enabled by default, use @option{-noautoscale} to disable it. When autoscale is
+disabled, all output frames of filter graph might not be in the same resolution
+and may be inadequate for some encoder/muxer. Therefore, it is not recommended
+to disable it unless you really know what you are doing.
+Disable autoscale at your own risk.
 @end table
 
 @section Advanced Video options
diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
index 7b6f802..8beba6c 100644
--- a/fftools/ffmpeg.h
+++ b/fftools/ffmpeg.h
@@ -230,6 +230,8 @@  typedef struct OptionsContext {
     int        nb_time_bases;
     SpecifierOpt *enc_time_bases;
     int        nb_enc_time_bases;
+    SpecifierOpt *autoscale;
+    int        nb_autoscale;
 } OptionsContext;
 
 typedef struct InputFilter {
@@ -479,6 +481,7 @@  typedef struct OutputStream {
     int force_fps;
     int top_field_first;
     int rotate_overridden;
+    int autoscale;
     double rotate_override_value;
 
     AVRational frame_aspect_ratio;
diff --git a/fftools/ffmpeg_filter.c b/fftools/ffmpeg_filter.c
index 40cc4c1..46c8ea8 100644
--- a/fftools/ffmpeg_filter.c
+++ b/fftools/ffmpeg_filter.c
@@ -469,7 +469,7 @@  static int configure_output_video_filter(FilterGraph *fg, OutputFilter *ofilter,
     if (ret < 0)
         return ret;
 
-    if (ofilter->width || ofilter->height) {
+    if ((ofilter->width || ofilter->height) && ofilter->ost->autoscale) {
         char args[255];
         AVFilterContext *filter;
         AVDictionaryEntry *e = NULL;
diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
index 12d4488..a6f4216 100644
--- a/fftools/ffmpeg_opt.c
+++ b/fftools/ffmpeg_opt.c
@@ -1405,6 +1405,8 @@  static OutputStream *new_output_stream(OptionsContext *o, AVFormatContext *oc, e
         ost->encoder_opts  = filter_codec_opts(o->g->codec_opts, ost->enc->id, oc, st, ost->enc);
 
         MATCH_PER_STREAM_OPT(presets, str, preset, oc, st);
+        ost->autoscale = 1;
+        MATCH_PER_STREAM_OPT(autoscale, i, ost->autoscale, oc, st);
         if (preset && (!(ret = get_preset_file_2(preset, ost->enc->name, &s)))) {
             do  {
                 buf = get_line(s);
@@ -3650,6 +3652,9 @@  const OptionDef options[] = {
     { "autorotate",       HAS_ARG | OPT_BOOL | OPT_SPEC |
                           OPT_EXPERT | OPT_INPUT,                                { .off = OFFSET(autorotate) },
         "automatically insert correct rotate filters" },
+    { "autoscale",        HAS_ARG | OPT_BOOL | OPT_SPEC |
+                          OPT_EXPERT | OPT_OUTPUT,                               { .off = OFFSET(autoscale) },
+        "automatically insert a scale filter at the end of the filter graph" },
 
     /* audio options */
     { "aframes",        OPT_AUDIO | HAS_ARG  | OPT_PERFILE | OPT_OUTPUT,           { .func_arg = opt_audio_frames },