Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cache busting parameter added to fragment request #76

Closed
somerobmitchell opened this issue May 6, 2015 · 7 comments
Closed

Cache busting parameter added to fragment request #76

somerobmitchell opened this issue May 6, 2015 · 7 comments
Labels
flag: seeking PR We are actively seeking PRs for this; we do not currently expect the core team will resolve this status: archived Archived and locked; will not be updated type: question A question from the community

Comments

@somerobmitchell
Copy link

Hi All,

A cache buster is added to every dash fragment requested by the player.
This can been seen here.

The reason given for this is that cached responses can ruin the bandwidth estimation but it has caused another issue with our dash stream.

We are using Unified Streaming to stream a dash manifest that is created on the fly.
This means that fragment generation (especially with encrypted streams) can take >1s. To overcome this we use the Level3 CDN to cache the fragments, this reduces the load on the origin server and makes the fragments available quicker. The cache-buster parameter meant that Level3 was treating every fragment request as a new request.

I can ask Level3 to ignore any query strings and just look at the path but wanted to understand the reasoning behind adding the cache buster param.

@kkarablinkbox
Copy link

Hi,

Just wanted to say that we've done exactly the same thing, asked Level3 to ignore the query string params and it has worked for us. We've tried other approaches but none really worked in an easy way. I'll let Joey respond to the reasoning, if there's any other than what you've already mentioned around bandwidth estimation.

@somerobmitchell
Copy link
Author

I am leaning towards removing the code. We are using the dash.js player as well and have not seen any problems with not having the cache buster.

@tdrews
Copy link
Contributor

tdrews commented May 6, 2015

Hi,

Thanks for your input on this. We added the cache-busting feature to simplify our bandwidth estimation code, as without it we found it difficult to get reliable estimates using our weighted average approach; however, as we now support custom bandwidth estimators and plan on supporting custom bit-rate managers (see #48) this is something that we will revisit for <= v1.4.0.

@tdrews tdrews added this to the v1.4.0 milestone May 13, 2015
@joeyparrish
Copy link
Member

The cache-busting parameter has been a pain point for others, as well. If there were a way to reliably detect when segments are being served from the local cache, we could do away with cache-busting and simply discard throughput measurements for cached responses. If anyone has ideas on this point, I would be very happy to hear them.

@joeyparrish joeyparrish added the flag: seeking PR We are actively seeking PRs for this; we do not currently expect the core team will resolve this label May 22, 2015
@joeyparrish
Copy link
Member

Oh, and to add context, if you never seek, cached segments will never trouble you, even without cache-busting params.

The trouble is when you have very little bandwidth and have downloaded a bunch of 360p segments, then you seek backward and suddenly you appear to have enough bandwidth for 1080p and the player adapts up. Since you don't actually have the bandwidth for that, the player adapts back down, possibly after a stall. Then you get cache hits again for your 360p segments, then you adapt back up, etc.

@joeyparrish joeyparrish removed this from the v1.4.0 milestone May 22, 2015
joeyparrish pushed a commit that referenced this issue Jun 26, 2015
Adds an API to IBandwithEstimator to enable BandwithEstimator
implementations to allow/disallow caching.

Issue #76

Change-Id: I842994981a0821e22de412478f094850414b6289
@joeyparrish joeyparrish changed the title Cache busting parameter to added to fragment request Cache busting parameter added to fragment request Jul 1, 2015
@joeyparrish joeyparrish added the type: question A question from the community label Jul 1, 2015
@joeyparrish
Copy link
Member

v1.4.0 is out now, including a new method on IBandwidthEstimator so that a custom implementation can disable cache-busting. Please let us know whether or not the new APIs resolve this issue for you.

@tdrews
Copy link
Contributor

tdrews commented Aug 3, 2015

Please feel free to re-open this issue if you have any issues with IBandwidthEstimator.supportsCaching().

@tdrews tdrews closed this as completed Aug 3, 2015
joeyparrish added a commit that referenced this issue Dec 14, 2015
This does not mean that Shaka v1 is suddenly cache-friendly, but this
gives users the option to weigh the problems caused by caching in
Shaka v1 against the problems caused by the cache-buster for certain
CDNs or authentication schemes.  Previously, this would have required
local changes.  Now it can be done in a standard build.

The parameter name is extra long and wordy to make sure that it cannot
be used without acknowledging that there are trade-offs involved.

Closes #235, #238, #76

Change-Id: I934a5703183d31250458b80cb38d2b5deae410df
@shaka-project shaka-project locked and limited conversation to collaborators Mar 22, 2018
@shaka-bot shaka-bot added the status: archived Archived and locked; will not be updated label Apr 15, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
flag: seeking PR We are actively seeking PRs for this; we do not currently expect the core team will resolve this status: archived Archived and locked; will not be updated type: question A question from the community
Projects
None yet
Development

No branches or pull requests

6 participants