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 5029C4A114 for ; Sat, 20 Apr 2024 16:48:12 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 1B7D468D1CF; Sat, 20 Apr 2024 19:48:10 +0300 (EEST) Received: from EUR01-DB5-obe.outbound.protection.outlook.com (mail-db5eur01olkn2037.outbound.protection.outlook.com [40.92.64.37]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 249E768CE03 for ; Sat, 20 Apr 2024 19:48:02 +0300 (EEST) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=oStR4oIYFT5cNPzV2duNOk0QrKOIw0/IqTN9+YC1NXvKYM5veuyzgZosO8dci50pOVNqXD0RgsB8unbrtLYZwFFNSDXVNuQYKR7wTx7lXaYUxMGxLfhz5e0pPbA68ezUX26CsVX3ni8M9ewGIIf9zI9W0wB/bStE6nAlxLVhcPnISSqmIf1p+zAJ5yjW4tIqbZmIWbAwvd+pIQciGIaal5S5A6kUJDu90RMaSG/8akmHXR40DKc3WakvLI5/VqFIdcYNYICfipS3odDj3AvalMDuKQxKjD7RZpqaHVeu/uHP14+lDpMT/9nfdgadEMTESr5SFQR1JbYG9zve8p7J3Q== 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=yX9/f42B2m06x+Mouf5nvk5Kompv1F3om4kBBQh8zkE=; b=LTG7xJOb2MQAt26OIBU4mn/4Hp1ST49aVBY0Cwrk+RLrWssjk8QvTQQG9HqM1e6RP0kKx8OgJ6EJpDyvEuNNET3iVf3y7THF2iCJdPyA4A1Xf7U8d7IY7cygyB3eHZOCQtqwy29XuYdI5f1QzakNjWxezgF8RU9AQ+yuIf6lbJCWDrFLkOQqvp42lMIN04j8rQarGIspuXJZJVNjqeFFUTsbelVXlt5m29moPJRJ+fELyWHzPuF9eqd3ByrtgFAC9h+8FLFWH+ErVIjAm6QYDS/nIc6kao2rvTk9ciF3f3ylkd6sHcriFeBxZENRGAQjXhGolHxyLRKNtlNBsMCdGQ== 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=yX9/f42B2m06x+Mouf5nvk5Kompv1F3om4kBBQh8zkE=; b=QzFgSXusLSfvwkFqg0YqRq78INecd0LE24JiN5uFQVzvhO2ayeouxbucWhaC4bqlO/rymZRCF4vJ300/goeAdkUROS8e3+v5vFBsrYBKYEBk3UtO0+GZoy3t9j9MgUK/b2xWGsJRjKprdfdQbEQXsysqAz/okKrAJETJtLxFZ1jydjfKGTaql4c6It+LLM1moWzoEEkjy8QkJnl97xQpBhjy3vkEAw+a98EV3ItcagMeYjJjXyp2n+5XebkO0mgQHtDwP6KqVrQ2HUkjFnSuxkvqduzokr6Z5HLhX8rliDNBW6cbfRTCXRsrBSPlWFCGkph0VM76rljY5M/SwndxCg== Received: from AS8P250MB0744.EURP250.PROD.OUTLOOK.COM (2603:10a6:20b:541::14) by DU2P250MB0143.EURP250.PROD.OUTLOOK.COM (2603:10a6:10:274::12) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.7472.44; Sat, 20 Apr 2024 16:48:01 +0000 Received: from AS8P250MB0744.EURP250.PROD.OUTLOOK.COM ([fe80::1f29:8206:b8c3:45bb]) by AS8P250MB0744.EURP250.PROD.OUTLOOK.COM ([fe80::1f29:8206:b8c3:45bb%3]) with mapi id 15.20.7409.042; Sat, 20 Apr 2024 16:48:01 +0000 Message-ID: Date: Sat, 20 Apr 2024 18:47:58 +0200 User-Agent: Mozilla Thunderbird To: ffmpeg-devel@ffmpeg.org References: <20240420114835.35645-1-stefasab@gmail.com> <20240420114835.35645-3-stefasab@gmail.com> Content-Language: en-US From: Andreas Rheinhardt In-Reply-To: X-TMN: [2bd53+gkiRzFspusYzpNpsswpo6Wo3edXyBIhCoLBlc=] X-ClientProxiedBy: ZR0P278CA0209.CHEP278.PROD.OUTLOOK.COM (2603:10a6:910:6a::26) To AS8P250MB0744.EURP250.PROD.OUTLOOK.COM (2603:10a6:20b:541::14) X-Microsoft-Original-Message-ID: <85eb3fa8-a4ed-45c5-a00c-ea4592b1184d@outlook.com> MIME-Version: 1.0 X-MS-Exchange-MessageSentRepresentingType: 1 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: AS8P250MB0744:EE_|DU2P250MB0143:EE_ X-MS-Office365-Filtering-Correlation-Id: cedd4414-52b1-4446-fca6-08dc6159a3e1 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: I4urdZzIWuM5YaJCnrBDZh5YLgx2h2XOLL4+8SvaJ+MGXvpwOin+d2IIwevqF5DbtQ+Y0Fv8ZYpwC9ztE3WS5OemNFVarQJYQuLK7svPye+Pn3aH6ElGFUTDMbxCRAgpkfM8naIxUgeRsTHZArwTVK+dqyZPp4qzSlTX0eEExoVzDVZz8WjiueO/eUC6RZZ4WwngjYp7CxYxCZWbmKpgCPHergO/Y2PW9cjDauy5dEB6iHiMAqWr+RRa9zJlw2Dt2ily2eGDUokfjud6b9zVNwGYAM61JV03UCDgapWrk4h/OZ/DjQ78+2U8f9ocxTcFlzplSekl36aNGOu09x63FMeoyGIgGJz/oFq7CC/TSDcW1D8kjzwHRe0O/eqK9MW6Myv/om+wItRzGT5PYfNSPZJTVNTU4o6MLQKs2cRtHeD4JkM2a0Dvgu7VFM+Kva+Nr7LjacOpZ2YAJ3gLk6CywqvlTtgeEI7NtPT1ke4BNfuIHw/9EkxZy5pzvQ1z0XDSqykRkbiHpsTWSFo+A0t+Qh7UXTlWT2WrgACRCOmkZ4kolRLegnR4qTr6/1T8y6XL X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?OXV4Yzl4dE9KQ2JtdGhrRm84bEpWcm13SjR2ckpFcEFVTU9nMktaR1NBV0xZ?= =?utf-8?B?SGNLYWtROU51ZG4vMzZBa2VWV2c3N1hOdDdsaWpDTUcxZkczTGJZeStYYitU?= =?utf-8?B?RUdhekt5Uk50ODlxdWFyTkIyNHY0Rmx1c2dtaWJDckxveWYvZ0h4d2tvMEt5?= =?utf-8?B?Z3k5c2FoS21GblJYQnZseGRjcDMrTE1KRmJCazFnMTdxdjFxSFJnRStaREpy?= =?utf-8?B?MU9HQUhPU3NKRktWTnFJY1d5Tk1veDZkNC8va0RhSzJVejFDV3JseDRlWUo5?= =?utf-8?B?dm04RmlmdC8rU29LNjMzMXQ1bEYyQ29FeTRleE5BUGREZldmRjE2dndobHAv?= =?utf-8?B?MnZFZ0YyWHdYTWh1ektkOWV6YVQzYno0djNyQ2d5cWxvNFcxMHJ0YWo1V0xQ?= =?utf-8?B?Sm14YVBFVDZHRVE4eGI3UmU0VTFncDZaK3J1dkRhR3ZZSHlVYmo3Und6MDNr?= =?utf-8?B?dk5ubWV6ZEoyY3VHYS9za3drRkthU251ek83d2QrRG1Ca1BBZ3F6M1RodmZi?= =?utf-8?B?dk1iQlFVcnMxMFhpZDE5Z1lBT2JyR1I2dlZhUm54dk1LcUsvSlFrNXExTy9y?= =?utf-8?B?TnloNURPdG8rMktaU21oWHJ6Y3BlOEF4VGRORDZCamZUc2dRR1dpRHAwb1FG?= =?utf-8?B?anEraktCWlR0T1NDS0R3UmNFaWJXQ01TL2VnQXpWUmhqTjY0eUdUYXR0eVA4?= =?utf-8?B?WEUzOFhPRHlLZytqN29ybXZ0NkdQeHUrTERHVm1ZV0ZDVW5rVFROZkVZVktE?= =?utf-8?B?SXhtdGdVZDhtWS9kSlR5R0J0Rmw5bWg3WVVDYk1pNXcvYk4zMUdwaGVKRTVS?= =?utf-8?B?WDdMMmd3cVhmY2w4TTkyakVHa2k5bWhHT1cvZjdRVU45V0ZOU1VKRFlNUEhy?= =?utf-8?B?NGVwdjJnYThwcWtvcUFjWmVRN3lxNmZCTERrTkhURmhEcVZKQmtwRHlYaUt6?= =?utf-8?B?OE9yS1pMcVdpV3hOLzVOdlJHZ01PVW4zVTJLOGV6ZklHZW9uV01HN0dTWU9Q?= =?utf-8?B?UzVkTG9SbW4wYkFXMWhxRWN5eVlTOEpUZUVTYmJBSEJ6Q0VHU1NtU1BPdUFw?= =?utf-8?B?alJwVjNkS1JUemVaa2VGQUxHZFRNRnJMbDFsQ2NTTGVtTnlIN0piNXFLUG1q?= =?utf-8?B?NmVCMTlJczlrbnhQZ3llbjVrbDlTbmJIWDFPeUo2UzA5cEpwZFN5VlBsZkpq?= =?utf-8?B?MXAvelF3eE1tVWZYN2tDdEwzMWEyTmNOMm1PVjc4UElYNGxpaXQwWWU5SXA3?= =?utf-8?B?QTRCdi9KN0ZmbjBta2V3ZUxmNHU0bTdWVGczRlZ4alR3Wm5DeVB5cy9yakRp?= =?utf-8?B?ZlNHYmsvTnY0bmU0V3JLZFBjUGF3UWhmV0JXa1oxenoxWDZCVjd2NmM4Q08w?= =?utf-8?B?RVR0TVVMaEQrdzBvZm9HR0NjWmpYWVRvRFkxZjVTdHdvNXI5ZEg0K3A3TUg5?= =?utf-8?B?cG5hWFUvL1l1cUR4QVJCNklMOGIzQm1RMlZRSUg4aHF4SCtjSnBVNWNnZDds?= =?utf-8?B?UlRraVVzTjd1TnZueE02S2l5MEhzYXprVEgwbVJLblJwSEY3SUZJTGNrT2hm?= =?utf-8?B?TTJMaTZXS3NsYk5XbWJKbjJNL3l4L0NOZitGbW82aGFzRWYxSm95by9JVncw?= =?utf-8?B?WVZtY0dvSHJDbWlvb0Q0MUE4VGdFeU5GdndiZnlvdGNmbDc3Lytab1Nhczh4?= =?utf-8?B?UTJNOUVhNUVpWGJNSHRCYVVacjdNamxNODlBVW50eW93UnN4YkUrcklRPT0=?= X-OriginatorOrg: outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: cedd4414-52b1-4446-fca6-08dc6159a3e1 X-MS-Exchange-CrossTenant-AuthSource: AS8P250MB0744.EURP250.PROD.OUTLOOK.COM X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 20 Apr 2024 16:48:01.1468 (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: DU2P250MB0143 Subject: Re: [FFmpeg-devel] [PATCH 2/3] lavf/mkvtimestamp_v2: review implementation to match mkvextract behavior 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: Stefano Sabatini: > On date Saturday 2024-04-20 15:18:39 +0200, Andreas Rheinhardt wrote: >> Stefano Sabatini: >>> Harmonize internal implementation with the mkvextract behavior: >>> - print PTS in place of DTS values >>> - ignore NOPTS values >>> - sort PTS values >>> --- >>> libavformat/mkvtimestamp_v2.c | 69 +++++++++++++++++++++++++++++++++-- >>> 1 file changed, 65 insertions(+), 4 deletions(-) >>> >>> diff --git a/libavformat/mkvtimestamp_v2.c b/libavformat/mkvtimestamp_v2.c >>> index 1eb2daf10a..c6446ed489 100644 >>> --- a/libavformat/mkvtimestamp_v2.c >>> +++ b/libavformat/mkvtimestamp_v2.c >>> @@ -22,30 +22,91 @@ >>> #include "avformat.h" >>> #include "internal.h" >>> #include "mux.h" >>> +#include "libavutil/qsort.h" >>> + >>> +#define PTSS_MAX_SIZE 128 >>> +#define PTSS_HALF_SIZE (PTSS_MAX_SIZE >> 1) >>> + >>> +struct MkvTimestampContext { >>> + int64_t ptss[PTSS_MAX_SIZE]; >>> + size_t ptss_size; >>> +}; >>> >>> static int write_header(AVFormatContext *s) >>> { >>> - static const char *header = "# timecode format v2\n"; >>> + static const char *header = "# timestamp format v2\n"; >>> avio_write(s->pb, header, strlen(header)); >>> avpriv_set_pts_info(s->streams[0], 64, 1, 1000); >>> + >>> return 0; >>> } >>> >>> +static int cmp_int64(const void *p1, const void *p2) >>> +{ >>> + int64_t left = *(const int64_t *)p1; >>> + int64_t right = *(const int64_t *)p2; >>> + return FFDIFFSIGN(left, right); >>> +} >>> + >>> static int write_packet(AVFormatContext *s, AVPacket *pkt) >>> { >>> char buf[256]; >>> + int i; >>> + struct MkvTimestampContext *m = s->priv_data; >>> + >>> if (pkt->stream_index) >>> av_log(s, AV_LOG_WARNING, "More than one stream unsupported\n"); >>> - snprintf(buf, sizeof(buf), "%" PRId64 "\n", pkt->dts); >>> - avio_write(s->pb, buf, strlen(buf)); >>> + >>> + if (pkt->pts == AV_NOPTS_VALUE) { >>> + av_log(s, AV_LOG_WARNING, "Found PTS with no value, ignored\n"); >>> + return 0; >>> + } >>> + >>> + if (m->ptss_size > PTSS_MAX_SIZE) { >>> + // sort all PTSs >>> + AV_QSORT(m->ptss, PTSS_MAX_SIZE, int64_t, cmp_int64); >>> + >>> + // write only the first half and copy the second half to the >>> + // beginning of the array >>> + for (i = 0; i < PTSS_HALF_SIZE; i++) { >>> + snprintf(buf, sizeof(buf), "%" PRId64 "\n", m->ptss[i]); >>> + avio_write(s->pb, buf, strlen(buf)); >>> + m->ptss[i] = m->ptss[i + PTSS_HALF_SIZE]; >>> + } >>> + >>> + m->ptss_size = PTSS_HALF_SIZE; >>> + } else { >>> + m->ptss[m->ptss_size++] = pkt->pts; >>> + } >>> + >>> + return 0; >>> +} >>> + >>> +static int write_trailer(struct AVFormatContext *s) >>> +{ >>> + struct MkvTimestampContext *m = s->priv_data; >>> + char buf[256]; >>> + int i; >>> + >>> + // sort all PTSs >>> + AV_QSORT(m->ptss, m->ptss_size, int64_t, cmp_int64); >>> + >>> + /* flush remaining timestamps */ >>> + for (i = 0; i < m->ptss_size; i++) { >>> + snprintf(buf, sizeof(buf), "%" PRId64 "\n", m->ptss[i]); >>> + avio_write(s->pb, buf, strlen(buf)); >>> + } >>> + >>> return 0; >>> } >>> >>> const FFOutputFormat ff_mkvtimestamp_v2_muxer = { >>> .p.name = "mkvtimestamp_v2", >>> - .p.long_name = NULL_IF_CONFIG_SMALL("mkvtoolnix v2 timecode format"), >>> + .p.long_name = NULL_IF_CONFIG_SMALL("mkvtoolnix v2 timestamp format"), >>> .p.audio_codec = AV_CODEC_ID_NONE, >>> .p.video_codec = AV_CODEC_ID_RAWVIDEO, >>> .write_header = write_header, >>> .write_packet = write_packet, >>> + .write_trailer = write_trailer, >>> + .priv_data_size = sizeof(struct MkvTimestampContext), >>> }; >> > >> 1. This does not match mkvextract behaviour. mkvextract does not force a >> 1ms timebase. > > From your past comment: >> The accuracy of the timestamps output by mkvextract is determined by the >> TimestampScale of the file in question; it is most often 1ms when the >> file has video. > "most often" != "force" > Note that this is only used for video. I don't know how this > TimestampScale is computed, but I wonder what was the point of this > muxer. Probably it was only used to extract the timestamps with a > fixed timescale, and the fact that it was similar to mkvextract was > not really an important factor. > mkvextract simply shows the timestamps as decimal number in ms (given that the timebase of Matroska files is always a multiple of 1 nanosecond, at most six digits after the decimal point are necessary for this). One would need to remove the avpriv_set_pts_info and convert the numbers to fractional ms representation to do likewise. >> 2. AV_QSORT should only be used in speed-critical code, which this here >> is definitely not. > > Why? What should I use instead? qsort > >> 3. I still don't think that this muxer should exist at all. > > I tend to agree. But: > > We don't really know why this weird muxer was added, today we have > better tools for that (we could use ffprobe, or even a bitstream > filter similar to showinfo to get the same result), but if it was > added probably there was a reason. So my plan is to make the format a > bit more useful (DTS => PTS+sort), and possibly deprecate it and point > to better available tools and drop this in two major releses. > > I don't think the point of the format was to really make the behavior > exactly equal to mkvextract, the thing with TimeScale is probably not > very important, at least for the original author that was not really > an issue, he was probably only looking for some way to dump timestamps > and took free inspiration from mkvtoolnix. > This thing has been added in 1c5670dbb204369477ee1b5d967f9e8b4f4a33b8. I can't find any discussion in the mailing list archives for this, but the file description was "extract pts as timecode v2, as defined by mkvtoolnix" at the time. Furthermore, its author is the one who started the Matroska muxer. So I think its aim was really to mimic mkvextract. (I am not certain wrt MKVToolNix handling of fractional millisecond; old versions of mkvextract may really have simply rounded/truncated to milliseconds.) > If this is true, we might point that this format is not exactly > equivalent to timestamp_v2 in the doc. Which makes this thing even more pointless. - 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".