[FFmpeg-devel] [PATCH 0/3] avformat/hlsenc: Some HLS improvements

[FFmpeg-devel] [PATCH 0/3] avformat/hlsenc: Some HLS improvements

[ffmpegagent]

Includes:

 * Segment and init file completion logging
 * Fix path handling on Windows

softworkz (3):
  avformat/hlsenc: Log information about segments written
  avformat/hlsenc: Log InitFileComplete message
  avformat/hlsenc: Fix path handling for Windows

 libavformat/hlsenc.c | 52 +++++++++++++++++++++++++++++++-------------
 1 file changed, 37 insertions(+), 15 deletions(-)


base-commit: f3bf5356966b4ad9334034e720fa224c2d9464a6
Published-As: https://github.com/ffstaging/FFmpeg/releases/tag/pr-ffstaging-97%2Fsoftworkz%2Fsubmit_hls_logging-v1
Fetch-It-Via: git fetch https://github.com/ffstaging/FFmpeg pr-ffstaging-97/softworkz/submit_hls_logging-v1
Pull-Request: https://github.com/ffstaging/FFmpeg/pull/97
-- 
ffmpeg-codebot
_______________________________________________
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".

[FFmpeg-devel] [PATCH 1/3] avformat/hlsenc: Log information about segments written

[softworkz]

From: softworkz <softworkz@hotmail.com>

Signed-off-by: softworkz <softworkz@hotmail.com>
---
 libavformat/hlsenc.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
index a93d35ab75..a3e523a5df 100644
--- a/libavformat/hlsenc.c
+++ b/libavformat/hlsenc.c
@@ -2703,6 +2703,12 @@ static int hls_write_packet(AVFormatContext *s, AVPacket *pkt)
         if (ret < 0)
             return ret;
 
+        if (oc->url[0]) {
+            av_log(s, AV_LOG_VERBOSE, "SegmentComplete=%s:%d Index=%d StartPts=%"PRId64" EndPts=%"PRId64" Duration=%f filename=%s\n",
+                   av_get_media_type_string(vs->streams[stream_index]->codecpar->codec_type),
+                   stream_index, vs->number, vs->start_pts, vs->end_pts, vs->duration, av_basename(oc->url));
+        }
+
         old_filename = av_strdup(oc->url);
         if (!old_filename) {
             return AVERROR(ENOMEM);
-- 
ffmpeg-codebot

_______________________________________________
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".

[FFmpeg-devel] [PATCH 2/3] avformat/hlsenc: Log InitFileComplete message

[softworkz]

From: softworkz <softworkz@hotmail.com>

Signed-off-by: softworkz <softworkz@hotmail.com>
---
 libavformat/hlsenc.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
index a3e523a5df..f81385d0b4 100644
--- a/libavformat/hlsenc.c
+++ b/libavformat/hlsenc.c
@@ -2467,6 +2467,9 @@ static int hls_init_file_resend(AVFormatContext *s, VariantStream *vs)
     avio_write(vs->out, vs->init_buffer, vs->init_range_length);
     hlsenc_io_close(s, &vs->out, hls->fmp4_init_filename);
 
+    av_log(s, AV_LOG_VERBOSE, "InitFileComplete, VariantStream=%d basename=%s filename=%s\n",
+           vs->number, vs->basename, av_basename(vs->base_output_dirname));
+
     return ret;
 }
 
-- 
ffmpeg-codebot

_______________________________________________
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".

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

[softworkz]

