Re: [FFmpeg-devel] [PATCH 3/3] avformat/hlsenc: Fix path handling for Windows

["softworkz ." <softworkz-at-hotmail.com@ffmpeg.org>]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> Marton Balint
> Sent: Freitag, 13. Juni 2025 23:36
> To: FFmpeg development discussions and patches <ffmpeg-
> 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 <softworkz@hotmail.com>
> >
> 
> Can you give an example where the path handling is wrong and where
> this
> patch fixes it?

c:\hls\video1\master.m3u8



> Is there a trac ticket?

Good question, there well may be.



> 
> > Signed-off-by: softworkz <softworkz@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.


> > +{
> > +    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()



> > +#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?



> > +        if (av_strncasecmp(master_url, media_url, pos)) {
> >             av_log(NULL, AV_LOG_WARNING, "Unable to find relative
> url\n");
> >             return NULL;
> >         }
> > @@ -1423,7 +1436,7 @@ static const char* get_relative_url(const
> char *master_url, const char *media_ur
> >         return media_url;
> >     }
> >
> > -    return media_url + base_len + 1;
> > +    return media_url + pos + 1;
> > }
> >
> > static int64_t get_stream_bit_rate(AVStream *stream)
> > @@ -3151,13 +3164,13 @@ static int hls_init(AVFormatContext *s)
> >                     vs->fmp4_init_filename = expanded;
> >                 }
> >
> > -                p = strrchr(vs->m3u8_name, '/');
> > -                if (p) {
> > -                    char tmp = *(++p);
> > -                    *p = '\0';
> > -                    vs->base_output_dirname =
> av_asprintf("%s%s", vs->m3u8_name,
> > -                                                          vs-
> >fmp4_init_filename);
> > -                    *p = tmp;
> > +                int pos = get_last_separator_pos(vs->m3u8_name);
> > +                if (pos >= 0) {
> > +                    AVBPrint buf;
> > +                    av_bprint_init(&buf, 0,
> AV_BPRINT_SIZE_UNLIMITED);
> > +                    av_bprint_append_data(&buf, vs->m3u8_name,
> pos + 1);
> > +                    av_bprintf(&buf, "%s", vs-
> >fmp4_init_filename);
> > +                    av_bprint_finalize(&buf, &vs-
> >base_output_dirname);
> 
> av_bprintf here is unneeded, because in the end you have to malloc
> the
> result anyway. Keep using av_asprintf() with "%*s" to limit the
> length.

Ah, right, that's better, will do.


Thanks again,
sw
_______________________________________________
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".