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

Accessing/Manipulating multiple player sources #2443

Closed
chemoish opened this issue Aug 6, 2015 · 6 comments
Closed

Accessing/Manipulating multiple player sources #2443

chemoish opened this issue Aug 6, 2015 · 6 comments

Comments

@chemoish
Copy link
Member

chemoish commented Aug 6, 2015

Has there been any thought about being able to access any/all sources provided to the player?

https://github.com/videojs/video.js/blob/master/docs/api/vjs.Player.md#src-source-

myPlayer.src([
  { type: "video/mp4", src: "http://www.example.com/path/to/video.mp4" },
  { type: "video/webm", src: "http://www.example.com/path/to/video.webm" },
  { type: "video/ogg", src: "http://www.example.com/path/to/video.ogv" }
]);

I am guessing for the typical case it doesn't matter which it plays, as long as it plays.

The use case might be...

myPlayer.src([
  { type: "application/dash+xml", src: "480.mpd" },
  { type: "application/dash+xml", src: "720.mpd" },
  { type: "application/dash+xml", src: "1080.mpd" }
]);

This may be more easily achieved if your utilizing source tags, but for src's with larger configuration/protection data. I don't believe you have the luxury.


The end goal might be to create a resolution changer.

The closest thing I found, but didn't really work was vjs('player').player().options().sources // []


Similarly, with playroll (videojs-contrib-ads), you cannot hot swap sources with protected data, because there is no way of obtaining sources that are dynamically attached.

i think

@heff
Copy link
Member

heff commented Aug 6, 2015

If you're just looking for a file-based resolution changer, I think there's still one in the plugins listing. I'm not sure exactly how it pulls this off, but that type of functionality makes the most sense as a plugin. However we could consider adding some way of accessing the original sources provided. The options() path would only work if you used source elements to provide the multiple sources, instead of the src() api.

@chemoish
Copy link
Member Author

chemoish commented Aug 7, 2015

There is 2 one that is new and one that isn't supported anymore.

https://github.com/vidcaster/video-js-resolutions/blob/master/video-js-resolutions.js#L424 (old)
https://github.com/dominic-p/videojs-resolution-selector/blob/master/video-quality-selector.js#L185 (new)

Both players rely on options()—which still has the issue of interacting with dynamic sources (same with pre/mid/postrolls—pretty much any src swap).

It would be great if sources can have the same/similar interaction API, whether or not they are inline or not.

It seems like tracks are already handled for this case (albeit kind of weird):

videojs('player').player().options().tracks

// vs.

videojs('player').textTracks

Thinking of solutions…

Additionally, I noticed that:

https://github.com/videojs/video.js/blob/master/src/js/player.js#L1675

src() // returns this.techGet('src');

https://github.com/videojs/video.js/blob/master/src/js/player.js#L1780

currentSrc() // returns this.techGet('src') || this.cache_.src || '';

Not sure if these were intended to be doing similar, but different operations.

My first thought would be to modify src to be both a set and a get, but that would cause breaking changes.

https://github.com/videojs/video.js/blob/master/src/js/player.js#L1738

And sourceList() seems to already be taken…


Potentially, this has already been thought of as its @todo https://github.com/videojs/video.js/blob/master/src/js/player.js#L2372.

I am willing to make the jump to 5.0 rc if I can leverage this feature.

And I am willing to contribute to the core, but I am not very familiar with the tools or architectural direction yet.

@chemoish
Copy link
Member Author

What about managing this in two parts?

this.sourceList_(source);

if (vjs.obj.isArray(source)) {
    this.cache_.sourceList = source; // save raw source list [The only time there would be more than 1]

    this.sourceList_(source);
    ...

this.cache_.src = source.src;

this.cache_.src = source.src;
this.cache_.source = source; // save raw source
this.currentType_ = source.type || '';

this.cache_.src = source.src;

this.cache_.src = source.src;
this.cache_.source = source; // save raw source

rawSourceList: function () {
    return this.cache_.sourceList.concat(this.player_.options_.sources); 
}

rawSource: function () {
    return this.cache_.source;
}

It is still possible that Tech can set the source (this.techCall('src')) and the this.cache_.source will not be accurate, but in the cases which the normal src flows into src or sourceList_—it should be fine?

I am not sure if this is how this.cache_ was intended to be used. Nor, am I sure that modifying the this.cache_ to include sourceList, before Tech.canPlaySource is executed is the correct approach either.

@heff Thoughts?

@heff
Copy link
Member

heff commented Aug 12, 2015

Overall, I think this could be a valuable addition. @videojs/core-committers any other opinions?

src()
currentSrc()
Not sure if these were intended to be doing similar, but different operations.

currentSrc tells you what the url of the currently playing source. src only tells you the value that was set using the the src attribute on on the video tag or the src property of the video element. If you provide a list of sources using source tags inside the video element, src will be empty. With a video.js player it doesn't work exactly like that but we try to mimic the video element as closely as possible.

It is still possible that Tech can set the source (this.techCall('src')) and the this.cache_.source will not be accurate

We heavily discourage talking to the tech direction, or using techCall, because of reasons like this. So I don't think that should be an issue.

@heff Thoughts?

I like the idea of adding:

player.currentSource()
player.currentSources()

Sources can be set in a few different ways so I think if we can simplify accessing that information it could be helpful. I think using a current[x] naming and standardizing the data that's returned would be more valuable than just exposing the raw data that passed in. That makes this a little more complicated.

For example, when you set a source with the src() function, all we have is the url.

player.src('http://foo.com/video.mp4');

player.currentSrc(); // --> 'http://foo.com/video.mp4'
player.currentSource(); // --> { src: 'http://foo.com/video.mp4' }
player.currentSources(); // --> [ { src: 'http://foo.com/video.mp4' } ]

The last two add some complexity but I think it's worth it to make the return values consistent.

And I think you can use the cache_ object to store the various values.

@chemoish
Copy link
Member Author

It feels weird not to have a correctly configured src, but that may be fine since the tech handles that already.

This still aligns with the end goal of being able to use player.currentSource() or player.currentSources() to reset/update the player.src() when necessary across drm/bitrate updates (rather than assume type and src is the end all be all).

And…

// yes have the tech worry about generating the additional details—no talk
player.src('http://foo.com/video.mp4');

Ideally we would want player.currentSource() and player.currentSources() to consistently return the configuration of:

  • <source>
  • src({…})

I'll have to double check, but I am not sure at what point does the player have access to values other than raw data.

Definitely agree and am in favor of consistency. I will look into achieving those @returns tomorrow.

Though still open to other alternatives.

@heff
Copy link
Member

heff commented Aug 13, 2015

Great, it sounds like we're thinking in the same direction.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants