[FFmpeg-devel] [PATCH] avformat/hlsenc: hold old key info when append list

[Steven Liu]

2017-05-05 12:29 GMT+08:00 Aaron Levinson <alevinsn at aracnet.com>:

> On 5/4/2017 9:15 PM, Steven Liu wrote:
>
>> 2017-05-03 9:49 GMT+08:00 Aaron Levinson <alevinsn at aracnet.com>:
>>
>> On 4/27/2017 7:21 PM, Steven Liu wrote:
>>>
>>> 2017-04-26 7:30 GMT+08:00 Steven Liu <lq at chinaffmpeg.org>:
>>>>
>>>> fix ticket id: #6353
>>>>
>>>>>
>>>>> Signed-off-by: Steven Liu <lq at chinaffmpeg.org>
>>>>> ---
>>>>>  libavformat/hlsenc.c | 24 ++++++++++++++++++++++++
>>>>>  1 file changed, 24 insertions(+)
>>>>>
>>>>> diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
>>>>> index 27c8e3355d..3ec0f330fb 100644
>>>>> --- a/libavformat/hlsenc.c
>>>>> +++ b/libavformat/hlsenc.c
>>>>> @@ -810,6 +810,7 @@ static int parse_playlist(AVFormatContext *s, const
>>>>> char *url)
>>>>>      int64_t new_start_pos;
>>>>>      char line[1024];
>>>>>      const char *ptr;
>>>>> +    const char *end;
>>>>>
>>>>>      if ((ret = ffio_open_whitelist(&in, url, AVIO_FLAG_READ,
>>>>>                                     &s->interrupt_callback, NULL,
>>>>> @@ -842,6 +843,29 @@ static int parse_playlist(AVFormatContext *s,
>>>>> const
>>>>> char *url)
>>>>>          } else if (av_strstart(line, "#EXTINF:", &ptr)) {
>>>>>              is_segment = 1;
>>>>>              hls->duration = atof(ptr);
>>>>> +        } else if (av_stristart(line, "#EXT-X-KEY:", &ptr)) {
>>>>> +            ptr = av_stristr(line, "URI=\"");
>>>>> +            if (ptr) {
>>>>> +                ptr += strlen("URI=\"");
>>>>> +                end = av_stristr(ptr, ",");
>>>>> +                if (end) {
>>>>> +                    av_strlcpy(hls->key_uri, ptr, end - ptr);
>>>>> +                } else {
>>>>> +                    av_strlcpy(hls->key_uri, ptr,
>>>>> sizeof(hls->key_uri));
>>>>>
>>>>>
>>>> I know that this patch has already been applied (although it didn't get
>>> any reviews on the ML), but I think that there are some issues with it.
>>> First, it seems that both av_strlcpy() cases above will copy the
>>> terminating '\"' character into hls->key_uri.  Perhaps that won't cause
>>> an
>>> issue in practice, but it is likely not the intended result.  Second,
>>> with
>>> both av_strlcpy() calls that use a length of end - ptr, this could in
>>> theory result in a buffer overrun depending on how long the URI is, since
>>> the destination buffers have a fixed size.  This issue is prevented in
>>> the
>>> second call to av_strlcpy(), since it uses sizeof(hls->key_uri), but it
>>> is
>>> a potential issue with the first calls (note that this comment applies to
>>> the IV=0x code below).  If you want to use av_strlcpy(), either make sure
>>> that end - ptr is less than or equal to sizeof(hls->key_uri) or do
>>> something like *end = 0 first and then use av_strlcpy(hls->key_uri, ptr,
>>> sizeof(hls->key_uri)).
>>>
>>> In addition, based on the EXT-X-KEY example at
>>> https://developer.apple.com/library/content/technotes/tn2288/_index.html
>>> , it would appear that an EXT-X-KEY declaration may span multiple lines.
>>> Your solution will not work properly in this case.
>>>
>>> Hi Aaron,
>>      I think the libavformat/hls.c maybe have the same problem, because
>> both of hlsenc and hls use  read_chomp_line to read one line,
>>      that need check the '\' tail a line, and read the next line into a
>> buffer, that maybe better, is that right?
>>
>
> Yes, I was thinking the same thing, that read_chomp_line() needs to be
> enhanced to deal with declarations that span multiple lines.  That probably
> belongs in a separate patch though, even if it is only relevant for
> EXT-X-KEY.

Yes, I think the better way is move them to a public space for the hlsenc
and hls.

>
>
> Aaron Levinson
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>