From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by master.gitmailbox.com (Postfix) with ESMTP id 495264A261 for ; Mon, 25 Mar 2024 18:27:53 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id BD8A068D4B4; Mon, 25 Mar 2024 20:27:51 +0200 (EET) Received: from iq.passwd.hu (iq.passwd.hu [217.27.212.140]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id ADB4368CDD1 for ; Mon, 25 Mar 2024 20:27:45 +0200 (EET) Received: from localhost (localhost [127.0.0.1]) by iq.passwd.hu (Postfix) with ESMTP id D7000E9B19 for ; Mon, 25 Mar 2024 19:27:44 +0100 (CET) X-Virus-Scanned: amavisd-new at passwd.hu Received: from iq.passwd.hu ([127.0.0.1]) by localhost (iq.passwd.hu [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id f9fRkfCS1Xnr for ; Mon, 25 Mar 2024 19:27:41 +0100 (CET) Received: from iq (iq [217.27.212.140]) by iq.passwd.hu (Postfix) with ESMTPS id 6B847E9BD1 for ; Mon, 25 Mar 2024 19:27:41 +0100 (CET) Date: Mon, 25 Mar 2024 19:27:41 +0100 (CET) From: Marton Balint To: FFmpeg development discussions and patches In-Reply-To: <20240323111414.25547-1-cus@passwd.hu> Message-ID: <9c3dfe56-22f1-4101-f397-76c83a2d7f18@passwd.hu> References: <20240323111414.25547-1-cus@passwd.hu> MIME-Version: 1.0 Subject: Re: [FFmpeg-devel] [PATCH v2 1/2] avutil/timestamp: introduce av_ts_make_time_string2 for better precision X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: 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 > > Signed-off-by: Marton Balint > --- > 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-request@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-request@ffmpeg.org with subject "unsubscribe".