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

Audio Support #1540

Closed
wants to merge 3 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@mmcc
Member

mmcc commented Sep 29, 2014

Just some basic changes to make VJS work a little better out of the box for audio support.

@@ -24,7 +24,9 @@ vjs.PosterImage = vjs.Button.extend({
this.src(player.poster());
}));
player.on('play', vjs.bind(this, this.hide));
if (!player.isAudio()) {

This comment has been minimized.

@gkatsev

gkatsev Sep 29, 2014

Member

keep the poster showing if not audio, right?

@gkatsev

gkatsev Sep 29, 2014

Member

keep the poster showing if not audio, right?

This comment has been minimized.

@mmcc

mmcc Sep 29, 2014

Member

No, keep the poster showing if it is audio. The thought being you could put album art or something and it would be more interesting than a black frame.

@mmcc

mmcc Sep 29, 2014

Member

No, keep the poster showing if it is audio. The thought being you could put album art or something and it would be more interesting than a black frame.

This comment has been minimized.

@gkatsev

gkatsev Sep 29, 2014

Member

er, yeah, that's what I meant. Idk where that not camre from.

@gkatsev

gkatsev Sep 29, 2014

Member

er, yeah, that's what I meant. Idk where that not camre from.

Show outdated Hide outdated src/js/player.js
@@ -80,6 +80,11 @@ vjs.Player = vjs.Component.extend({
this.addClass('vjs-controls-disabled');
}
// Disable inactivity timeout if it's an audio source.
if (this.isAudio()) {
this.options()['inactivityTimeout'] = 0;

This comment has been minimized.

@gkatsev

gkatsev Sep 29, 2014

Member

if you then give this player a video, the inactivity timeout would be disabled still, no?

@gkatsev

gkatsev Sep 29, 2014

Member

if you then give this player a video, the inactivity timeout would be disabled still, no?

This comment has been minimized.

@mmcc

mmcc Sep 29, 2014

Member

Yes, if it's a video element it (should) behave exactly as it does now. This is related to the poster thing above, where I think it makes sense to leave the controls up for audio.

@mmcc

mmcc Sep 29, 2014

Member

Yes, if it's a video element it (should) behave exactly as it does now. This is related to the poster thing above, where I think it makes sense to leave the controls up for audio.

This comment has been minimized.

@gkatsev

gkatsev Sep 29, 2014

Member

Hm... maybe I'm over thinking things.

Are audio and video vjs players separate? I.e., can you have something like

var player = videojs(document.createElement('audio'));
player.src({src: 'http://example.com/audio.mp3', type: 'audio/mp3'}); // or whatever the mimetype is

//now switch to a video
player.src({ src: 'http://example.com/video.mp4', type: 'video/mp4'});

Do we want to allow that switch? If we allow that switch, the inactivity timeout will be disabled for the video-player it was disabled for the audio here.

@gkatsev

gkatsev Sep 29, 2014

Member

Hm... maybe I'm over thinking things.

Are audio and video vjs players separate? I.e., can you have something like

var player = videojs(document.createElement('audio'));
player.src({src: 'http://example.com/audio.mp3', type: 'audio/mp3'}); // or whatever the mimetype is

//now switch to a video
player.src({ src: 'http://example.com/video.mp4', type: 'video/mp4'});

Do we want to allow that switch? If we allow that switch, the inactivity timeout will be disabled for the video-player it was disabled for the audio here.

This comment has been minimized.

@mmcc

mmcc Sep 29, 2014

Member

That's a good point. I think we definitely want to allow the switch, so we should probably come up with a different way to handle the inactivityTimeout.

@mmcc

mmcc Sep 29, 2014

Member

That's a good point. I think we definitely want to allow the switch, so we should probably come up with a different way to handle the inactivityTimeout.

This comment has been minimized.

@heff

heff Sep 29, 2014

Member

I think we do want to enable that switch. I think the most consistent way to make this work would be through CSS classes. Something like vjs-audio, and then something like .vjs-audio.vjs-user-inactive .vjs-control-bar { visibility: visible; }. That way we're not changing what user activity is, we're changing how components react to it in different player states.

@heff

heff Sep 29, 2014

Member

I think we do want to enable that switch. I think the most consistent way to make this work would be through CSS classes. Something like vjs-audio, and then something like .vjs-audio.vjs-user-inactive .vjs-control-bar { visibility: visible; }. That way we're not changing what user activity is, we're changing how components react to it in different player states.

@heff heff closed this in 18b6d25 Oct 1, 2014

@mmcc mmcc referenced this pull request Oct 1, 2014

Closed

Support Audio #537

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment