diff mbox

[FFmpeg-devel] av_format/hlsenc: fix %v handling by format_name function

Message ID 19b3fdd0-734f-f4a6-168c-707fc8a24b6a@vivanet.hu
State Accepted
Commit 09a4853930e7950f423e9161004871afe659ed84
Headers show

Commit Message

Bodecs Bela June 17, 2019, 9:05 p.m. UTC 
Hi All,

When multiple variant streams are specified by var_stream_map option, %v
placeholder in various names ensures that each variant has its unique
names. Most of %v handlng is done in format_name function. Currently
in this function the result buffer is the same as the
input pattern buffer, so you must allocate it before calling format_name
function. It also means, that it is silently assumed that the result 
string will NOT be
longer that the pattern string. It is true most of the time, because %v
may appear only once in the pattern string and number of variant streams
is less than 100 in practical cases. But theoretically it will fail if
specified number of variant streams is greater than 100 (i.e. longer 
than 2 digits).
This patch fixes this behaviour by altering format_name function to 
allocate the
result buffer and return it to the caller.

Please, review this patch.

best,

Bela
From 6377ebee8a106a9684d41b270c7d6c8e57cd3e7b Mon Sep 17 00:00:00 2001
From: Bela Bodecs <bodecsb@vivanet.hu>
Date: Mon, 17 Jun 2019 14:31:36 +0200
Subject: [PATCH] av_format/hlsenc: fix %v handling by format_name function

When multiple variant streams are specified by var_stream_map option, %v
placeholder in various names ensures that each variant has its unique
names. Most of %v handlng is done in format_name function. Currently
in this function the result buffer is the same as the input pattern
buffer, so you must allocate it before calling format_name function. It
also means, that it is silently assumed that the result string will NOT
be longer that the pattern string. It is true most of the time, because
%v may appear only once in the pattern string and number of variant
streams is less than 100 in practical cases. But theoretically it will
fail if specified number of variant streams is greater than 100. This
patch fixes this behaviour by altering format_name function to allocate
the result buffer and return it to the caller.

Signed-off-by: Bela Bodecs <bodecsb@vivanet.hu>
---
 libavformat/hlsenc.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

Comments

Steven Liu June 18, 2019, 2:02 a.m. UTC | #1 
Bodecs Bela <bodecsb@vivanet.hu> 于2019年6月18日周二 上午5:57写道：
>
> Hi All,
>
> When multiple variant streams are specified by var_stream_map option, %v
> placeholder in various names ensures that each variant has its unique
> names. Most of %v handlng is done in format_name function. Currently
> in this function the result buffer is the same as the
> input pattern buffer, so you must allocate it before calling format_name
> function. It also means, that it is silently assumed that the result
> string will NOT be
> longer that the pattern string. It is true most of the time, because %v
> may appear only once in the pattern string and number of variant streams
> is less than 100 in practical cases. But theoretically it will fail if
> specified number of variant streams is greater than 100 (i.e. longer
> than 2 digits).
> This patch fixes this behaviour by altering format_name function to
> allocate the
> result buffer and return it to the caller.
>
> Please, review this patch.
>
> best,
>
> Bela
>
> _______________________________________________
> 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".


LGTM

Thanks
Steven
diff mbox

Patch

diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
index 9884f74d51..ebe1ab62e5 100644
--- a/libavformat/hlsenc.c
+++ b/libavformat/hlsenc.c
@@ -1761,33 +1761,34 @@  fail:
     return ret;
 }
 
-static int format_name(char *buf, int buf_len, int index)
+static int format_name(const char *buf, char **s, int index)
 {
     const char *proto, *dir;
-    char *orig_buf_dup = NULL, *mod_buf = NULL, *mod_buf_dup = NULL;
+    char *orig_buf_dup = NULL, *mod_buf_dup = NULL;
     int ret = 0;
 
-    if (!av_stristr(buf, "%v"))
-        return ret;
-
     orig_buf_dup = av_strdup(buf);
     if (!orig_buf_dup) {
         ret = AVERROR(ENOMEM);
         goto fail;
     }
 
-    if (replace_int_data_in_filename(&mod_buf, orig_buf_dup, 'v', index) < 1) {
+    if (!av_stristr(buf, "%v")) {
+        *s = orig_buf_dup;
+        return ret;
+    }
+
+    if (replace_int_data_in_filename(s, orig_buf_dup, 'v', index) < 1) {
         ret = AVERROR(EINVAL);
         goto fail;
     }
-    av_strlcpy(buf, mod_buf, buf_len);
 
     proto = avio_find_protocol_name(orig_buf_dup);
     dir = av_dirname(orig_buf_dup);
 
     /* if %v is present in the file's directory, create sub-directory */
     if (av_stristr(dir, "%v") && proto && !strcmp(proto, "file")) {
-        mod_buf_dup = av_strdup(buf);
+        mod_buf_dup = av_strdup(*s);
         if (!mod_buf_dup) {
             ret = AVERROR(ENOMEM);
             goto fail;
@@ -1803,7 +1804,6 @@  static int format_name(char *buf, int buf_len, int index)
 fail:
     av_freep(&orig_buf_dup);
     av_freep(&mod_buf_dup);
-    av_freep(&mod_buf);
     return ret;
 }
 
@@ -2634,7 +2634,7 @@  static int hls_init(AVFormatContext *s)
             ret = AVERROR(ENOMEM);
             goto fail;
         }
-        ret = format_name(vs->m3u8_name, strlen(s->url) + 1, i);
+        ret = format_name(s->url, i, vs->m3u8_name);
         if (ret < 0)
             goto fail;