From 84ecec197545109ba7974ebf1836ab539c903a09 Mon Sep 17 00:00:00 2001 From: Rob Walch Date: Wed, 14 Apr 2021 13:55:21 -0400 Subject: [PATCH] Fix switching to track after track has been loaded --- src/controller/audio-track-controller.ts | 10 +- .../unit/controller/audio-track-controller.ts | 140 +++++++++++++++--- 2 files changed, 122 insertions(+), 28 deletions(-) diff --git a/src/controller/audio-track-controller.ts b/src/controller/audio-track-controller.ts index c06f0d60827..a6690567ab5 100644 --- a/src/controller/audio-track-controller.ts +++ b/src/controller/audio-track-controller.ts @@ -171,11 +171,6 @@ class AudioTrackController extends BasePlaylistController { private setAudioTrack(newId: number): void { const tracks = this.tracksInGroup; - const track = tracks[newId]; - // noop on audio track already set - if (track?.details) { - return; - } // check if level idx is valid if (newId < 0 || newId >= tracks.length) { @@ -188,6 +183,7 @@ class AudioTrackController extends BasePlaylistController { const lastTrack = tracks[this.trackId]; this.log(`Now switching to audio-track index ${newId}`); + const track = tracks[newId]; const { id, groupId = '', name, type, url } = track; this.trackId = newId; this.trackName = name; @@ -199,6 +195,10 @@ class AudioTrackController extends BasePlaylistController { type, url, }); + // Do not reload track unless live + if (track.details && !track.details.live) { + return; + } const hlsUrlParameters = this.switchParams(track.url, lastTrack?.details); this.loadPlaylist(hlsUrlParameters); } diff --git a/tests/unit/controller/audio-track-controller.ts b/tests/unit/controller/audio-track-controller.ts index cda12af90e3..389c99a3d07 100644 --- a/tests/unit/controller/audio-track-controller.ts +++ b/tests/unit/controller/audio-track-controller.ts @@ -1,5 +1,7 @@ import AudioTrackController from '../../../src/controller/audio-track-controller'; -import Hls from '../../../src/hls'; +import Hls, { MediaPlaylist } from '../../../src/hls'; +import { AttrList } from '../../../src/utils/attr-list'; +import { LevelDetails } from '../../../src/loader/level-details'; import { Events } from '../../../src/events'; import * as sinon from 'sinon'; @@ -11,45 +13,83 @@ chai.use(sinonChai); const expect = chai.expect; describe('AudioTrackController', function () { - const tracks = [ + const tracks: MediaPlaylist[] = [ { + attrs: new AttrList({}), + bitrate: 0, + autoselect: false, + default: true, + forced: false, groupId: '1', id: 0, - default: true, name: 'A', + type: 'AUDIO', + url: '', }, { + attrs: new AttrList({}), + bitrate: 0, + autoselect: false, + default: false, + forced: false, groupId: '1', id: 1, - default: false, name: 'B', + type: 'AUDIO', + url: '', }, { + attrs: new AttrList({}), + bitrate: 0, + autoselect: false, + default: false, + forced: false, groupId: '1', id: 2, name: 'C', + type: 'AUDIO', + url: '', }, { + attrs: new AttrList({}), + bitrate: 0, + autoselect: false, + default: true, + forced: false, groupId: '2', id: 0, - default: true, name: 'A', + type: 'AUDIO', + url: '', }, { + attrs: new AttrList({}), + bitrate: 0, + autoselect: false, + default: false, + forced: false, groupId: '2', id: 1, - default: false, name: 'B', + type: 'AUDIO', + url: '', }, { + attrs: new AttrList({}), + bitrate: 0, + autoselect: false, + default: false, + forced: false, groupId: '2', id: 2, name: 'C', + type: 'AUDIO', + url: '', }, ]; - let hls; - let audioTrackController; + let hls; //: Hls; + let audioTrackController; //: AudioTrackController; const levels = [ { @@ -79,16 +119,9 @@ describe('AudioTrackController', function () { }); describe('onLevelLoading', function () { - it('should set the audioTracks contained in the event data and trigger AUDIO_TRACKS_UPDATED', function (done) { - hls.on(Hls.Events.AUDIO_TRACKS_UPDATED, (event, data) => { - expect(data.audioTracks).to.eql([ - { groupId: '2', id: 0, default: true, name: 'A' }, - { groupId: '2', id: 1, default: false, name: 'B' }, - { groupId: '2', id: 2, name: 'C' }, - ]); - expect(audioTrackController.tracks).to.equal(tracks); - done(); - }); + it('should set the audioTracks contained in the event data and trigger AUDIO_TRACKS_UPDATED', function () { + const audioTracksUpdatedCallback = sinon.spy(); + hls.on(Hls.Events.AUDIO_TRACKS_UPDATED, audioTracksUpdatedCallback); audioTrackController.onManifestParsed(Events.MANIFEST_PARSED, { audioTracks: tracks, @@ -96,6 +129,15 @@ describe('AudioTrackController', function () { audioTrackController.onLevelLoading(Events.LEVEL_LOADING, { level: 0, }); + + expect(audioTrackController.tracks).to.equal(tracks); + expect(audioTracksUpdatedCallback).to.be.calledOnce; + expect(audioTracksUpdatedCallback).to.be.calledWith( + Events.AUDIO_TRACKS_UPDATED, + { + audioTracks: tracks.slice(3, 6), + } + ); }); }); @@ -117,24 +159,76 @@ describe('AudioTrackController', function () { // current track name const audioTrackName = tracks[audioTrackController.audioTrack].name; - audioTrackController.onManifestParsed({ + audioTrackController.onManifestParsed(Events.MANIFEST_PARSED, { audioTracks: tracks, }); // group has switched - expect(audioTrackController.audioGroupId).to.equal(newGroupId); + expect(audioTrackController.groupId).to.equal(newGroupId); // name is still the same expect(tracks[audioTrackController.audioTrack].name).to.equal( audioTrackName ); }); + it('should always switch tracks when audioTrack is set to a valid index', function () { + const audioTracksUpdatedCallback = sinon.spy(); + const audioTrackSwitchingCallback = sinon.spy(); + hls.on(Hls.Events.AUDIO_TRACKS_UPDATED, audioTracksUpdatedCallback); + hls.on(Hls.Events.AUDIO_TRACK_SWITCHING, audioTrackSwitchingCallback); + + audioTrackController.onManifestParsed(Events.MANIFEST_PARSED, { + audioTracks: tracks, + }); + audioTrackController.onLevelLoading(Events.LEVEL_LOADING, { + level: 0, + }); + expect(audioTracksUpdatedCallback, 'AUDIO_TRACKS_UPDATED').to.have.been + .calledOnce; + expect( + audioTrackSwitchingCallback, + 'AUDIO_TRACK_SWITCHING to initial track 0' + ).to.have.been.calledOnce; + + audioTrackController.onAudioTrackLoaded(Events.AUDIO_TRACK_LOADED, { + details: new LevelDetails(''), + id: 0, + groupId: '1', + networkDetails: null, + stats: { loading: {} }, + deliveryDirectives: null, + }); + expect(audioTrackController.tracksInGroup[0], 'tracksInGroup[0]') + .to.have.property('details') + .which.is.an('object'); + + audioTrackController.audioTrack = 1; + expect(audioTrackSwitchingCallback, 'AUDIO_TRACK_SWITCHING to track 1').to + .have.been.calledTwice; + + audioTrackController.onAudioTrackLoaded(Events.AUDIO_TRACK_LOADED, { + details: new LevelDetails(''), + id: 1, + groupId: '1', + networkDetails: null, + stats: { loading: {} }, + deliveryDirectives: null, + }); + expect(audioTrackController.tracksInGroup[1], 'tracksInGroup[1]') + .to.have.property('details') + .which.is.an('object'); + + audioTrackController.audioTrack = 0; + expect(audioTrackSwitchingCallback, 'AUDIO_TRACK_SWITCHING back to track 0') + .to.have.been.calledThrice; + }); + describe('shouldLoadTrack', function () { it('should not need loading because the audioTrack is embedded in the main playlist', function () { audioTrackController.canLoad = true; expect(audioTrackController.shouldLoadTrack({ details: { live: true } })) .to.be.false; - expect(audioTrackController.shouldLoadTrack({ details: null })).to.be + expect(audioTrackController.shouldLoadTrack({ details: undefined })).to.be .false; }); @@ -186,7 +280,7 @@ describe('AudioTrackController', function () { ); // group has switched - expect(audioTrackController.audioGroupId).to.equal(newGroupId); + expect(audioTrackController.groupId).to.equal(newGroupId); // name is still the same expect(tracks[audioTrackController.audioTrack].name).to.equal( audioTrackName @@ -223,7 +317,7 @@ describe('AudioTrackController', function () { audioTrackController.onLevelLoading(Events.LEVEL_LOADING, { level: 0, }); - audioTrackController.startLoad(0); + audioTrackController.startLoad(); expect(shouldLoadTrack).to.have.been.calledTwice; expect(shouldLoadTrack).to.have.been.calledWith(trackWithUrl);