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 B1D524218B for ; Sat, 15 Jan 2022 21:28:20 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id C99B568A77A; Sat, 15 Jan 2022 23:28:17 +0200 (EET) Received: from NAM12-DM6-obe.outbound.protection.outlook.com (mail-dm6nam12olkn2061.outbound.protection.outlook.com [40.92.22.61]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 1285368A255 for ; Sat, 15 Jan 2022 23:28:11 +0200 (EET) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=d/rqNMFGxz9sAI8ZAgc/EXXL3RBM/Rh7N02B1olemNpqitBACy+hQVnoPlcdqxaGmgWgx9N578CD+Tn2DxTE/v7NDcK0N80HSuutuxoPrr2jMPDfYSSzqxI37LJwbbEiczGkek5/MCHWPbQO6SEASIdU8DqF7CeeOoqiIxZ5lhWQR5UkkDg2NBmJy5FqfZG1pVx1bLz6V1JJXUk79sV/nNkmNThkqWZlAImpAFY9aAeGmRPehXLAh4aSllEYJLhStWiNZHT04hpnA7xeULBtMbESCNk/fu2RyAQOeuYww3SXUCqE4gz8z09ZqYTGWMNYJGA61rBnlWiYQJWJs55tlg== 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=ilBFo+LzpUVy1ORyhKPaycIHIdFXOYfAAAuvFwpNuN4=; b=ixIEBmci/+vL0yXVq9hcDomx7u+FpvTF+9AGNIZ5m4sHt9B46eIeuGOdAAzfGK/y0gQT9ZO3uITOP0FwJ8fFvjwz1wluA5YWRzQdagUDxYBkgBAqhZsnr95mC/XulL/lVVWpdM+PisUrnsdOkfK/UKScsAcDTPZMXl1IjsEaWmnArXUYSZ5Ia/7E7zH5LsuwmTMp8OqgTHaO+iDJX6CCMOY7U7cy3zz7vwZG7Prg8KKZrc3RbjPaH8AQV8a/5HbehODPgqNVbyASbaXw2R3t5ePNMVcEIJHOgCJz40pn+e+KY8tpwXbKNT2NZCRdBufvLaD4OPm+9N1aefFMTxvr2w== 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=hotmail.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=ilBFo+LzpUVy1ORyhKPaycIHIdFXOYfAAAuvFwpNuN4=; b=fqx+zgiWoFkAMEe/qB2o67n7JQU/L6XiZBmxwK51paycq4ePTEuG7Ehsv5Uw0dPIV1z0tzmfPpECzIHvHaD7H2erLnweGg7OTThOi0Cg4B6ZmJR/N7oEOx4uiBLYEtMPC0YEv5HqPsCECTGEHzUOYh6L1TidlOYC0Qnl1P95qvIA/b4NLrsc17xMB+YZqg/m4EU4GPEtiGVRnTPW8TkAc6oy8+FoSB9ZZfEWbWBKIuYyLGSbDSCgt38badeZkNQiVo1KzlGIoixvwMhYFaN+YASmsG+iMXMUo/34heRl61wezz9qG+TVMYX/OzXmQhFSLkzqxb5rj/vU1dMywl2dzg== Received: from DM8P223MB0365.NAMP223.PROD.OUTLOOK.COM (2603:10b6:8:b::20) by BL3P223MB0338.NAMP223.PROD.OUTLOOK.COM (2603:10b6:208:34d::16) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4888.9; Sat, 15 Jan 2022 21:28:08 +0000 Received: from DM8P223MB0365.NAMP223.PROD.OUTLOOK.COM ([fe80::54ae:66eb:e304:96d6]) by DM8P223MB0365.NAMP223.PROD.OUTLOOK.COM ([fe80::54ae:66eb:e304:96d6%7]) with mapi id 15.20.4888.013; Sat, 15 Jan 2022 21:28:08 +0000 From: Soft Works To: FFmpeg development discussions and patches Thread-Topic: [FFmpeg-devel] [PATCH] avformat/hlsenc: Fix path handling on Windows Thread-Index: AQHYCbb0X5En51serUyBtJh6PLfb5KxjohcAgAAHj7CAAL++AIAAE8rwgAAc5eA= Date: Sat, 15 Jan 2022 21:28:08 +0000 Message-ID: References: In-Reply-To: Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-tmn: [CXwghbFdUIH0oW+vy3q8YHAQIi7wEgZ6] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: fb64bd18-47b5-41f2-969b-08d9d86dec96 x-ms-traffictypediagnostic: BL3P223MB0338:EE_ x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: xOgqxoT1hUcR/poOyjUE+vthY5LOMHZUINGilvYvWPedYWcFu9fyaWQkuf+r4+ObaT7kRsYK2aNun3yOapQglKs+7InvhfweBNY0lKKODBcqOdqqWim7oR6K7drRtJqKhm5EWrj5XfpS7vjkm5mCgwUAviFrVEulxngwDk+xJ8RTHMakbB075asJrIV0XjoXYnfErXvm2lOMc+fIGK6HPyfxhPvEVE8vTsY4XZXFyTj6UXE8d8ZUB+MOAZQ6N42kE4S5WUbO9ivb+XQwFGTk2d9Php72MjHm3lhedpj2kAmJuRR1cBxvy/kAZFWzWS4Srtxo7HHrGyUaOQZ6p/ZaAmpIWlcbmE0gpQw9+gBXOoBghiyfb3ne7j16ofRqlhKy3pvejOWilCT0qpatjioQh8/hXnB9AZGhL7rpTOjjcQfUTlqaZt5TPHU0B9PhemeoAzYUvh6zdFQU+TImNl9LIL+/rg4SHl5eVY1JYA2n+Dn70+IVNloW5pe9+yRZbPPW0rDnyXtIYusAzumbz+rYT/OFdFBxfeN5xZJntrD4cBjDhCW9V4m+lOneHYaraqXBSYtb8gsKdBaGjmk04eok64F8TRK8ItALCgH5l6VzGz4EMfVYNmNyvf4xlrdzcrsvXkjh+HdVwc7sG08qv/qgQiGhK5CoF36iWile9goKZqU= x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?utf-8?B?dk5qdjJJeCt1WEhTN1doeW1jL2tYMUljVGp6QndWbEFyV3BRb1dXc0pJUk43?= =?utf-8?B?eVdWQVVXczg2dEhPUlN3c1lWOG9xS2Eycm5GNjR0TkpWNTlzMDI5Qm9nRS9V?= =?utf-8?B?b2Z4eW9vK2hoYXNwaXV0cFhxb2RqSkVLbExjeTJ0eGpac2lYbHQ2TnAwMFFp?= =?utf-8?B?RmlkR0t6Z0k4aGp4ZFRWWTBwcG1xM0Z3a0dFcFB6T1ZHTkRjMk1jcFdPUHVo?= =?utf-8?B?ZDlXcGRlcWQ3Vlh3YjhYOFo2d2ZrendETzVVMG5GYytlb2FKUUF6b2lHMDA2?= =?utf-8?B?NVhDQjRBdkl2TzFYUmpaSHBhbmQyemhwbHJubjFGVllmTFBjaGp0cllNWS94?= =?utf-8?B?ZzBuZnJkQzg5ekUwMlU0VFE5MmEvblpTVU50ckcvUnIrTkUwL2IwcS90alRx?= =?utf-8?B?clVMN2RJVFlyT1ZTemJVZXVNTnRpOUJFSW1NdWRsMHpDdVd1SXhna1doWmsz?= =?utf-8?B?eUZUVlRxcjB2WTYvU1ROSktBTVZOY3RxWDliQW80M0xuYUVyWU1nUG82aE9D?= =?utf-8?B?NkJiSjRjNzB0Q3o1b2tNMkxKVjRia1FXTlJ5WVI3NDdNL2o3RnVYSmtJQUM5?= =?utf-8?B?aVJxRFJSMWdVTXA2NUtLNUdtOENFbnVTa2wzcjlmM3BTRks3dlE5RHY3QmtD?= =?utf-8?B?ZlljZnVpQmxkOVdEU2RFRTd1ajM4RCtMcEg5d2Rhb0FKTC81ZmQ1a0phUFFE?= =?utf-8?B?WjdFQ2tmNFdvaitBZmh5V0ZXaEpady9tM2pDdEtLV2U3ZjZobjcxenV4WUkw?= =?utf-8?B?SmhPT09JQUtqRHhVUXJPWjZxS0JDZVlwQzFWUWhpTnNrSm9uMVc3OTNMOTRo?= =?utf-8?B?eVM4Z3pUMXc1eTllQlRJVEgydWEzdU1wNnRUWDUyc0JzbGdjc3RySlQ0STdn?= =?utf-8?B?RndTQ211Vi8rckZWbkROZXJHZVljNFEwTzNIbXRXcjB6dEJtSmRrMWMwU2tm?= =?utf-8?B?NGdHdmFJUFR2aUxtbEtBaGovWVBFbmcvM1k1K2dZQ1BUbDUyQVZidG1na1Bh?= =?utf-8?B?L1ZEYitlQmllM0JUSXh3aDJKNkxvSzF6cVE1cXZEM2wzRGgzYlZWanRUbDBZ?= =?utf-8?B?akdxTEVZS1JFZ1NJa0FKdVN2cHpUUHBybkhmVnkzeTFZMThYVUVLTDlBaW52?= =?utf-8?B?WkRqNmlxZ0ZoWkgyTEpseGwraGtXeVppbElxLzdEK0VzR1ZhYTJVVTcya0Z0?= =?utf-8?B?Y1pvRWFhVXU2L3RHWUNINERCRDJmVUJ4T0VoQndzeG5xaFcvTXdNNCs5OVEw?= =?utf-8?B?S0VVc1l2aVNZUVBwSFRva01jdWtpb1U3eFR0SGYySGxWVlpRdz09?= MIME-Version: 1.0 X-OriginatorOrg: sct-15-20-4755-11-msonline-outlook-1ff67.templateTenant X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: DM8P223MB0365.NAMP223.PROD.OUTLOOK.COM X-MS-Exchange-CrossTenant-RMS-PersistedConsumerOrg: 00000000-0000-0000-0000-000000000000 X-MS-Exchange-CrossTenant-Network-Message-Id: fb64bd18-47b5-41f2-969b-08d9d86dec96 X-MS-Exchange-CrossTenant-originalarrivaltime: 15 Jan 2022 21:28:08.0557 (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: BL3P223MB0338 Subject: Re: [FFmpeg-devel] [PATCH] avformat/hlsenc: Fix path handling on Windows 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: > -----Original Message----- > From: ffmpeg-devel On Behalf Of Soft Works > Sent: Saturday, January 15, 2022 10:18 PM > To: FFmpeg development discussions and patches > Subject: Re: [FFmpeg-devel] [PATCH] avformat/hlsenc: Fix path handling on > Windows > > > > > -----Original Message----- > > From: ffmpeg-devel On Behalf Of Andreas > > Rheinhardt > > Sent: Saturday, January 15, 2022 7:34 PM > > To: ffmpeg-devel@ffmpeg.org > > Subject: Re: [FFmpeg-devel] [PATCH] avformat/hlsenc: Fix path handling on > > Windows > > > > Soft Works: > > > > > > > > >> -----Original Message----- > > >> From: ffmpeg-devel On Behalf Of > Andreas > > >> Rheinhardt > > >> Sent: Saturday, January 15, 2022 7:40 AM > > >> To: ffmpeg-devel@ffmpeg.org > > >> Subject: Re: [FFmpeg-devel] [PATCH] avformat/hlsenc: Fix path handling > on > > >> Windows > > >> > > >> ffmpegagent: > > >>> From: softworkz > > >>> > > >>> Signed-off-by: softworkz > > >>> --- > > >>> avformat/hlsenc: Fix path handling on Windows > > >>> > > >>> Handling for DOS path separators was missing > > >>> > > >>> Published-As: https://github.com/ffstaging/FFmpeg/releases/tag/pr- > > >> ffstaging-19%2Fsoftworkz%2Fsubmit_hlspath-v1 > > >>> Fetch-It-Via: git fetch https://github.com/ffstaging/FFmpeg pr- > ffstaging- > > >> 19/softworkz/submit_hlspath-v1 > > >>> Pull-Request: https://github.com/ffstaging/FFmpeg/pull/19 > > >>> > > >>> libavformat/hlsenc.c | 4 ++++ > > >>> 1 file changed, 4 insertions(+) > > >>> > > >>> diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c > > >>> index ef8973cea1..eff7f4212e 100644 > > >>> --- a/libavformat/hlsenc.c > > >>> +++ b/libavformat/hlsenc.c > > >>> @@ -3028,6 +3028,10 @@ static int hls_init(AVFormatContext *s) > > >>> } > > >>> > > >>> p = strrchr(vs->m3u8_name, '/'); > > >>> +#if HAVE_DOS_PATHS > > >>> + p = FFMAX(p, strrchr(vs->m3u8_name, '\\')); > > >>> +#endif > > >>> + > > >>> if (p) { > > >>> char tmp = *(++p); > > >>> *p = '\0'; > > >>> > > >>> base-commit: c936c319bd54f097cc1d75b1ee1c407d53215d71 > > >>> > > >> > > > > > > Thanks for reviewing. > > > > > >> 1. You seem to be under the impression that NULL <= all other pointers. > > >> This is wrong. Relational operators acting on pointers are only defined > > >> when both point to the same object (the case of "one past the last > > >> element of an array" is also allowed) and are undefined behaviour > > otherwise. > > > > > > The case about NULL is interesting - I wasn't aware of that. > > > Is it practically relevant, i.e. is there any platform where casting > > > (void *)0 does not evaluate to 0 ? > > > > > > > "An integer constant expression with the value 0, or such an expression > > cast to type > > void *, is called a null pointer constant." (C11, 6.3.2.3 3) (void*)0 > > is therefore a valid null pointer constant and is also commonly used for > > the NULL macro. (void*)0 == 0 is always true, because the right hand > > side is converted to the type of the pointer (namely to a null pointer) > > and all null pointers compare equal. But this is irrelevant to > > relational comparisons, because checking for equality of pointers is not > > subject to these pointers pointing to the same object (or one past the > > last element of an array...), whereas this is so for relational operations. > > > > (If one uses unsigned for pointers, then one only needs to reserve two > > values that can not be used as part of an object: 0 and the max value > > (the latter can't be used for an object, because using a pointer one > > past the object is legal and has to be consistent with "<=" (and anyway > > said pointer must compare unequal to NULL)); if one used signed > > comparisons for pointers > > On initial read I had thought you were implying something like "signed > pointers". But maybe one should really create a computer system with > "symmetric addressing" where pointers are always pointing to the center > of a memory area (odd-number sized). I think it would be a great > invention for bored and under-challenged developers who love to > deep-dive into academic details - hihi.. > > > (I'd bet, concepts and theory exist somewhere anywhere) > > > one would have to reserve -1, 0 and the max > > value, the former because a one past the end array element needs to > > compare unequal to NULL and the latter to be consistent with <= and a > > potential one-past-the-end element. But this is a very small advantage. > > Honestly, I don't know whether compilers consistently use unsigned > > comparisons for pointer comparisons at all (even when restricted to > > compilers for systems with HAVE_DOS_PATHS). The fact that comparisons of > > pointers to different objects is UB means that compiler writers actually > > can choose what they want.) > > So one could cast both pointers to unsigned to make the relational > comparison valid, as long as either (1) both are pointing to the same object, > or (2) one or (3) both are NULL/0? > > C++ defines NULL as 0 (which I had actually also assumed to be the case > in C), so this wouldn't be an issue there I guess? > (I mean "hypothetical" issue same as what it is in C) > > BTW, thanks for the insight. That part is really interesting. > > Now for the other one.. > > > (Furthermore, it is not guaranteed by the spec that zeroing a pointer > > via memset (or calloc) generates a valid null pointer. E.g. the > > documentation of calloc has this footnote: "Note that this [the bitwise > > zero-initialization] need not be the same as the representation of > > floating-point zero or a null pointer constant." But I don't know a > > system where this is not so and we definitely require it to be so.) > > > > >> 2. Apart from that: Your code would potentially evaluate strrchr() > > >> multiple times which is bad style (given that this function is likely > > >> marked as pure the compiler could probably optimize the second call > > >> away, but this is not a given). > > > > > > It's not my code. It's code copied from avstring.c - so please blame > > > whoever that wrote. > > > > > > > I couldn't find strrchr() being evaluated multiple times unnecessarily > > due to a macro in avstring.c. > > I don't understand. av_basename() does this: > > p = strrchr(path, '/'); > #if HAVE_DOS_PATHS > q = strrchr(path, '\\'); > d = strchr(path, ':'); > p = FFMAX3(p, q, d); > #endif > > and I do this: > > p = strrchr(vs->m3u8_name, '/'); > #if HAVE_DOS_PATHS > p = FFMAX(p, strrchr(vs->m3u8_name, '\\')); > #endif > > So, for Windows, I'm counting 3 vs. 2... > > > > > Regarding performance, I'm not sure whether this is relevant in any way, > > > given the low frequency of execution and putting it into relation to > > > all the other things that ffmpeg is doing usually. > > > > > > > The above would be a valid point if there were a tradeoff between > > writing the code without repeated evaluations and writing clear code. > > (And even then you'd be ignoring that the performance difference might > > be negligible for code only run very infrequently, but bloated code > > takes more space in the binary even when executed infrequently.) But > > there is no such tradeoff here. > > A HLS segment typically has 0.5 to 20 MB. Let's assume 1 MB. Even when the > file name/path would be re-evaluated for each segment, it would be looping > over something like 100 byte _additionally_ per segment. To produce the > segment, those 1 MB data would need to be "touched" or looped over multiple > times (effectively), even when only remuxing. Let's say 10x. > That leaves us with 100 iterations vs. 10 Million iterations. > Static ffmpeg binaries have like 60 MB. How many bytes does it add to the > code? 20? => makes 20 vs. 60 Million bytes > > > > >> 3. The code in av_basename() is also wrong. > > > > > > ... > > > > > >> 4. Is there actually a reason why you don't use av_basename() directly > > here? > > > > > > Yes, multiple: > > > > > > 1. Different behavior - those functions are returning a "." when not > found > > > > av_basename() also has precise guarantees when this happens. > > by "those", I meant av_basename and av_dirname(). I wanted to do an > absolute minimal fix that is safe without rewriting the code and without > having to do much testing. I don't even use that code, the fix was > a favor for somebody else and I've been once again stupid enough to > submit it to the ML - thinking it might be useful for somebody. > > > > > 2. The docs tell it's required to copy a string before supplying it to > > > those functions (as they may changing the string). > > > > You are confusing av_basename() and av_dirname(). > > No. av_dirname() is actually the one that is needed. > > > > 3. The hlsenc code changes the string temporarily and restores it after > > > wards. The same couldn't be done when using the avstring functions. > > > > > > > Why? > > I don't know. You need to ask the author. > > > (Actually, your code is still slightly different from av_basename(): > ---^^^^--- > > Are you aware that I haven't written a single line of this code besides > the 3 lines of the patch? > > I know nothing about the code. I have just fixed that single thing. > To do it in a way that is conforming and acceptable, I looked around > how it's done elsewhere. avstring.c is a central part of the code base, > used virtually everywhere, so I used the same approach, assuming that > it must be the most acceptable one. I didn't even spend a second for > thinking about it, because it's pointless here anyway: Somebody will > always come with another opinion. > But when even the project's own code is not good enough for submission, > then it will sooner or later inflate. deflate _______________________________________________ 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".