diff mbox series

[FFmpeg-devel,v2] ffmpeg: add -rmax to clamp output framerate

Message ID 20210201132227.6535-1-ffmpeg@gyani.pro
State New
Headers show
Series [FFmpeg-devel,v2] ffmpeg: add -rmax to clamp output framerate | expand

Checks

Context Check Description
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

Gyan Doshi Feb. 1, 2021, 1:22 p.m. UTC 
Useful when encoding in batch or with aberrant inputs.
---
 doc/ffmpeg.texi      |  7 +++++++
 fftools/ffmpeg.c     |  7 ++++++-
 fftools/ffmpeg.h     |  3 +++
 fftools/ffmpeg_opt.c | 24 +++++++++++++++++++++---
 4 files changed, 37 insertions(+), 4 deletions(-)

Forgot to nullify rmax with r set.

Comments

Gyan Doshi Feb. 3, 2021, 4:08 a.m. UTC | #1 
Plan to push this in a day.

On 01-02-2021 06:52 pm, Gyan Doshi wrote:
> Useful when encoding in batch or with aberrant inputs.
> ---
>   doc/ffmpeg.texi      |  7 +++++++
>   fftools/ffmpeg.c     |  7 ++++++-
>   fftools/ffmpeg.h     |  3 +++
>   fftools/ffmpeg_opt.c | 24 +++++++++++++++++++++---
>   4 files changed, 37 insertions(+), 4 deletions(-)
>
> Forgot to nullify rmax with r set.
>
> diff --git a/doc/ffmpeg.texi b/doc/ffmpeg.texi
> index 8eb012b7c0..7726f25082 100644
> --- a/doc/ffmpeg.texi
> +++ b/doc/ffmpeg.texi
> @@ -759,6 +759,13 @@ If in doubt use @option{-framerate} instead of the input option @option{-r}.
>   As an output option, duplicate or drop input frames to achieve constant output
>   frame rate @var{fps}.
>   
> +@item -rmax[:@var{stream_specifier}] @var{fps} (@emph{output,per-stream})
> +Set maximum frame rate (Hz value, fraction or abbreviation).
> +
> +Clamps output frame rate when output framerate is auto-set and is higher than this value.
> +Useful in batch processing or when input framerate is wrongly detected as very high.
> +Ignored when either @code{-r} is set or during streamcopy.
> +
>   @item -s[:@var{stream_specifier}] @var{size} (@emph{input/output,per-stream})
>   Set frame size.
>   
> diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
> index d7c833be63..add5a3e505 100644
> --- a/fftools/ffmpeg.c
> +++ b/fftools/ffmpeg.c
> @@ -3376,7 +3376,7 @@ static int init_output_stream_encode(OutputStream *ost, AVFrame *frame)
>               ost->frame_rate = ist->framerate;
>           if (ist && !ost->frame_rate.num)
>               ost->frame_rate = ist->st->r_frame_rate;
> -        if (ist && !ost->frame_rate.num) {
> +        if (ist && !ost->frame_rate.num && !ost->max_frame_rate.num) {
>               ost->frame_rate = (AVRational){25, 1};
>               av_log(NULL, AV_LOG_WARNING,
>                      "No information "
> @@ -3386,6 +3386,11 @@ static int init_output_stream_encode(OutputStream *ost, AVFrame *frame)
>                      ost->file_index, ost->index);
>           }
>   
> +        if (ost->max_frame_rate.num &&
> +            (av_q2d(ost->frame_rate) > av_q2d(ost->max_frame_rate) ||
> +            !ost->frame_rate.den))
> +            ost->frame_rate = ost->max_frame_rate;
> +
>           if (ost->enc->supported_framerates && !ost->force_fps) {
>               int idx = av_find_nearest_q_idx(ost->frame_rate, ost->enc->supported_framerates);
>               ost->frame_rate = ost->enc->supported_framerates[idx];
> diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
> index 8046e75026..3662130da4 100644
> --- a/fftools/ffmpeg.h
> +++ b/fftools/ffmpeg.h
> @@ -108,6 +108,8 @@ typedef struct OptionsContext {
>       int        nb_audio_sample_rate;
>       SpecifierOpt *frame_rates;
>       int        nb_frame_rates;
> +    SpecifierOpt *max_frame_rates;
> +    int        nb_max_frame_rates;
>       SpecifierOpt *frame_sizes;
>       int        nb_frame_sizes;
>       SpecifierOpt *frame_pix_fmts;
> @@ -479,6 +481,7 @@ typedef struct OutputStream {
>   
>       /* video only */
>       AVRational frame_rate;
> +    AVRational max_frame_rate;
>       int is_cfr;
>       int force_fps;
>       int top_field_first;
> diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
> index bf2eb26246..2080499e65 100644
> --- a/fftools/ffmpeg_opt.c
> +++ b/fftools/ffmpeg_opt.c
> @@ -55,6 +55,7 @@ static const char *const opt_name_codec_names[]               = {"c", "codec", "
>   static const char *const opt_name_audio_channels[]            = {"ac", NULL};
>   static const char *const opt_name_audio_sample_rate[]         = {"ar", NULL};
>   static const char *const opt_name_frame_rates[]               = {"r", NULL};
> +static const char *const opt_name_max_frame_rates[]           = {"rmax", NULL};
>   static const char *const opt_name_frame_sizes[]               = {"s", NULL};
>   static const char *const opt_name_frame_pix_fmts[]            = {"pix_fmt", NULL};
>   static const char *const opt_name_ts_scale[]                  = {"itsscale", NULL};
> @@ -1688,7 +1689,7 @@ static OutputStream *new_video_stream(OptionsContext *o, AVFormatContext *oc, in
>       AVStream *st;
>       OutputStream *ost;
>       AVCodecContext *video_enc;
> -    char *frame_rate = NULL, *frame_aspect_ratio = NULL;
> +    char *frame_rate = NULL, *max_frame_rate = NULL, *frame_aspect_ratio = NULL;
>   
>       ost = new_output_stream(o, oc, AVMEDIA_TYPE_VIDEO, source_index);
>       st  = ost->st;
> @@ -1699,8 +1700,22 @@ static OutputStream *new_video_stream(OptionsContext *o, AVFormatContext *oc, in
>           av_log(NULL, AV_LOG_FATAL, "Invalid framerate value: %s\n", frame_rate);
>           exit_program(1);
>       }
> -    if (frame_rate && video_sync_method == VSYNC_PASSTHROUGH)
> -        av_log(NULL, AV_LOG_ERROR, "Using -vsync 0 and -r can produce invalid output files\n");
> +
> +    MATCH_PER_STREAM_OPT(max_frame_rates, str, max_frame_rate, oc, st);
> +    if (max_frame_rate && av_parse_video_rate(&ost->max_frame_rate, max_frame_rate) < 0) {
> +        av_log(NULL, AV_LOG_FATAL, "Invalid maximum framerate value: %s\n", max_frame_rate);
> +        exit_program(1);
> +    }
> +
> +    if (frame_rate && max_frame_rate) {
> +        av_log(NULL, AV_LOG_WARNING, "-rmax for stream %d:%d will have no effect as"
> +               " -r is also set.\n", ost->file_index, ost->index);
> +        ost->max_frame_rate = (AVRational){0, 1};
> +    }
> +
> +    if ((frame_rate || max_frame_rate) &&
> +        video_sync_method == VSYNC_PASSTHROUGH)
> +        av_log(NULL, AV_LOG_ERROR, "Using -vsync 0 and -r/-rmax can produce invalid output files\n");
>   
>       MATCH_PER_STREAM_OPT(frame_aspect_ratios, str, frame_aspect_ratio, oc, st);
>       if (frame_aspect_ratio) {
> @@ -3596,6 +3611,9 @@ const OptionDef options[] = {
>       { "r",            OPT_VIDEO | HAS_ARG  | OPT_STRING | OPT_SPEC |
>                         OPT_INPUT | OPT_OUTPUT,                                    { .off = OFFSET(frame_rates) },
>           "set frame rate (Hz value, fraction or abbreviation)", "rate" },
> +    { "rmax",         OPT_VIDEO | HAS_ARG  | OPT_STRING | OPT_SPEC |
> +                      OPT_OUTPUT,                                                { .off = OFFSET(max_frame_rates) },
> +        "set max frame rate (Hz value, fraction or abbreviation)", "rate" },
>       { "s",            OPT_VIDEO | HAS_ARG | OPT_SUBTITLE | OPT_STRING | OPT_SPEC |
>                         OPT_INPUT | OPT_OUTPUT,                                    { .off = OFFSET(frame_sizes) },
>           "set frame size (WxH or abbreviation)", "size" },
James Almer Feb. 3, 2021, 4:17 a.m. UTC | #2 
On 2/3/2021 1:08 AM, Gyan Doshi wrote:
> Plan to push this in a day.

It's been a day since you submitted this patch, so you can give people 
some more time to look at it. There's no hurry.

> 
> On 01-02-2021 06:52 pm, Gyan Doshi wrote:
>> Useful when encoding in batch or with aberrant inputs.
>> ---
>>   doc/ffmpeg.texi      |  7 +++++++
>>   fftools/ffmpeg.c     |  7 ++++++-
>>   fftools/ffmpeg.h     |  3 +++
>>   fftools/ffmpeg_opt.c | 24 +++++++++++++++++++++---
>>   4 files changed, 37 insertions(+), 4 deletions(-)
>>
>> Forgot to nullify rmax with r set.
>>
>> diff --git a/doc/ffmpeg.texi b/doc/ffmpeg.texi
>> index 8eb012b7c0..7726f25082 100644
>> --- a/doc/ffmpeg.texi
>> +++ b/doc/ffmpeg.texi
>> @@ -759,6 +759,13 @@ If in doubt use @option{-framerate} instead of 
>> the input option @option{-r}.
>>   As an output option, duplicate or drop input frames to achieve 
>> constant output
>>   frame rate @var{fps}.
>> +@item -rmax[:@var{stream_specifier}] @var{fps} 
>> (@emph{output,per-stream})
>> +Set maximum frame rate (Hz value, fraction or abbreviation).
>> +
>> +Clamps output frame rate when output framerate is auto-set and is 
>> higher than this value.
>> +Useful in batch processing or when input framerate is wrongly 
>> detected as very high.
>> +Ignored when either @code{-r} is set or during streamcopy.
>> +
>>   @item -s[:@var{stream_specifier}] @var{size} 
>> (@emph{input/output,per-stream})
>>   Set frame size.
>> diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
>> index d7c833be63..add5a3e505 100644
>> --- a/fftools/ffmpeg.c
>> +++ b/fftools/ffmpeg.c
>> @@ -3376,7 +3376,7 @@ static int 
>> init_output_stream_encode(OutputStream *ost, AVFrame *frame)
>>               ost->frame_rate = ist->framerate;
>>           if (ist && !ost->frame_rate.num)
>>               ost->frame_rate = ist->st->r_frame_rate;
>> -        if (ist && !ost->frame_rate.num) {
>> +        if (ist && !ost->frame_rate.num && !ost->max_frame_rate.num) {
>>               ost->frame_rate = (AVRational){25, 1};
>>               av_log(NULL, AV_LOG_WARNING,
>>                      "No information "
>> @@ -3386,6 +3386,11 @@ static int 
>> init_output_stream_encode(OutputStream *ost, AVFrame *frame)
>>                      ost->file_index, ost->index);
>>           }
>> +        if (ost->max_frame_rate.num &&
>> +            (av_q2d(ost->frame_rate) > av_q2d(ost->max_frame_rate) ||
>> +            !ost->frame_rate.den))
>> +            ost->frame_rate = ost->max_frame_rate;
>> +
>>           if (ost->enc->supported_framerates && !ost->force_fps) {
>>               int idx = av_find_nearest_q_idx(ost->frame_rate, 
>> ost->enc->supported_framerates);
>>               ost->frame_rate = ost->enc->supported_framerates[idx];
>> diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
>> index 8046e75026..3662130da4 100644
>> --- a/fftools/ffmpeg.h
>> +++ b/fftools/ffmpeg.h
>> @@ -108,6 +108,8 @@ typedef struct OptionsContext {
>>       int        nb_audio_sample_rate;
>>       SpecifierOpt *frame_rates;
>>       int        nb_frame_rates;
>> +    SpecifierOpt *max_frame_rates;
>> +    int        nb_max_frame_rates;
>>       SpecifierOpt *frame_sizes;
>>       int        nb_frame_sizes;
>>       SpecifierOpt *frame_pix_fmts;
>> @@ -479,6 +481,7 @@ typedef struct OutputStream {
>>       /* video only */
>>       AVRational frame_rate;
>> +    AVRational max_frame_rate;
>>       int is_cfr;
>>       int force_fps;
>>       int top_field_first;
>> diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
>> index bf2eb26246..2080499e65 100644
>> --- a/fftools/ffmpeg_opt.c
>> +++ b/fftools/ffmpeg_opt.c
>> @@ -55,6 +55,7 @@ static const char *const 
>> opt_name_codec_names[]               = {"c", "codec", "
>>   static const char *const opt_name_audio_channels[]            = 
>> {"ac", NULL};
>>   static const char *const opt_name_audio_sample_rate[]         = 
>> {"ar", NULL};
>>   static const char *const opt_name_frame_rates[]               = 
>> {"r", NULL};
>> +static const char *const opt_name_max_frame_rates[]           = 
>> {"rmax", NULL};
>>   static const char *const opt_name_frame_sizes[]               = 
>> {"s", NULL};
>>   static const char *const opt_name_frame_pix_fmts[]            = 
>> {"pix_fmt", NULL};
>>   static const char *const opt_name_ts_scale[]                  = 
>> {"itsscale", NULL};
>> @@ -1688,7 +1689,7 @@ static OutputStream 
>> *new_video_stream(OptionsContext *o, AVFormatContext *oc, in
>>       AVStream *st;
>>       OutputStream *ost;
>>       AVCodecContext *video_enc;
>> -    char *frame_rate = NULL, *frame_aspect_ratio = NULL;
>> +    char *frame_rate = NULL, *max_frame_rate = NULL, 
>> *frame_aspect_ratio = NULL;
>>       ost = new_output_stream(o, oc, AVMEDIA_TYPE_VIDEO, source_index);
>>       st  = ost->st;
>> @@ -1699,8 +1700,22 @@ static OutputStream 
>> *new_video_stream(OptionsContext *o, AVFormatContext *oc, in
>>           av_log(NULL, AV_LOG_FATAL, "Invalid framerate value: %s\n", 
>> frame_rate);
>>           exit_program(1);
>>       }
>> -    if (frame_rate && video_sync_method == VSYNC_PASSTHROUGH)
>> -        av_log(NULL, AV_LOG_ERROR, "Using -vsync 0 and -r can produce 
>> invalid output files\n");
>> +
>> +    MATCH_PER_STREAM_OPT(max_frame_rates, str, max_frame_rate, oc, st);
>> +    if (max_frame_rate && av_parse_video_rate(&ost->max_frame_rate, 
>> max_frame_rate) < 0) {
>> +        av_log(NULL, AV_LOG_FATAL, "Invalid maximum framerate value: 
>> %s\n", max_frame_rate);
>> +        exit_program(1);
>> +    }
>> +
>> +    if (frame_rate && max_frame_rate) {
>> +        av_log(NULL, AV_LOG_WARNING, "-rmax for stream %d:%d will 
>> have no effect as"
>> +               " -r is also set.\n", ost->file_index, ost->index);
>> +        ost->max_frame_rate = (AVRational){0, 1};
>> +    }
>> +
>> +    if ((frame_rate || max_frame_rate) &&
>> +        video_sync_method == VSYNC_PASSTHROUGH)
>> +        av_log(NULL, AV_LOG_ERROR, "Using -vsync 0 and -r/-rmax can 
>> produce invalid output files\n");
>>       MATCH_PER_STREAM_OPT(frame_aspect_ratios, str, 
>> frame_aspect_ratio, oc, st);
>>       if (frame_aspect_ratio) {
>> @@ -3596,6 +3611,9 @@ const OptionDef options[] = {
>>       { "r",            OPT_VIDEO | HAS_ARG  | OPT_STRING | OPT_SPEC |
>>                         OPT_INPUT | 
>> OPT_OUTPUT,                                    { .off = 
>> OFFSET(frame_rates) },
>>           "set frame rate (Hz value, fraction or abbreviation)", 
>> "rate" },
>> +    { "rmax",         OPT_VIDEO | HAS_ARG  | OPT_STRING | OPT_SPEC |
>> +                      
>> OPT_OUTPUT,                                                { .off = 
>> OFFSET(max_frame_rates) },
>> +        "set max frame rate (Hz value, fraction or abbreviation)", 
>> "rate" },
>>       { "s",            OPT_VIDEO | HAS_ARG | OPT_SUBTITLE | 
>> OPT_STRING | OPT_SPEC |
>>                         OPT_INPUT | 
>> OPT_OUTPUT,                                    { .off = 
>> OFFSET(frame_sizes) },
>>           "set frame size (WxH or abbreviation)", "size" },
> 
> _______________________________________________
> 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".
Anton Khirnov Feb. 3, 2021, 8:11 a.m. UTC | #3 
Quoting Gyan Doshi (2021-02-01 14:22:27)
> Useful when encoding in batch or with aberrant inputs.
> ---
>  doc/ffmpeg.texi      |  7 +++++++
>  fftools/ffmpeg.c     |  7 ++++++-
>  fftools/ffmpeg.h     |  3 +++
>  fftools/ffmpeg_opt.c | 24 +++++++++++++++++++++---
>  4 files changed, 37 insertions(+), 4 deletions(-)
> 
> Forgot to nullify rmax with r set.
> 
> diff --git a/doc/ffmpeg.texi b/doc/ffmpeg.texi
> index 8eb012b7c0..7726f25082 100644
> --- a/doc/ffmpeg.texi
> +++ b/doc/ffmpeg.texi
> @@ -759,6 +759,13 @@ If in doubt use @option{-framerate} instead of the input option @option{-r}.
>  As an output option, duplicate or drop input frames to achieve constant output
>  frame rate @var{fps}.
>  
> +@item -rmax[:@var{stream_specifier}] @var{fps} (@emph{output,per-stream})

Name could be more descriptive, "rmax" does not tell you much. How about
-framerate_clamp or something along these lines?
Anton Khirnov Feb. 3, 2021, 8:13 a.m. UTC | #4 
Quoting Gyan Doshi (2021-02-01 14:22:27)
> @@ -1699,8 +1700,22 @@ static OutputStream *new_video_stream(OptionsContext *o, AVFormatContext *oc, in
>          av_log(NULL, AV_LOG_FATAL, "Invalid framerate value: %s\n", frame_rate);
>          exit_program(1);
>      }
> -    if (frame_rate && video_sync_method == VSYNC_PASSTHROUGH)
> -        av_log(NULL, AV_LOG_ERROR, "Using -vsync 0 and -r can produce invalid output files\n");
> +
> +    MATCH_PER_STREAM_OPT(max_frame_rates, str, max_frame_rate, oc, st);
> +    if (max_frame_rate && av_parse_video_rate(&ost->max_frame_rate, max_frame_rate) < 0) {
> +        av_log(NULL, AV_LOG_FATAL, "Invalid maximum framerate value: %s\n", max_frame_rate);
> +        exit_program(1);
> +    }
> +
> +    if (frame_rate && max_frame_rate) {
> +        av_log(NULL, AV_LOG_WARNING, "-rmax for stream %d:%d will have no effect as"
> +               " -r is also set.\n", ost->file_index, ost->index);
> +        ost->max_frame_rate = (AVRational){0, 1};

Invalid option combinations should be an error.
Gyan Doshi Feb. 3, 2021, 8:15 a.m. UTC | #5 
On 03-02-2021 01:41 pm, Anton Khirnov wrote:
> Quoting Gyan Doshi (2021-02-01 14:22:27)
>> Useful when encoding in batch or with aberrant inputs.
>> ---
>>   doc/ffmpeg.texi      |  7 +++++++
>>   fftools/ffmpeg.c     |  7 ++++++-
>>   fftools/ffmpeg.h     |  3 +++
>>   fftools/ffmpeg_opt.c | 24 +++++++++++++++++++++---
>>   4 files changed, 37 insertions(+), 4 deletions(-)
>>
>> Forgot to nullify rmax with r set.
>>
>> diff --git a/doc/ffmpeg.texi b/doc/ffmpeg.texi
>> index 8eb012b7c0..7726f25082 100644
>> --- a/doc/ffmpeg.texi
>> +++ b/doc/ffmpeg.texi
>> @@ -759,6 +759,13 @@ If in doubt use @option{-framerate} instead of the input option @option{-r}.
>>   As an output option, duplicate or drop input frames to achieve constant output
>>   frame rate @var{fps}.
>>   
>> +@item -rmax[:@var{stream_specifier}] @var{fps} (@emph{output,per-stream})
> Name could be more descriptive, "rmax" does not tell you much. How about
> -framerate_clamp or something along these lines?
>
We use -r for video framerate, so -rmax is intuitive. But -fpsmax can 
also work.

Regards,
Gyan

P.S. Will make both -r/-rmax an error.
Anton Khirnov Feb. 3, 2021, 9:50 a.m. UTC | #6 
Quoting Gyan Doshi (2021-02-03 09:15:35)
> 
> 
> On 03-02-2021 01:41 pm, Anton Khirnov wrote:
> > Quoting Gyan Doshi (2021-02-01 14:22:27)
> >> Useful when encoding in batch or with aberrant inputs.
> >> ---
> >>   doc/ffmpeg.texi      |  7 +++++++
> >>   fftools/ffmpeg.c     |  7 ++++++-
> >>   fftools/ffmpeg.h     |  3 +++
> >>   fftools/ffmpeg_opt.c | 24 +++++++++++++++++++++---
> >>   4 files changed, 37 insertions(+), 4 deletions(-)
> >>
> >> Forgot to nullify rmax with r set.
> >>
> >> diff --git a/doc/ffmpeg.texi b/doc/ffmpeg.texi
> >> index 8eb012b7c0..7726f25082 100644
> >> --- a/doc/ffmpeg.texi
> >> +++ b/doc/ffmpeg.texi
> >> @@ -759,6 +759,13 @@ If in doubt use @option{-framerate} instead of the input option @option{-r}.
> >>   As an output option, duplicate or drop input frames to achieve constant output
> >>   frame rate @var{fps}.
> >>   
> >> +@item -rmax[:@var{stream_specifier}] @var{fps} (@emph{output,per-stream})
> > Name could be more descriptive, "rmax" does not tell you much. How about
> > -framerate_clamp or something along these lines?
> >
> We use -r for video framerate, so -rmax is intuitive. But -fpsmax can 
> also work.

IIUC this option is useful only under quite specific circumstances. In
such a case, I think it's better to use longer more descriptive names.
Gyan Doshi Feb. 3, 2021, 9:58 a.m. UTC | #7 
On 03-02-2021 03:20 pm, Anton Khirnov wrote:
> Quoting Gyan Doshi (2021-02-03 09:15:35)
>>
>> On 03-02-2021 01:41 pm, Anton Khirnov wrote:
>>> Quoting Gyan Doshi (2021-02-01 14:22:27)
>>>> Useful when encoding in batch or with aberrant inputs.
>>>> ---
>>>>    doc/ffmpeg.texi      |  7 +++++++
>>>>    fftools/ffmpeg.c     |  7 ++++++-
>>>>    fftools/ffmpeg.h     |  3 +++
>>>>    fftools/ffmpeg_opt.c | 24 +++++++++++++++++++++---
>>>>    4 files changed, 37 insertions(+), 4 deletions(-)
>>>>
>>>> Forgot to nullify rmax with r set.
>>>>
>>>> diff --git a/doc/ffmpeg.texi b/doc/ffmpeg.texi
>>>> index 8eb012b7c0..7726f25082 100644
>>>> --- a/doc/ffmpeg.texi
>>>> +++ b/doc/ffmpeg.texi
>>>> @@ -759,6 +759,13 @@ If in doubt use @option{-framerate} instead of the input option @option{-r}.
>>>>    As an output option, duplicate or drop input frames to achieve constant output
>>>>    frame rate @var{fps}.
>>>>    
>>>> +@item -rmax[:@var{stream_specifier}] @var{fps} (@emph{output,per-stream})
>>> Name could be more descriptive, "rmax" does not tell you much. How about
>>> -framerate_clamp or something along these lines?
>>>
>> We use -r for video framerate, so -rmax is intuitive. But -fpsmax can
>> also work.
> IIUC this option is useful only under quite specific circumstances. In
> such a case, I think it's better to use longer more descriptive names.
How about fpsmax? framerate_clamp is too long.

Gyan

>
Anton Khirnov Feb. 3, 2021, 11 a.m. UTC | #8 
Quoting Gyan Doshi (2021-02-03 10:58:36)
> 
> 
> On 03-02-2021 03:20 pm, Anton Khirnov wrote:
> > Quoting Gyan Doshi (2021-02-03 09:15:35)
> >>
> >> On 03-02-2021 01:41 pm, Anton Khirnov wrote:
> >>> Quoting Gyan Doshi (2021-02-01 14:22:27)
> >>>> Useful when encoding in batch or with aberrant inputs.
> >>>> ---
> >>>>    doc/ffmpeg.texi      |  7 +++++++
> >>>>    fftools/ffmpeg.c     |  7 ++++++-
> >>>>    fftools/ffmpeg.h     |  3 +++
> >>>>    fftools/ffmpeg_opt.c | 24 +++++++++++++++++++++---
> >>>>    4 files changed, 37 insertions(+), 4 deletions(-)
> >>>>
> >>>> Forgot to nullify rmax with r set.
> >>>>
> >>>> diff --git a/doc/ffmpeg.texi b/doc/ffmpeg.texi
> >>>> index 8eb012b7c0..7726f25082 100644
> >>>> --- a/doc/ffmpeg.texi
> >>>> +++ b/doc/ffmpeg.texi
> >>>> @@ -759,6 +759,13 @@ If in doubt use @option{-framerate} instead of the input option @option{-r}.
> >>>>    As an output option, duplicate or drop input frames to achieve constant output
> >>>>    frame rate @var{fps}.
> >>>>    
> >>>> +@item -rmax[:@var{stream_specifier}] @var{fps} (@emph{output,per-stream})
> >>> Name could be more descriptive, "rmax" does not tell you much. How about
> >>> -framerate_clamp or something along these lines?
> >>>
> >> We use -r for video framerate, so -rmax is intuitive. But -fpsmax can
> >> also work.
> > IIUC this option is useful only under quite specific circumstances. In
> > such a case, I think it's better to use longer more descriptive names.
> How about fpsmax? framerate_clamp is too long.

Fine with me.
diff mbox series

Patch

diff --git a/doc/ffmpeg.texi b/doc/ffmpeg.texi
index 8eb012b7c0..7726f25082 100644
--- a/doc/ffmpeg.texi
+++ b/doc/ffmpeg.texi
@@ -759,6 +759,13 @@  If in doubt use @option{-framerate} instead of the input option @option{-r}.
 As an output option, duplicate or drop input frames to achieve constant output
 frame rate @var{fps}.
 
+@item -rmax[:@var{stream_specifier}] @var{fps} (@emph{output,per-stream})
+Set maximum frame rate (Hz value, fraction or abbreviation).
+
+Clamps output frame rate when output framerate is auto-set and is higher than this value.
+Useful in batch processing or when input framerate is wrongly detected as very high.
+Ignored when either @code{-r} is set or during streamcopy.
+
 @item -s[:@var{stream_specifier}] @var{size} (@emph{input/output,per-stream})
 Set frame size.
 
diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
index d7c833be63..add5a3e505 100644
--- a/fftools/ffmpeg.c
+++ b/fftools/ffmpeg.c
@@ -3376,7 +3376,7 @@  static int init_output_stream_encode(OutputStream *ost, AVFrame *frame)
             ost->frame_rate = ist->framerate;
         if (ist && !ost->frame_rate.num)
             ost->frame_rate = ist->st->r_frame_rate;
-        if (ist && !ost->frame_rate.num) {
+        if (ist && !ost->frame_rate.num && !ost->max_frame_rate.num) {
             ost->frame_rate = (AVRational){25, 1};
             av_log(NULL, AV_LOG_WARNING,
                    "No information "
@@ -3386,6 +3386,11 @@  static int init_output_stream_encode(OutputStream *ost, AVFrame *frame)
                    ost->file_index, ost->index);
         }
 
+        if (ost->max_frame_rate.num &&
+            (av_q2d(ost->frame_rate) > av_q2d(ost->max_frame_rate) ||
+            !ost->frame_rate.den))
+            ost->frame_rate = ost->max_frame_rate;
+
         if (ost->enc->supported_framerates && !ost->force_fps) {
             int idx = av_find_nearest_q_idx(ost->frame_rate, ost->enc->supported_framerates);
             ost->frame_rate = ost->enc->supported_framerates[idx];
diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
index 8046e75026..3662130da4 100644
--- a/fftools/ffmpeg.h
+++ b/fftools/ffmpeg.h
@@ -108,6 +108,8 @@  typedef struct OptionsContext {
     int        nb_audio_sample_rate;
     SpecifierOpt *frame_rates;
     int        nb_frame_rates;
+    SpecifierOpt *max_frame_rates;
+    int        nb_max_frame_rates;
     SpecifierOpt *frame_sizes;
     int        nb_frame_sizes;
     SpecifierOpt *frame_pix_fmts;
@@ -479,6 +481,7 @@  typedef struct OutputStream {
 
     /* video only */
     AVRational frame_rate;
+    AVRational max_frame_rate;
     int is_cfr;
     int force_fps;
     int top_field_first;
diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
index bf2eb26246..2080499e65 100644
--- a/fftools/ffmpeg_opt.c
+++ b/fftools/ffmpeg_opt.c
@@ -55,6 +55,7 @@  static const char *const opt_name_codec_names[]               = {"c", "codec", "
 static const char *const opt_name_audio_channels[]            = {"ac", NULL};
 static const char *const opt_name_audio_sample_rate[]         = {"ar", NULL};
 static const char *const opt_name_frame_rates[]               = {"r", NULL};
+static const char *const opt_name_max_frame_rates[]           = {"rmax", NULL};
 static const char *const opt_name_frame_sizes[]               = {"s", NULL};
 static const char *const opt_name_frame_pix_fmts[]            = {"pix_fmt", NULL};
 static const char *const opt_name_ts_scale[]                  = {"itsscale", NULL};
@@ -1688,7 +1689,7 @@  static OutputStream *new_video_stream(OptionsContext *o, AVFormatContext *oc, in
     AVStream *st;
     OutputStream *ost;
     AVCodecContext *video_enc;
-    char *frame_rate = NULL, *frame_aspect_ratio = NULL;
+    char *frame_rate = NULL, *max_frame_rate = NULL, *frame_aspect_ratio = NULL;
 
     ost = new_output_stream(o, oc, AVMEDIA_TYPE_VIDEO, source_index);
     st  = ost->st;
@@ -1699,8 +1700,22 @@  static OutputStream *new_video_stream(OptionsContext *o, AVFormatContext *oc, in
         av_log(NULL, AV_LOG_FATAL, "Invalid framerate value: %s\n", frame_rate);
         exit_program(1);
     }
-    if (frame_rate && video_sync_method == VSYNC_PASSTHROUGH)
-        av_log(NULL, AV_LOG_ERROR, "Using -vsync 0 and -r can produce invalid output files\n");
+
+    MATCH_PER_STREAM_OPT(max_frame_rates, str, max_frame_rate, oc, st);
+    if (max_frame_rate && av_parse_video_rate(&ost->max_frame_rate, max_frame_rate) < 0) {
+        av_log(NULL, AV_LOG_FATAL, "Invalid maximum framerate value: %s\n", max_frame_rate);
+        exit_program(1);
+    }
+
+    if (frame_rate && max_frame_rate) {
+        av_log(NULL, AV_LOG_WARNING, "-rmax for stream %d:%d will have no effect as"
+               " -r is also set.\n", ost->file_index, ost->index);
+        ost->max_frame_rate = (AVRational){0, 1};
+    }
+
+    if ((frame_rate || max_frame_rate) &&
+        video_sync_method == VSYNC_PASSTHROUGH)
+        av_log(NULL, AV_LOG_ERROR, "Using -vsync 0 and -r/-rmax can produce invalid output files\n");
 
     MATCH_PER_STREAM_OPT(frame_aspect_ratios, str, frame_aspect_ratio, oc, st);
     if (frame_aspect_ratio) {
@@ -3596,6 +3611,9 @@  const OptionDef options[] = {
     { "r",            OPT_VIDEO | HAS_ARG  | OPT_STRING | OPT_SPEC |
                       OPT_INPUT | OPT_OUTPUT,                                    { .off = OFFSET(frame_rates) },
         "set frame rate (Hz value, fraction or abbreviation)", "rate" },
+    { "rmax",         OPT_VIDEO | HAS_ARG  | OPT_STRING | OPT_SPEC |
+                      OPT_OUTPUT,                                                { .off = OFFSET(max_frame_rates) },
+        "set max frame rate (Hz value, fraction or abbreviation)", "rate" },
     { "s",            OPT_VIDEO | HAS_ARG | OPT_SUBTITLE | OPT_STRING | OPT_SPEC |
                       OPT_INPUT | OPT_OUTPUT,                                    { .off = OFFSET(frame_sizes) },
         "set frame size (WxH or abbreviation)", "size" },