From patchwork Sun Apr 7 07:47:26 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Diego Felix de Souza via ffmpeg-devel X-Patchwork-Id: 12630 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 90FBC448897 for ; Sun, 7 Apr 2019 10:47:43 +0300 (EEST) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 7155D68AC37; Sun, 7 Apr 2019 10:47:43 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from sonic307-56.consmr.mail.ne1.yahoo.com (sonic307-56.consmr.mail.ne1.yahoo.com [66.163.190.31]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id C794868ABA1 for ; Sun, 7 Apr 2019 10:47:36 +0300 (EEST) X-YMail-OSG: kamSf_YVM1kD3bbGxo8piomHgYPFGCyucPMWc5lQaSqSthmTtJ6vxHu.INPVFne n3i.Ns.Da6r_K628Fu7MqlPibelH0ByCGXNZ38jXkIzo3LEZhYLqhS43ktRkMFDH4m9KGPpOh4qh 7a_oXSVxfKQVXml_Y.FxBw4a3Kl5DD67h23zJBD7MmYqEQ2.4zpaplWrO3QbhZ_yix0qFY3PvxbR 3PQy2fiH65bXnmAQLa_f8h1QWu_vTLCGa5wEs7eBgrl1XAYEg3jix5QMooJ2vIpPDPohgeY0EX0_ 6PMHJKOITIseLZ3B78ImGJa88tz9.Ilxa6BETbQh.NvPuMo.hE4zbJkQY0SrOLfJo2XgbF5v_23t 8a2cQ6BgXRI_4FlzhsstlUw5Q6YZmR4BUa3SBa4W.aCWQA69OLk31y1zmrtyBeIo9XY1uZSfQVoS IXs7qFaa1bEvJmvTGhlcPmVx9UC0d2k_DQxZ.T4N9ZAeOrAz3OABxl2byOT4R9VoNYUU0tSLV8.d 7bBj8_Rk1textPYjBi.AkFAo5Ko7h4wARimyk_cXYpcbqNBeZaKaV0gPp0KTMfpnaLTF0POQzg1r RR.s7Ab7TXLhxV9LMxRUkJJ1YmnDpKDQqozOgEQp2HQKmsqjh5qYYBDP7rYv.KAIvnSzJLVfL9i7 mtyfKQFK2yRU1BG17oY4dbbeNr8S2amkXTjpHfTz3ZVWwgqvi_dgmlZroLIrtUkrzaGqYwD9ZWAd OYQMg8SPf9vIs_xLk2VBV.RdB0ws.UDpVigyNOyZXNSdLz.guhsjE2TiMIgE0.a9.oFaNTA7wAe9 GWl8KXKns_v8jDYeD55q7jv1sR0Bh8veit07YMhzb_0D9h6WYY.gdUmFFmmTboIz.PWirsAv1cmd figQku8MRrD1yEFsu_73FLH7M.bxxs3XPn0bBjVqH0JhnmTA3b1KU1WN.MSFdRlddJEYsxYWRDFz wGYM6jCbgNL2n2KjevbMmvCCJ9MVoWb77g6vzlaTDruBRZornatba9.dsUuY3Jjt0Futv44qSbO_ S446cWGnK1iwtR7w4Tx9vQwZ.O58GK6qdCtX1tJdgDcx6WNCx Received: from sonic.gate.mail.ne1.yahoo.com by sonic307.consmr.mail.ne1.yahoo.com with HTTP; Sun, 7 Apr 2019 07:47:34 +0000 Received: from c-24-56-228-226.customer.broadstripe.net (EHLO [172.16.42.35]) ([24.56.228.226]) by smtp428.mail.ne1.yahoo.com (Oath Hermes SMTP Server) with ESMTPA ID 58c9429e1c02c0c570bfd8b2b9321197 for ; Sun, 07 Apr 2019 07:47:32 +0000 (UTC) To: ffmpeg-devel@ffmpeg.org Message-ID: Date: Sun, 7 Apr 2019 00:47:26 -0700 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:60.0) Gecko/20100101 Thunderbird/60.6.1 MIME-Version: 1.0 Content-Language: en-US Subject: [FFmpeg-devel] [PATCH] libavutil/timestamp.h: Fix loss of precision in timestamps for silencedetect on 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: , X-Patchwork-Original-From: Allan Cady via ffmpeg-devel From: Diego Felix de Souza via ffmpeg-devel Reply-To: FFmpeg development discussions and patches Cc: Allan Cady Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" [Second try submitting to the list. This patch now passes fate.] 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. This is insufficient precision for my purposes. I propose changing the format to "%.6f", which will give microsecond precision (probably overkill but safe) 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. From 5492506534bf863cbf1ee8f09a5e59b4ee111226 Mon Sep 17 00:00:00 2001 From: Allan Cady Date: Sun, 7 Apr 2019 00:07:58 -0700 Subject: [PATCH] libavutil/timestamp.h: Fix loss of precision in timestamps for silencedetect on long files 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. This is insufficient precision for my purposes. I propose changing the format to "%.6f", which will give microsecond precision (probably overkill but safe) 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 | 13 ++++++++----- tests/ref/fate/filter-metadata-silencedetect | 2 +- 3 files changed, 17 insertions(+), 12 deletions(-) diff --git a/libavfilter/af_silencedetect.c b/libavfilter/af_silencedetect.c index 3a71f39..2da8dbe 100644 --- a/libavfilter/af_silencedetect.c +++ b/libavfilter/af_silencedetect.c @@ -32,6 +32,8 @@ #include "avfilter.h" #include "internal.h" +const char TIMESTAMP_FMT_SILENCEDETECT[] = "%.6f"; + typedef struct SilenceDetectContext { const AVClass *class; double noise; ///< noise amplitude ratio @@ -86,11 +88,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, TIMESTAMP_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, TIMESTAMP_FMT_SILENCEDETECT)); } } } else { @@ -101,15 +103,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, TIMESTAMP_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, TIMESTAMP_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, TIMESTAMP_FMT_SILENCEDETECT), + av_ts2timestr_fmt(duration_ts, &time_base, TIMESTAMP_FMT_SILENCEDETECT)); } s->nb_null_samples[channel] = 0; s->start[channel] = INT64_MIN; diff --git a/libavutil/timestamp.h b/libavutil/timestamp.h index e082f01..fd9baaf 100644 --- a/libavutil/timestamp.h +++ b/libavutil/timestamp.h @@ -31,6 +31,7 @@ #endif #define AV_TS_MAX_STRING_SIZE 32 +#define TIMESTAMP_FMT_GENERAL "%.6g" /** * Fill the provided buffer with a string containing a timestamp @@ -55,24 +56,26 @@ static inline char *av_ts_make_string(char *buf, int64_t ts) /** * Fill the provided buffer with a string containing a timestamp time - * representation. + * 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 format string for timestamp output, e.g. "%.5f". * @return the buffer in input */ -static inline char *av_ts_make_time_string(char *buf, int64_t ts, AVRational *tb) +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, "%.6g", av_q2d(*tb) * ts); + else snprintf(buf, AV_TS_MAX_STRING_SIZE, format, av_q2d(*tb) * ts); 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, TIMESTAMP_FMT_GENERAL) #endif /* AVUTIL_TIMESTAMP_H */ diff --git a/tests/ref/fate/filter-metadata-silencedetect b/tests/ref/fate/filter-metadata-silencedetect index 16a9d07..5d8063c 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