Skip to content

Commit

Permalink
fix(player): memory leak on provider change
Browse files Browse the repository at this point in the history
  • Loading branch information
mihar-22 committed Oct 17, 2023
1 parent 7659ac1 commit 24f451f
Show file tree
Hide file tree
Showing 6 changed files with 20 additions and 18 deletions.
4 changes: 2 additions & 2 deletions packages/vidstack/src/components/provider/source-select.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ import {
computed,
effect,
peek,
scoped,
tick,
untrack,
type ReadSignal,
type WriteSignal,
} from 'maverick.js';
Expand Down Expand Up @@ -177,7 +177,7 @@ export class SourceSelection {
if (!provider || provider[SETUP]) return;

if (this._media.$state.canLoad()) {
untrack(() => provider.setup(this._media));
scoped(() => provider.setup(this._media), provider.scope);
provider[SETUP] = true;
return;
}
Expand Down
8 changes: 1 addition & 7 deletions packages/vidstack/src/core/state/media-state-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -207,16 +207,10 @@ export class MediaStateManager extends MediaPlayerController {
if (prevProvider?.type === newProvider?.type) return;

prevProvider?.destroy?.();
prevProvider?.scope?.dispose();
this._media.$provider.set(event.detail);

if (prevProvider && event.detail === null) this._resetMediaState(event);

this['stream-type-change'](
this.createEvent('stream-type-change', {
detail: 'unknown',
trigger: event,
}),
);
}

['provider-loader-change'](event: ME.MediaProviderLoaderChangeEvent) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ export class HTMLMediaEvents {
this._attachEventListener('stalled', this._onStalled),
this._attachEventListener('suspend', this._onSuspend),
);

this._attachedLoadStart = true;
}

Expand Down
4 changes: 3 additions & 1 deletion packages/vidstack/src/providers/html/provider.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { onDispose } from 'maverick.js';
import { createScope, onDispose } from 'maverick.js';
import { isString, setAttribute } from 'maverick.js/std';

import type { MediaSrc } from '../../core/api/types';
Expand All @@ -14,6 +14,8 @@ import { NativeAudioTracks } from './native-audio-tracks';
* @see {@link https://developer.mozilla.org/en-US/docs/Web/API/HTMLMediaElement}
*/
export class HTMLMediaProvider implements MediaProviderAdapter {
readonly scope = createScope();

protected _currentSrc: MediaSrc | null = null;

constructor(protected _media: HTMLMediaElement) {}
Expand Down
3 changes: 3 additions & 0 deletions packages/vidstack/src/providers/types.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import type { Scope } from 'maverick.js';

import type { MediaPlayer } from '../components/player';
import type { MediaContext } from '../core/api/media-context';
import type { MediaState } from '../core/api/player-state';
Expand Down Expand Up @@ -25,6 +27,7 @@ export interface MediaProviderAdapter
MediaState,
'paused' | 'muted' | 'currentTime' | 'volume' | 'playsinline' | 'playbackRate'
> {
readonly scope: Scope;
readonly type: string;
readonly currentSrc: MediaSrc | null;
readonly fullscreen?: MediaFullscreenAdapter;
Expand Down
18 changes: 10 additions & 8 deletions packages/vidstack/src/providers/video/provider.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { onDispose } from 'maverick.js';
import { onDispose, scoped } from 'maverick.js';

import type { MediaContext } from '../../core/api/media-context';
import {
Expand Down Expand Up @@ -49,13 +49,15 @@ export class VideoProvider extends HTMLMediaProvider implements MediaProviderAda

constructor(video: HTMLVideoElement, context: MediaContext) {
super(video);
if (canUseVideoPresentation(video)) {
const presentation = new VideoPresentation(video, context);
this.fullscreen = new FullscreenPresentationAdapter(presentation);
this.pictureInPicture = new PIPPresentationAdapter(presentation);
} else if (canUsePictureInPicture(video)) {
this.pictureInPicture = new VideoPictureInPicture(video, context);
}
scoped(() => {
if (canUseVideoPresentation(video)) {
const presentation = new VideoPresentation(video, context);
this.fullscreen = new FullscreenPresentationAdapter(presentation);
this.pictureInPicture = new PIPPresentationAdapter(presentation);
} else if (canUsePictureInPicture(video)) {
this.pictureInPicture = new VideoPictureInPicture(video, context);
}
}, this.scope);
}

override setup(context: MediaSetupContext): void {
Expand Down

0 comments on commit 24f451f

Please sign in to comment.