[FFmpeg-devel] [PATCH 1/2] avfilter/vf_fps: clean-up filter options

Tobias Rapp t.rapp at noa-archive.com
Mon Sep 25 12:51:26 EEST 2017


On 23.09.2017 17:05, Michael Niedermayer wrote:
> On Fri, Sep 22, 2017 at 08:52:26AM +0200, Tobias Rapp wrote:
>> On 22.09.2017 01:58, Michael Niedermayer wrote:
>>> On Thu, Sep 21, 2017 at 04:55:51PM +0200, Tobias Rapp wrote:
>>>> Align order of "start_time" option to documentation and add missing
>>>> AV_OPT_FLAG_FILTERING_PARAM flag. Fix indent of "round" named constants
>>>> and clear unused field values.
>>>>
>>>> Also fix some documentation cosmetics.
>>>>
>>>> Signed-off-by: Tobias Rapp <t.rapp at noa-archive.com>
>>>> ---
>>>>   doc/filters.texi     |  4 ++--
>>>>   libavfilter/vf_fps.c | 12 ++++++------
>>>>   2 files changed, 8 insertions(+), 8 deletions(-)
>>>>
>>>> diff --git a/doc/filters.texi b/doc/filters.texi
>>>> index 830de54..96b3dfd 100644
>>>> --- a/doc/filters.texi
>>>> +++ b/doc/filters.texi
>>>> @@ -8670,12 +8670,12 @@ It accepts the following parameters:
>>>>   The desired output frame rate. The default is @code{25}.
>>>>   @item round
>>>> -Rounding method.
>>>> +Timestamp (PTS) rounding method.
>>>>   Possible values are:
>>>>   @table @option
>>>>   @item zero
>>>> -zero round towards 0
>>>> +round towards 0
>>>>   @item inf
>>>>   round away from 0
>>>>   @item down
>>>> diff --git a/libavfilter/vf_fps.c b/libavfilter/vf_fps.c
>>>> index 1e5d07e..0db2b48 100644
>>>> --- a/libavfilter/vf_fps.c
>>>> +++ b/libavfilter/vf_fps.c
>>>> @@ -65,13 +65,13 @@ typedef struct FPSContext {
>>>>   #define F AV_OPT_FLAG_FILTERING_PARAM
>>>>   static const AVOption fps_options[] = {
>>>>       { "fps", "A string describing desired output framerate", OFFSET(framerate), AV_OPT_TYPE_VIDEO_RATE, { .str = "25" }, 0, INT_MAX, V|F },
>>>> -    { "start_time", "Assume the first PTS should be this value.", OFFSET(start_time), AV_OPT_TYPE_DOUBLE, { .dbl = DBL_MAX}, -DBL_MAX, DBL_MAX, V },
>>>>       { "round", "set rounding method for timestamps", OFFSET(rounding), AV_OPT_TYPE_INT, { .i64 = AV_ROUND_NEAR_INF }, 0, 5, V|F, "round" },
>>>> -    { "zero", "round towards 0",      OFFSET(rounding), AV_OPT_TYPE_CONST, { .i64 = AV_ROUND_ZERO     }, 0, 5, V|F, "round" },
>>>> -    { "inf",  "round away from 0",    OFFSET(rounding), AV_OPT_TYPE_CONST, { .i64 = AV_ROUND_INF      }, 0, 5, V|F, "round" },
>>>> -    { "down", "round towards -infty", OFFSET(rounding), AV_OPT_TYPE_CONST, { .i64 = AV_ROUND_DOWN     }, 0, 5, V|F, "round" },
>>>> -    { "up",   "round towards +infty", OFFSET(rounding), AV_OPT_TYPE_CONST, { .i64 = AV_ROUND_UP       }, 0, 5, V|F, "round" },
>>>> -    { "near", "round to nearest",     OFFSET(rounding), AV_OPT_TYPE_CONST, { .i64 = AV_ROUND_NEAR_INF }, 0, 5, V|F, "round" },
>>>> +        { "zero", "round towards 0",                 0, AV_OPT_TYPE_CONST, { .i64 = AV_ROUND_ZERO     }, 0, 0, V|F, "round" },
>>>> +        { "inf",  "round away from 0",               0, AV_OPT_TYPE_CONST, { .i64 = AV_ROUND_INF      }, 0, 0, V|F, "round" },
>>>> +        { "down", "round towards -infty",            0, AV_OPT_TYPE_CONST, { .i64 = AV_ROUND_DOWN     }, 0, 0, V|F, "round" },
>>>> +        { "up",   "round towards +infty",            0, AV_OPT_TYPE_CONST, { .i64 = AV_ROUND_UP       }, 0, 0, V|F, "round" },
>>>> +        { "near", "round to nearest",                0, AV_OPT_TYPE_CONST, { .i64 = AV_ROUND_NEAR_INF }, 0, 0, V|F, "round" },
>>>> +    { "start_time", "Assume the first PTS should be this value.", OFFSET(start_time), AV_OPT_TYPE_DOUBLE, { .dbl = DBL_MAX}, -DBL_MAX, DBL_MAX, V|F },
>>>>       { NULL }
>>>
>>> This breaks shorthand notation like fps=30:-0.01
>>
>> The order of options according to the documentation is
>> [fps]:[round]:[start_time] (see
>> https://ffmpeg.org/ffmpeg-filters.html#fps). It even explicitly
>> says:
>>
>> "Alternatively, the options can be specified as a flat string: fps[:round]."
>>
>> But if preferred I can update the documentation instead.
> 
> Whichever makes more users happy.
> 
> (we maybe should avoid 2 different implementation behaviours if we
>   dont already have 2, i have not checked if we had a different order in
>   the code in the past)

Looking at the history of vf_fps.c apparently the current order of 
options exists since August 2013 (commit 
b69b075ac607419a840da798b089de8ea7630d4b). So it might be better to just 
update documentation to the established implementation.

Regards,
Tobias



More information about the ffmpeg-devel mailing list