[FFmpeg-devel] [PATCH] lavfi/trim: use AV_OPT_TYPE_DURATION

[Stefano Sabatini]

On date Thursday 2013-07-11 13:31:27 +0000, Paul B Mahol encoded:
> Workarounds for rounding differences between platforms should not be
> needed any more.
> 
> Signed-off-by: Paul B Mahol <onemda at gmail.com>
> ---
>  doc/filters.texi   | 18 ++++++++++--------
>  libavfilter/trim.c | 27 ++++++++++++---------------
>  2 files changed, 22 insertions(+), 23 deletions(-)

Missing micro bump since this is extending the syntax.

> diff --git a/doc/filters.texi b/doc/filters.texi
> index 92f8612..0e0ceb0 100644
> --- a/doc/filters.texi
> +++ b/doc/filters.texi
> @@ -949,13 +949,14 @@ Trim the input so that the output contains one continuous subpart of the input.
>  This filter accepts the following options:
>  @table @option
>  @item start
> -Timestamp (in seconds) of the start of the kept section. I.e. the audio sample
> +Specify time of the start of the kept section, i.e. the audio sample
>  with the timestamp @var{start} will be the first sample in the output.
> +See @code{time duration syntax}.
>  
>  @item end
> -Timestamp (in seconds) of the first audio sample that will be dropped. I.e. the
> +Specify time of the first audio sample that will be dropped, i.e. the
>  audio sample immediately preceding the one with the timestamp @var{end} will be
> -the last sample in the output.
> +the last sample in the output. See @code{time duration syntax}.
>  
>  @item start_pts
>  Same as @var{start}, except this option sets the start timestamp in samples
> @@ -966,7 +967,7 @@ Same as @var{end}, except this option sets the end timestamp in samples instead
>  of seconds.
>  
>  @item duration
> -Maximum duration of the output in seconds.
> +Specify maximum duration of the output. See @code{time duration syntax}.
>  
>  @item start_sample
>  Number of the first sample that should be passed to output.
> @@ -7066,13 +7067,14 @@ Trim the input so that the output contains one continuous subpart of the input.
>  This filter accepts the following options:
>  @table @option
>  @item start
> -Timestamp (in seconds) of the start of the kept section. I.e. the frame with the
> +Specify time of the start of the kept section, i.e. the frame with the
>  timestamp @var{start} will be the first frame in the output.

> +See @code{time duration syntax}.

Why @code{...}?

>  
>  @item end
> -Timestamp (in seconds) of the first frame that will be dropped. I.e. the frame
> +Specify time of the first frame that will be dropped, i.e. the frame
>  immediately preceding the one with the timestamp @var{end} will be the last
> -frame in the output.
> +frame in the output. See @code{time duration syntax}.
>  
>  @item start_pts
>  Same as @var{start}, except this option sets the start timestamp in timebase
> @@ -7083,7 +7085,7 @@ Same as @var{end}, except this option sets the end timestamp in timebase units
>  instead of seconds.
>  
>  @item duration
> -Maximum duration of the output in seconds.
> +Specify maximum duration of the output. See @code{time duration syntax}.

I suggest to skip the "See @code{time duration syntax}", and add a
paragraph like:

@option{start}, @option{end}, @option{duration} are expressed as time
duration specifications, check the "Time duration" section in the
ffmpeg-utils manual.

Unfortunately there is no way to reference sections in other manuals
portably with texinfo/man.

[...]
>  #define OFFSET(x) offsetof(TrimContext, x)
>  #define COMMON_OPTS                                                                                                                                                         \
> -    { "start",       "Timestamp in seconds of the first frame that "                                                                                                        \
> -        "should be passed",                                              OFFSET(start_time),  AV_OPT_TYPE_DOUBLE, { .dbl = DBL_MAX },       -DBL_MAX, DBL_MAX,     FLAGS }, \
> -    { "end",         "Timestamp in seconds of the first frame that "                                                                                                        \
> -        "should be dropped again",                                       OFFSET(end_time),    AV_OPT_TYPE_DOUBLE, { .dbl = DBL_MAX },       -DBL_MAX, DBL_MAX,     FLAGS }, \
> +    { "start",       "Timestamp of the first frame that "                                                                                                        \
> +        "should be passed",                                              OFFSET(start_time),  AV_OPT_TYPE_DURATION, { .i64 = INT64_MAX },    INT64_MIN, INT64_MAX, FLAGS }, \
> +    { "end",         "Timestamp of the first frame that "                                                                                                        \
> +        "should be dropped again",                                       OFFSET(end_time),    AV_OPT_TYPE_DURATION, { .i64 = INT64_MAX },    INT64_MIN, INT64_MAX, FLAGS }, \

unrelated, but "timestamp" is not the same as "time"

[...]

LGTM otherwise, thanks.
-- 
FFmpeg = Fierce & Fiendish Mythic Pure Elitist Game