[FFmpeg-devel] [PATCH] libavutil/timestamp.h: Fix loss of precision in timestamps for silencedetect on long files

[Allan Cady via ffmpeg-devel <ffmpeg-devel@ffmpeg.org>]

When the silencedetect filter is run against long files, the output
timestamps gradually lose precision as the scan proceeds further into
the file. This is because the output is formatted (in
libavutil/timestamp.h) as "%.6g", which limits the total field
length. Eventually, for offsets greater than 100000 seconds (about 28
hours), fractions of a second disappear altogether, and the
timestamps are logged as whole seconds.

I propose changing the format to "%.6f", which will
give microsecond precision for all timestamps, regardless of
offset. Trailing zeros can be trimmed from the fraction, without
losing precision. If the length of the fixed-precision formatted
timestamp exceeds the length of the allocated buffer
(AV_TS_MAX_STRING_SIZE, currently 32, less one for the
terminating null), then we can fall back to scientific notation, though
this seems almost certain to never occur, because 32 characters would
allow a maximum timestamp value of (32 - 1 - 6 - 1) = 24 characters.
By my calculation, 10^24 seconds is about six orders of magnitude
greater than the age of the universe.

The fix proposed here follows the following logic:

1) Try formatting the number of seconds using "%.6f". This will
always result in a string with six decimal digits in the fraction,
possibly including trailing zeros. (e.g. "897234.73200").

2) Check if that string would overflow the buffer. If it would, then
format it using scientific notation ("%.8g").

3) If the original fixed-point format fits, then trim any trailing
zeros and decimal point, and return that result.

Making this change broke two fate tests, filter-metadata-scdet,
and filter-metadata-silencedetect. To correct this, I've modified
tests/ref/fate/filter-metadata-scdet and
tests/ref/fate/filter-metadata-silencedetect to match the
new output.

Signed-off-by: Allan Cady <allancady@yahoo.com>
---
 libavutil/timestamp.h                        | 53 +++++++++++++++++++-
 tests/ref/fate/filter-metadata-scdet         | 12 ++---
 tests/ref/fate/filter-metadata-silencedetect |  2 +-
 3 files changed, 58 insertions(+), 9 deletions(-)

diff --git a/libavutil/timestamp.h b/libavutil/timestamp.h
index 2b37781eba..2f04f9bb2b 100644
--- a/libavutil/timestamp.h
+++ b/libavutil/timestamp.h
@@ -25,6 +25,7 @@
 #define AVUTIL_TIMESTAMP_H
 
 #include "avutil.h"
+#include <stdbool.h>
 
 #if defined(__cplusplus) && !defined(__STDC_FORMAT_MACROS) && !defined(PRId64)
 #error missing -D__STDC_FORMAT_MACROS / #define __STDC_FORMAT_MACROS
@@ -53,6 +54,32 @@ static inline char *av_ts_make_string(char *buf, int64_t ts)
  */
 #define av_ts2str(ts) av_ts_make_string((char[AV_TS_MAX_STRING_SIZE]){0}, ts)
 
+/**
+ * Strip trailing zeros and decimal point from a string. Performed
+ * in-place on input buffer. For local use only by av_ts_make_time_string.
+ * 
+ * e.g.:
+ * "752.378000" -> "752.378"
+ *        "4.0" -> "4"
+ *      "97300" -> "97300"
+ */
+static inline void av_ts_strip_trailing_zeros_and_decimal_point(char *str) {
+    if (strchr(str, '.'))
+    {
+        int i;
+        for (i = strlen(str) - 1; i >= 0 && str[i] == '0'; i--) {
+            str[i] = '\0';
+        }
+        
+        // Remove decimal point if it's the last character
+        if (i >= 0 && str[i] == '.') {
+            str[i] = '\0';
+        }
+
+        // String was modified in place; no need for return value.
+    }
+}
+
 /**
  * Fill the provided buffer with a string containing a timestamp time
  * representation.
@@ -65,8 +92,30 @@ static inline char *av_ts_make_string(char *buf, int64_t ts)
 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);
+    if (ts == AV_NOPTS_VALUE)
+    {
+        snprintf(buf, AV_TS_MAX_STRING_SIZE, "NOPTS");
+    }
+    else
+    {
+        const int max_fraction_digits = 6;
+
+        // Convert 64-bit timestamp to double, using rational timebase
+        double seconds = av_q2d(*tb) * ts;
+
+        int length = snprintf(NULL, 0, "%.*f", max_fraction_digits, seconds);
+        if (length <= AV_TS_MAX_STRING_SIZE - 1)
+        {
+            snprintf(buf, AV_TS_MAX_STRING_SIZE, "%.*f", max_fraction_digits, seconds);
+            av_ts_strip_trailing_zeros_and_decimal_point(buf);
+        }
+        else
+        {
+            snprintf(buf, AV_TS_MAX_STRING_SIZE, "%.8g", seconds);
+        }
+
+    }
+
     return buf;
 }
 
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.34.1

_______________________________________________
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".