If manifest returns 301/302, build the segment list relative to the redirect

[rgc]

If manifest returns 301/302, build the segment list relative to the redirect. Update code to make requests relative to the URL returned by the 301/302.

Most HTML5 players used to behave badly with 301/302, since the XMLHttpRequest object did not, until recently, expose the final manifest location. HTML5 based players can now utilize the responseUrl attribute to fix this.

The segment list should be built off of the redirected manifest.

Broken Player Flow
http://some.host.org/path/manifest.m3u8 -> 302 -> http://other.host.org/path/manifest.m3u8
http://some.host.org/path/chunk1.ts (302)
http://some.host.org/path/chunk2.ts (302)

If the server has been configured to 302 redirect for everything, this will result in each of the above chunks also having a 302 redirect. If the server hasn't been configured, then they will fail.

Correct Player Flow
http://some.host.org/path/manifest.m3u8 -> 302 -> http://other.host.org/path/manifest.m3u8
http://other.host.org/path/chunk1.ts (200)
http://other.host.org/path/chunk2.ts (200)
(manifest update)
http://some.host.org/path/manifest.m3u8 -> 302 -> http://another.host.org/path/manifest.m3u8
http://another.host.org/path/chunk1.ts (200)
http://another.host.org/path/chunk2.ts (200)