From: softworkz <softworkz@hotmail.com>

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)
+{
+    if (!path || *path == '\0')
+        return -1;
+
+    char *p = strrchr(path, '/');
+#if HAVE_DOS_PATHS
+    char *q = strrchr(path, '\\');
+    p = FFMAX(p, q);
+#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);
 
-    if (p) {
-        base_len = p - master_url;
-        if (av_strncasecmp(master_url, media_url, base_len)) {
+    if (pos >= 0) {
+        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);
                 } else {
                     vs->base_output_dirname = av_strdup(vs->fmp4_init_filename);
                 }
-- 
ffmpeg-codebot
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 1/3] avformat/hlsenc: Log information about segments written

[Marton Balint]

On Fri, 13 Jun 2025, softworkz wrote:

> From: softworkz <softworkz@hotmail.com>
>
> Signed-off-by: softworkz <softworkz@hotmail.com>
> ---
> libavformat/hlsenc.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
> index a93d35ab75..a3e523a5df 100644
> --- a/libavformat/hlsenc.c
> +++ b/libavformat/hlsenc.c
> @@ -2703,6 +2703,12 @@ static int hls_write_packet(AVFormatContext *s, AVPacket *pkt)
>         if (ret < 0)
>             return ret;
>
> +        if (oc->url[0]) {
> +            av_log(s, AV_LOG_VERBOSE, "SegmentComplete=%s:%d Index=%d StartPts=%"PRId64" EndPts=%"PRId64" Duration=%f filename=%s\n",
> +                   av_get_media_type_string(vs->streams[stream_index]->codecpar->codec_type),
> +                   stream_index, vs->number, vs->start_pts, vs->end_pts, vs->duration, av_basename(oc->url));

snake_case consistently please here and in the following patches.

Thanks,
Marton

> +        }
> +
>         old_filename = av_strdup(oc->url);
>         if (!old_filename) {
>             return AVERROR(ENOMEM);
> -- 
> ffmpeg-codebot
>
> _______________________________________________
> 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".
>
_______________________________________________
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".

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

[Marton Balint]

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? Is there a trac ticket?

> 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

> +{
> +    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.

> +#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) {
> +        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.

Regards,
Marton

>                 } else {
>                     vs->base_output_dirname = av_strdup(vs->fmp4_init_filename);
>                 }
> -- 
> ffmpeg-codebot
> _______________________________________________
> 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".
>
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 1/3] avformat/hlsenc: Log information about segments written

[softworkz .]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> Marton Balint
> Sent: Freitag, 13. Juni 2025 23:03
> To: FFmpeg development discussions and patches <ffmpeg-
> devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH 1/3] avformat/hlsenc: Log
> information about segments written
> 
> 
> 
> On Fri, 13 Jun 2025, softworkz wrote:
> 
> > From: softworkz <softworkz@hotmail.com>
> >
> > Signed-off-by: softworkz <softworkz@hotmail.com>
> > ---
> > libavformat/hlsenc.c | 6 ++++++
> > 1 file changed, 6 insertions(+)
> >
> > diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
> > index a93d35ab75..a3e523a5df 100644
> > --- a/libavformat/hlsenc.c
> > +++ b/libavformat/hlsenc.c
> > @@ -2703,6 +2703,12 @@ static int
> hls_write_packet(AVFormatContext *s, AVPacket *pkt)
> >         if (ret < 0)
> >             return ret;
> >
> > +        if (oc->url[0]) {
> > +            av_log(s, AV_LOG_VERBOSE, "SegmentComplete=%s:%d
> Index=%d StartPts=%"PRId64" EndPts=%"PRId64" Duration=%f
> filename=%s\n",
> > +                   av_get_media_type_string(vs-
> >streams[stream_index]->codecpar->codec_type),
> > +                   stream_index, vs->number, vs->start_pts, vs-
> >end_pts, vs->duration, av_basename(oc->url));
> 
> snake_case consistently please here and in the following patches.
> 
> Thanks,
> Marton


Oh, ouch, hadn't thought about it. I'll withdraw the logging patches, 
there's no mutual benefit.
(my fault - it's right for FFmpeg to require consistency in output)


Best regards
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".

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

[softworkz .]

> -----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".

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

[softworkz]

From: softworkz <softworkz@hotmail.com>

Signed-off-by: softworkz <softworkz@hotmail.com>
---
    avformat/hlsenc: Some HLS improvements
    
    Includes only a single patch now:
    
     * Fix path handling on Windows
    
    
    Versions
    ========
    
    
    V2
    ==
    
     * Fix path handling for Windows;
       * Keep using av_asprintf() with "%*s" to limit the length (as per
         review by Marton - thanks!)
       * Use int64:_t instead of int
     * Removed logging patches
    
    .

Published-As: https://github.com/ffstaging/FFmpeg/releases/tag/pr-ffstaging-97%2Fsoftworkz%2Fsubmit_hls_logging-v2
Fetch-It-Via: git fetch https://github.com/ffstaging/FFmpeg pr-ffstaging-97/softworkz/submit_hls_logging-v2
Pull-Request: https://github.com/ffstaging/FFmpeg/pull/97

Range-diff vs v1:

 1:  58988164f2 < -:  ---------- avformat/hlsenc: Log information about segments written
 2:  1a4268d7e1 < -:  ---------- avformat/hlsenc: Log InitFileComplete message
 3:  99dfda2420 ! 1:  93e1a502be avformat/hlsenc: Fix path handling for Windows
     @@ libavformat/hlsenc.c: static int hlsenc_io_close(AVFormatContext *s, AVIOContext
           return ret;
       }
       
     -+static int get_last_separator_pos(const char *path)
     ++static int64_t get_last_separator_pos(const char *path)
      +{
      +    if (!path || *path == '\0')
      +        return -1;
     @@ libavformat/hlsenc.c: static int hls_rename_temp_file(AVFormatContext *s, AVForm
      -    size_t base_len = 0;
      -
      -    if (!p) p = strrchr(master_url, '\\');
     -+    int pos = get_last_separator_pos(master_url);
     ++    int64_t pos = get_last_separator_pos(master_url);
       
      -    if (p) {
      -        base_len = p - master_url;
     @@ libavformat/hlsenc.c: static int hls_init(AVFormatContext *s)
      -                    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);
     ++                int64_t 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);
     ++                    vs->base_output_dirname = av_asprintf("%.*s%s", pos + 1, vs->m3u8_name,
     +                                                           vs->fmp4_init_filename);
     +-                    *p = tmp;
                       } else {
                           vs->base_output_dirname = av_strdup(vs->fmp4_init_filename);
                       }


 libavformat/hlsenc.c | 38 ++++++++++++++++++++++++--------------
 1 file changed, 24 insertions(+), 14 deletions(-)

diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
index a93d35ab75..965ed9c1c0 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 int64_t get_last_separator_pos(const char *path)
+{
+    if (!path || *path == '\0')
+        return -1;
+
+    char *p = strrchr(path, '/');
+#if HAVE_DOS_PATHS
+    char *q = strrchr(path, '\\');
+    p = FFMAX(p, q);
+#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, '\\');
+    int64_t pos = get_last_separator_pos(master_url);
 
-    if (p) {
-        base_len = p - master_url;
-        if (av_strncasecmp(master_url, media_url, base_len)) {
+    if (pos >= 0) {
+        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)
@@ -3142,13 +3155,10 @@ 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,
+                int64_t pos = get_last_separator_pos(vs->m3u8_name);
+                if (pos >= 0) {
+                    vs->base_output_dirname = av_asprintf("%.*s%s", pos + 1, vs->m3u8_name,
                                                           vs->fmp4_init_filename);
-                    *p = tmp;
                 } else {
                     vs->base_output_dirname = av_strdup(vs->fmp4_init_filename);
                 }

base-commit: f3bf5356966b4ad9334034e720fa224c2d9464a6
-- 
ffmpeg-codebot
_______________________________________________
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".

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

[softworkz .]

> -----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? Is there a trac ticket?
> 
> > Signed-off-by: softworkz <softworkz@hotmail.com>
> > ---
> > libavformat/hlsenc.c | 43 ++++++++++++++++++++++++++++-----------
> ----
> > 1 file changed, 28 insertions(+), 15 deletions(-)

> > +{
> > +    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.


Hi Marton,

actually, there's a prequel to this patch, a simple form of which I had
submitted in 2022 and ended up in a discussion with Andreas:

https://lists.ffmpeg.org/pipermail/ffmpeg-devel/2022-January/291657.html


Obviously, this wasn't about performance-critical code in any way, but
as Andreas had insisted so much on it at that time, and there was also 
topic about null-pointer comparison, I actually wanted to go a very 
different route this time.
Looking at av_basename() (when HAVE_DOS_PATHS is enabled), it seemed 
to be easy to beat the tripled search by doing a single (reverse) 
search for all three chars.

So I created a function av_strrcspn(). The naming should be correct, 
while there's no strrcspn function in reality.

I tried various optimizations but I wasn't able to beta the triple
strrchar().

Code and benchmark are here, in case anybody is interested:
https://github.com/FFmpeg/FFmpeg/compare/master...softworkz:FFmpeg:test_av_strrcspn

I suspect that there's some parallelism/SIMD in strrchar() which 
makes it faster than expected.

Finally, I ditched that way and resorted to doing the same/similar
like in av_basename(). 
Now, either both a wrong or both are right...


Best regards,
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".

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

[Marton Balint]

On Fri, 13 Jun 2025, softworkz . wrote:

>
>
>> -----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

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@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-request@ffmpeg.org with subject "unsubscribe".