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

[Bodecs Bela]

2019.06.19. 23:49 keltezéssel, Bodecs Bela írta:
>
> 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
>
I think I know the solution. This patch 
(0001-avformat-hlsenc-better-checking-var_stream_map-conte.patch)

was applied by Steven Liu very shortly before Michael tested it.

And I mistakenly thought that Michael tested another of my patches. 
(0003-avformat-hlsenc-enhanced-v-handling-with-variant-nam.patch)

I did not read the Subject line of the email .

Sorry for confusing you.


best,

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