Skip to content

Commit

Permalink
Fix: Retain quality setting when setting/switching audio tracks (#720)
Browse files Browse the repository at this point in the history
  • Loading branch information
Jeremy Press authored Mar 29, 2018
1 parent f3b83d6 commit adc84e1
Show file tree
Hide file tree
Showing 2 changed files with 134 additions and 58 deletions.
64 changes: 44 additions & 20 deletions src/lib/viewers/media/DashViewer.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ class DashViewer extends VideoBaseViewer {
this.loadeddataHandler = this.loadeddataHandler.bind(this);
this.adaptationHandler = this.adaptationHandler.bind(this);
this.shakaErrorHandler = this.shakaErrorHandler.bind(this);
this.shakaManifestLoadedHandler = this.shakaManifestLoadedHandler.bind(this);
this.requestFilter = this.requestFilter.bind(this);
this.handleQuality = this.handleQuality.bind(this);
this.handleSubtitle = this.handleSubtitle.bind(this);
Expand Down Expand Up @@ -152,10 +153,11 @@ class DashViewer extends VideoBaseViewer {
this.adapting = true;
this.player = new shaka.Player(this.mediaEl);
this.player.addEventListener('adaptation', this.adaptationHandler);
this.player.addEventListener('streaming', this.shakaManifestLoadedHandler);
this.player.addEventListener('error', this.shakaErrorHandler);
this.player.configure({
abr: {
enabled: true
enabled: false
},
streaming: {
bufferingGoal: MAX_BUFFER,
Expand Down Expand Up @@ -239,6 +241,25 @@ class DashViewer extends VideoBaseViewer {
}
}

/**
* Given a an audio ID (e.g. english track audio ID), enables the track with that audio ID
* while maintaining the SAME VIDEO as the active track.
*
* @private
* @param {number} role - The role of the audio used in the variant (provided by Shaka)
* @return {void}
*/
enableAudioId(role) {
const tracks = this.player.getVariantTracks();
const activeTrack = this.getActiveTrack();
// We select a track that has the desired audio role but maintains the same video ID as our currently active track.
const newTrack = tracks.find((track) => track.roles[0] === role && track.videoId === activeTrack.videoId);
if (newTrack && newTrack.audioId !== activeTrack.audioId) {
this.showLoadingIcon(newTrack.id);
this.player.selectVariantTrack(newTrack, true);
}
}

/**
* Enables or disables automatic adaptation
*
Expand Down Expand Up @@ -280,9 +301,9 @@ class DashViewer extends VideoBaseViewer {
*/
handleAudioTrack() {
const audioIdx = parseInt(this.cache.get('media-audiotracks'), 10);
if (this.audioTracks[audioIdx] !== undefined) {
const track = this.audioTracks[audioIdx];
this.player.selectAudioLanguage(track.language, track.role);
const newAudioTrack = this.audioTracks[audioIdx];
if (newAudioTrack !== undefined) {
this.enableAudioId(newAudioTrack.role);
}
}

Expand Down Expand Up @@ -387,6 +408,25 @@ class DashViewer extends VideoBaseViewer {
}
}

/**
* Handles streaming event which is the first time the manifest is available. See https://shaka-player-demo.appspot.com/docs/api/shaka.Player.html#event:StreamingEvent
*
* @private
* @param {Object} shakaError - Error to handle
* @return {void}
*/
shakaManifestLoadedHandler() {
this.calculateVideoDimensions();
this.loadUI();

if (this.hdVideoId !== -1) {
this.mediaControls.enableHDSettings();
}

this.loadSubtitles();
this.loadAlternateAudio();
}

/**
* Adds event listeners to the media controls.
* Makes changes to the media element.
Expand Down Expand Up @@ -463,15 +503,10 @@ class DashViewer extends VideoBaseViewer {
this.autoplay();
}

this.calculateVideoDimensions();
this.loadUI();
this.loadFilmStrip();
this.resize();
this.handleVolume();
this.startBandwidthTracking();
this.handleQuality(); // should come after gettings rep ids
this.loadSubtitles();
this.loadAlternateAudio();
this.showPlayButton();

this.loaded = true;
Expand All @@ -483,17 +518,6 @@ class DashViewer extends VideoBaseViewer {
this.mediaContainerEl.focus();
}

/**
* @inheritdoc
*/
loadUI() {
super.loadUI();

if (this.hdVideoId !== -1) {
this.mediaControls.enableHDSettings();
}
}

/**
* Loads the film strip
*
Expand Down
128 changes: 90 additions & 38 deletions src/lib/viewers/media/__tests__/DashViewer-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ describe('lib/viewers/media/DashViewer', () => {

dash.mediaControls = {
addListener: () => {},
enableHDSettings: () => {},
destroy: () => {},
initFilmstrip: () => {},
initSubtitles: () => {},
Expand Down Expand Up @@ -215,6 +216,7 @@ describe('lib/viewers/media/DashViewer', () => {
dash.mediaUrl = 'url';
sandbox.stub(shaka, 'Player').returns(dash.player);
stubs.mockPlayer.expects('addEventListener').withArgs('adaptation', sinon.match.func);
stubs.mockPlayer.expects('addEventListener').withArgs('streaming', sinon.match.func);
stubs.mockPlayer.expects('addEventListener').withArgs('error', sinon.match.func);
stubs.mockPlayer.expects('configure');
stubs.mockPlayer
Expand Down Expand Up @@ -362,6 +364,53 @@ describe('lib/viewers/media/DashViewer', () => {
});
});

describe('enableAudioId()', () => {
it('should enable audioId while maintaining the same video ID', () => {
const variant1 = { id: 1, videoId: 1, audioId: 5, active: false, roles: ['1'] };
const variant2 = { id: 2, videoId: 2, audioId: 5, active: false, roles: ['1'] };
const variant3 = { id: 3, videoId: 1, audioId: 6, active: false, roles: ['2'] };
const variant4 = { id: 4, videoId: 2, audioId: 6, active: true, roles: ['2'] };
const variant5 = { id: 5, videoId: 1, audioId: 7, active: false, roles: ['3'] };
const variant6 = { id: 6, videoId: 2, audioId: 7, active: false, roles: ['3'] };
stubs.mockPlayer
.expects('getVariantTracks')
.returns([variant1, variant2, variant3, variant4, variant5, variant6]);
sandbox.stub(dash, 'getActiveTrack').returns(variant4);
sandbox.stub(dash, 'showLoadingIcon');
stubs.mockPlayer.expects('selectVariantTrack').withArgs(variant6, true);

dash.enableAudioId('3');

expect(dash.showLoadingIcon).to.be.calledWith(6);
});

it('should do nothing if enabling a audioId which is already active', () => {
const variant1 = { id: 1, videoId: 1, audioId: 5, active: false, roles: ['1'] };
const variant2 = { id: 2, videoId: 2, audioId: 6, active: true, roles: ['2'] };
stubs.mockPlayer.expects('getVariantTracks').returns([variant1, variant2]);
sandbox.stub(dash, 'getActiveTrack').returns(variant2);
sandbox.stub(dash, 'showLoadingIcon');
stubs.mockPlayer.expects('selectVariantTrack').never();

dash.enableAudioId('2');

expect(dash.showLoadingIcon).to.not.be.called;
});

it('should do nothing if enabling an invalid audioId', () => {
const variant1 = { id: 1, videoId: 1, audioId: 5, active: false, roles: ['1'] };
const variant2 = { id: 2, videoId: 2, audioId: 6, active: true, roles: ['2'] };
stubs.mockPlayer.expects('getVariantTracks').returns([variant1, variant2]);
sandbox.stub(dash, 'getActiveTrack').returns(variant2);
sandbox.stub(dash, 'showLoadingIcon');
stubs.mockPlayer.expects('selectVariantTrack').never();

dash.enableAudioId(-1);

expect(dash.showLoadingIcon).to.not.be.called;
});
});

describe('enableAdaptation()', () => {
it('should configure player to enable adaptation by default', () => {
stubs.mockPlayer.expects('configure').withArgs({ abr: { enabled: true } });
Expand Down Expand Up @@ -540,6 +589,40 @@ describe('lib/viewers/media/DashViewer', () => {
});
});

describe('shakaManifestLoadedHandler()', () => {
beforeEach(() => {
sandbox.stub(dash, 'calculateVideoDimensions');
sandbox.stub(dash, 'loadUI');
sandbox.stub(dash, 'loadSubtitles');
sandbox.stub(dash, 'loadAlternateAudio');
});

it('should calculate video dimensions and load UI', () => {
dash.shakaManifestLoadedHandler();

expect(dash.calculateVideoDimensions).to.be.called;
expect(dash.loadUI).to.be.called;
});

it('should enable HD settings if there is an HD rep', () => {
dash.hdVideoId = -1;
stubs.mockControls.expects('enableHDSettings').never();
dash.shakaManifestLoadedHandler();

dash.hdVideoId = 1;
stubs.mockControls.expects('enableHDSettings');
dash.shakaManifestLoadedHandler();

});

it('should load subtitles and additional audio tracks', () => {
dash.shakaManifestLoadedHandler();

expect(dash.loadSubtitles).to.be.called;
expect(dash.loadAlternateAudio).to.be.called;
});
});

describe('addEventListenersForMediaControls()', () => {
const listenerFunc = DashViewer.prototype.addEventListenersForMediaControls;

Expand Down Expand Up @@ -573,60 +656,24 @@ describe('lib/viewers/media/DashViewer', () => {
sandbox.stub(dash, 'showMedia');
sandbox.stub(dash, 'isAutoplayEnabled').returns(true);
sandbox.stub(dash, 'autoplay');
sandbox.stub(dash, 'calculateVideoDimensions');
sandbox.stub(dash, 'loadUI');
sandbox.stub(dash, 'loadFilmStrip');
sandbox.stub(dash, 'loadAlternateAudio');
sandbox.stub(dash, 'resize');
sandbox.stub(dash, 'handleVolume');
sandbox.stub(dash, 'startBandwidthTracking');
sandbox.stub(dash, 'handleQuality');
sandbox.stub(dash, 'loadSubtitles');
sandbox.stub(dash, 'showPlayButton');

dash.loadeddataHandler();
expect(dash.autoplay).to.be.called;
expect(dash.showMedia).to.be.called;
expect(dash.showPlayButton).to.be.called;
expect(dash.loadSubtitles).to.be.called;
expect(dash.loadAlternateAudio).to.be.called;
expect(dash.emit).to.be.calledWith(VIEWER_EVENT.load);
expect(dash.loaded).to.be.true;
expect(document.activeElement).to.equal(dash.mediaContainerEl);
expect(dash.mediaControls.show).to.be.called;
});
});

describe('loadUI()', () => {
beforeEach(() => {
stubs.loadUI = DashViewer.prototype.loadUI;
dash.mediaControls = {
enableHDSettings: sandbox.stub(),
removeListener: sandbox.stub(),
removeAllListeners: sandbox.stub(),
destroy: sandbox.stub()
};

Object.defineProperty(VideoBaseViewer.prototype, 'loadUI', { value: sandbox.mock() });
});

afterEach(() => {
Object.defineProperty(VideoBaseViewer.prototype, 'loadUI', { value: stubs.loadUI });
});

it('should enable HD settings if an HD rep exists', () => {
dash.hdVideoId = 3;
dash.loadUI();
expect(dash.mediaControls.enableHDSettings).to.be.called;
});

it('should do nothing if there is no HD rep', () => {
dash.hdVideoId = -1;
dash.loadUI();
expect(dash.mediaControls.enableHDSettings).to.not.be.called;
});
});

describe('loadFilmStrip()', () => {
beforeEach(() => {
dash.options = {
Expand Down Expand Up @@ -858,16 +905,20 @@ describe('lib/viewers/media/DashViewer', () => {
});

describe('handleAudioTrack()', () => {
beforeEach(() => {
sandbox.stub(dash, 'enableAudioId');
});

it('should select correct audio', () => {
dash.audioTracks = [
{ language: 'eng', role: 'audio0' },
{ language: 'eng', role: 'audio1' },
{ language: 'eng', role: 'audio2' }
];
sandbox.stub(dash.cache, 'get').returns('1');
stubs.mockPlayer.expects('selectAudioLanguage').withArgs('eng', 'audio1');

dash.handleAudioTrack();
expect(dash.enableAudioId).to.be.calledWith('audio1')
});

it('should not select audio if index out of bounds', () => {
Expand All @@ -877,9 +928,10 @@ describe('lib/viewers/media/DashViewer', () => {
{ language: 'eng', role: 'audio2' }
];
sandbox.stub(dash.cache, 'get').returns('3');
stubs.mockPlayer.expects('selectAudioLanguage').never();

dash.handleAudioTrack();
expect(dash.enableAudioId).to.not.be.called;

});
});

Expand Down

0 comments on commit adc84e1

Please sign in to comment.