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

HTTP redirects should be "sticky" for manifest updates #1367

Closed
avelad opened this issue Mar 20, 2018 · 15 comments · Fixed by #1880
Closed

HTTP redirects should be "sticky" for manifest updates #1367

avelad opened this issue Mar 20, 2018 · 15 comments · Fixed by #1880
Assignees
Labels
flag: good first issue This might be a relatively easy issue; good for new contributors status: archived Archived and locked; will not be updated type: enhancement New feature or request
Milestone

Comments

@avelad
Copy link
Member

avelad commented Mar 20, 2018

Have you read the FAQ and checked for duplicate open issues?: Yes

What version of Shaka Player are you using?: 2.3.3

Can you reproduce the issue with our latest release version?: Yes

Can you reproduce the issue with the latest code from master?: Yes

Are you using the demo app or your own custom app?: Both

If custom app, can you reproduce the issue using our demo app?: Yes

What browser and OS are you using?: Chrome 65 - Ubuntu 17.10

What are the manifest and license server URIs?:
(NOTE: you can send the URIs to shaka-player-issues@google.com instead, but please use GitHub and the template for the rest)
(NOTE: a copy of the manifest text or an attached manifest will not be enough to reproduce your issue, and we will ask you to send a URI instead)

What did you do?
Load a Live MPD with redirect

What did you expect to happen?
All segments and refresh playlist should use the location included in the first redirect
Eg:
any.mpd (balancer url) --> HTTP 302 --> edge url
1.mp4 --> edge url
2.mp2 --> edge url
any.mpd --> edge url
3.mp4 --> edge url
....

What actually happened?
All segments are using the location included in the redirect, when there a new playlist refresh go to the initial domain and then redirect to new location, and the next segments using this new redirect, and so on.

Eg:
any.mpd (balancer url) --> HTTP 302 --> edge url
1.mp4 --> edge url
2.mp2 --> edge url
any.mpd (balancer url) --> HTTP 302 --> edge url (2)
3.mp4 --> edge url (2)
4.mp4 --> edge url (2)
any.mpd (balancer url) --> HTTP 302 --> edge url (3)
....

@TheModMaker
Copy link
Contributor

The problem here seems to be that the manifest update goes to the original URL rather than the redirected one. (I think) The DASH spec doesn't say anything about how to handle redirects, so there is no guidance on the correct way to handle this case. Usually, a redirect is transparent to JavaScript; the browser will handle the redirect and won't even tell us the response wasn't from the original URL. On some browsers we can use the responseURL property to detect a redirect, but it isn't supported on all browsers. Since redirects are usually transparent, and 302 is a temporary redirect, I would think we should query the original URL.

However, there is a correct way to do this in DASH. There is a <Location> element that does just what you want. It is a child if <MPD> and the body is a full URL where we should send all future manifest requests to. It also acts as the base URL for segments. So all you need to do is have your balancer give us a different URL in the <Location> for the first request and all future requests will be made to that URL.

@TheModMaker TheModMaker added the type: question A question from the community label Mar 20, 2018
@ismena ismena added the status: waiting on response Waiting on a response from the reporter(s) of the issue label Mar 21, 2018
@avelad
Copy link
Member Author

avelad commented Mar 23, 2018

I have transmitted the message to the provider, I am waiting for what they tell me.

@shaka-bot shaka-bot removed the status: waiting on response Waiting on a response from the reporter(s) of the issue label Mar 23, 2018
@ismena
Copy link
Contributor

ismena commented Mar 23, 2018

Thanks, let us know how it goes!

@ismena ismena added the status: waiting on response Waiting on a response from the reporter(s) of the issue label Mar 23, 2018
@avelad
Copy link
Member Author

avelad commented Mar 26, 2018

Are there another type of redirection (301?, 307?, ...) that address our need?

@shaka-bot shaka-bot removed the status: waiting on response Waiting on a response from the reporter(s) of the issue label Mar 26, 2018
@TheModMaker
Copy link
Contributor

No, all types of redirects are the same from our perspective. In fact, when using XMLHttpRequest, we can't even determine the type of redirect that occurred; all we can determine is whether a redirect occurred at all. Our logic is to always query the original manifest URL for updates, unless there is a <Location> element.

@joeyparrish
Copy link
Member

We assumed that an HTTP redirect would be for load-balancing purposes, and should probably go back to the load-balancer for a manifest update. Segments URLs, though, are relative to the redirected URL (when available from the platform).

Were we wrong about that? Should redirects for manifests be "sticky"?

@avelad
Copy link
Member Author

avelad commented Mar 27, 2018

Yes, HTTP redirect is for load-balancing (regional) and minor latency.

I see the behaviour in DASH-IF, Theoplayer y ShakaPlayer
dashif
shaka-player
theoplayer

DASH-IF and Theoplayer have the expected behavior. And only ShakaPlayer go to load-balancer in the manifest update.

So, should redirects for manifests be "sticky"? Yes

@joeyparrish
Copy link
Member

Okay, that's reasonable. I will rename the issue and classify as an enhancement for v2.5. Thanks!

@joeyparrish joeyparrish added type: enhancement New feature or request and removed type: question A question from the community labels Mar 27, 2018
@joeyparrish joeyparrish changed the title Issue with the redirection in Live streams HTTP redirects should be "sticky" for manifest updates Mar 27, 2018
@shaka-bot shaka-bot added this to the Backlog milestone Mar 27, 2018
@avelad
Copy link
Member Author

avelad commented Mar 28, 2018

I see that the milestone is backlog. Can you add it to 2.5?

@joeyparrish joeyparrish modified the milestones: Backlog, v2.5 Mar 28, 2018
@joeyparrish
Copy link
Member

Done. Thanks for catching that!

@joeyparrish joeyparrish added the flag: good first issue This might be a relatively easy issue; good for new contributors label Jul 9, 2018
@waxidiotic
Copy link

Hey @joeyparrish - there hasn't been an update on this since March so I just want to make sure this still looks like it will make it in to 2.5.0. We have a customer that has a similar issue, but not exactly. They're pointing the player to a php file which does a 302 redirect to the MPD. Each time Shaka requests the master manifest, it requests the php again which adds time to each request.

This looks like this enhancement to make the redirects "sticky" would relieve this as well, right?

@waxidiotic
Copy link

@joeyparrish My apologies - the customer is apparently not using PHP redirects but just normal 302's. The person who reported it to me was just using PHP to simulate it.

Either way, is there an ETA on 2.5.0?

@TobbeEdgeware
Copy link
Contributor

We are also using MPD redirect as our main way of handling sessions and it is working in all other clients, so we'd like this to be supported in 2.5.0. Using the <Location> element in the MPD is a fallback, but requires manifest rewrite so it is heavier.

@waxidiotic
Copy link

@joeyparrish - Just checking in to see if you still think this is on track for 2.5.0?

@joeyparrish
Copy link
Member

We just received a pull request for this, so I think it's safe to say it will make it into v2.5.0. Thanks, @avelad, for contributing!

@shaka-project shaka-project locked and limited conversation to collaborators Jun 14, 2019
@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: good first issue This might be a relatively easy issue; good for new contributors status: archived Archived and locked; will not be updated type: enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants