[FFmpeg-devel] [PATCH] avutil/timestamp: keep microsecond precision in av_ts_make_time_string

[Marton Balint <cus@passwd.hu>]

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.

Signed-off-by: Marton Balint <cus@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@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".