> On Sat, 23 Mar 2024, Marton Balint wrote:
>> av_ts_make_time_string() used "%.6g" format, 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. Also time base
>> parameter of the function was *AVRational instead of AVRational.
>>
>> Resolve these problems by introducing a new function,
>> av_ts_make_time_string2().
>>
>> We change the used format to "%.*f", use a precision of 6, except when
>> printing
>> values near 0, in which case we calculate the precision dynamically to aim
>> for
>> a similar precision in normal form as with %.6g. No longer using scientific
>> representation can make parsing the timestamp easier for the users, we can
>> safely do this because the theoretical maximum of INT64_MAX*INT32_MAX still
>> fits into the string buffer in normal form.
>>
>> We somewhat imitate %g by trimming ending zeroes and the potential decimal
>> point characters. In order not to trim "inf" as well, we assume that the
>> decimal point string does not contain the letter "f". Note that depending on
>> printf %f implementation, we might trim "infinity" to "inf".
>>
>> Thanks for Allan Cady for bringing up this issue.
> Will apply the series, so it can make it into 7.0.
> Regards,
> Marton
This looks great. I ran the patch against my test program with a couple
dozen vectors of inputs, and it looks perfect to me.
Thanks to both of you Marton and Andreas for seeing this through. I
really appreciate it.
Allan
>>
>> Signed-off-by: Marton Balint <c...@passwd.hu>
>> ---
>> doc/APIchanges | 5 +++++
>> libavutil/Makefile | 1 +
>> libavutil/timestamp.c | 36 ++++++++++++++++++++++++++++++++++++
>> libavutil/timestamp.h | 8 ++++++++
>> libavutil/version.h | 2 +-
>> 5 files changed, 51 insertions(+), 1 deletion(-)
>> create mode 100644 libavutil/timestamp.c
>>
>> diff --git a/doc/APIchanges b/doc/APIchanges
>> index 2796b4d0c2..9c6fa381e1 100644
>> --- a/doc/APIchanges
>> +++ b/doc/APIchanges
>> @@ -2,6 +2,11 @@ The last version increases of all libraries were on
>> 2024-03-07
>>
>> API changes, most recent first:
>>
>> +2024-03-xx - xxxxxxxxxx - lavu 59.5.100 - timestamp.h
>> + Add av_ts_make_time_string2() for better timestamp precision, the new
>> + function accepts AVRational as time base instead of *AVRational, and is
>> not
>> + an inline function like its predecessor.
>> +
>> 2024-03-xx - xxxxxxxxxx - lavc 61.3.100 - jni.h
>> Add av_jni_set_android_app_ctx() and av_jni_get_android_app_ctx().
>>
>> diff --git a/libavutil/Makefile b/libavutil/Makefile
>> index 008fcfcd9c..6e6fa8d800 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..2a3e3012a4
>> --- /dev/null
>> +++ b/libavutil/timestamp.c
>> @@ -0,0 +1,36 @@
>> +/*
>> + * 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_string2(char *buf, int64_t ts, AVRational tb)
>> +{
>> + if (ts == AV_NOPTS_VALUE) {
>> + snprintf(buf, AV_TS_MAX_STRING_SIZE, "NOPTS");
>> + } else {
>> + double val = av_q2d(tb) * ts;
>> + double log = floor(log10(fabs(val)));
>> + int precision = (isfinite(log) && log < 0) ? -log + 5 : 6;
>> + int last = snprintf(buf, AV_TS_MAX_STRING_SIZE, "%.*f", precision,
>> val);
>> + 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..7e6da894df 100644
>> --- a/libavutil/timestamp.h
>> +++ b/libavutil/timestamp.h
>> @@ -62,6 +62,14 @@ static inline char *av_ts_make_string(char *buf, int64_t
>> ts)
>> * @param tb the timebase of the timestamp
>> * @return the buffer in input
>> */
>> +char *av_ts_make_time_string2(char *buf, int64_t ts, AVRational tb);
>> +
>> +/**
>> + * Fill the provided buffer with a string containing a timestamp
>> + * representation.
>> + *
>> + * @see av_ts_make_time_string2
>> + */
>> static inline char *av_ts_make_time_string(char *buf, int64_t ts,
>> const AVRational *tb)
>> {
>> diff --git a/libavutil/version.h b/libavutil/version.h
>> index 882003f719..8a1ecd4451 100644
>> --- a/libavutil/version.h
>> +++ b/libavutil/version.h
>> @@ -79,7 +79,7 @@
>> */
>>
>> #define LIBAVUTIL_VERSION_MAJOR 59
>> -#define LIBAVUTIL_VERSION_MINOR 4
>> +#define LIBAVUTIL_VERSION_MINOR 5
>> #define LIBAVUTIL_VERSION_MICRO 100
>>
>> #define LIBAVUTIL_VERSION_INT AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \
>> --
>> 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-requ...@ffmpeg.org with subject "unsubscribe".
>>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
