On Fri, 13 Jun 2025, softworkz . wrote:
-----Original Message-----
From: ffmpeg-devel <ffmpeg-devel-boun...@ffmpeg.org> On Behalf Of
Marton Balint
Sent: Freitag, 13. Juni 2025 23:36
To: FFmpeg development discussions and patches <ffmpeg-
de...@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 <softwo...@hotmail.com>
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 <softwo...@hotmail.com>
---
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-requ...@ffmpeg.org with subject "unsubscribe".
