[FFmpeg-devel] http: honor response headers in redirect caching

[FFmpeg-devel] http: honor response headers in redirect caching

[Eran Kornblau]

[-- Attachment #1: Type: text/plain, Size: 2093 bytes --]

Hi all,

Recently I’ve submitted a patch that adds a config option to disable the caching of http redirects.
We planned this as a workaround to the fact there’s a limit on the expiration that can be set on S3 pre-signed URLs.
The idea was to have a service that generates signed S3 URLs and redirects to them, ffmpeg would hit this service
on every seek/disconnect, and get a fresh S3 URL.

We found that in some cases, ffmpeg seeks on every frame (probably due to some gap between the positions of
video/audio frames in the file). In these cases, completely disabling the redirect caching becomes quite inefficient -
our S3 signing service gets called ~20 times/sec by a single encoding task.

The attached patch provides a more complete/correct implementation of redirect caching – the decision whether
to cache/for how long is now determined according to the response from the server - status code (e.g. 301 vs 302),
expires header & cache-control header. The behavior (detailed in the comment of the commit) is more aligned
with how browsers handle it.

In high level, these are the changes that were implemented in the patch –

  1.  Added a dictionary on HTTPContext for keeping the cache – since I need to save both the target URL
and the expiration, I’m formatting them together on a single string (“expiry;target-url”)
  2.  Added a string on HTTPContext to save the value of the location header – this makes the existing
flags new_location/location_changed redundant, and they were removed
  3.  Added functions for parsing Cache-Control/Expires – for Expires I used the existing function for
parsing cookie expiration time. The result is saved on a new integer on HTTPContent -
a zero value means that no such header was encountered yet, a negative value means the response
should not be cached.
Once this patch is merged, IMHO we can remove the flag that I added in the previous patch (=always treat it as 0,
and have the caching work based on the new dictionary). I will happily submit another patch for this.

Thank you!

Eran

[-- Attachment #2: 0001-http-honor-response-headers-in-redirect-caching.patch --]
[-- Type: application/octet-stream, Size: 13646 bytes --]

[-- Attachment #3: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] http: honor response headers in redirect caching

[Ronald S. Bultje]

Hi,

On Tue, Jan 11, 2022 at 9:43 AM Eran Kornblau <eran.kornblau@kaltura.com>
wrote:

> Hi all,
>
> Recently I’ve submitted a patch that adds a config option to disable the
> caching of http redirects.
> We planned this as a workaround to the fact there’s a limit on the
> expiration that can be set on S3 pre-signed URLs.
> The idea was to have a service that generates signed S3 URLs and redirects
> to them, ffmpeg would hit this service
> on every seek/disconnect, and get a fresh S3 URL.
>
> We found that in some cases, ffmpeg seeks on every frame (probably due to
> some gap between the positions of
> video/audio frames in the file). In these cases, completely disabling the
> redirect caching becomes quite inefficient -
> our S3 signing service gets called ~20 times/sec by a single encoding task.
>
> The attached patch provides a more complete/correct implementation of
> redirect caching – the decision whether
> to cache/for how long is now determined according to the response from the
> server - status code (e.g. 301 vs 302),
> expires header & cache-control header. The behavior (detailed in the
> comment of the commit) is more aligned
> with how browsers handle it.
>
> In high level, these are the changes that were implemented in the patch –
>
>   1.  Added a dictionary on HTTPContext for keeping the cache – since I
> need to save both the target URL
> and the expiration, I’m formatting them together on a single string
> (“expiry;target-url”)
>   2.  Added a string on HTTPContext to save the value of the location
> header – this makes the existing
> flags new_location/location_changed redundant, and they were removed
>   3.  Added functions for parsing Cache-Control/Expires – for Expires I
> used the existing function for
> parsing cookie expiration time. The result is saved on a new integer on
> HTTPContent -
> a zero value means that no such header was encountered yet, a negative
> value means the response
> should not be cached.
> Once this patch is merged, IMHO we can remove the flag that I added in the
> previous patch (=always treat it as 0,
> and have the caching work based on the new dictionary). I will happily
> submit another patch for this.
>
> Thank you!
>

Will merge this in 1-2 days if there's no further comments.

Ronald
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] http: honor response headers in redirect caching

[Ronald S. Bultje]

Hi,

On Mon, Jan 17, 2022 at 7:10 AM Ronald S. Bultje <rsbultje@gmail.com> wrote:

> Hi,
>
> On Tue, Jan 11, 2022 at 9:43 AM Eran Kornblau <eran.kornblau@kaltura.com>
> wrote:
>
>> Hi all,
>>
>> Recently I’ve submitted a patch that adds a config option to disable the
>> caching of http redirects.
>> We planned this as a workaround to the fact there’s a limit on the
>> expiration that can be set on S3 pre-signed URLs.
>> The idea was to have a service that generates signed S3 URLs and
>> redirects to them, ffmpeg would hit this service
>> on every seek/disconnect, and get a fresh S3 URL.
>>
>> We found that in some cases, ffmpeg seeks on every frame (probably due to
>> some gap between the positions of
>> video/audio frames in the file). In these cases, completely disabling the
>> redirect caching becomes quite inefficient -
>> our S3 signing service gets called ~20 times/sec by a single encoding
>> task.
>>
>> The attached patch provides a more complete/correct implementation of
>> redirect caching – the decision whether
>> to cache/for how long is now determined according to the response from
>> the server - status code (e.g. 301 vs 302),
>> expires header & cache-control header. The behavior (detailed in the
>> comment of the commit) is more aligned
>> with how browsers handle it.
>>
>> In high level, these are the changes that were implemented in the patch –
>>
>>   1.  Added a dictionary on HTTPContext for keeping the cache – since I
>> need to save both the target URL
>> and the expiration, I’m formatting them together on a single string
>> (“expiry;target-url”)
>>   2.  Added a string on HTTPContext to save the value of the location
>> header – this makes the existing
>> flags new_location/location_changed redundant, and they were removed
>>   3.  Added functions for parsing Cache-Control/Expires – for Expires I
>> used the existing function for
>> parsing cookie expiration time. The result is saved on a new integer on
>> HTTPContent -
>> a zero value means that no such header was encountered yet, a negative
>> value means the response
>> should not be cached.
>> Once this patch is merged, IMHO we can remove the flag that I added in
>> the previous patch (=always treat it as 0,
>> and have the caching work based on the new dictionary). I will happily
>> submit another patch for this.
>>
>> Thank you!
>>
>
> Will merge this in 1-2 days if there's no further comments.
>

Merged.

Ronald
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] http: honor response headers in redirect caching

[Eran Kornblau]

[-- Attachment #1: Type: text/plain, Size: 3488 bytes --]

> 
> 
> Hi,
> 
> On Mon, Jan 17, 2022 at 7:10 AM Ronald S. Bultje <rsbultje@gmail.com> wrote:
> 
> > Hi,
> >
> > On Tue, Jan 11, 2022 at 9:43 AM Eran Kornblau 
> > <eran.kornblau@kaltura.com>
> > wrote:
> >
> >> Hi all,
> >>
> >> Recently I’ve submitted a patch that adds a config option to disable 
> >> the caching of http redirects.
> >> We planned this as a workaround to the fact there’s a limit on the 
> >> expiration that can be set on S3 pre-signed URLs.
> >> The idea was to have a service that generates signed S3 URLs and 
> >> redirects to them, ffmpeg would hit this service on every 
> >> seek/disconnect, and get a fresh S3 URL.
> >>
> >> We found that in some cases, ffmpeg seeks on every frame (probably 
> >> due to some gap between the positions of video/audio frames in the 
> >> file). In these cases, completely disabling the redirect caching 
> >> becomes quite inefficient - our S3 signing service gets called ~20 
> >> times/sec by a single encoding task.
> >>
> >> The attached patch provides a more complete/correct implementation of 
> >> redirect caching – the decision whether to cache/for how long is now 
> >> determined according to the response from the server - status code 
> >> (e.g. 301 vs 302), expires header & cache-control header. The 
> >> behavior (detailed in the comment of the commit) is more aligned with 
> >> how browsers handle it.
> >>
> >> In high level, these are the changes that were implemented in the 
> >> patch –
> >>
> >>   1.  Added a dictionary on HTTPContext for keeping the cache – since 
> >> I need to save both the target URL and the expiration, I’m formatting 
> >> them together on a single string
> >> (“expiry;target-url”)
> >>   2.  Added a string on HTTPContext to save the value of the location 
> >> header – this makes the existing flags new_location/location_changed 
> >> redundant, and they were removed
> >>   3.  Added functions for parsing Cache-Control/Expires – for Expires 
> >> I used the existing function for parsing cookie expiration time. The 
> >> result is saved on a new integer on HTTPContent - a zero value means 
> >> that no such header was encountered yet, a negative value means the 
> >> response should not be cached.
> >> Once this patch is merged, IMHO we can remove the flag that I added 
> >> in the previous patch (=always treat it as 0, and have the caching 
> >> work based on the new dictionary). I will happily submit another 
> >> patch for this.
> >>
> >> Thank you!
> >>
> >
> > Will merge this in 1-2 days if there's no further comments.
> >
> 
> Merged.
> 
> Ronald

Ronald, thank you for your continued support on this one!

As promised, sending another patch to remove the `cache_redirect` option, as it seems to be no longer needed.

Thanks

Eran
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://eur02.safelinks.protection.outlook.com/?url=https%3A%2F%2Fffmpeg.org%2Fmailman%2Flistinfo%2Fffmpeg-devel&amp;data=04%7C01%7C%7Cf4976d428a7c45cd411d08d9dad2f9ab%7C0c503748de3f4e2597e26819d53a42b6%7C1%7C0%7C637781421948601254%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=yqZc6UTV%2FQ9KXD1eCwJXAPha%2FKZqas5TrRlnH7cUPmQ%3D&amp;reserved=0
> 
> To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>

[-- Attachment #2: 0001-http-remove-cache_redirect-option.patch --]
[-- Type: application/octet-stream, Size: 2592 bytes --]

[-- Attachment #3: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] http: honor response headers in redirect caching

[Eran Kornblau]

> 
> -----Original Message-----
> From: Eran Kornblau 
> Sent: Wednesday, 19 January 2022 9:14
> To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> Subject: RE: [FFmpeg-devel] http: honor response headers in redirect caching
> 
> > 
> > 
> > Hi,
> > 
> > On Mon, Jan 17, 2022 at 7:10 AM Ronald S. Bultje <rsbultje@gmail.com> wrote:
> > 
> > > Hi,
> > >
> > > On Tue, Jan 11, 2022 at 9:43 AM Eran Kornblau 
> > > <eran.kornblau@kaltura.com>
> > > wrote:
> > >
> > >> Hi all,
> > >>
> > >> Recently I’ve submitted a patch that adds a config option to 
> > >> disable the caching of http redirects.
> > >> We planned this as a workaround to the fact there’s a limit on the 
> > >> expiration that can be set on S3 pre-signed URLs.
> > >> The idea was to have a service that generates signed S3 URLs and 
> > >> redirects to them, ffmpeg would hit this service on every 
> > >> seek/disconnect, and get a fresh S3 URL.
> > >>
> > >> We found that in some cases, ffmpeg seeks on every frame (probably 
> > >> due to some gap between the positions of video/audio frames in the 
> > >> file). In these cases, completely disabling the redirect caching 
> > >> becomes quite inefficient - our S3 signing service gets called ~20 
> > >> times/sec by a single encoding task.
> > >>
> > >> The attached patch provides a more complete/correct implementation 
> > >> of redirect caching – the decision whether to cache/for how long is 
> > >> now determined according to the response from the server - status 
> > >> code (e.g. 301 vs 302), expires header & cache-control header. The 
> > >> behavior (detailed in the comment of the commit) is more aligned 
> > >> with how browsers handle it.
> > >>
> > >> In high level, these are the changes that were implemented in the 
> > >> patch –
> > >>
> > >>   1.  Added a dictionary on HTTPContext for keeping the cache – 
> > >> since I need to save both the target URL and the expiration, I’m 
> > >> formatting them together on a single string
> > >> (“expiry;target-url”)
> > >>   2.  Added a string on HTTPContext to save the value of the 
> > >> location header – this makes the existing flags 
> > >> new_location/location_changed redundant, and they were removed
> > >>   3.  Added functions for parsing Cache-Control/Expires – for 
> > >> Expires I used the existing function for parsing cookie expiration 
> > >> time. The result is saved on a new integer on HTTPContent - a zero 
> > >> value means that no such header was encountered yet, a negative 
> > >> value means the response should not be cached.
> > >> Once this patch is merged, IMHO we can remove the flag that I added 
> > >> in the previous patch (=always treat it as 0, and have the caching 
> > >> work based on the new dictionary). I will happily submit another 
> > >> patch for this.
> > >>
> > >> Thank you!
> > >>
> > >
> > > Will merge this in 1-2 days if there's no further comments.
> > >
> > 
> > Merged.
> > 
> > Ronald
> 
> Ronald, thank you for your continued support on this one!
> 
> As promised, sending another patch to remove the `cache_redirect` option, as it seems to be no longer needed.
> 
> Thanks
> 
> Eran

Ping

> > _______________________________________________
> > ffmpeg-devel mailing list
> > ffmpeg-devel@ffmpeg.org
> > https://eur02.safelinks.protection.outlook.com/?url=https%3A%2F%2Fffmp
> > eg.org%2Fmailman%2Flistinfo%2Fffmpeg-devel&amp;data=04%7C01%7C%7Cf4976
> > d428a7c45cd411d08d9dad2f9ab%7C0c503748de3f4e2597e26819d53a42b6%7C1%7C0
> > %7C637781421948601254%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQ
> > IjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=yqZc6UTV%2
> > FQ9KXD1eCwJXAPha%2FKZqas5TrRlnH7cUPmQ%3D&amp;reserved=0
> > 
> > To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
> >
>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] http: honor response headers in redirect caching

[Ronald S. Bultje]

Hi,

On Sun, Jan 30, 2022 at 1:15 AM Eran Kornblau <eran.kornblau@kaltura.com>
wrote:

> >
> > -----Original Message-----
> > From: Eran Kornblau
> > Sent: Wednesday, 19 January 2022 9:14
> > To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> > Subject: RE: [FFmpeg-devel] http: honor response headers in redirect
> caching
> >
> > >
> > >
> > > Hi,
> > >
> > > On Mon, Jan 17, 2022 at 7:10 AM Ronald S. Bultje <rsbultje@gmail.com>
> wrote:
> > >
> > > > Hi,
> > > >
> > > > On Tue, Jan 11, 2022 at 9:43 AM Eran Kornblau
> > > > <eran.kornblau@kaltura.com>
> > > > wrote:
> > > >
> > > >> Hi all,
> > > >>
> > > >> Recently I’ve submitted a patch that adds a config option to
> > > >> disable the caching of http redirects.
> > > >> We planned this as a workaround to the fact there’s a limit on the
> > > >> expiration that can be set on S3 pre-signed URLs.
> > > >> The idea was to have a service that generates signed S3 URLs and
> > > >> redirects to them, ffmpeg would hit this service on every
> > > >> seek/disconnect, and get a fresh S3 URL.
> > > >>
> > > >> We found that in some cases, ffmpeg seeks on every frame (probably
> > > >> due to some gap between the positions of video/audio frames in the
> > > >> file). In these cases, completely disabling the redirect caching
> > > >> becomes quite inefficient - our S3 signing service gets called ~20
> > > >> times/sec by a single encoding task.
> > > >>
> > > >> The attached patch provides a more complete/correct implementation
> > > >> of redirect caching – the decision whether to cache/for how long is
> > > >> now determined according to the response from the server - status
> > > >> code (e.g. 301 vs 302), expires header & cache-control header. The
> > > >> behavior (detailed in the comment of the commit) is more aligned
> > > >> with how browsers handle it.
> > > >>
> > > >> In high level, these are the changes that were implemented in the
> > > >> patch –
> > > >>
> > > >>   1.  Added a dictionary on HTTPContext for keeping the cache –
> > > >> since I need to save both the target URL and the expiration, I’m
> > > >> formatting them together on a single string
> > > >> (“expiry;target-url”)
> > > >>   2.  Added a string on HTTPContext to save the value of the
> > > >> location header – this makes the existing flags
> > > >> new_location/location_changed redundant, and they were removed
> > > >>   3.  Added functions for parsing Cache-Control/Expires – for
> > > >> Expires I used the existing function for parsing cookie expiration
> > > >> time. The result is saved on a new integer on HTTPContent - a zero
> > > >> value means that no such header was encountered yet, a negative
> > > >> value means the response should not be cached.
> > > >> Once this patch is merged, IMHO we can remove the flag that I added
> > > >> in the previous patch (=always treat it as 0, and have the caching
> > > >> work based on the new dictionary). I will happily submit another
> > > >> patch for this.
> > > >>
> > > >> Thank you!
> > > >>
> > > >
> > > > Will merge this in 1-2 days if there's no further comments.
> > > >
> > >
> > > Merged.
> > >
> > > Ronald
> >
> > Ronald, thank you for your continued support on this one!
> >
> > As promised, sending another patch to remove the `cache_redirect`
> option, as it seems to be no longer needed.
> >
> > Thanks
> >
> > Eran
>
> Ping
>

Got it, will apply this later today. I realize this is an API change but
since the option only existed for 2 weeks, it should be OK.

Ronald
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] http: honor response headers in redirect caching

[Ronald S. Bultje]

Hi,

On Mon, Jan 31, 2022 at 9:37 AM Ronald S. Bultje <rsbultje@gmail.com> wrote:

> Hi,
>
> On Sun, Jan 30, 2022 at 1:15 AM Eran Kornblau <eran.kornblau@kaltura.com>
> wrote:
>
>> >
>> > -----Original Message-----
>> > From: Eran Kornblau
>> > Sent: Wednesday, 19 January 2022 9:14
>> > To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org
>> >
>> > Subject: RE: [FFmpeg-devel] http: honor response headers in redirect
>> caching
>> >
>> > >
>> > >
>> > > Hi,
>> > >
>> > > On Mon, Jan 17, 2022 at 7:10 AM Ronald S. Bultje <rsbultje@gmail.com>
>> wrote:
>> > >
>> > > > Hi,
>> > > >
>> > > > On Tue, Jan 11, 2022 at 9:43 AM Eran Kornblau
>> > > > <eran.kornblau@kaltura.com>
>> > > > wrote:
>> > > >
>> > > >> Hi all,
>> > > >>
>> > > >> Recently I’ve submitted a patch that adds a config option to
>> > > >> disable the caching of http redirects.
>> > > >> We planned this as a workaround to the fact there’s a limit on the
>> > > >> expiration that can be set on S3 pre-signed URLs.
>> > > >> The idea was to have a service that generates signed S3 URLs and
>> > > >> redirects to them, ffmpeg would hit this service on every
>> > > >> seek/disconnect, and get a fresh S3 URL.
>> > > >>
>> > > >> We found that in some cases, ffmpeg seeks on every frame (probably
>> > > >> due to some gap between the positions of video/audio frames in the
>> > > >> file). In these cases, completely disabling the redirect caching
>> > > >> becomes quite inefficient - our S3 signing service gets called ~20
>> > > >> times/sec by a single encoding task.
>> > > >>
>> > > >> The attached patch provides a more complete/correct implementation
>> > > >> of redirect caching – the decision whether to cache/for how long
>> is
>> > > >> now determined according to the response from the server - status
>> > > >> code (e.g. 301 vs 302), expires header & cache-control header. The
>> > > >> behavior (detailed in the comment of the commit) is more aligned
>> > > >> with how browsers handle it.
>> > > >>
>> > > >> In high level, these are the changes that were implemented in the
>> > > >> patch –
>> > > >>
>> > > >>   1.  Added a dictionary on HTTPContext for keeping the cache –
>> > > >> since I need to save both the target URL and the expiration, I’m
>> > > >> formatting them together on a single string
>> > > >> (“expiry;target-url”)
>> > > >>   2.  Added a string on HTTPContext to save the value of the
>> > > >> location header – this makes the existing flags
>> > > >> new_location/location_changed redundant, and they were removed
>> > > >>   3.  Added functions for parsing Cache-Control/Expires – for
>> > > >> Expires I used the existing function for parsing cookie expiration
>> > > >> time. The result is saved on a new integer on HTTPContent - a zero
>> > > >> value means that no such header was encountered yet, a negative
>> > > >> value means the response should not be cached.
>> > > >> Once this patch is merged, IMHO we can remove the flag that I
>> added
>> > > >> in the previous patch (=always treat it as 0, and have the caching
>> > > >> work based on the new dictionary). I will happily submit another
>> > > >> patch for this.
>> > > >>
>> > > >> Thank you!
>> > > >>
>> > > >
>> > > > Will merge this in 1-2 days if there's no further comments.
>> > > >
>> > >
>> > > Merged.
>> > >
>> > > Ronald
>> >
>> > Ronald, thank you for your continued support on this one!
>> >
>> > As promised, sending another patch to remove the `cache_redirect`
>> option, as it seems to be no longer needed.
>> >
>> > Thanks
>> >
>> > Eran
>>
>> Ping
>>
>
> Got it, will apply this later today. I realize this is an API change but
> since the option only existed for 2 weeks, it should be OK.
>

Merged.

Ronald
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".