[FFmpeg-devel] [PATCH 3/3] avformat/hlsenc: Fix path handling for Windows
Marton Balint
cus at passwd.hu
Sat Jun 14 18:34:56 EEST 2025
On Fri, 13 Jun 2025, softworkz . wrote:
>
>
>> -----Original Message-----
>> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of
>> Marton Balint
>> Sent: Freitag, 13. Juni 2025 23:36
>> To: FFmpeg development discussions and patches <ffmpeg-
>> devel at 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 <softworkz at 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 <softworkz at 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
More information about the ffmpeg-devel
mailing list