Skip to content

Commit

Permalink
fix(subtitles): Fix duplication of subtitles, subtitles not being rem…
Browse files Browse the repository at this point in the history
…oved when subtitle track is disabled and resample set to default.
  • Loading branch information
brys0 committed Nov 23, 2024
1 parent ecad69d commit 4e0e2a5
Show file tree
Hide file tree
Showing 4 changed files with 55 additions and 39 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -87,14 +87,17 @@
</VRow>
<VRow align="center">
<VCol :cols="4">
<label>{{ $t('preciseSubtitle') }}</label>
<label for="usePreciseControls">
{{ $t('preciseSubtitle') }}
</label>
</VCol>
<VCol
:cols="8"
class="text-right">
<VSwitch
v-model="playerElement.usePreciseSubtitles.value"
color="primary"
id="usePreciseControls"
hide-details />
</VCol>
</VRow>
Expand Down
4 changes: 3 additions & 1 deletion frontend/src/components/Buttons/SubtitleSelectionButton.vue
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
:close-on-content-click="false"
transition="slide-y-transition"
location="top">
<VList class="uno-overflow-hidden">
<VList class="uno-overflow-hidden" :disabled="loading">
<VListItem
v-for="track of tracks"
:key="track.srcIndex"
Expand Down Expand Up @@ -59,4 +59,6 @@ const tracks = computed(() => {
...(subs ?? [])
];
});
const loading = computed(() => playbackManager.subtitleLoading as boolean);
</script>
11 changes: 10 additions & 1 deletion frontend/src/store/playback-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ interface PlaybackManagerState {
playbackInitiator: BaseItemDto | undefined;
playbackInitMode: InitMode;
playbackSpeed: number;
subtitleLoading: boolean;
}

/**
Expand Down Expand Up @@ -296,6 +297,13 @@ class PlaybackManagerStore extends CommonStore<PlaybackManagerState> {
}
}

public get subtitleLoading(): boolean {
return this._state.subtitleLoading;
}

public set subtitleLoading(loading: boolean) {
this._state.subtitleLoading = loading;
}
/**
* Filters the external subtitle tracks
*/
Expand Down Expand Up @@ -1029,7 +1037,8 @@ class PlaybackManagerStore extends CommonStore<PlaybackManagerState> {
playSessionId: undefined,
playbackInitiator: undefined,
playbackInitMode: InitMode.Unknown,
playbackSpeed: 1
playbackSpeed: 1,
subtitleLoading: false,
}));
/**
* Logic is divided by concerns and scope. Watchers for callbacks
Expand Down
74 changes: 38 additions & 36 deletions frontend/src/store/player-element.ts
Original file line number Diff line number Diff line change
Expand Up @@ -125,28 +125,31 @@ class PlayerElementStore extends CommonStore<PlayerElementState> {
}
};

private _fetchSSATrack = async (trackSrc: string) => {
fetch(trackSrc).then((body) => body.text()).then((text) => {
this._asssub = new ASSSUB(text, mediaElementRef.value as (HTMLVideoElement), { resampling: 'video_width' });
});
private readonly _fetchSSATrack = async (trackSrc: string) => {
const ax = remote.sdk.api?.axiosInstance
if (!ax) return

/**
* Before doing anything let's free the current assjs renderer (if any)
* and make sure the user cannot keep switching subtitles when one is already trying to load.
*/
this._freeSSATrack();
playbackManager.subtitleLoading = true;

/**
* Let's fetch the subtitle track from the server, we assume the data returned will always be a string.
*/
const subtitleTrackText = await (await ax.get(trackSrc)).data as string;
this._asssub = new ASSSUB(subtitleTrackText, mediaElementRef.value as (HTMLVideoElement), { /** TODO: Make this a player setting/config option */ resampling: 'video_height' });
playbackManager.subtitleLoading = false;
};

private readonly _setSsaTrack = (trackSrc: string): void => {
if (
!this._asssub
&& mediaElementRef.value
&& mediaElementRef.value instanceof HTMLVideoElement
) {

// AssJS require sub contents in advance, we can not just give it a URL
this._fetchSSATrack(trackSrc);
} else {
// Destroy ASS subtitle player if it already exists
this._asssub?.destroy();
if (!mediaElementRef.value || !(mediaElementRef.value instanceof HTMLVideoElement)) return
if (this._asssub) this._freeSSATrack()

this._fetchSSATrack(trackSrc);
}
}
this._fetchSSATrack(trackSrc);
};

private readonly _setPgsTrack = (trackSrc: string): void => {
if (
Expand All @@ -171,6 +174,13 @@ class PlayerElementStore extends CommonStore<PlayerElementState> {
this._pgssub = undefined;
};

private readonly _freeSSATrack = (): void => {
// Function was called but the assjs class was already destroyed
if (!this._asssub) return

this._asssub.destroy();
this._asssub = undefined;
};
/**
* Applies PGS subtitles to the media element.
*/
Expand Down Expand Up @@ -213,36 +223,26 @@ class PlayerElementStore extends CommonStore<PlayerElementState> {
* Applies SSA (SubStation Alpha) subtitles to the media element.
*/
private readonly _applySsaSubtitles = async (): Promise<void> => {
if (
mediaElementRef.value
&& this.currentExternalSubtitleTrack
) {
const subtitleTrack = this.currentExternalSubtitleTrack;
if (!this.currentExternalSubtitleTrack || !mediaElementRef) return;

const subtitleTrack = this.currentExternalSubtitleTrack;

/**
* Check if client is able to display custom subtitle track
* otherwise use ASSUB to render subtitles
*/
let applyASSUB = !this._useCustomSubtitleTrack;

* Check if client is able to display custom subtitle track
*/
if (this._useCustomSubtitleTrack) {
const data = await genericWorker.parseSsaFile(subtitleTrack.src);

/**
* If style isn't basic (animations, custom typographics, etc.)
* fallback to rendering subtitles with ASSUB
* Check if worker returned that the sub data is 'basic', when true use basic renderer method
*/
if (data?.isBasic) {
this.currentExternalSubtitleTrack.parsed = data;
} else {
applyASSUB = true;
return;
}
}

if (applyASSUB) {
this._setSsaTrack(subtitleTrack.src);
}
}
this._setSsaTrack(subtitleTrack.src);
};

/**
Expand All @@ -266,6 +266,7 @@ class PlayerElementStore extends CommonStore<PlayerElementState> {
}
}

this._freeSSATrack();
this._freePgsTrack();
this.currentExternalSubtitleTrack = undefined;

Expand Down Expand Up @@ -329,6 +330,7 @@ class PlayerElementStore extends CommonStore<PlayerElementState> {
*/
watch(videoContainerRef, () => {
if (!videoContainerRef.value) {
this._freeSSATrack();
this._freePgsTrack();
}
}, { flush: 'sync' });
Expand Down

0 comments on commit 4e0e2a5

Please sign in to comment.