[FFmpeg-devel] [PATCH] avutil/timestamp: keep microsecond precision in av_ts_make_time_string
Marton Balint
cus at passwd.hu
Tue Mar 19 23:14:32 EET 2024
On Mon, 18 Mar 2024, Marton Balint wrote:
> av_ts_make_time_string() used "%.6g" format in the past, but this format was
> losing precision even when the timestamp to be printed was not that large. For
> example for 3 hours (10800) seconds, only 1 decimal digit was printed, which
> made this format inaccurate when it was used in e.g. the silencedetect filter.
> Other detection filters printing timestamps had similar issues.
>
> So let's change the used format to "%.6f" instead, we have plenty of space in
> the string buffer, we can somewhat imitate existing behaviour of %g by trimming
> ending zeroes and the potential decimal point, which can be any non-numeric
> character. In order not to trim "inf" as well, we assume that the decimal
> point does not contain the letter "f".
>
> We also no longer use scientific representation to make parsing and printing
> easier, because the theoretical maximum of INT64_MAX*INT32_MAX still fits into
> the string buffer in normal form.
>
> Since the additional processing yields more code, inlineing this function no
> longer make sense, so this commit also changes the API to actually export the
> function instead of having it inlinable in the header.
>
> Thanks for Allan Cady for bringing up this issue.
Ping for this. Considering the API and behaviour change, I prefer we apply
this before branching 7.0.
Thanks,
Marton
>
> Signed-off-by: Marton Balint <cus at passwd.hu>
> ---
> doc/APIchanges | 3 ++
> libavutil/Makefile | 1 +
> libavutil/timestamp.c | 33 ++++++++++++++++++++
> libavutil/timestamp.h | 8 +----
> libavutil/version.h | 2 +-
> tests/ref/fate/filter-metadata-scdet | 12 +++----
> tests/ref/fate/filter-metadata-silencedetect | 2 +-
> 7 files changed, 46 insertions(+), 15 deletions(-)
> create mode 100644 libavutil/timestamp.c
>
> diff --git a/doc/APIchanges b/doc/APIchanges
> index a44c8e4f10..1afde062a5 100644
> --- a/doc/APIchanges
> +++ b/doc/APIchanges
> @@ -2,6 +2,9 @@ The last version increases of all libraries were on 2024-03-07
>
> API changes, most recent first:
>
> +2024-03-xx - xxxxxxxxxx - lavu 59.3.100 - timestamp.h
> + av_ts_make_time_string() is no longer an inline function. It is now exported.
> +
> 2024-03-xx - xxxxxxxxxx - lavu 59.2.100 - channel_layout.h
> Add AV_CHANNEL_LAYOUT_RETYPE_FLAG_CANONICAL.
>
> diff --git a/libavutil/Makefile b/libavutil/Makefile
> index e7709b97d0..5e75aa1855 100644
> --- a/libavutil/Makefile
> +++ b/libavutil/Makefile
> @@ -174,6 +174,7 @@ OBJS = adler32.o \
> threadmessage.o \
> time.o \
> timecode.o \
> + timestamp.o \
> tree.o \
> twofish.o \
> utils.o \
> diff --git a/libavutil/timestamp.c b/libavutil/timestamp.c
> new file mode 100644
> index 0000000000..06fb47e94c
> --- /dev/null
> +++ b/libavutil/timestamp.c
> @@ -0,0 +1,33 @@
> +/*
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +
> +#include "timestamp.h"
> +
> +char *av_ts_make_time_string(char *buf, int64_t ts, const AVRational *tb)
> +{
> + if (ts == AV_NOPTS_VALUE) {
> + snprintf(buf, AV_TS_MAX_STRING_SIZE, "NOPTS");
> + } else {
> + int last = snprintf(buf, AV_TS_MAX_STRING_SIZE, "%.6f", av_q2d(*tb) * ts);
> + last = FFMIN(last, AV_TS_MAX_STRING_SIZE - 1) - 1;
> + for (; last && buf[last] == '0'; last--);
> + for (; last && buf[last] != 'f' && (buf[last] < '0' || buf[0] > '9'); last--);
> + buf[last + 1] = '\0';
> + }
> + return buf;
> +}
> diff --git a/libavutil/timestamp.h b/libavutil/timestamp.h
> index 2b37781eba..a02d873060 100644
> --- a/libavutil/timestamp.h
> +++ b/libavutil/timestamp.h
> @@ -62,13 +62,7 @@ static inline char *av_ts_make_string(char *buf, int64_t ts)
> * @param tb the timebase of the timestamp
> * @return the buffer in input
> */
> -static inline char *av_ts_make_time_string(char *buf, int64_t ts,
> - const AVRational *tb)
> -{
> - if (ts == AV_NOPTS_VALUE) snprintf(buf, AV_TS_MAX_STRING_SIZE, "NOPTS");
> - else snprintf(buf, AV_TS_MAX_STRING_SIZE, "%.6g", av_q2d(*tb) * ts);
> - return buf;
> -}
> +char *av_ts_make_time_string(char *buf, int64_t ts, const AVRational *tb);
>
> /**
> * Convenience macro, the return value should be used only directly in
> diff --git a/libavutil/version.h b/libavutil/version.h
> index 57cad02ec0..5027b025be 100644
> --- a/libavutil/version.h
> +++ b/libavutil/version.h
> @@ -79,7 +79,7 @@
> */
>
> #define LIBAVUTIL_VERSION_MAJOR 59
> -#define LIBAVUTIL_VERSION_MINOR 2
> +#define LIBAVUTIL_VERSION_MINOR 3
> #define LIBAVUTIL_VERSION_MICRO 100
>
> #define LIBAVUTIL_VERSION_INT AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \
> diff --git a/tests/ref/fate/filter-metadata-scdet b/tests/ref/fate/filter-metadata-scdet
> index ca5dbaaefc..d385920fcd 100644
> --- a/tests/ref/fate/filter-metadata-scdet
> +++ b/tests/ref/fate/filter-metadata-scdet
> @@ -1,11 +1,11 @@
> pts=1620|tag:lavfi.scd.score=59.252|tag:lavfi.scd.mafd=60.175|tag:lavfi.scd.time=2.7
> pts=4140|tag:lavfi.scd.score=36.070|tag:lavfi.scd.mafd=44.209|tag:lavfi.scd.time=6.9
> -pts=5800|tag:lavfi.scd.score=55.819|tag:lavfi.scd.mafd=55.819|tag:lavfi.scd.time=9.66667
> +pts=5800|tag:lavfi.scd.score=55.819|tag:lavfi.scd.mafd=55.819|tag:lavfi.scd.time=9.666667
> pts=6720|tag:lavfi.scd.score=18.580|tag:lavfi.scd.mafd=22.505|tag:lavfi.scd.time=11.2
> pts=8160|tag:lavfi.scd.score=49.240|tag:lavfi.scd.mafd=49.444|tag:lavfi.scd.time=13.6
> -pts=9760|tag:lavfi.scd.score=51.497|tag:lavfi.scd.mafd=51.801|tag:lavfi.scd.time=16.2667
> -pts=14080|tag:lavfi.scd.score=34.165|tag:lavfi.scd.mafd=34.337|tag:lavfi.scd.time=23.4667
> -pts=15700|tag:lavfi.scd.score=58.310|tag:lavfi.scd.mafd=58.315|tag:lavfi.scd.time=26.1667
> -pts=18500|tag:lavfi.scd.score=16.504|tag:lavfi.scd.mafd=19.603|tag:lavfi.scd.time=30.8333
> +pts=9760|tag:lavfi.scd.score=51.497|tag:lavfi.scd.mafd=51.801|tag:lavfi.scd.time=16.266667
> +pts=14080|tag:lavfi.scd.score=34.165|tag:lavfi.scd.mafd=34.337|tag:lavfi.scd.time=23.466667
> +pts=15700|tag:lavfi.scd.score=58.310|tag:lavfi.scd.mafd=58.315|tag:lavfi.scd.time=26.166667
> +pts=18500|tag:lavfi.scd.score=16.504|tag:lavfi.scd.mafd=19.603|tag:lavfi.scd.time=30.833333
> pts=20040|tag:lavfi.scd.score=13.764|tag:lavfi.scd.mafd=19.060|tag:lavfi.scd.time=33.4
> -pts=21760|tag:lavfi.scd.score=64.451|tag:lavfi.scd.mafd=64.551|tag:lavfi.scd.time=36.2667
> +pts=21760|tag:lavfi.scd.score=64.451|tag:lavfi.scd.mafd=64.551|tag:lavfi.scd.time=36.266667
> diff --git a/tests/ref/fate/filter-metadata-silencedetect b/tests/ref/fate/filter-metadata-silencedetect
> index bc53fea047..e66ffe5fdd 100644
> --- a/tests/ref/fate/filter-metadata-silencedetect
> +++ b/tests/ref/fate/filter-metadata-silencedetect
> @@ -1,5 +1,5 @@
> pts=0|tag:lavfi.silence_duration=0.523107|tag:lavfi.silence_end=0.690023|tag:lavfi.silence_start=0.736417
> -pts=46080|tag:lavfi.silence_start=1.27626|tag:lavfi.silence_end=1.80751|tag:lavfi.silence_duration=0.531247
> +pts=46080|tag:lavfi.silence_start=1.276259|tag:lavfi.silence_end=1.807506|tag:lavfi.silence_duration=0.531247
> pts=92160
> pts=138240
> pts=184320
> --
> 2.35.3
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
>
More information about the ffmpeg-devel
mailing list