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 3E9F049DA1 for ; Mon, 11 Mar 2024 14:27:04 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 58C8368D065; Mon, 11 Mar 2024 16:27:01 +0200 (EET) Received: from EUR05-AM6-obe.outbound.protection.outlook.com (mail-am6eur05olkn2056.outbound.protection.outlook.com [40.92.91.56]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id ED3F768CD68 for ; Mon, 11 Mar 2024 16:26:54 +0200 (EET) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=kKeK0uS2Yl9dotoHp+/Pa/KoRcreFv8WC9nknoz9/0RSlyazNvR7RL+mXkJWb9VsElMWDYEwfP8iEraHx8JkM44GkFn32xx/v33lonfeR9zPxnltaU4qurZ6La6Bb5p1oSb/shRzIvuxwM1aVpfcF+iXWr0N5H5QrbNfbtgbzX/aQEcS6pfYt2cmtpIv5sRMNgCZHVLvNoe4Jxx+YEWP4EEirW9s8rRxbQoneKu4F8l/6vlg2dBjZIoQZVUIj54Gb0VvF+/SWxSFEosA323LFpr9gdNJXcvGYGQTVULM+0U4frBQ5C9amOA/CTw6fuqa9x6+a1/Eyv0NIAHSsnac7Q== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=Qq/OrrFp5B9WW1orUAfJiBuIVok5p+8C2kvKWyHaxzQ=; b=gMm+emiNUza7BUQVpDeZAk4n5LhxSD1GeVTqu7/U8XquhWjPbYFwAhM4mEYk58NeKfqDtFbb5RTkA/obdQfPdLylY6WJkdkVBRO3vs1sDIpXRWo9SS2RcauaqOj2FDGpOL/KR+iPWdOPV8iNDb+deuMF6WFFgjtEBg4o0N8ZDuBYCi+nzw16bmTVhejyoMGKw4Ph82/GKWM9y5JQuKVwh8XngrkiAUMAzzNjAN70sBX8GGTSKrbvj87ggJn3d1e1Pt2nONel8tcDxeEyTUVSOAfNAPUgqPTkIX8MeJBzPZH+YB1MaOO8qAieW3ZlvLgrXH95jrHz+oqO9B89E9zBMA== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=none; dmarc=none; dkim=none; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=outlook.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=Qq/OrrFp5B9WW1orUAfJiBuIVok5p+8C2kvKWyHaxzQ=; b=MAjXrwWdKXbWn9XLMDgAXqG9qO7Q8F0mMIVhq5pGL6WeMHjBkGE7F0P0aN8sHUqpnAqHOmpZXOlTxMh+6y2h1M8+utDapFIy5665h8GbZLAtlk6AIljKwGknuLk27mHDJp1AE8oj/ubdmfg+rI52NggMHvgRhcxSvK3GzLDlnZ7uUiyrtX4tExsVaXrqfArZUg5Um3GQrMB7eC4lkE8XZUcNXtKfGveLyrB7IITfEpwvKhIjztW2YzUyaxpJUxEOKoVCzDpWFC2FNO/ildkmuCDXGqL1MNuDxAFcPlSswZuHlc+ftOefcxO26kfELp3DP44u7JHev3Fx8C/d33TAMg== Received: from AS8P250MB0744.EURP250.PROD.OUTLOOK.COM (2603:10a6:20b:541::14) by PAXP250MB0447.EURP250.PROD.OUTLOOK.COM (2603:10a6:102:281::13) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.7362.34; Mon, 11 Mar 2024 14:26:53 +0000 Received: from AS8P250MB0744.EURP250.PROD.OUTLOOK.COM ([fe80::228d:8c6f:ed10:82eb]) by AS8P250MB0744.EURP250.PROD.OUTLOOK.COM ([fe80::228d:8c6f:ed10:82eb%7]) with mapi id 15.20.7362.035; Mon, 11 Mar 2024 14:26:53 +0000 Message-ID: Date: Mon, 11 Mar 2024 15:26:51 +0100 User-Agent: Mozilla Thunderbird Content-Language: en-US To: ffmpeg-devel@ffmpeg.org References: <20240311025607.3468624-1-allancady@yahoo.com> From: Andreas Rheinhardt In-Reply-To: X-TMN: [i2A8T0Apt+7KK/ErLs8/9R6am5+tOfoLLXkYVsGgGHI=] X-ClientProxiedBy: ZR0P278CA0049.CHEP278.PROD.OUTLOOK.COM (2603:10a6:910:1d::18) To AS8P250MB0744.EURP250.PROD.OUTLOOK.COM (2603:10a6:20b:541::14) X-Microsoft-Original-Message-ID: <26c07ce2-4ccc-48ad-af56-a613b703b18f@outlook.com> MIME-Version: 1.0 X-MS-Exchange-MessageSentRepresentingType: 1 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: AS8P250MB0744:EE_|PAXP250MB0447:EE_ X-MS-Office365-Filtering-Correlation-Id: 1b571ce8-a682-410b-749f-08dc41d74c04 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: 8tGcCGYDRAY2GqdI7MKDLpO8bHylRpkk86KGM/u1UI+7JUYLUlDJCKoyJPmwGMGDHsKmkNQAqh4cz6aDG5/WkSgbF8T8X2p36e5ojcvqHemhaVLbCKufvA390uxyFHeD65fWHYdoqEHpyQsaKwtYY+4tFq6l180zPyyEga2ejGQEJFOMYirpKX9FbilcUNE3IK8o4dsy5LTxbNU5tn/itWk5l0ZBdX3VTPjtPhnweBd6I2n07V9lSmzWqCVg9mf1MBabPhPoOIIu/LhmPsXiNB+FEbEvUHC6GruvoH4sdwai3Omkb5YUfg+zVYwktVyfadhW63geEMoHmfHHvW5Yi+DgpGItdoLFkTxho3JGMczHxu4EhD4qxy0R0LgxzGbBEX/r+8a60yC1YMKSV9ATF5T7TggqnCs1qNhWtDBHPRodegx35kMQAHeOXA1chkMlxqIZXJfADP9YHxaXpd4bZre8jAJ/ofrePb0Dwh7u6WWNnnyqHfAFWLq1dJhs44e97qr0j6bUt/qXtH4K432DIArsNzY27lrnhesDMYFX3bX1kulpvhWJVVRFgU/ZwZh0 X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?Tmc0SUIwU3hjVmlTdWZBc0M4bXMveVFVMy9QZC83d0VQV1pUM3hKUnRLUCtJ?= =?utf-8?B?WGY4Z2l4MStuSC9IWUVkdDNtTllldXhlMWJRd3JZQXVkOG4yL0ZqVGJweTFq?= =?utf-8?B?ZDg5dGlzUVJRU2YvMVBrZkJxbTBFT1FTZEUyYmttVFdOeHk0OEhGRER5VjZ5?= =?utf-8?B?eG5LUzEvWGdMdzk4SWtJd0dXNWxIQkx0eFJDZG84Q1pvZ2tRYlo4eEFFWHBD?= =?utf-8?B?alllUW5zYXJLNmhkMkFFdGtua1NsODJZb1NvK0NnUnpsZWxINkJCbVdOM1Nk?= =?utf-8?B?T01DRFpBNFhYRUxMTUpqSm5nUGtSeFVMd2J6QmFsWTNvbU53dXFmeVM4N0xM?= =?utf-8?B?ek92NVRERDZqcHBBVEJORzBsS2V0VXU2VjRPVXJTR015Y0l4Zks2Ump1a1Jj?= =?utf-8?B?VHNjd0ViRGhacHZxWWtjaUNJUGZaaitXcDZ1OGJ0K25Fa09JMmhCbEdWdDcr?= =?utf-8?B?U3ZaU000MzZkRTUwdWZQL3JHNkxVZVBET0RLRW0vd1BhMGZWenlhU1RBVEVm?= =?utf-8?B?eFpxZ1UrMWM2Rzg1bFRXdjVmUWpJL2RQMTRPbEI3QjI4L0ZEcER2cE9IMUw3?= =?utf-8?B?UE9oQXBwbGtvcXArWE1ZMFVhK0RuNGpFdm53TjAyZ0NBR080RFYwYWJveVQx?= =?utf-8?B?QzczcktRSGR1bUV3YzNWRFZ1QkVFeXllNkUwcXNQSEYvQUVRaFVjUDhpWU4r?= =?utf-8?B?Sit4eE5Bc2F4NlY4aGVvYUN4OFhhV2JLVHZvbWFSU1crdHpqYkhOZ3JuRzg1?= =?utf-8?B?OUplak5hc0RXQm9lNHNJc0F5SWlTcmV0WFI4RHRPSG42UWwxUXRJTzhtTFpi?= =?utf-8?B?L2dqM29qV1ovWWFmTlZETVR5QmxRTnJkTExkSEVUWUNrMWVyVjhoczhCU2xH?= =?utf-8?B?Z0hJWlc2NW5TSjEzd3dsajNBMEhzUWZWamhzNjg3MUt3VHp5N09NY0RpNDJ0?= =?utf-8?B?d2o0NFZ5R3hXQjBVZTJUb0IwSXVrRFZpQTNRN0J3Z3Q1WnkwNGFueDBRSDlj?= =?utf-8?B?V1c3VHByYnhrYld5ZGMwNzR2K01IUGNRaDE3S29UMVdYMDNiblpVK1Y5aUhB?= =?utf-8?B?TlorV1pRaCsvSkVOQllBaVRyTUsvZGs1N1RGMjlBYngvMTNvNjlTL2kyVlFm?= =?utf-8?B?WnVaSTZRWUpMNGZHK0xoNGZ4VXpuamJ5NkFvSkVManh1RkVkenZoQUpXRkh0?= =?utf-8?B?SjJjMUdaQTR4Ym05SXNVeGlYcUdZTnVOa0NYa3duaFJBcW5nUkhsbFpFV3hz?= =?utf-8?B?bUIxT0hzcTVuT2YwcEhMQ1VBVEVJQXBoWVgxekNleVgrMm0xcTFtOFUzR0o4?= =?utf-8?B?TVBBemJNWkpOWG83M0c0SUlTYlkxNkVZOUxCRmpFbmwzU3YvM2FHY0hXYm9C?= =?utf-8?B?Wm5aMENYTlozRnhGZ1VSMzBOV05XeE9hQTJZQ09NWE9xWlRMa0lKb3cxTDlG?= =?utf-8?B?RUtEdldUTmNFUVdpV1lrb1BSaW01bXR0VnhFbUMyK2pPYkMweis0TVpWVGl6?= =?utf-8?B?T3NuYUgrc2I3SjBSZVRWVlZ1OGdJSW5HNGtIUWYyeDRqS1RVdmYrTDVhNUdB?= =?utf-8?B?UWpyZnRvdFI5MjdNcUFsMTZHcC9FY0tSTWhwUEtadkJyeDBvRjNBMmFaUWc2?= =?utf-8?B?WUM5TzRCT0NpSzFsR2QxUGtwekIrSlc0NnJoTlRYOVplTSsvcUJPSmRYaEg2?= =?utf-8?B?OTZCU3hYbndrKzNBUFg1a2dSZlZDVzNzdHc4QnlmSmg1S1Z2RDZsbTd3PT0=?= X-OriginatorOrg: outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 1b571ce8-a682-410b-749f-08dc41d74c04 X-MS-Exchange-CrossTenant-AuthSource: AS8P250MB0744.EURP250.PROD.OUTLOOK.COM X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 11 Mar 2024 14:26:53.0742 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 84df9e7f-e9f6-40af-b435-aaaaaaaaaaaa X-MS-Exchange-CrossTenant-RMS-PersistedConsumerOrg: 00000000-0000-0000-0000-000000000000 X-MS-Exchange-Transport-CrossTenantHeadersStamped: PAXP250MB0447 Subject: Re: [FFmpeg-devel] [PATCH] 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. 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-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: Andreas Rheinhardt: > Allan Cady via ffmpeg-devel: >> From: "Allan Cady" >> >> I propose changing the format to "%.6f", which will >> give microsecond precision for all timestamps, regardless of >> offset. Trailing zeros can be trimmed from the fraction, without >> losing precision. If the length of the fixed-precision formatted >> timestamp exceeds the length of the allocated buffer >> (AV_TS_MAX_STRING_SIZE, currently 32, less one for the >> terminating null), then we can fall back to scientific notation, though >> this seems almost certain to never occur, because 32 characters would >> allow a maximum timestamp value of (32 - 1 - 6 - 1) = 24 characters. >> By my calculation, 10^24 seconds is about six orders of magnitude >> greater than the age of the universe. >> >> The fix proposed here follows the following logic: >> >> 1) Try formatting the number of seconds using "%.6f". This will >> always result in a string with six decimal digits in the fraction, >> possibly including trailing zeros. (e.g. "897234.73200"). >> >> 2) Check if that string would overflow the buffer. If it would, then >> format it using scientific notation ("%.8g"). >> >> 3) If the original fixed-point format fits, then trim any trailing >> zeros and decimal point, and return that result. >> >> Making this change broke two fate tests, filter-metadata-scdet, >> and filter-metadata-silencedetect. To correct this, I've modified >> tests/ref/fate/filter-metadata-scdet and >> tests/ref/fate/filter-metadata-silencedetect to match the >> new output. >> >> Signed-off-by: Allan Cady >> --- >> libavutil/timestamp.h | 53 +++++++++++++++++++- >> tests/ref/fate/filter-metadata-scdet | 12 ++--- >> tests/ref/fate/filter-metadata-silencedetect | 2 +- >> 3 files changed, 58 insertions(+), 9 deletions(-) >> >> diff --git a/libavutil/timestamp.h b/libavutil/timestamp.h >> index 2b37781eba..2f04f9bb2b 100644 >> --- a/libavutil/timestamp.h >> +++ b/libavutil/timestamp.h >> @@ -25,6 +25,7 @@ >> #define AVUTIL_TIMESTAMP_H >> >> #include "avutil.h" >> +#include >> >> #if defined(__cplusplus) && !defined(__STDC_FORMAT_MACROS) && !defined(PRId64) >> #error missing -D__STDC_FORMAT_MACROS / #define __STDC_FORMAT_MACROS >> @@ -53,6 +54,32 @@ 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) >> >> +/** >> + * Strip trailing zeros and decimal point from a string. Performed >> + * in-place on input buffer. For local use only by av_ts_make_time_string. >> + * >> + * e.g.: >> + * "752.378000" -> "752.378" >> + * "4.0" -> "4" >> + * "97300" -> "97300" >> + */ >> +static inline void av_ts_strip_trailing_zeros_and_decimal_point(char *str) { >> + if (strchr(str, '.')) >> + { >> + int i; >> + for (i = strlen(str) - 1; i >= 0 && str[i] == '0'; i--) { >> + str[i] = '\0'; >> + } >> + >> + // Remove decimal point if it's the last character >> + if (i >= 0 && str[i] == '.') { >> + str[i] = '\0'; >> + } >> + >> + // String was modified in place; no need for return value. >> + } >> +} >> + >> /** >> * Fill the provided buffer with a string containing a timestamp time >> * representation. >> @@ -65,8 +92,30 @@ 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, >> 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); >> + if (ts == AV_NOPTS_VALUE) >> + { >> + snprintf(buf, AV_TS_MAX_STRING_SIZE, "NOPTS"); >> + } >> + else >> + { >> + const int max_fraction_digits = 6; >> + >> + // Convert 64-bit timestamp to double, using rational timebase >> + double seconds = av_q2d(*tb) * ts; >> + >> + int length = snprintf(NULL, 0, "%.*f", max_fraction_digits, seconds); >> + if (length <= AV_TS_MAX_STRING_SIZE - 1) >> + { >> + snprintf(buf, AV_TS_MAX_STRING_SIZE, "%.*f", max_fraction_digits, seconds); >> + av_ts_strip_trailing_zeros_and_decimal_point(buf); >> + } >> + else >> + { >> + snprintf(buf, AV_TS_MAX_STRING_SIZE, "%.8g", seconds); >> + } >> + >> + } >> + >> return buf; >> } >> > > 1. What makes you believe that all users want the new format and that it > does not cause undesired behaviour for some (maybe a lot) of them? The > number of characters written by the earlier code stayed pretty constant > even when the times became big (in this case, it just printed 8 chars if > ts>=0), yet your code will really make use of the whole buffer. > Granted, we could tell our users that they have no right to complain > about this, given that we always had a "right" to use the full buffer, > but I consider this a violation of the principle of least surprise. Why > don't you just change silencedetect or add another function? > 2. For very small timestamps (< 10^-4), the new code will print a lot of > useless leading zeros (after the decimal point). In fact, it can be so > many that the new code has less precision than the old code, despite > using the fill buffer. > 2. This is way too much code for an inline function. > 3. Anyway, your placement of {} on their own lines does not match the > project coding style. > In addition to this, there is another issue here: Your av_ts_strip_trailing_zeros_and_decimal_point() presumes that the "decimal-point character" is always '.', but this can be changed via setlocale(). - Andreas _______________________________________________ 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".