[FFmpeg-devel] [PATCH v2] doc/filters.texi: complete rewrite of fps filter doc, v2.

[Carl Eugen Hoyos]

Am Fr., 1. Mai 2020 um 06:05 Uhr schrieb <list+ffmpeg-dev at jdlh.com>:
>
> From: Jim DeLaHunt <from+ffmpeg-dev at jdlh.com>
>
> Fix unclear wording and spelling mistakes based on review.

> Reduce overall word count by 11%.

This is heavily misleading and definitely not ok.

> Ready for patch review.
>
> No other docs, and no executable code, are changed.
>
> Signed-off-by: Jim DeLaHunt <from+ffmpeg-dev at jdlh.com>
> ---
>  doc/filters.texi | 157 +++++++++++++++++++++++++++++++++++++++++------
>  1 file changed, 138 insertions(+), 19 deletions(-)
>
> diff --git a/doc/filters.texi b/doc/filters.texi
> index d19fd346ae..0d7d15c448 100644
> --- a/doc/filters.texi
> +++ b/doc/filters.texi
> @@ -11194,25 +11194,30 @@ format=pix_fmts=yuv420p|yuv444p|yuv410p
>  @anchor{fps}
>  @section fps

> -Convert the video to specified constant frame rate by duplicating or dropping
> -frames as necessary.
> +Make a new video from the frames and presentation time stamps (PTSs) of the

While I am not a native speaker, the idea that the fps filter "makes a
new video"
is absurd imo (independently of what I may have written in the past, the
documentation has higher standards than the user mailing list).

> +input. The new video has a specified constant frame rate, and new PTSs. It
> +generally keeps frames from the old video, but might repeat or drop some frames.
> +You can choose the method for rounding from input PTS to output PTS. This
> +affects which frames @var{fps} keeps, repeats, or drops.
>
>  It accepts the following parameters:
>  @table @option
>
>  @item fps
> -The desired output frame rate. The default is @code{25}.
> +The output frame rate, in frames per second. May be an integer, real, or
> +rational number, or an abbreviation. The default is @code{25}.

I seriously wonder who really profits from this change but it may be ok.

>  @item start_time
> -Assume the first PTS should be the given value, in seconds. This allows for
> -padding/trimming at the start of stream. By default, no assumption is made
> -about the first frame's expected PTS, so no padding or trimming is done.
> -For example, this could be set to 0 to pad the beginning with duplicates of
> -the first frame if a video stream starts after the audio stream or to trim any
> -frames with a negative PTS.

> +A time, in seconds from the start of the input stream

I don't think this is correct, somebody has to confirm this before the change
is acceptable.
Since it contradicts the text above, this should be explicitely mentioned in
the commit message (if correct).

> , which @var{fps} converts
> +to an input starting PTS and an output starting PTS. If set,
> + at var{fps} drops input frames which have PTSs less than the input starting
> +PTS. If not set, the input and output starting PTSs are zero, but
> + at var{fps} drops no input frames based on PTS. (See details below.)
>
>  @item round
> -Timestamp (PTS) rounding method.
> +Rounding method to use when calculating output PTSs from input PTSs.
> +If the calculated output PTS is not exactly an integer, then the value
> +determines which neighbouring integer value @var{fps} selects.
>
>  Possible values are:
>  @table @option
> @@ -11225,43 +11230,157 @@ round towards -infinity
>  @item up
>  round towards +infinity
>  @item near
> -round to nearest
> +round to nearest (midpoints round away from 0)
>  @end table
>  The default is @code{near}.
>
>  @item eof_action
> -Action performed when reading the last frame.
> +Action which @var{fps} takes with the final input frame. The input video passes
> +in a final input PTS, which @var{fps} converts to an output PTS limit.

> + at var{fps} drops any input frames with a PTS at or after this limit.

This sounds wrong to me: In which situation are frames dropped?

>  Possible values are:
>  @table @option
>  @item round
> -Use same timestamp rounding method as used for other frames.
> +Use same rounding method as for other frames.

>  @item pass
> -Pass through last frame if input duration has not been reached yet.
> +Round the ending input PTS using @code{up}. This might make @ref{fps} include
> +one last input frame.
>  @end table

This seems to contradict the original description. Is this intended?

> +
>  The default is @code{round}.
>
>  @end table
>
> -Alternatively, the options can be specified as a flat string:
> +Alternatively, the options may be specified as a flat string:
>  @var{fps}[:@var{start_time}[:@var{round}]].
>
> -See also the @ref{setpts} filter.
> + at var{fps} makes an output video with consecutive integer PTSs, and with a
> +time base set to the inverse of the given frame rate. @var{fps} keeps, repeats,
> +or drops input frames, in sequence, to the output video. It does so according

> +to their input PTSs, as converted to seconds (via the input time base),

Why would the fps filter convert the PTS to seconds?
And how is this relevant for users of the filter?

> +then rounded to output PTSs.

In which situation does fps round?

