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

Upgrade: Upgrade Shaka-player to fix infinite 401 issue #164

Merged
merged 2 commits into from
Jun 6, 2017
Merged

Upgrade: Upgrade Shaka-player to fix infinite 401 issue #164

merged 2 commits into from
Jun 6, 2017

Conversation

bhh1988
Copy link
Contributor

@bhh1988 bhh1988 commented Jun 6, 2017

This is effectively is an upgrade to 2.1.2 with a fix cherry-picked on
top: shaka-project/shaka-player#842. This fixes an
issue where 401s would get infinitely retried.

@@ -129,6 +129,7 @@ const MANIFEST = 'manifest.mpd';
this.adapting = true;
this.player = new shaka.Player(this.mediaEl);
this.player.addEventListener('adaptation', this.adaptationHandler);
this.player.addEventListener('error', this.shakaErrorHandler);
Copy link
Contributor

@tonyjin tonyjin Jun 6, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the future, this.shakaErrorHandler should be self-bound in the constructor. It'll look like this:

class DashViewer extends VideoBaseViewer {
    constructor() {
        super();
    
        // Bind event handlers
        this.shakaErrorHandler = this.shakaErrorHandler.bind(this);
        this.adaptationHandler = this.adaptationHandler.bind(this);
    }

This way, we can remove autobind. This pattern follows #4 described here: https://medium.freecodecamp.com/react-binding-patterns-5-approaches-for-handling-this-92c651b5af56. We can't use #5 due to issues with arrow class properties and inheritance (React doesn't use inheritance, but composition). #1 isn't relevant. #2 has some performance implications and is also easy to forget to add the .bind. #3 has the same issues as #2 and also can't be unbound.

`Shaka error. Code = ${shakaError.detail.code}, Category = ${shakaError.detail.category}, Severity = ${shakaError.detail.severity}`
);
error.displayMessage = __('error_refresh');
if (shakaError.detail.severity > 1) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: I'd add a line break before this.

Bryan Huh and others added 2 commits June 6, 2017 15:05
This is effectively is an upgrade to 2.1.2 with a fix cherry-picked on
top: shaka-project/shaka-player#842. This fixes an
issue where 401s would get infinitely retried. In addition, the viewer
will bind to error-events thrown from shaka-player and propagate it so
that the preview fails faster on error
@bhh1988 bhh1988 merged commit 5646a79 into box:master Jun 6, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants