-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Fix Crossfade randomly muting music until manual unmute #4307
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -188,7 +188,8 @@ export default createPlugin< | |||||||||
| onPlayerApiReady() { | ||||||||||
| let transitionAudio: Howl; // Howler audio used to fade out the current music | ||||||||||
| let firstVideo = true; | ||||||||||
| let waitForTransition: Promise<unknown>; | ||||||||||
| let waitForTransition: Promise<unknown> = Promise.resolve(); | ||||||||||
| let originalVolume: number = 1; | ||||||||||
|
|
||||||||||
| const getStreamURL = async (videoID: string): Promise<string> => | ||||||||||
| this.ipc?.invoke('audio-url', videoID) as Promise<string>; | ||||||||||
|
|
@@ -199,6 +200,13 @@ export default createPlugin< | |||||||||
| const isReadyToCrossfade = () => | ||||||||||
| transitionAudio && transitionAudio.state() === 'loaded'; | ||||||||||
|
|
||||||||||
| const ensureVideoVolume = () => { | ||||||||||
| const video = document.querySelector('video'); | ||||||||||
| if (video && video.volume === 0 && originalVolume > 0) { | ||||||||||
| video.volume = originalVolume; | ||||||||||
| } | ||||||||||
| }; | ||||||||||
|
Comment on lines
+203
to
+208
|
||||||||||
|
|
||||||||||
| const watchVideoIDChanges = (cb: (id: string) => void) => { | ||||||||||
| window.navigation.addEventListener('navigate', (event) => { | ||||||||||
| const currentVideoID = getVideoIDFromURL( | ||||||||||
|
|
@@ -216,6 +224,7 @@ export default createPlugin< | |||||||||
| cb(nextVideoID); | ||||||||||
| }); | ||||||||||
| } else { | ||||||||||
| ensureVideoVolume(); | ||||||||||
| cb(nextVideoID); | ||||||||||
| firstVideo = false; | ||||||||||
| } | ||||||||||
|
|
@@ -239,6 +248,10 @@ export default createPlugin< | |||||||||
| const syncVideoWithTransitionAudio = () => { | ||||||||||
| const video = document.querySelector('video')!; | ||||||||||
|
|
||||||||||
| if (video.volume > 0) { | ||||||||||
| originalVolume = video.volume; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| const videoFader = new VolumeFader(video, { | ||||||||||
| fadeScaling: this.config?.fadeScaling, | ||||||||||
| fadeDuration: this.config?.fadeInDuration, | ||||||||||
|
|
@@ -247,29 +260,40 @@ export default createPlugin< | |||||||||
| transitionAudio.play(); | ||||||||||
| transitionAudio.seek(video.currentTime); | ||||||||||
|
|
||||||||||
| video.addEventListener('seeking', () => { | ||||||||||
| const onSeeking = () => { | ||||||||||
| transitionAudio.seek(video.currentTime); | ||||||||||
| }); | ||||||||||
| }; | ||||||||||
|
|
||||||||||
| video.addEventListener('pause', () => { | ||||||||||
| const onPause = () => { | ||||||||||
| transitionAudio.pause(); | ||||||||||
| }); | ||||||||||
| }; | ||||||||||
|
|
||||||||||
| video.addEventListener('play', () => { | ||||||||||
| const onPlay = () => { | ||||||||||
| transitionAudio.play(); | ||||||||||
| transitionAudio.seek(video.currentTime); | ||||||||||
|
|
||||||||||
| // Fade in | ||||||||||
| const videoVolume = video.volume; | ||||||||||
| const videoVolume = originalVolume || video.volume || 1; | ||||||||||
| video.volume = 0; | ||||||||||
| videoFader.fadeTo(videoVolume); | ||||||||||
|
Comment on lines
275
to
278
|
||||||||||
| }); | ||||||||||
| }; | ||||||||||
|
|
||||||||||
| video.addEventListener('seeking', onSeeking); | ||||||||||
| video.addEventListener('pause', onPause); | ||||||||||
| video.addEventListener('play', onPlay); | ||||||||||
|
Comment on lines
+281
to
+283
|
||||||||||
|
|
||||||||||
| if (!video.paused) { | ||||||||||
| const videoVolume = originalVolume || video.volume || 1; | ||||||||||
| if (video.volume === 0) { | ||||||||||
| videoFader.fadeTo(videoVolume); | ||||||||||
| } | ||||||||||
| } | ||||||||||
|
|
||||||||||
| // Exit just before the end for the transition | ||||||||||
| const transitionBeforeEnd = () => { | ||||||||||
| if ( | ||||||||||
| video.currentTime >= | ||||||||||
| video.duration - (this.config?.secondsBeforeEnd ?? 0) && | ||||||||||
| video.duration - (this.config?.secondsBeforeEnd ?? 0) && | ||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🚫 [eslint] <prettier/prettier> reported by reviewdog 🐶
Suggested change
|
||||||||||
| isReadyToCrossfade() | ||||||||||
| ) { | ||||||||||
| video.removeEventListener('timeupdate', transitionBeforeEnd); | ||||||||||
|
|
@@ -284,6 +308,7 @@ export default createPlugin< | |||||||||
|
|
||||||||||
| const crossfade = (cb: () => void) => { | ||||||||||
| if (!isReadyToCrossfade()) { | ||||||||||
| ensureVideoVolume() | ||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🚫 [eslint] <prettier/prettier> reported by reviewdog 🐶
Suggested change
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🚫 [eslint] <stylistic/semi> reported by reviewdog 🐶
Suggested change
|
||||||||||
| cb(); | ||||||||||
|
Comment on lines
310
to
312
|
||||||||||
| return; | ||||||||||
| } | ||||||||||
|
|
@@ -295,8 +320,12 @@ export default createPlugin< | |||||||||
|
|
||||||||||
| const video = document.querySelector('video')!; | ||||||||||
|
|
||||||||||
| if (video.volume > 0) { | ||||||||||
| originalVolume = video.volume; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| const fader = new VolumeFader(transitionAudio._sounds[0]._node, { | ||||||||||
| initialVolume: video.volume, | ||||||||||
| initialVolume: originalVolume, | ||||||||||
| fadeScaling: this.config?.fadeScaling, | ||||||||||
| fadeDuration: this.config?.fadeOutDuration, | ||||||||||
| }); | ||||||||||
|
|
@@ -313,6 +342,7 @@ export default createPlugin< | |||||||||
| await waitForTransition; | ||||||||||
| const url = await getStreamURL(videoID); | ||||||||||
| if (!url) { | ||||||||||
| ensureVideoVolume(); | ||||||||||
| return; | ||||||||||
| } | ||||||||||
|
|
||||||||||
|
|
||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change introduces non-trivial volume state recovery behavior (tracking/restoring previous volume across navigations). There are Playwright tests in the repo (including plugin-level unit tests), but this plugin has no coverage for crossfade/volume handling. Adding a regression test (unit test for extracted helper or an e2e test that reproduces the “volume stuck at 0 after transition” case) would help prevent future regressions.