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

[Bodecs Bela]

2019.06.19. 19:37 keltezéssel, Michael Niedermayer írta:
> On Wed, Jun 19, 2019 at 10:03:51AM +0000, Bodecs Bela wrote:
>> ffmpeg | branch: master | Bodecs Bela <bodecsb at vivanet.hu> | Mon Jun 17 23:05:21 2019 +0200| [09a4853930e7950f423e9161004871afe659ed84] | committer: Steven Liu
>>
>> av_format/hlsenc: fix %v handling by format_name function
>>
>> 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 at 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 at vivanet.hu>
> This broke fate (segfaults) also it produces new compiler warnings which
> probably are pointing to the cause
>
> libavformat/hlsenc.c: In function ‘hls_init’:
> libavformat/hlsenc.c:2643:9: warning: passing argument 2 of ‘format_name’ makes pointer from integer without a cast [enabled by default]
>           ret = format_name(s->url, i, vs->m3u8_name);
>           ^
> libavformat/hlsenc.c:1764:12: note: expected ‘char **’ but argument is of type ‘int’
>   static int format_name(const char *buf, char **s, int index)
>              ^
> libavformat/hlsenc.c:2643:9: warning: passing argument 3 of ‘format_name’ makes integer from pointer without a cast [enabled by default]
>           ret = format_name(s->url, i, vs->m3u8_name);
>           ^
> libavformat/hlsenc.c:1764:12: note: expected ‘int’ but argument is of type ‘char *’
>   static int format_name(const char *buf, char **s, int index)
>              ^
> libavformat/hlsenc.c:2712:13: warning: passing argument 2 of ‘format_name’ makes pointer from integer without a cast [enabled by default]
>               ret = format_name(vs->basename, basename_size, i);
>               ^
> libavformat/hlsenc.c:1764:12: note: expected ‘char **’ but argument is of type ‘int’
>   static int format_name(const char *buf, char **s, int index)
>              ^
> libavformat/hlsenc.c:2767:25: warning: passing argument 2 of ‘format_name’ makes pointer from integer without a cast [enabled by default]
>                           format_name(vs->fmp4_init_filename, fmp4_init_filename_len, i);
>                           ^
> libavformat/hlsenc.c:1764:12: note: expected ‘char **’ but argument is of type ‘int’
>   static int format_name(const char *buf, char **s, int index)
>              ^
> libavformat/hlsenc.c:2832:17: warning: passing argument 2 of ‘format_name’ makes pointer from integer without a cast [enabled by default]
>                   ret = format_name(vs->vtt_m3u8_name, vtt_basename_size, i);
>                   ^
> libavformat/hlsenc.c:1764:12: note: expected ‘char **’ but argument is of type ‘int’
>   static int format_name(const char *buf, char **s, int index)
>
>
> [...]
>
Dear Michael,

thank you for review my patch.

This patch requires one of my earlier other patch to be applied. Steven 
Liu applied that earlier patch very shortly before you tested this patch.

Maybe you applied this patch against an earlier head than Seven Liu 
applied the necessary patch?

I have tested the current head with my sent public patch file without 
any problem. The commands I have used at 2019jun19 22:40

git clone https://git.ffmpeg.org/ffmpeg.git ffmpeg
cd ffmpeg
git apply --stat 
0003-avformat-hlsenc-enhanced-v-handling-with-variant-nam.patch
git apply --check 
0003-avformat-hlsenc-enhanced-v-handling-with-variant-nam.patch
git am --signoff < 
0003-avformat-hlsenc-enhanced-v-handling-with-variant-nam.patch
configure ....
make
make fate-rsync SAMPLES=fate-suite/
make fate SAMPLES=fate-suite/


Can You or anybody else confirm it?

thank you,

Bela


> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".