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 0CCA049739 for ; Tue, 19 Mar 2024 21:14:44 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id F37F268D02A; Tue, 19 Mar 2024 23:14:41 +0200 (EET) Received: from iq.passwd.hu (iq.passwd.hu [217.27.212.140]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 150B968C0C3 for ; Tue, 19 Mar 2024 23:14:36 +0200 (EET) Received: from localhost (localhost [127.0.0.1]) by iq.passwd.hu (Postfix) with ESMTP id DD31BEA1B2 for ; Tue, 19 Mar 2024 22:14:35 +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 FGLMELrpaQXG for ; Tue, 19 Mar 2024 22:14:32 +0100 (CET) Received: from iq (iq [217.27.212.140]) by iq.passwd.hu (Postfix) with ESMTPS id A2C49E9BD0 for ; Tue, 19 Mar 2024 22:14:32 +0100 (CET) Date: Tue, 19 Mar 2024 22:14:32 +0100 (CET) From: Marton Balint To: FFmpeg development discussions and patches In-Reply-To: <20240317234021.13180-1-cus@passwd.hu> Message-ID: References: <902017378.2075771.1710309823107@mail.yahoo.com> <20240317234021.13180-1-cus@passwd.hu> MIME-Version: 1.0 Subject: Re: [FFmpeg-devel] [PATCH] avutil/timestamp: keep microsecond precision in av_ts_make_time_string 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 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 > --- > 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". > _______________________________________________ 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".