| Message ID | 20180403011246.30530-1-rshaffer@tunein.com |
|---|---|
| State | Accepted |
| Commit | 6a1be7561c870a8cd3cee86a57aabdffb19e3870 |
| Headers | show |
> On 3 Apr 2018, at 09:12, rshaffer@tunein.com wrote: > > From: Richard Shaffer <rshaffer@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. > 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
On Mon, Apr 2, 2018 at 8:31 PM, Steven Liu <lq@chinaffmpeg.org> wrote: > > >> On 3 Apr 2018, at 09:12, rshaffer@tunein.com wrote: >> >> From: Richard Shaffer <rshaffer@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? >> 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 > > > > >
> On 3 Apr 2018, at 12:33, Richard Shaffer <rshaffer@tunein.com> wrote: > > On Mon, Apr 2, 2018 at 8:31 PM, Steven Liu <lq@chinaffmpeg.org> wrote: >> >> >>> On 3 Apr 2018, at 09:12, rshaffer@tunein.com wrote: >>> >>> From: Richard Shaffer <rshaffer@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. > >>> 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
On Mon, Apr 2, 2018 at 10:15 PM, Steven Liu <lq@chinaffmpeg.org> wrote: > > >> On 3 Apr 2018, at 12:33, Richard Shaffer <rshaffer@tunein.com> wrote: >> >> On Mon, Apr 2, 2018 at 8:31 PM, Steven Liu <lq@chinaffmpeg.org> wrote: >>> >>> >>>> On 3 Apr 2018, at 09:12, rshaffer@tunein.com wrote: >>>> >>>> From: Richard Shaffer <rshaffer@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@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
> On 3 Apr 2018, at 14:06, Richard Shaffer <rshaffer@tunein.com> wrote: > > On Mon, Apr 2, 2018 at 10:15 PM, Steven Liu <lq@chinaffmpeg.org> wrote: >> >> >>> On 3 Apr 2018, at 12:33, Richard Shaffer <rshaffer@tunein.com> wrote: >>> >>> On Mon, Apr 2, 2018 at 8:31 PM, Steven Liu <lq@chinaffmpeg.org> wrote: >>>> >>>> >>>>> On 3 Apr 2018, at 09:12, rshaffer@tunein.com wrote: >>>>> >>>>> From: Richard Shaffer <rshaffer@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@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
On Tue, Apr 3, 2018 at 12:13 AM, Steven Liu <lq@chinaffmpeg.org> wrote: > > > > On 3 Apr 2018, at 14:06, Richard Shaffer <rshaffer@tunein.com> wrote: > > > > On Mon, Apr 2, 2018 at 10:15 PM, Steven Liu <lq@chinaffmpeg.org> wrote: > >> > >> > >>> On 3 Apr 2018, at 12:33, Richard Shaffer <rshaffer@tunein.com> wrote: > >>> > >>> On Mon, Apr 2, 2018 at 8:31 PM, Steven Liu <lq@chinaffmpeg.org> wrote: > >>>> > >>>> > >>>>> On 3 Apr 2018, at 09:12, rshaffer@tunein.com wrote: > >>>>> > >>>>> From: Richard Shaffer <rshaffer@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 >
>> >> 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. no logic problem, just that option be used to HTTP, is that ok? BTW, how should i reproduce the problem you said? " 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. " > >
2018-04-04 7:53 GMT+08:00 Steven Liu <lingjiujianke@gmail.com>: >>> >>> 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. > > no logic problem, just that option be used to HTTP, is that ok? > > BTW, how should i reproduce the problem you said? > > " > 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. > " So i think add option timeout to do it maybe better than this way. you can find another formats do it like this. > >> >>
On Tue, Apr 3, 2018 at 5:11 PM, Steven Liu <lingjiujianke@gmail.com> wrote: > 2018-04-04 7:53 GMT+08:00 Steven Liu <lingjiujianke@gmail.com>: > >>> > >>> 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. > > > > no logic problem, just that option be used to HTTP, is that ok? > This is a logic problem for me, because I'm not understanding your logic. What is the reason that avio_opts should only be used for HTTP options? > > > > BTW, how should i reproduce the problem you said? > If you pass the rw_timeout option to either the command-line or to avformat_open_input, it will be applied only to the URL passed on the command line or to the function. When the HLS demuxer tries to open other URLs, such as keying URIs, media playlists or segments, it will not apply the rw_timeout option. As a result, if we fail to receive data from the remote server, it can block the process instead of returning the expected AVERROR(ETIMEDOUT) error. > > > > " > > 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. > > " > > So i think add option timeout to do it maybe better than this way. you > can find another formats do it like this. > I think the HLS demuxer is pretty unique. Except for dash, I don't know which other demuxers have to open additional avio to do their work. If there is an example you can show me that illustrates your point, that would be appreciated. > > > >> > >> > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > Hi Steven, I apologize for the late response. I have been traveling. I understand that you might be busy, and that English is not everyone's first language. However, if you could take the time to give a more thorough response, I'd really appreciate it. It's a little bit frustrating for me: I'm asking why avio_opts needs to be restricted to HTTP options, and the answer I seem to keep getting back is "because it's for HTTP options." That answer isn't very satisfying, and doesn't help me understand /why/ you think it should only be used for HTTP options. It may be that you have a good reason, but if I can't understand what it is, I'm not going to be able to provide a better fix. I can't force anyone to accept my changes or provide more detailed feedback. However, if we can't understand each other, then at some point I will have to give up. I'll apply a patch that fixes my problem to a local fork of ffmpeg. Other people outside of my company won't benefit from the fix. My company will also have to maintain our own copy of ffmpeg. This could make it less likely for us to share our changes with the community, and it will also make it harder for us to benefit from improvements and fixes to ffmpeg. I don't want to do that, because nobody wins in that scenario. That is why I'm asking for your help to understand your objection and suggestion. Thanks, -Richard
> 在 2018年4月11日，上午3:14，Richard Shaffer <rshaffer@tunein.com> 写道： > > On Tue, Apr 3, 2018 at 5:11 PM, Steven Liu <lingjiujianke@gmail.com> wrote: > >> 2018-04-04 7:53 GMT+08:00 Steven Liu <lingjiujianke@gmail.com>: >>>>> >>>>> 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. >>> >>> no logic problem, just that option be used to HTTP, is that ok? >> > > This is a logic problem for me, because I'm not understanding your logic. > What is the reason that avio_opts should only be used for HTTP options? > > >>> >>> BTW, how should i reproduce the problem you said? >> > > If you pass the rw_timeout option to either the command-line or to > avformat_open_input, it will be applied only to the URL passed on the > command line or to the function. When the HLS demuxer tries to open other > URLs, such as keying URIs, media playlists or segments, it will not apply > the rw_timeout option. As a result, if we fail to receive data from the > remote server, it can block the process instead of returning the expected > AVERROR(ETIMEDOUT) error. > > >>> >>> " >>> 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. >>> " >> >> So i think add option timeout to do it maybe better than this way. you >> can find another formats do it like this. >> > > I think the HLS demuxer is pretty unique. Except for dash, I don't know > which other demuxers have to open additional avio to do their work. If > there is an example you can show me that illustrates your point, that would > be appreciated. > > >>> >>>> >>>> >> _______________________________________________ >> ffmpeg-devel mailing list >> ffmpeg-devel@ffmpeg.org >> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >> > > Hi Steven, > > I apologize for the late response. I have been traveling. > > I understand that you might be busy, and that English is not everyone's > first language. However, if you could take the time to give a more thorough > response, I'd really appreciate it. It's a little bit frustrating for me: > I'm asking why avio_opts needs to be restricted to HTTP options, and the > answer I seem to keep getting back is "because it's for HTTP options." That > answer isn't very satisfying, and doesn't help me understand /why/ you > think it should only be used for HTTP options. It may be that you have a > good reason, but if I can't understand what it is, I'm not going to be able > to provide a better fix. > > I can't force anyone to accept my changes or provide more detailed > feedback. However, if we can't understand each other, then at some point I > will have to give up. I'll apply a patch that fixes my problem to a local > fork of ffmpeg. Other people outside of my company won't benefit from the > fix. My company will also have to maintain our own copy of ffmpeg. This > could make it less likely for us to share our changes with the community, > and it will also make it harder for us to benefit from improvements and > fixes to ffmpeg. I don't want to do that, because nobody wins in that > scenario. That is why I'm asking for your help to understand your objection > and suggestion. Sorry for my poor English, English is not my first language, thanks, you can contribute. After the last patch you submit(https://patchwork.ffmpeg.org/patch/8378/), i think i can understand this patch. > > Thanks, > > -Richard > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
On Tue, Apr 10, 2018 at 4:00 PM, Liu Steven <lq@chinaffmpeg.org> wrote: > > > 在 2018年4月11日，上午3:14，Richard Shaffer <rshaffer@tunein.com> 写道： > > > > On Tue, Apr 3, 2018 at 5:11 PM, Steven Liu <lingjiujianke@gmail.com> > wrote: > > > >> 2018-04-04 7:53 GMT+08:00 Steven Liu <lingjiujianke@gmail.com>: > >>>>> > >>>>> 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. > >>> > >>> no logic problem, just that option be used to HTTP, is that ok? > >> > > > > This is a logic problem for me, because I'm not understanding your logic. > > What is the reason that avio_opts should only be used for HTTP options? > > > > > >>> > >>> BTW, how should i reproduce the problem you said? > >> > > > > If you pass the rw_timeout option to either the command-line or to > > avformat_open_input, it will be applied only to the URL passed on the > > command line or to the function. When the HLS demuxer tries to open other > > URLs, such as keying URIs, media playlists or segments, it will not apply > > the rw_timeout option. As a result, if we fail to receive data from the > > remote server, it can block the process instead of returning the expected > > AVERROR(ETIMEDOUT) error. > > > > > >>> > >>> " > >>> 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. > >>> " > >> > >> So i think add option timeout to do it maybe better than this way. you > >> can find another formats do it like this. > >> > > > > I think the HLS demuxer is pretty unique. Except for dash, I don't know > > which other demuxers have to open additional avio to do their work. If > > there is an example you can show me that illustrates your point, that > would > > be appreciated. > > > > > >>> > >>>> > >>>> > >> _______________________________________________ > >> ffmpeg-devel mailing list > >> ffmpeg-devel@ffmpeg.org > >> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > >> > > > > Hi Steven, > > > > I apologize for the late response. I have been traveling. > > > > I understand that you might be busy, and that English is not everyone's > > first language. However, if you could take the time to give a more > thorough > > response, I'd really appreciate it. It's a little bit frustrating for me: > > I'm asking why avio_opts needs to be restricted to HTTP options, and the > > answer I seem to keep getting back is "because it's for HTTP options." > That > > answer isn't very satisfying, and doesn't help me understand /why/ you > > think it should only be used for HTTP options. It may be that you have a > > good reason, but if I can't understand what it is, I'm not going to be > able > > to provide a better fix. > > > > I can't force anyone to accept my changes or provide more detailed > > feedback. However, if we can't understand each other, then at some point > I > > will have to give up. I'll apply a patch that fixes my problem to a local > > fork of ffmpeg. Other people outside of my company won't benefit from the > > fix. My company will also have to maintain our own copy of ffmpeg. This > > could make it less likely for us to share our changes with the community, > > and it will also make it harder for us to benefit from improvements and > > fixes to ffmpeg. I don't want to do that, because nobody wins in that > > scenario. That is why I'm asking for your help to understand your > objection > > and suggestion. > > Sorry for my poor English, English is not my first language, thanks, you > can contribute. > After the last patch you submit(https://patchwork.ffmpeg.org/patch/8378/), > i think i can understand this patch. > > Thanks, Steven. I will wait a few days. If there are no additional comments, I will ask you or another maintainer to apply the patches. > > > > > Thanks, > > > > -Richard > > _______________________________________________ > > 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 >
> On 11 Apr 2018, at 07:13, Richard Shaffer <rshaffer@tunein.com> wrote: > > > > On Tue, Apr 10, 2018 at 4:00 PM, Liu Steven <lq@chinaffmpeg.org> wrote: > > > 在 2018年4月11日，上午3:14，Richard Shaffer <rshaffer@tunein.com> 写道： > > > > On Tue, Apr 3, 2018 at 5:11 PM, Steven Liu <lingjiujianke@gmail.com> wrote: > > > >> 2018-04-04 7:53 GMT+08:00 Steven Liu <lingjiujianke@gmail.com>: > >>>>> > >>>>> 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. > >>> > >>> no logic problem, just that option be used to HTTP, is that ok? > >> > > > > This is a logic problem for me, because I'm not understanding your logic. > > What is the reason that avio_opts should only be used for HTTP options? > > > > > >>> > >>> BTW, how should i reproduce the problem you said? > >> > > > > If you pass the rw_timeout option to either the command-line or to > > avformat_open_input, it will be applied only to the URL passed on the > > command line or to the function. When the HLS demuxer tries to open other > > URLs, such as keying URIs, media playlists or segments, it will not apply > > the rw_timeout option. As a result, if we fail to receive data from the > > remote server, it can block the process instead of returning the expected > > AVERROR(ETIMEDOUT) error. > > > > > >>> > >>> " > >>> 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. > >>> " > >> > >> So i think add option timeout to do it maybe better than this way. you > >> can find another formats do it like this. > >> > > > > I think the HLS demuxer is pretty unique. Except for dash, I don't know > > which other demuxers have to open additional avio to do their work. If > > there is an example you can show me that illustrates your point, that would > > be appreciated. > > > > > >>> > >>>> > >>>> > >> _______________________________________________ > >> ffmpeg-devel mailing list > >> ffmpeg-devel@ffmpeg.org > >> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > >> > > > > Hi Steven, > > > > I apologize for the late response. I have been traveling. > > > > I understand that you might be busy, and that English is not everyone's > > first language. However, if you could take the time to give a more thorough > > response, I'd really appreciate it. It's a little bit frustrating for me: > > I'm asking why avio_opts needs to be restricted to HTTP options, and the > > answer I seem to keep getting back is "because it's for HTTP options." That > > answer isn't very satisfying, and doesn't help me understand /why/ you > > think it should only be used for HTTP options. It may be that you have a > > good reason, but if I can't understand what it is, I'm not going to be able > > to provide a better fix. > > > > I can't force anyone to accept my changes or provide more detailed > > feedback. However, if we can't understand each other, then at some point I > > will have to give up. I'll apply a patch that fixes my problem to a local > > fork of ffmpeg. Other people outside of my company won't benefit from the > > fix. My company will also have to maintain our own copy of ffmpeg. This > > could make it less likely for us to share our changes with the community, > > and it will also make it harder for us to benefit from improvements and > > fixes to ffmpeg. I don't want to do that, because nobody wins in that > > scenario. That is why I'm asking for your help to understand your objection > > and suggestion. > > Sorry for my poor English, English is not my first language, thanks, you can contribute. > After the last patch you submit(https://patchwork.ffmpeg.org/patch/8378/), i think i can understand this patch. > > Thanks, Steven. I will wait a few days. If there are no additional comments, I will ask you or another maintainer to apply the patches. pushed > > > > > Thanks, > > > > -Richard > > _______________________________________________ > > 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
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 }; const char * const * opt = opts; uint8_t *buf; int ret = 0;
From: Richard Shaffer <rshaffer@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(-)