Skip to content

Commit

Permalink
Merge pull request #3777 from video-dev/bugfix/track-switch-after-loaded
Browse files Browse the repository at this point in the history 
Fix switching to track after track has been loaded
  • Loading branch information
@robwalch
robwalch committed Apr 14, 2021
2 parents fe97657 + 84ecec1 commit c9149f9
Show file tree
Hide file tree
Showing 2 changed files with 122 additions and 28 deletions.
10 changes: 5 additions & 5 deletions src/controller/audio-track-controller.ts
View file
Expand Up @@ -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) {
Expand All @@ -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;
Expand All @@ -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);
}
Expand Down
140 changes: 117 additions & 23 deletions tests/unit/controller/audio-track-controller.ts
View file
@@ -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';
Expand All @@ -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 = [
{
Expand Down Expand Up @@ -79,23 +119,25 @@ 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,
});
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),
}
);
});
});

Expand All @@ -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;
});

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit c9149f9

Please sign in to comment.