From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ffbox0-bg.ffmpeg.org (ffbox0-bg.ffmpeg.org [79.124.17.100]) by master.gitmailbox.com (Postfix) with ESMTPS id 1283A4CEA2 for ; Sat, 14 Jun 2025 15:38:05 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.ffmpeg.org (Postfix) with ESMTP id BE5E468CD4E; Sat, 14 Jun 2025 18:38:01 +0300 (EEST) Received: from iq.passwd.hu (iq.passwd.hu [217.27.212.140]) by ffbox0-bg.ffmpeg.org (Postfix) with ESMTP id 8C4D668C90D for ; Sat, 14 Jun 2025 18:37:54 +0300 (EEST) Received: from localhost (localhost [127.0.0.1]) by iq.passwd.hu (Postfix) with ESMTP id C475DE5190 for ; Sat, 14 Jun 2025 17:34:58 +0200 (CEST) X-Virus-Scanned: amavisd-new at passwd.hu Received: from iq.passwd.hu ([127.0.0.1]) by localhost (iq.passwd.hu [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id bGBz3DkrUOFh for ; Sat, 14 Jun 2025 17:34:57 +0200 (CEST) Received: from iq (iq [217.27.212.140]) by iq.passwd.hu (Postfix) with ESMTPS id D3F9FE5165 for ; Sat, 14 Jun 2025 17:34:56 +0200 (CEST) Date: Sat, 14 Jun 2025 17:34:56 +0200 (CEST) From: Marton Balint To: FFmpeg development discussions and patches In-Reply-To: Message-ID: <4b6d8494-2cfe-e118-516d-4ed9c94b757e@passwd.hu> References: <99dfda242076be814b80d9adb0722e860f95cf6a.1749835195.git.ffmpegagent@gmail.com> <9199955b-d517-2a9b-fd2b-ddda1833493c@passwd.hu> MIME-Version: 1.0 Subject: Re: [FFmpeg-devel] [PATCH 3/3] avformat/hlsenc: Fix path handling for 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-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: On Fri, 13 Jun 2025, softworkz . wrote: > > >> -----Original Message----- >> From: ffmpeg-devel On Behalf Of >> Marton Balint >> Sent: Freitag, 13. Juni 2025 23:36 >> To: FFmpeg development discussions and patches > devel@ffmpeg.org> >> Subject: Re: [FFmpeg-devel] [PATCH 3/3] avformat/hlsenc: Fix path >> handling for Windows >> >> >> >> On Fri, 13 Jun 2025, softworkz wrote: >> >>> From: softworkz >>> >> >> Can you give an example where the path handling is wrong and where >> this >> patch fixes it? > > c:\hls\video1\master.m3u8 What I meant is that you should try to explain "fix" better in the commit message, like: When base_output_dirname was determined only '/' was searched for as path separator. get_relative_url() on the other hand searched for both forward and backward slash regardless of OS. Fix these issues by factorizing the separator finder function, only search for backslash for Windows/DOS and use that in both places. > > > >> Is there a trac ticket? > > Good question, there well may be. > Its worth doing a quick search, if there is one you might want to reference it in the commit message. > > >> >>> Signed-off-by: softworkz >>> --- >>> libavformat/hlsenc.c | 43 ++++++++++++++++++++++++++++----------- >> ---- >>> 1 file changed, 28 insertions(+), 15 deletions(-) >>> >>> diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c >>> index f81385d0b4..ba1e74e999 100644 >>> --- a/libavformat/hlsenc.c >>> +++ b/libavformat/hlsenc.c >>> @@ -329,6 +329,23 @@ static int hlsenc_io_close(AVFormatContext >> *s, AVIOContext **pb, char *filename) >>> return ret; >>> } >>> >>> +static int get_last_separator_pos(const char *path) >> >> size_t > > Cannot return -1. Indeed, actually ptrdiff_t would be the proper type. > > >>> +{ >>> + if (!path || *path == '\0') >>> + return -1; >>> + >>> + char *p = strrchr(path, '/'); >>> +#if HAVE_DOS_PATHS >>> + char *q = strrchr(path, '\\'); >>> + p = FFMAX(p, q); >> >> You are comparing potentially NULL pointers here. > > > It's the same like in av_basename() or av_dirname() And those are likely wrong too. > > > >>> +#endif >>> + >>> + if (!p) >>> + return -1; >>> + >>> + return p - path; >>> +} >>> + >>> static void set_http_options(AVFormatContext *s, AVDictionary >> **options, HLSContext *c) >>> { >>> int http_base_proto = ff_is_http_proto(s->url); >>> @@ -1408,14 +1425,10 @@ static int >> hls_rename_temp_file(AVFormatContext *s, AVFormatContext *oc) >>> >>> static const char* get_relative_url(const char *master_url, const >> char *media_url) >>> { >>> - const char *p = strrchr(master_url, '/'); >>> - size_t base_len = 0; >>> - >>> - if (!p) p = strrchr(master_url, '\\'); >>> + int pos = get_last_separator_pos(master_url); >> >> size_t, and you can keep using base_len variable, you don't have to >> rename, it is used for the same purpose as before. > > >> >>> >>> - if (p) { >>> - base_len = p - master_url; >>> - if (av_strncasecmp(master_url, media_url, base_len)) { >>> + if (pos >= 0) { > > That's the check for not being -1 > > int64_t should cover the whole range - can I use that instead? ptrdiff_t. Thanks, Marton _______________________________________________ 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".