On Tue, Apr 3, 2018 at 12:13 AM, Steven Liu <l...@chinaffmpeg.org> wrote:
> > > > On 3 Apr 2018, at 14:06, Richard Shaffer <rshaf...@tunein.com> wrote: > > > > On Mon, Apr 2, 2018 at 10:15 PM, Steven Liu <l...@chinaffmpeg.org> wrote: > >> > >> > >>> On 3 Apr 2018, at 12:33, Richard Shaffer <rshaf...@tunein.com> wrote: > >>> > >>> On Mon, Apr 2, 2018 at 8:31 PM, Steven Liu <l...@chinaffmpeg.org> wrote: > >>>> > >>>> > >>>>> On 3 Apr 2018, at 09:12, rshaf...@tunein.com wrote: > >>>>> > >>>>> From: Richard Shaffer <rshaf...@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. > > That's pretty clear. What I was asking is why the options are stored both in these fields as well as avio_opts, and this doesn't answer my question. I was also asking why you object to storing the timeout option in avio_opts, but I'm not understanding the logic in your responses. > > > >>> > >>>>> const char * const * opt = opts; > >>>>> uint8_t *buf; > >>>>> int ret = 0; > >>>>> -- > >>>>> 2.15.1 (Apple Git-101) > >>>>> > >>>>> _______________________________________________ > >>>>> ffmpeg-devel mailing list > >>>>> ffmpeg-devel@ffmpeg.org > >>>>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > >>>> > >>>> Thanks > >>>> Steven > >>>> > >>>> > >>>> > >>>> > >>>> > >>> _______________________________________________ > >>> ffmpeg-devel mailing list > >>> ffmpeg-devel@ffmpeg.org > >>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > >> > >> Thanks > >> Steven > >> > >> > >> > >> > >> > >> _______________________________________________ > >> ffmpeg-devel mailing list > >> ffmpeg-devel@ffmpeg.org > >> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > _______________________________________________ > > ffmpeg-devel mailing list > > ffmpeg-devel@ffmpeg.org > > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > Thanks > Steven > > > > > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel