[FFmpeg-devel] [PATCH] avformat/hls: copy rw_timeout from parent to child AVIOContexts.

[Richard Shaffer]

On Mon, Apr 2, 2018 at 10:15 PM, Steven Liu <lq at chinaffmpeg.org> wrote:
>
>
>> On 3 Apr 2018, at 12:33, Richard Shaffer <rshaffer at tunein.com> wrote:
>>
>> On Mon, Apr 2, 2018 at 8:31 PM, Steven Liu <lq at chinaffmpeg.org> wrote:
>>>
>>>
>>>> On 3 Apr 2018, at 09:12, rshaffer at tunein.com wrote:
>>>>
>>>> From: Richard Shaffer <rshaffer at tunein.com>
>>>>
>>>> The rw_timeout option is currently not applied when opening media playlist,
>>>> segment, or encryption key URLs. This can cause the HLS demuxer to block
>>>> indefinitely, even when the rw_timeout option has been specified. This change
>>>> simply enables carrying over the rw_timeout option when the demuxer opens these
>>>> URLs.
>>>> ---
>>>> libavformat/hls.c | 2 +-
>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/libavformat/hls.c b/libavformat/hls.c
>>>> index c578bf86e3..6663244ddf 100644
>>>> --- a/libavformat/hls.c
>>>> +++ b/libavformat/hls.c
>>>> @@ -1661,7 +1661,7 @@ static int save_avio_options(AVFormatContext *s)
>>>> {
>>>>    HLSContext *c = s->priv_data;
>>>>    static const char * const opts[] = {
>>>> -        "headers", "http_proxy", "user_agent", "user-agent", "cookies", "referer", NULL };
>>>> +        "headers", "http_proxy", "user_agent", "user-agent", "cookies", "referer", "rw_timeout", NULL };
>>> This table is used for http header.
>>> You could add the option into hls_options.
>>
>> Thanks for looking at the change. While the options currently in the
>> table are related to HTTP and rw_timeout is more general, I'm not
>> aware of a reason not to preserve the rw_timeout option here as well.
>> It seems unnecessary to define another HLS-specific option for
>> rw_timeout when the existing option exists and does what is intended.
>> I'm not sure whether you're objecting to the change and/or have a
>> different suggestion. Do you mind elaborating on your comment?
> Is the rw_timeout in to HTTP RFC? If yes, this is ok, If not, i think that is a ffmpeg option , not a http header content.

http_proxy isn't tied to an HTTP header value, either. There isn't any
indication that avio_opts is intended for specifically HTTP options,
or that using it to store other avio options would break something. If
you have a reason why this shouldn't be used for other (non-HTTP)
options, can you please help me understand your thinking?

I also want to spend some more time working on this, because I see
that there are multiple fields in HLSContext dealing with avio
options: There is the avio_opts field, but also referer, user_agent,
cookies, headers and http_proxy. avio_opts seems to be used when
opening segments and encryption key URIs, but the other fields are
used when reloading a playlist or parsing variant playlist URLs from a
master playlist. It's not clear why the options are stored in separate
places or whether that is necessary. If you have any insight into that
as well, it would be appreciated.

>>
>>>>    const char * const * opt = opts;
>>>>    uint8_t *buf;
>>>>    int ret = 0;
>>>> --
>>>> 2.15.1 (Apple Git-101)
>>>>
>>>> _______________________________________________
>>>> ffmpeg-devel mailing list
>>>> ffmpeg-devel at ffmpeg.org
>>>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>>
>>> Thanks
>>> Steven
>>>
>>>
>>>
>>>
>>>
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel at ffmpeg.org
>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> Thanks
> Steven
>
>
>
>
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel