[FFmpeg-devel] libavformat/hls: Observe Set-Cookie headers

Signed-off-by: Micah Galizia <micahgalizia@gmail.com>
---
 libavformat/hls.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

On Tue,  2 May 2017 20:47:06 -0400
Micah Galizia <micahgalizia@gmail.com> wrote:

> Signed-off-by: Micah Galizia <micahgalizia@gmail.com>
> ---
>  libavformat/hls.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/libavformat/hls.c b/libavformat/hls.c
> index bac53a4350..643d50e1da 100644
> --- a/libavformat/hls.c
> +++ b/libavformat/hls.c
> @@ -630,8 +630,14 @@ static int open_url(AVFormatContext *s, AVIOContext **pb, const char *url,
>      ret = s->io_open(s, pb, url, AVIO_FLAG_READ, &tmp);
>      if (ret >= 0) {
>          // update cookies on http response with setcookies.
> -        void *u = (s->flags & AVFMT_FLAG_CUSTOM_IO) ? NULL : s->pb;
> -        update_options(&c->cookies, "cookies", u);
> +        char *new_cookies = NULL;
> +
> +        av_opt_get(*pb, "cookies", AV_OPT_SEARCH_CHILDREN, (uint8_t**)&new_cookies);
> +        if (new_cookies) {
> +            av_free(c->cookies);
> +            c->cookies = new_cookies;
> +        }
> +
>          av_dict_set(&opts, "cookies", c->cookies, 0);
>      }
>  
> @@ -1608,7 +1614,7 @@ static int hls_close(AVFormatContext *s)
>  
>  static int hls_read_header(AVFormatContext *s)
>  {
> -    void *u = (s->flags & AVFMT_FLAG_CUSTOM_IO) ? NULL : s->pb;
> +    void *u = s->pb;
>      HLSContext *c = s->priv_data;
>      int ret = 0, i;
>      int highest_cur_seq_no = 0;

Not comfortable with this without knowing what the purpose of this
CUSTOM_IO check was.

On 2017-05-02 09:04 PM, wm4 wrote:
> On Tue,  2 May 2017 20:47:06 -0400
> Micah Galizia <micahgalizia@gmail.com> wrote:
>
>> Signed-off-by: Micah Galizia <micahgalizia@gmail.com>
>> ---
>>   libavformat/hls.c | 12 +++++++++---
>>   1 file changed, 9 insertions(+), 3 deletions(-)
>>
>> diff --git a/libavformat/hls.c b/libavformat/hls.c
>> index bac53a4350..643d50e1da 100644
>> --- a/libavformat/hls.c
>> +++ b/libavformat/hls.c
>> @@ -630,8 +630,14 @@ static int open_url(AVFormatContext *s, AVIOContext **pb, const char *url,
>>       ret = s->io_open(s, pb, url, AVIO_FLAG_READ, &tmp);
>>       if (ret >= 0) {
>>           // update cookies on http response with setcookies.
>> -        void *u = (s->flags & AVFMT_FLAG_CUSTOM_IO) ? NULL : s->pb;
>> -        update_options(&c->cookies, "cookies", u);
>> +        char *new_cookies = NULL;
>> +
>> +        av_opt_get(*pb, "cookies", AV_OPT_SEARCH_CHILDREN, (uint8_t**)&new_cookies);
>> +        if (new_cookies) {
>> +            av_free(c->cookies);
>> +            c->cookies = new_cookies;
>> +        }
>> +
>>           av_dict_set(&opts, "cookies", c->cookies, 0);
>>       }
>>   
>> @@ -1608,7 +1614,7 @@ static int hls_close(AVFormatContext *s)
>>   
>>   static int hls_read_header(AVFormatContext *s)
>>   {
>> -    void *u = (s->flags & AVFMT_FLAG_CUSTOM_IO) ? NULL : s->pb;
>> +    void *u = s->pb;
>>       HLSContext *c = s->priv_data;
>>       int ret = 0, i;
>>       int highest_cur_seq_no = 0;
> Not comfortable with this without knowing what the purpose of this
> CUSTOM_IO check was.

Sorry, I misunderstood a prior email. Regarding why we check the custom 
IO flags, I was able to track down the following: 
https://lists.ffmpeg.org/pipermail/ffmpeg-trac/2013-February/013020.html 
for the hls_read_header. It was originally added in commit 
db6e2e848b21d988fb108995387a8c7836da4dc7 back in 2013.

After reading that, I think the problem it was trying to solve was about 
treating pb->opaque as a urlcontext -- but the code no longer does that, 
unless I've misunderstood all the casting in av_opt_get. What I believe 
it does now is call av_opt_get(*pb...), which eventually winds up in 
av_opt_find2 where it pulls options out the AVClass (av_class) contained 
in AVIOContext and starts reading its options. It doesn't seem to be 
touching opaque field in AVIOContext anymore.

Given this, I _think_ what I did is still ok. However, I can also change 
the patch to leave hls_read_header alone and add an if (s->flags ^ 
AVFMT_FLAG_CUSTOM_IO) before getting the new_cookies. This, effectively, 
fixes the bug without disregarding the check against 
AVFMT_FLAG_CUSTOM_IO -- best of both worlds and a smaller patch.

If nobody can confirm my analysis that opaque is now being left alone 
(thus, no longer requiring the check against the custom IO flag) then 
I'll just put the smaller patch up.

Thanks,

Hi,

This is a simpler version of the other patch that still observes AVFMT_FLAG_CUSTOM_IO.

Thanks in advance.