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

[Steven Liu]

> On 3 Apr 2018, at 14:06, Richard Shaffer <rshaffer at tunein.com> wrote:
> 
> 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.

Look at the code:

 205     char *referer;                       ///< holds HTTP referer set as an AVOption to the HTTP protocol context
 206     char *user_agent;                    ///< holds HTTP user agent set as an AVOption to the HTTP protocol context
 207     char *cookies;                       ///< holds HTTP cookie values set in either the initial response or as an AVOption to the HTTP protocol context
 208     char *headers;                       ///< holds HTTP headers set as an AVOption to the HTTP protocol context
 209     char *http_proxy;                    ///< holds the address of the HTTP proxy server

There have some comment for the options.
and reference the code in: hls_read_header / open_input and use the options.


> 
>>> 
>>>>>   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
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Thanks
Steven