Re: [FFmpeg-devel] [PATCH] http: make caching of redirect url optional

Re: [FFmpeg-devel] [PATCH] http: make caching of redirect url optional

[Eran Kornblau]

> 
> -----Original Message-----
> From: Eran Kornblau 
> Sent: Thursday, 9 December 2021 22:24
> To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> Subject: RE: [FFmpeg-devel] [PATCH] http: make caching of redirect url optional
> 
> > 
> > -----Original Message-----
> > From: Eran Kornblau 
> > Sent: Thursday, 2 December 2021 9:16
> > To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> > Subject: RE: [FFmpeg-devel] [PATCH] http: make caching of redirect url optional
> > 
> > Another ping... resending with the original patch
> > 
> > Eran 
> > 
> > -----Original Message-----
> > From: Eran Kornblau 
> > Sent: Thursday, 25 November 2021 8:55
> > To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> > Subject: RE: [FFmpeg-devel] [PATCH] http: make caching of redirect url optional
> > 
> > > > > 
> > > > > On Tue, 2 Nov 2021, Eran Kornblau wrote:
> > > > > 
> > > > > > Hi all,
> > > > > >
> > > > > > The attached patch makes the default behavior of caching HTTP redirects optional.
> > > > > 
> > > > > Is caching a redirected URL allowed per the HTTP spec? If not, then no caching should take place, or the caching should be optional, and not the default.
> > > > > 
> > > > From a quick search on this, it seems that browsers by default cache 301 redirects, while 302/307 are not cached by default.
> > > > However, in both cases, the browser is expected to honor any Cache-Control/Expires headers.
> > > > 
> > > > Changing the implementation to imitate the behavior of a browser would be more complex, since currently there is only a single "slot" in the cache.
> > > > For example, if we have a chain of redirects 307->301->200, and we want to cache the 301 but not the 307, we can't really do it.
> > > > Solving this one would require a more complex data structure, and if we add expiration/TTL to the mix, it becomes even more complex...
> > > > 
> > > > Assuming we don't want to add a large chunk of code to handle this, the options I see are - 1. A simple boolean option - as proposed in this patch. We can argue about what the default should be :) but it felt safer to me to have the default behavior as it was before.
> > > > 2. Cache only 301's directly linked to the original URL - e.g. if we have 301->301->307->301->200, we cache only the first 2 redirects.
> > > > My concern here is that this would break/slow down applications that rely on the existing behavior of caching any redirect.
> > > > 
> > > 
> > > Hi,
> > > 
> > > Looking for some guidance how to proceed with this...
> > > 
> > > On one side, we have the minimal alternative of adding an option to the http module (as in the patch I submitted), on the other side we have the alternative of fully imitating the behavior of a browser - respect cache-control/expires, and if these headers are missing, default according to status code (301/302/307).
> > > We can also go with something in between - e.g., we can cache 301's that are returned directly from the original url, and avoid caching any 302/307, as well as 301's that come after a 302/307.
> > > 
> > 
> > Ping... 
> > 
> > Any objection to merge the patch that adds an option to disable the caching of redirect?
> > 
> > Thanks!
> > 
> > Eran
> > 
> > > Thank you
> > > 
> > > Eran
> > > 
> > > > Any thoughts/suggestions are welcome,
> > > > 
> > > > Thank you,
> > > > 
> > > > Eran
> > > > 
> > > > > Thanks,
> > > > > Marton
> > > > >
> > > >
> > >
> > 
> Trying again... we're stuck on this one...
> 
> To recap - I want to somehow prevent ffmpeg from caching HTTP redirects. 
> The patch I submitted implements it using a new option, but I'm open to other suggestions - doing it based on HTTP status code etc.
> Just need some guidance on which direction to take here, so that some solution for this could be merged. 
> 
> Thank you,
> 
> Eran
> 
> 
Anyone?

Eran

_______________________________________________
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] [PATCH] http: make caching of redirect url optional

[Ronald S. Bultje]

Hi,