• We have resolved this issue on Shaka player (Add support for 302 redirect shaka-project/shaka-player#266)
• It has also been resolved on google chrome cast (https://code.google.com/p/google-cast-sdk/issues/detail?id=746)
• It has been resolved upstream in Dash.js

[arendjr]

I'd love to use a patch like this. Unfortunately, it seems you're the third person to attempt to write a patch like this (previous ones: #646 and #647) and both the previous ones got rejected on the grounds it's not supported in IE.

So I'd like to take this opportunity to address the maintainers to please give this patch fair consideration for inclusion. If 3 people independently develop a patch for this (I would've been the fourth, given that the older two don't work against master anymore, had I not checked again to see if there are any more solutions since yesterday when I also investigated this) it seems there's a real demand for this.

Also, the argument it doesn't work in IE is irrelevant at least to us because IE11 seems to have issues with the HLS plugin anyway (documented here: https://github.com/videojs/videojs-contrib-hls#ie11), which is why we don't use this plugin for IE to begin with.

So I hope this patch gets tidied up (I doubt the failing tests are the result of this patch, but I guess at least another comment in the README to warn people trying to use this functionality with IE would be warranted) and gets approved, so we can prevent some duplication of work for the future. Thanks!

[rgc]

arendjr,

I apologize for not checking the pull request history -- thank you for the considerate and informative reply.

I understand the desire to stay consistent across platforms, but would argue that it's more important to adhere to the RFC and allow the platform developers to do the same.

When dealing with a URI, we should attempt to conform to RFC3986, which states in section 5.1.3 that "if the retrieval was the result of a redirected request, the last URI used (i.e., the URI that resulted in the actual retrieval of the representation) is the base URI".

HTML5 players have been slow to implement this since XMLHttpRequest has historically hidden the fact that a redirect has occurred. Only recently the XMLHttpRequest spec 4.6.1 has added the responseUrl attribute.

I appreciate the consideration and time of the maintainers, please let me know what other steps are required to tidy up this patch.

Thank you!

[arendjr]

Btw, in order for us to get this patch to work, we also needed the following change:

diff --git a/src/playlist-loader.js b/src/playlist-loader.js
index d1fd48f..98ddd32 100644
--- a/src/playlist-loader.js
+++ b/src/playlist-loader.js
@@ -478,7 +483,9 @@ const PlaylistLoader = function(srcUrl, hls, withCredentials) {

       loader.state = 'HAVE_MASTER';

-      parser.manifest.uri = srcUrl;
+      if (parser.manifest.uri != req.responseURL) {
+        parser.manifest.uri = req.responseURL;
+      }

       // loaded a master playlist
       if (parser.manifest.playlists) {

This change originally came from PR #646.

[arendjr]

Furthermore, I'd suggest adding this to the README:

diff --git a/README.md b/README.md
index fa3030b..4d17c63 100644
--- a/README.md
+++ b/README.md
@@ -441,6 +441,10 @@ SourceBuffers functionality. This leads to various issues, such as
 videos stopping playback with media decode errors. The known workaround
 for this issues is to force the player to use flash when running on IE11.

+Also it is known that IE cannot load videos if a redirect is performed
+when loading the playlist URL. The workaround for this is to pass the
+destination URL directly to the player.
+
 ### Fragmented MP4 Support
 Edge has native support for HLS but only in the MPEG2-TS container. If
 you attempt to play an HLS stream with fragmented MP4 segments, Edge

[rgc]

Arend,

I thought that perhaps adding options to enable this would be a good
option. By default, it would operate as it does now, but we can make
"follow redirect" and "sticky redirects" options.

On Mon, Nov 21, 2016 at 8:25 AM, Arend van Beelen jr. <
notifications@github.com> wrote:

Furthermore, I'd suggest adding this to the README:

diff --git a/README.md b/README.md
index fa3030b..4d17c63 100644
--- a/README.md
+++ b/README.md
@@ -441,6 +441,10 @@ SourceBuffers functionality. This leads to various issues, such as
videos stopping playback with media decode errors. The known workaround
for this issues is to force the player to use flash when running on IE11.

+Also it is known that IE cannot load videos if a redirect is performed
+when loading the playlist URL. The workaround for this is to pass the
+destination URL directly to the player.
+

Fragmented MP4 Support

Edge has native support for HLS but only in the MPEG2-TS container. If
you attempt to play an HLS stream with fragmented MP4 segments, Edge

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#912 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AACh2fWiPMGuHATXr5_95SQ4iHgbnSVOks5rAZvQgaJpZM4Kyp-B
.

[arendjr]

Sure, that sounds fine to me.

[rgc]

OK - will do.

I'm working with a specific use-case in mind, so I don't want to assume too
much about the general use of this plugin.

Thanks,

Rob

On Tue, Nov 22, 2016 at 10:12 AM, Arend van Beelen jr. <
notifications@github.com> wrote:

Sure, that sounds fine to me.

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#912 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AACh2bMixT-cKjf7lv3baR3b9zlG94gaks5rAwZUgaJpZM4Kyp-B
.

[dmlap]

@arendjr @rgc you make compelling arguments. I closed the other PRs on platform-consistency grounds but three independent contributions is a pretty strong indication I was probably wrong. Here's my question: is there any way we can warn people in IE that their video isn't working because of redirects? Speaking selfishly as one of the folks who reviews issues, I'd like to make this problem easy to diagnose.

[arendjr]

That's a good question. Best I can think of is to detect whether the responseURL property is supported (from what I read it's as easy as checking whether it's undefined) and use that in case loading of the playlist works, but loading of the chunklist fails with a 404 or similar and print a log in such case to check whether it's because of a redirect.

…

On Wed, 23 Nov 2016 at 21:53 David LaPalomento ***@***.***> wrote: @arendjr <https://github.com/arendjr> @rgc <https://github.com/rgc> you make compelling arguments. I closed the other PRs on platform-consistency grounds but three independent contributions is a pretty strong indication I was probably wrong. Here's my question: is there any way we can warn people in IE that their video isn't working because of redirects? Speaking selfishly as one of the folks who reviews issues, I'd like to make this problem easy to diagnose. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#912 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAgjLmV1pv8zxLYorA7etC9VtF2WgPK3ks5rBKfhgaJpZM4Kyp-B> .

[dmlap]

@arendjr that sounds like a pretty good idea. Would you or @rgc be interested on modifying this PR to do that?

[rgc]

Yes - I can do that... I've been a bit busy this week, but should have time later this week to add that and the other options I was speaking about. Also, I'll try to get a property spun up for testing. Thanks for the feedback and suggestions, Rob

…

On Wed, Dec 7, 2016 at 5:27 PM, David LaPalomento ***@***.***> wrote: @arendjr <https://github.com/arendjr> that sounds like a pretty good idea. Would you or @rgc <https://github.com/rgc> be interested on modifying this PR to do that? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#912 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AACh2ZXDNk_COz4MzLaiJZtt4MBfhnCQks5rFzLngaJpZM4Kyp-B> .

[gesinger]

Sorry this PR has gone on for a while without much response.

It seems that enough people want this feature that it would be good to add in, however, at least to start, it would be good behind a feature flag (particularly since the IE11 flash recommendation has been removed).

Would it be possible to add a couple tests as well (if it is still a problem, I know the PR has been here a while).

Thank you.

[gesinger]

Might be good to add a comment here that we check for the existence of responseURL to prevent breaking IE or other browsers that don't support it.

[forbesjo]

Looks like this has been handled by #1213, closing