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

Use track name over track index, fixes unordered track group switching #3731

Merged
merged 1 commit into from Apr 8, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions api-extractor/report/hls.js.api.md
Expand Up @@ -84,6 +84,8 @@ export interface AudioTrackSwitchingData {
// (undocumented)
id: number;
// (undocumented)
name: string;
// (undocumented)
type: MediaPlaylistType | 'main';
// (undocumented)
url: string;
Expand Down Expand Up @@ -2008,6 +2010,8 @@ export interface SubtitleTrackSwitchData {
// (undocumented)
id: number;
// (undocumented)
name?: string;
// (undocumented)
type?: MediaPlaylistType | 'main';
// (undocumented)
url?: string;
Expand Down
3 changes: 3 additions & 0 deletions demo/chart/timeline-chart.ts
Expand Up @@ -666,6 +666,9 @@ export class TimelineChart {
}

function stripDeliveryDirectives(url: string): string {
if (url === '') {
return url;
}
try {
const webUrl: URL = new self.URL(url);
webUrl.searchParams.delete('_HLS_msn');
Expand Down
16 changes: 10 additions & 6 deletions src/controller/audio-track-controller.ts
Expand Up @@ -19,6 +19,7 @@ class AudioTrackController extends BasePlaylistController {
private groupId: string | null = null;
private tracksInGroup: MediaPlaylist[] = [];
private trackId: number = -1;
private trackName: string = '';
private selectDefaultTrack: boolean = true;

constructor(hls: Hls) {
Expand Down Expand Up @@ -58,6 +59,7 @@ class AudioTrackController extends BasePlaylistController {
this.groupId = null;
this.tracksInGroup = [];
this.trackId = -1;
this.trackName = '';
this.selectDefaultTrack = true;
}

Expand Down Expand Up @@ -169,8 +171,9 @@ class AudioTrackController extends BasePlaylistController {

private setAudioTrack(newId: number): void {
const tracks = this.tracksInGroup;
// noop on same audio track id as already set
if (this.trackId === newId && tracks[newId]?.details) {
const track = tracks[newId];
// noop on audio track already set
if (track?.details) {
return;
}

Expand All @@ -184,11 +187,12 @@ class AudioTrackController extends BasePlaylistController {
this.clearTimer();

const lastTrack = tracks[this.trackId];
const track = tracks[newId];
this.log(`Now switching to audio-track index ${newId}`);
const { id, name, type, url } = track;
this.trackId = newId;
const { url, type, id } = track;
this.hls.trigger(Events.AUDIO_TRACK_SWITCHING, { id, type, url });
this.trackName = name;
this.selectDefaultTrack = false;
this.hls.trigger(Events.AUDIO_TRACK_SWITCHING, { id, name, type, url });
const hlsUrlParameters = this.switchParams(track.url, lastTrack?.details);
this.loadPlaylist(hlsUrlParameters);
}
Expand All @@ -199,7 +203,7 @@ class AudioTrackController extends BasePlaylistController {
audioTracks.length,
'Initial audio track should be selected when tracks are known'
);
const currentAudioTrackName = audioTracks[this.trackId]?.name;
const currentAudioTrackName = this.trackName;
const trackId =
this.findTrackId(currentAudioTrackName) || this.findTrackId();

Expand Down
4 changes: 2 additions & 2 deletions src/controller/subtitle-track-controller.ts
Expand Up @@ -354,8 +354,8 @@ class SubtitleTrackController extends BasePlaylistController {
this.log(`Switching to subtitle track ${newId}`);
this.trackId = newId;
if (track) {
const { url, type, id } = track;
this.hls.trigger(Events.SUBTITLE_TRACK_SWITCH, { id, type, url });
const { id, name, type, url } = track;
this.hls.trigger(Events.SUBTITLE_TRACK_SWITCH, { id, name, type, url });
const hlsUrlParameters = this.switchParams(track.url, lastTrack?.details);
this.loadPlaylist(hlsUrlParameters);
} else {
Expand Down
10 changes: 6 additions & 4 deletions src/types/events.ts
Expand Up @@ -153,9 +153,10 @@ export interface LevelPTSUpdatedData {
}

export interface AudioTrackSwitchingData {
url: string;
type: MediaPlaylistType | 'main';
id: number;
name: string;
type: MediaPlaylistType | 'main';
url: string;
}

export interface AudioTrackSwitchedData {
Expand All @@ -173,9 +174,10 @@ export interface SubtitleTracksUpdatedData {
}

export interface SubtitleTrackSwitchData {
url?: string;
type?: MediaPlaylistType | 'main';
id: number;
name?: string;
type?: MediaPlaylistType | 'main';
url?: string;
}

export interface SubtitleTrackLoadedData extends TrackLoadedData {}
Expand Down
2 changes: 2 additions & 0 deletions tests/unit/controller/subtitle-track-controller.js
Expand Up @@ -129,6 +129,7 @@ describe('SubtitleTrackController', function () {
'hlsSubtitleTrackSwitch',
{
id: 1,
name: 'English',
type: 'SUBTITLES',
url: 'bar',
}
Expand Down Expand Up @@ -163,6 +164,7 @@ describe('SubtitleTrackController', function () {
'hlsSubtitleTrackSwitch',
{
id: 0,
name: 'English',
type: 'SUBTITLES',
url: 'baz',
}
Expand Down