shaka-project / shaka-player

JavaScript player library / DASH & HLS client / MSE-EME player
Apache License 2.0
7.07k stars 1.33k forks source link

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

Closed avelad closed 5 years ago

avelad commented 6 years ago

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 commented 6 years ago

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.

avelad commented 6 years ago

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

ismena commented 6 years ago

Thanks, let us know how it goes!

avelad commented 6 years ago

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

TheModMaker commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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

avelad commented 6 years ago

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

joeyparrish commented 6 years ago

Done. Thanks for catching that!

waxidiotic commented 5 years ago

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 commented 5 years ago

@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 commented 5 years ago

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 commented 5 years ago

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

joeyparrish commented 5 years ago

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!