From patchwork Sat Jun 6 21:32:18 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Allan Cady X-Patchwork-Id: 20181 Return-Path: X-Original-To: patchwork@ffaux-bg.ffmpeg.org Delivered-To: patchwork@ffaux-bg.ffmpeg.org Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by ffaux.localdomain (Postfix) with ESMTP id 708A644A4E8 for ; Sun, 7 Jun 2020 00:32:30 +0300 (EEST) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 42C8868AFCC; Sun, 7 Jun 2020 00:32:30 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from sonic306-3.consmr.mail.bf2.yahoo.com (sonic306-3.consmr.mail.bf2.yahoo.com [74.6.132.42]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 7F558689C71 for ; Sun, 7 Jun 2020 00:32:23 +0300 (EEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=yahoo.com; s=s2048; t=1591479141; bh=5kWc7Eqw3vTbTY0eVnIf4trlX4R/h5DB3mZrOYOTLa4=; h=Date:From:To:Subject:References:From:Subject; b=fSvt715KiQe2yAJlKgXSPDoUCGIA3tKXGz/K7uxUhGQAfpZ6x5kk9zylLvODawWGWPfqa156xOMgwC+ie93ORAztYIbxLxQ1x2m0htfv6kfYOlnDF7nqQuAFvrJMppu0zDDasaB2TNC9IOfnaOqizXJXu4H9NHl53yL1wot1g506f7bskWoCo0mSIjOeT55yqNOCud99in2UfNnVsZhUUy5ypjdJafcYP6nARUWIehVKtsmAy6o77vMheKI4ytrGiOY//sOmVa+iG6y7poSkf28d6N97UFA3OGUgbKltnlB959oWahHxljZXiwMINLF36AeWaOzrkmBJcxSuitY0zw== X-YMail-OSG: TxmxU.oVM1mrDe5lBcs6rXCeMRri2XbbQEdf2szLP1EC6zSgWXi3Y8xmCkeaRSA Dj4e84Vk6AT3mSbTjsQUoMcoSOuYy24XCsc9cRtjmgpc8nycvn5HH960qDh0VKt7X1Z8SgJyina5 HtcBeBcw36l89yTBBQ82AvJarnvz4V0o02yLe6d_zzjiZRtvLT_n6su_4a1G.BogByDSOIyr6Mal GuFPrpAIN4kRxiyyq.Y2tEHUQ2HSUUIWEYXKGZwTB.iK5mO3EbMsGovhyiG4pMdkRLbH_QTso2_v X426abKwPngt9Af4AC99OGl3qQYMIdhEkzZalDbWx1tqvS0r0AhAgaQaSLkRvFdrxe1GVoVrzSyr IfA.ggkNNPjDXbR91RLsfcryRnOc.tkCrh.WEf_3hC_GBLxVGt0x_c5cTeCAKW2HkaCqGI1PdTEv VNlui9Ax02jsRAmg1734nyHToBx4n4a_7QPojw1LO.4o5YYHtr.dq8mq4NslemGTQ8sD.e1Yw4Ky ycMxUPMjAMovqedum0pN0hfVdj.p9Y8KFDIeLVf_XmYckxNsvPmF3BMs55PM8rmmjPeGNZ3mH3cI knat93aNpSKg2x6qJH9iUhyiizBu6J8AbNouTAMOhK96IgNAYdKU3Dr58_R3MWGuio0GFk1sxnCP saS13nv2__zEZZk0whtc4CVs9yAjq_WOCGn4CwBAB63k88KBsr7C195f2NWfT18vgqoOrqLEd8j7 FyKbBtq9_kNyZkrbx6ZZLc7RTRfxbHAHuYK5QRQaJSRipCDzV3VCFv.bEfncH5EiSrafnTz5QZkj GcHuZLYq7LeHglsPQNU2WgprOhqEQfCMVZSx0xEGtlteYJouAy4iOLLk261o9sGdPnQLJJYrJgU. dAMyM.PdmOsHw521QxZrk_NBCOot.FAjwMEd1woC0gSdmS024B2BtO8hRMIINl1dBkp6pYDoJy6X DD4dD6yYgS.qradu0Iu3teN1DZ42L5WlmQtf0FtTW.ueMQRKEupFMN06Ffmv11MRqDPAsu4tXslq HKH2s0pSrwlt__gDvZHNJnVn0UB27.fvaHjyLC6U6LxD10_xw26ysy.xNUOjKTW8y8TFsjJNSL38 3dOF4iTIs_VbHPhfEbPd226amKrQrbN6hrsSy3345vpSQ27uATCpBBge43s8g4AzxBMjtfERruhv 1PvX5D2IX5mbQmdPjbwx2f_mbW.Yz_DzXV35gfbTlgh3IhfXhhUerGnmYwlByqUqslyg7rxmJpAX M4fve2N3KF2EtugC5JZfH44CEUGm9BOiSTTxpE5GOYljQq9m0IcuPSnItt6Cmjpv7b0FN Received: from sonic.gate.mail.ne1.yahoo.com by sonic306.consmr.mail.bf2.yahoo.com with HTTP; Sat, 6 Jun 2020 21:32:21 +0000 Date: Sat, 6 Jun 2020 21:32:18 +0000 (UTC) From: Allan Cady To: FFmpeg Development Discussions and Patches Message-ID: <990375690.258086.1591479138605@mail.yahoo.com> MIME-Version: 1.0 References: <990375690.258086.1591479138605.ref@mail.yahoo.com> X-Mailer: WebService/1.1.16072 YMailNorrin Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/80.0.3987.122 Safari/537.36 Subject: [FFmpeg-devel] [PATCH] libavutil/timestamp.h: Fix loss of precision in silencedetect timestamps for long files X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.20 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 Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" [Repeat submission. I really don't know my way around git tools... this time I'm using "git imap-send" in hopes this will better please the ffmpeg submission gods. Thank you for your patience.] 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. The timestamp string length is limited to 32 characters (AV_TS_MAX_STRING_SIZE), so this should still be plenty long enough with the increased length (up to 10^25 seconds). My interest is in fixing this problem for silencedetect, which formats the timestamps by calling the macro av_ts2timestr, defined in timestamp.h. Since av_ts2timestr is also used in many other places (I count 21 c files), I have created a new macro, av_ts2timestr_format, with a format string added as a parameter, and left the original macro interface as is for other usages, to limit the scope of this change. The same or similar change could be made for other cases where better precision is desired. --- libavfilter/af_silencedetect.c              | 14 +++++----- libavutil/timestamp.h                        | 27 +++++++++++++++++--- tests/ref/fate/filter-metadata-silencedetect |  2 +- 3 files changed, 32 insertions(+), 11 deletions(-) diff --git a/libavfilter/af_silencedetect.c b/libavfilter/af_silencedetect.c index ff3b219e77..13c6f6f498 100644 --- a/libavfilter/af_silencedetect.c +++ b/libavfilter/af_silencedetect.c @@ -32,6 +32,8 @@ #include "avfilter.h" #include "internal.h" +const char TS_FMT_SILENCEDETECT[] = "%.6f"; + typedef struct SilenceDetectContext {     const AVClass *class;     double noise;              ///< noise amplitude ratio @@ -87,11 +89,11 @@ static av_always_inline void update(SilenceDetectContext *s, AVFrame *insamples,                 s->start[channel] = insamples->pts + av_rescale_q(current_sample / s->channels + 1 - nb_samples_notify * s->independent_channels / s->channels,                         (AVRational){ 1, s->last_sample_rate }, time_base);                 set_meta(insamples, s->mono ? channel + 1 : 0, "silence_start", -                        av_ts2timestr(s->start[channel], &time_base)); +                        av_ts2timestr_fmt(s->start[channel], &time_base, TS_FMT_SILENCEDETECT));                 if (s->mono)                     av_log(s, AV_LOG_INFO, "channel: %d | ", channel);                 av_log(s, AV_LOG_INFO, "silence_start: %s\n", -                        av_ts2timestr(s->start[channel], &time_base)); +                        av_ts2timestr_fmt(s->start[channel], &time_base, TS_FMT_SILENCEDETECT));             }         }     } else { @@ -102,15 +104,15 @@ static av_always_inline void update(SilenceDetectContext *s, AVFrame *insamples,             int64_t duration_ts = end_pts - s->start[channel];             if (insamples) {                 set_meta(insamples, s->mono ? channel + 1 : 0, "silence_end", -                        av_ts2timestr(end_pts, &time_base)); +                        av_ts2timestr_fmt(end_pts, &time_base, TS_FMT_SILENCEDETECT));                 set_meta(insamples, s->mono ? channel + 1 : 0, "silence_duration", -                        av_ts2timestr(duration_ts, &time_base)); +                        av_ts2timestr_fmt(duration_ts, &time_base, TS_FMT_SILENCEDETECT));             }             if (s->mono)                 av_log(s, AV_LOG_INFO, "channel: %d | ", channel);             av_log(s, AV_LOG_INFO, "silence_end: %s | silence_duration: %s\n", -                    av_ts2timestr(end_pts, &time_base), -                    av_ts2timestr(duration_ts, &time_base)); +                    av_ts2timestr_fmt(end_pts, &time_base, TS_FMT_SILENCEDETECT), +                    av_ts2timestr_fmt(duration_ts, &time_base, TS_FMT_SILENCEDETECT));         }         s->nb_null_samples[channel] = 0;         s->start[channel] = INT64_MIN; diff --git a/libavutil/timestamp.h b/libavutil/timestamp.h index e082f01b40..590c8642d8 100644 --- a/libavutil/timestamp.h +++ b/libavutil/timestamp.h @@ -32,6 +32,8 @@ #define AV_TS_MAX_STRING_SIZE 32 +#define AV_TS_FMT_DEFAULT "%.6g" + /**   * Fill the provided buffer with a string containing a timestamp   * representation. @@ -53,6 +55,23 @@ 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) +/** + * Fill the provided buffer with a string containing a timestamp time + * representation, allowing format specification. + * + * @param buf a buffer with size in bytes of at least AV_TS_MAX_STRING_SIZE + * @param ts the timestamp to represent + * @param tb the timebase of the timestamp + * @param format string for timestamp output, e.g. AV_TS_FMT_DEFAULT. + * @return the buffer in input + */ +static inline char *av_ts_make_time_string_format(char *buf, int64_t ts, AVRational *tb, const char *format) +{ +    if (ts == AV_NOPTS_VALUE) snprintf(buf, AV_TS_MAX_STRING_SIZE, "NOPTS"); +    else                      snprintf(buf, AV_TS_MAX_STRING_SIZE, format, av_q2d(*tb) * ts); +    return buf; +} + /**   * Fill the provided buffer with a string containing a timestamp time   * representation. @@ -64,15 +83,15 @@ 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, 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); +    av_ts_make_time_string_format(buf, ts, tb, AV_TS_FMT_DEFAULT);     return buf; } /** - * Convenience macro, the return value should be used only directly in + * Convenience macros, the return value should be used only directly in   * function arguments but never stand-alone.   */ -#define av_ts2timestr(ts, tb) av_ts_make_time_string((char[AV_TS_MAX_STRING_SIZE]){0}, ts, tb) +#define av_ts2timestr_fmt(ts, tb, fmt) av_ts_make_time_string_format((char[AV_TS_MAX_STRING_SIZE]){0}, ts, tb, fmt) +#define av_ts2timestr(ts, tb) av_ts_make_time_string_format((char[AV_TS_MAX_STRING_SIZE]){0}, ts, tb, AV_TS_FMT_DEFAULT) #endif /* AVUTIL_TIMESTAMP_H */ diff --git a/tests/ref/fate/filter-metadata-silencedetect b/tests/ref/fate/filter-metadata-silencedetect index 16a9d07692..5d8063cb71 100644 --- a/tests/ref/fate/filter-metadata-silencedetect +++ b/tests/ref/fate/filter-metadata-silencedetect @@ -1,5 +1,5 @@ pkt_pts=0|tag:lavfi.silence_duration=0.523107|tag:lavfi.silence_end=0.690023|tag:lavfi.silence_start=0.736417 -pkt_pts=46080|tag:lavfi.silence_start=1.27626|tag:lavfi.silence_end=1.80751|tag:lavfi.silence_duration=0.531247 +pkt_pts=46080|tag:lavfi.silence_start=1.276259|tag:lavfi.silence_end=1.807506|tag:lavfi.silence_duration=0.531247 pkt_pts=92160 pkt_pts=138240 pkt_pts=184320