(I was asked to respond since I'm listed as HTTP maintainer, not sure I
should be since I'm mostly working on video codecs nowadays.)

On Tue, Nov 2, 2021 at 9:00 AM Eran Kornblau <eran.kornblau@kaltura.com>
wrote:

> The motivation for this feature is S3 signatures – currently we have a
> problem where S3 signatures cannot be created
> with an expiration of more than 12H. In some cases, a transcoding task may
> execute for more than that.
> If we use a pre-signed S3 URL, and ffmpeg disconnects/seeks after the
> expiration of the URL, it will fail.
>
> The solution we are planning is to have some local server on the machine
> running ffmpeg that will generate an S3-signature,
> and redirect to the full pre-signed URL. For this to work, I need to
> disable the caching of redirects, and have ffmpeg always
> start from the initial URL.
> The nice thing about this solution is that the video data is pulled
> directly from S3 – in other words, the local server doesn’t
> hold any real load, it just builds the signature and returns a redirect.
>

Uhm... This is a really weird solution, but it does look right.

Generally speaking, we're typically concerned about the default being the
right behaviour. I would say that (maybe after some time, at the next ABI
break or so), 0 should be the default, not 1. This is the same as what
Marton/Hendrik said also, I think, so consider this consensus. I would just
do that with the appropriate ABI macros so the default behaviour changes at
the next bump.

Patch itself is fine, it has some whitespace change at line 430 and 621
that is not really necessary, but otherwise no objections from me.

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] [PATCH] http: make caching of redirect url optional

[Eran Kornblau]

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

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Ronald S. Bultje
> Sent: Sunday, 26 December 2021 16:07
> To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH] http: make caching of redirect url optional
> 
> Hi,
> 
> (I was asked to respond since I'm listed as HTTP maintainer, not sure I should be since I'm mostly working on video codecs nowadays.)
> 
> On Tue, Nov 2, 2021 at 9:00 AM Eran Kornblau <eran.kornblau@kaltura.com>
> wrote:
> 
> > The motivation for this feature is S3 signatures – currently we have a 
> > problem where S3 signatures cannot be created with an expiration of 
> > more than 12H. In some cases, a transcoding task may execute for more 
> > than that.
> > If we use a pre-signed S3 URL, and ffmpeg disconnects/seeks after the 
> > expiration of the URL, it will fail.
> >
> > The solution we are planning is to have some local server on the 
> > machine running ffmpeg that will generate an S3-signature, and 
> > redirect to the full pre-signed URL. For this to work, I need to 
> > disable the caching of redirects, and have ffmpeg always start from 
> > the initial URL.
> > The nice thing about this solution is that the video data is pulled 
> > directly from S3 – in other words, the local server doesn’t hold any 
> > real load, it just builds the signature and returns a redirect.
> >
> 
> Uhm... This is a really weird solution, but it does look right.
> 
> Generally speaking, we're typically concerned about the default being the right behaviour. I would say that (maybe after some time, at the next ABI break or so), 0 should be the default, not 1. This is the same as what Marton/Hendrik said also, I think, so consider this consensus. I would just do that with the appropriate ABI macros so the default behaviour changes at the next bump.
> 
Thank you, Ronald!

I attached a new patch with the change you requested, I hope I understood your intention correctly...

Also -
1. Rebased it against latest master - there were some changes in http since I submitted...
2. Fixed a leak that I created in the original patch - I tried to match the 'location' field when adding 'uri', but missed the fact that 'location' is freed automatically, being an option.

> Patch itself is fine, it has some whitespace change at line 430 and 621 that is not really necessary, but otherwise no objections from me.
> 
These 2 whitespaces were added intentionally - to have one block that handles 'location', and another block that handles the new 'uri' field.

> Ronald
> _______________________________________________
> 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%7C310f5d2e188648e50d0308d9c8790674%7C0c503748de3f4e2597e26819d53a42b6%7C1%7C1%7C637761244390439784%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=5YhHSjTVN5v7BQdMgkZ2tXUA5tZGbv8Yb1MN8x%2Ffsd4%3D&amp;reserved=0
> 
> To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>

Thanks,

Eran