> + at var{fps} sets output PTSs in terms of a timeline which starts at
> +zero. For any output frame, the integer PTS multiplied by the time base
> +gives a value in seconds on that timeline. If the @var{start_time}
> +parameter is not set, or is zero, the first output frame's PTS is zero.
> +Otherwise, the first PTS is the output starting PTS calculated
> +from the @var{start_time} parameter.
> +
> + at var{fps} interprets input PTSs in terms of the same timeline. It
> +multiplies each input frame's PTS by the input time base, to get a value
> +in seconds on the timeline. It rounds that value to an integer output PTS.
> +For example, if the input video has a frame rate of 30 fps, a time base
> +of 1/30 seconds, and a first frame with a PTS of 300, then @var{fps} treats that
> +first frame as occurring 10 seconds (= 300 * 1/30) after the start of the video,
> +even though it is the first frame.
> +
> +Setting a @code{start_time} value allows for padding/trimming at the
> +start of the input. For example, you can set @code{start_time} to 0, to pad the
> +beginning with repeats of the first frame if a video stream starts after
> +the audio stream, or to trim any frames with a negative PTS. When
> + at code{start_time} is not set, the @var{fps} filter does not pad or trim
> +starting frames, as long as they contain PTSs.
> +
> +See also the @ref{setpts} and @var{settb} filters.
> +
> + at subsection Details
> +
> + at var{fps} outputs exactly one frame for each output PTS. If there is
> +exactly one input frame with an input PTS which converts to the current output
> +PTS, @var{fps} keeps (outputs) that frame. If there are multiple frames which
> +convert to the same output PTS, @var{fps} outputs the final frame of that
> +group, and drops the previous frames.
> +
> +If the input frame PTS converts to an output PTS later than the current
> +output PTS, @var{fps} repeats the previously output frame as the current frame.
> +When this happens for the first input frame,  @var{fps} "pads" — outputs
> +repetitions of — that first frame until the output PTS reaches the value
> +converted from that first frame's input PTS.
> +
> + at var{fps} always drops input frames which have no PTS set, regardless
> +of the @var{start_time} parameter.
> +
> +The @var{frame rate} value must be zero or greater. It may be provided in a
> +variety of forms. Each form is converted into a rational number, with an
> +integer numerator and denominator.
> +
> + at itemize
> +
> + at item
> +An integer number, e.g. @code{25}. This converts to the rational number
> + at code{25/1}.
> +
> + at item
> +A real number, e.g. @code{3.14145926}. This converts to a rational number,
> +e.g. @code{954708/303893}
> +
> + at item
> +A rational number. The numerator and denominator may be either integers or real
> +numbers. e.g. @code{30/1.001} or @code{-30000/-1001}, which both convert to
> + at code{30000/1001}. The denominator must be non-zero.
> +
> + at item
> +An abbreviation. e.g @code{ntsc} as @code{30000/1001},
> + at code{ntsc-film} as @code{24000/1001}. See the complete list at
> + at ref{Video rate,,the "Video rate" section in the ffmpeg-utils(1) manual,ffmpeg-utils}.
> +
> + at end itemize
> +
> + at var{fps} defines a sync point on the timeline, where one input PTS and one
> +output PTS occur at the same moment. It calculates other PTSs as time offsets
> +from this sync point. This affects the details of rounding. If @var{start_time}
> +is set, then @var{fps} uses it to calculate input and output PTSs, and makes
> +them the sync point. Otherwise, input and output PTS of zero are the sync point.
> +
> +Note that @var{fps} does not assume that input frames are separated by exactly
> +1/frame_rate seconds. It takes the input PTSs literally. If the increment
> +of PTS between frames varies along the video, @var{fps} treats those frames as
> +happening at varying time intervals.
> +
> +An input video with PTSs starting past zero might yield unexpected results.
> +Suppose the input PTSs start at 300, and say that converts to 10 seconds.
> +Then @var{fps} repeats the first frame to fill the first 10 seconds of the
> +output video. (However, @command{ffmpeg} may suppress those repeated frames,
> +depending on the @option{-vsync} setting.) If you set @var{start_time} to 10
> +seconds, then @var{fps} sets the sync point to the PTSs converted from
> +10 seconds on the timeline. It no longer repeats the first frame. And, it starts
> +the output PTS at a value corresponding to 10 seconds, instead of zero.
>
>  @subsection Examples
>
>  @itemize
>  @item
> -A typical usage in order to set the fps to 25:
> +A typical usage to make a video with a frame rate of 25 frames per
> +second, from an input video with any frame rate:
>  @example
>  fps=fps=25
>  @end example
> -
> +The output frames have PTSs of 0, 1, 2, etc. The frame rate is @code{25/1}. The
> +time base is @code{1/25}.
>  @item
> -Sets the fps to 24, using abbreviation and rounding method to round to nearest:
> +Make a video with a frame rate of 24 frames per second, using an abbreviation,
> +and rounding method to round to nearest:
>  @example
>  fps=fps=film:round=near
>  @end example
> +
> + at item
> +Clean up a video with varying time between frames, and dropped frames.
> +The input video is supposed to have an NTSC standard frame rate of 29.97 frames
> +per second, and the time base is @code{3003/90000}, but the PTSs increment
> +variably at slightly more and less than that rate. The recorder dropped some
> +frames, but the PTSs still reflect when the remaining frames were captured.
> + at example
> +fps=fps=30/1.001:round=near
> + at end example
> +The output PTSs are 0, 1001, 2002, etc. The time between frames is exact.
> +The output frame rate is @code{30000/1001}. The time base is @code{1001/30000}.
> +Where frames were dropped by the recorder, @var{fps} repeated frames to fill
> +the gaps.

This sections are still far too long.

The patch is not ok.

Carl Eugen