[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 at gmail.com> wrote:

> Hi,
>
> On Sun, Jan 30, 2022 at 1:15 AM Eran Kornblau <eran.kornblau at kaltura.com>
> wrote:
>
>> >
>> > -----Original Message-----
>> > From: Eran Kornblau
>> > Sent: Wednesday, 19 January 2022 9:14
>> > To: FFmpeg development discussions and patches <ffmpeg-devel at 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 at gmail.com>
>> wrote:
>> > >
>> > > > Hi,
>> > > >
>> > > > On Tue, Jan 11, 2022 at 9:43 AM Eran Kornblau
>> > > > <eran.kornblau at 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