[-- Attachment #2: 0001-http-make-caching-of-redirect-url-optional.patch --]
[-- Type: application/octet-stream, Size: 4176 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] [PATCH] http: make caching of redirect url optional

[Ronald S. Bultje]

Hi,

On Mon, Dec 27, 2021 at 4:38 AM Eran Kornblau <eran.kornblau@kaltura.com>
wrote:

> > -----Original Message-----
> > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> Ronald S. Bultje
> > Sent: Sunday, 26 December 2021 16:07
> > To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> > Subject: Re: [FFmpeg-devel] [PATCH] http: make caching of redirect url
> optional
> >
> > Hi,
> >
> > (I was asked to respond since I'm listed as HTTP maintainer, not sure I
> should be since I'm mostly working on video codecs nowadays.)
> >
> > On Tue, Nov 2, 2021 at 9:00 AM Eran Kornblau <eran.kornblau@kaltura.com>
> > wrote:
> >
> > > The motivation for this feature is S3 signatures – currently we have a
> > > problem where S3 signatures cannot be created with an expiration of
> > > more than 12H. In some cases, a transcoding task may execute for more
> > > than that.
> > > If we use a pre-signed S3 URL, and ffmpeg disconnects/seeks after the
> > > expiration of the URL, it will fail.
> > >
> > > The solution we are planning is to have some local server on the
> > > machine running ffmpeg that will generate an S3-signature, and
> > > redirect to the full pre-signed URL. For this to work, I need to
> > > disable the caching of redirects, and have ffmpeg always start from
> > > the initial URL.
> > > The nice thing about this solution is that the video data is pulled
> > > directly from S3 – in other words, the local server doesn’t hold any
> > > real load, it just builds the signature and returns a redirect.
> > >
> >
> > Uhm... This is a really weird solution, but it does look right.
> >
> > Generally speaking, we're typically concerned about the default being
> the right behaviour. I would say that (maybe after some time, at the next
> ABI break or so), 0 should be the default, not 1. This is the same as what
> Marton/Hendrik said also, I think, so consider this consensus. I would just
> do that with the appropriate ABI macros so the default behaviour changes at
> the next bump.
> >
> Thank you, Ronald!
>
> I attached a new patch with the change you requested, I hope I understood
> your intention correctly...
>

Yes, LGTM. I'll give it a few days to let Marton/Hendrik respond before I
push.

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] [PATCH] http: make caching of redirect url optional

[Ronald S. Bultje]

Hi,

On Mon, Dec 27, 2021 at 2:08 PM Ronald S. Bultje <rsbultje@gmail.com> wrote:

> Hi,
>
> On Mon, Dec 27, 2021 at 4:38 AM Eran Kornblau <eran.kornblau@kaltura.com>
> wrote:
>
>> > -----Original Message-----
>> > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
>> Ronald S. Bultje
>> > Sent: Sunday, 26 December 2021 16:07
>> > To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org
>> >
>> > Subject: Re: [FFmpeg-devel] [PATCH] http: make caching of redirect url
>> optional
>> >
>> > Hi,
>> >
>> > (I was asked to respond since I'm listed as HTTP maintainer, not sure I
>> should be since I'm mostly working on video codecs nowadays.)
>> >
>> > On Tue, Nov 2, 2021 at 9:00 AM Eran Kornblau <eran.kornblau@kaltura.com
>> >
>> > wrote:
>> >
>> > > The motivation for this feature is S3 signatures – currently we have
>> a
>> > > problem where S3 signatures cannot be created with an expiration of
>> > > more than 12H. In some cases, a transcoding task may execute for more
>> > > than that.
>> > > If we use a pre-signed S3 URL, and ffmpeg disconnects/seeks after the
>> > > expiration of the URL, it will fail.
>> > >
>> > > The solution we are planning is to have some local server on the
>> > > machine running ffmpeg that will generate an S3-signature, and
>> > > redirect to the full pre-signed URL. For this to work, I need to
>> > > disable the caching of redirects, and have ffmpeg always start from
>> > > the initial URL.
>> > > The nice thing about this solution is that the video data is pulled
>> > > directly from S3 – in other words, the local server doesn’t hold any
>> > > real load, it just builds the signature and returns a redirect.
>> > >
>> >
>> > Uhm... This is a really weird solution, but it does look right.
>> >
>> > Generally speaking, we're typically concerned about the default being
>> the right behaviour. I would say that (maybe after some time, at the next
>> ABI break or so), 0 should be the default, not 1. This is the same as what
>> Marton/Hendrik said also, I think, so consider this consensus. I would just
>> do that with the appropriate ABI macros so the default behaviour changes at
>> the next bump.
>> >
>> Thank you, Ronald!
>>
>> I attached a new patch with the change you requested, I hope I understood
>> your intention correctly...
>>
>
> Yes, LGTM. I'll give it a few days to let Marton/Hendrik respond before I
> push.
>

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".