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

Update MW to require a factory, add *-mw #3969

Merged
merged 7 commits into from
Jan 27, 2017
Merged

Update MW to require a factory, add *-mw #3969

merged 7 commits into from
Jan 27, 2017

Conversation

gkatsev
Copy link
Member

@gkatsev gkatsev commented Jan 23, 2017

Description

As I was playing around with middleware, in the first RC, I realized that the current way of videojs.use with an object was lacking. Specifically, if you have multiple players on the page, the middleware would need to be able to handle tech management itself, which doesn't seem right. Also, the mw didn't know about what player it was attached to.
Now, videojs.use takes in a function that given a player, returns a middleware object. The middleware object looks the same as the current object we use for registration. This allows us to have per-player instances of middleware and because it's a factory, a user can use a class or Component or Plugin object for managing their middleware if they want, as long as the methods are available.
For example:

videojs.use('video/foo', (player) => ({
  setSource(src, next) {
    if (player.playbackRate() > 1) {
      next(null, {
       src:  'foo',
       type: 'video/mp4'
      });
    } else {
      next(new Error('we don't support this));
    }
  }
}));

Also, this PR adds the ability to register "star middleware" (* mw), which will always be run through right before selecting the tech.

Copy link
Member

@misteroneill misteroneill left a comment

Choose a reason for hiding this comment

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

One comment/question.

export function setSource(setTimeout, src, next) {
setTimeout(() => setSourceHelper(src, middlewares[src.type], next), 1);
export function setSource(player, setTimeout, src, next) {
setTimeout(() => setSourceHelper(src, middlewares[src.type], next, player), 1);
Copy link
Member

Choose a reason for hiding this comment

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

Should this be using window.setTimeout?

Copy link
Member Author

Choose a reason for hiding this comment

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

actually, I just realized I don't need to pass in setTimeout here. But it's the setTimeout from player, so, that if player is disposed`, so will this timeout.

@gkatsev gkatsev merged commit 0352916 into master Jan 27, 2017
@gkatsev gkatsev deleted the mw-updates branch January 27, 2017 20:09
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