Skip to content

Commit 5012498

Browse files
authored
Fix a few memory leaks while tearing down the player (#5698)
* Fix a few memory leaks while tearing down the player * Fix `ui` properties not getting cleared
1 parent ad320ac commit 5012498

File tree

2 files changed

+81
-20
lines changed

2 files changed

+81
-20
lines changed

src/renderer/components/ft-shaka-video-player/ft-shaka-video-player.js

Lines changed: 69 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1588,6 +1588,8 @@ export default defineComponent({
15881588

15891589
// #region custom player controls
15901590

1591+
const { ContextMenu: shakaContextMenu, Controls: shakaControls, OverflowMenu: shakaOverflowMenu } = shaka.ui
1592+
15911593
function registerAudioTrackSelection() {
15921594
/** @implements {shaka.extern.IUIElement.Factory} */
15931595
class AudioTrackSelectionFactory {
@@ -1596,8 +1598,8 @@ export default defineComponent({
15961598
}
15971599
}
15981600

1599-
shaka.ui.Controls.registerElement('ft_audio_tracks', new AudioTrackSelectionFactory())
1600-
shaka.ui.OverflowMenu.registerElement('ft_audio_tracks', new AudioTrackSelectionFactory())
1601+
shakaControls.registerElement('ft_audio_tracks', new AudioTrackSelectionFactory())
1602+
shakaOverflowMenu.registerElement('ft_audio_tracks', new AudioTrackSelectionFactory())
16011603
}
16021604

16031605
function registerTheatreModeButton() {
@@ -1614,8 +1616,8 @@ export default defineComponent({
16141616
}
16151617
}
16161618

1617-
shaka.ui.Controls.registerElement('ft_theatre_mode', new TheatreModeButtonFactory())
1618-
shaka.ui.OverflowMenu.registerElement('ft_theatre_mode', new TheatreModeButtonFactory())
1619+
shakaControls.registerElement('ft_theatre_mode', new TheatreModeButtonFactory())
1620+
shakaOverflowMenu.registerElement('ft_theatre_mode', new TheatreModeButtonFactory())
16191621
}
16201622

16211623
function registerFullWindowButton() {
@@ -1642,8 +1644,8 @@ export default defineComponent({
16421644
}
16431645
}
16441646

1645-
shaka.ui.Controls.registerElement('ft_full_window', new FullWindowButtonFactory())
1646-
shaka.ui.OverflowMenu.registerElement('ft_full_window', new FullWindowButtonFactory())
1647+
shakaControls.registerElement('ft_full_window', new FullWindowButtonFactory())
1648+
shakaOverflowMenu.registerElement('ft_full_window', new FullWindowButtonFactory())
16471649
}
16481650

16491651
function registerLegacyQualitySelection() {
@@ -1677,8 +1679,8 @@ export default defineComponent({
16771679
}
16781680
}
16791681

1680-
shaka.ui.Controls.registerElement('ft_legacy_quality', new LegacyQualitySelectionFactory())
1681-
shaka.ui.OverflowMenu.registerElement('ft_legacy_quality', new LegacyQualitySelectionFactory())
1682+
shakaControls.registerElement('ft_legacy_quality', new LegacyQualitySelectionFactory())
1683+
shakaOverflowMenu.registerElement('ft_legacy_quality', new LegacyQualitySelectionFactory())
16821684
}
16831685

16841686
function registerStatsButton() {
@@ -1699,7 +1701,7 @@ export default defineComponent({
16991701
}
17001702
}
17011703

1702-
shaka.ui.ContextMenu.registerElement('ft_stats', new StatsButtonFactory())
1704+
shakaContextMenu.registerElement('ft_stats', new StatsButtonFactory())
17031705
}
17041706

17051707
function registerScreenshotButton() {
@@ -1716,8 +1718,34 @@ export default defineComponent({
17161718
}
17171719
}
17181720

1719-
shaka.ui.Controls.registerElement('ft_screenshot', new ScreenshotButtonFactory())
1720-
shaka.ui.OverflowMenu.registerElement('ft_screenshot', new ScreenshotButtonFactory())
1721+
shakaControls.registerElement('ft_screenshot', new ScreenshotButtonFactory())
1722+
shakaOverflowMenu.registerElement('ft_screenshot', new ScreenshotButtonFactory())
1723+
}
1724+
1725+
/**
1726+
* As shaka-player doesn't let you unregister custom control factories,
1727+
* overwrite them with `null` instead so the referenced objects
1728+
* (e.g. {@linkcode events}, {@linkcode fullWindowEnabled}) can get gargabe collected
1729+
*/
1730+
function cleanUpCustomPlayerControls() {
1731+
shakaControls.registerElement('ft_audio_tracks', null)
1732+
shakaOverflowMenu.registerElement('ft_audio_tracks', null)
1733+
1734+
shakaControls.registerElement('ft_theatre_mode', null)
1735+
shakaOverflowMenu.registerElement('ft_theatre_mode', null)
1736+
1737+
shakaControls.registerElement('ft_full_window', null)
1738+
shakaOverflowMenu.registerElement('ft_full_window', null)
1739+
1740+
shakaControls.registerElement('ft_legacy_quality', null)
1741+
shakaOverflowMenu.registerElement('ft_legacy_quality', null)
1742+
1743+
shakaContextMenu.registerElement('ft_stats', null)
1744+
1745+
if (process.env.IS_ELECTRON) {
1746+
shakaControls.registerElement('ft_screenshot', null)
1747+
shakaOverflowMenu.registerElement('ft_screenshot', null)
1748+
}
17211749
}
17221750

17231751
// #endregion custom player controls
@@ -2636,12 +2664,7 @@ export default defineComponent({
26362664
resizeObserver = null
26372665
}
26382666

2639-
if (ui) {
2640-
// destroying the ui also destroys the player
2641-
ui.destroy()
2642-
ui = null
2643-
player = null
2644-
}
2667+
cleanUpCustomPlayerControls()
26452668

26462669
stopPowerSaveBlocker()
26472670
window.removeEventListener('beforeunload', stopPowerSaveBlocker)
@@ -2679,13 +2702,41 @@ export default defineComponent({
26792702
video.value.currentTime = time
26802703
}
26812704

2705+
/**
2706+
* Vue's lifecycle hooks are synchonous, so if we destroy the player in {@linkcode onBeforeUnmount},
2707+
* it won't be finished in time, as the player destruction is asynchronous.
2708+
* To workaround that we destroy the player first and wait for it to finish before we unmount this component.
2709+
*/
2710+
async function destroyPlayer() {
2711+
if (ui) {
2712+
// destroying the ui also destroys the player
2713+
await ui.destroy()
2714+
ui = null
2715+
player = null
2716+
} else if (player) {
2717+
await player.destroy()
2718+
player = null
2719+
}
2720+
2721+
// shaka-player doesn't clear these itself, which prevents shaka.ui.Overlay from being garbage collected
2722+
// Should really be fixed in shaka-player but it's easier just to do it ourselves
2723+
if (container.value) {
2724+
container.value.ui = null
2725+
}
2726+
2727+
if (video.value) {
2728+
video.value.ui = null
2729+
}
2730+
}
2731+
26822732
expose({
26832733
hasLoaded,
26842734

26852735
isPaused,
26862736
pause,
26872737
getCurrentTime,
2688-
setCurrentTime
2738+
setCurrentTime,
2739+
destroyPlayer
26892740
})
26902741

26912742
// #endregion functions used by the watch page

src/renderer/views/Watch/Watch.js

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,9 +54,14 @@ export default defineComponent({
5454
'watch-video-recommendations': WatchVideoRecommendations,
5555
'ft-age-restricted': FtAgeRestricted
5656
},
57-
beforeRouteLeave: function (to, from, next) {
57+
beforeRouteLeave: async function (to, from, next) {
5858
this.handleRouteChange()
5959
window.removeEventListener('beforeunload', this.handleWatchProgress)
60+
61+
if (this.$refs.player) {
62+
await this.$refs.player.destroyPlayer()
63+
}
64+
6065
next()
6166
},
6267
data: function () {
@@ -229,8 +234,13 @@ export default defineComponent({
229234
},
230235
},
231236
watch: {
232-
$route() {
237+
async $route() {
233238
this.handleRouteChange()
239+
240+
if (this.$refs.player) {
241+
await this.$refs.player.destroyPlayer()
242+
}
243+
234244
// react to route changes...
235245
this.videoId = this.$route.params.id
236246

0 commit comments

Comments
 (0)