From 65894cd71e772d4fcd52cb4f697ad30d16b5b945 Mon Sep 17 00:00:00 2001 From: Thijs Triemstra Date: Thu, 24 Oct 2019 12:18:21 +0200 Subject: [PATCH] fix destroy in MediaElement backend (#1778) * fix destroy in MediaElement backend * add pr nr * make var public * move var to webaudio backend --- CHANGES.md | 10 +++-- src/mediaelement.js | 96 +++++++++++++++++++++++++++------------------ src/wavesurfer.js | 9 +++-- src/webaudio.js | 5 +++ 4 files changed, 74 insertions(+), 46 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index f43400d89..001df2ff2 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -13,15 +13,17 @@ wavesurfer.js changelog a long duration. You can also supply peaks data, so the entire audio file does not have to be decoded. For example: - ` wavesurfer.load(url | HTMLMediaElement, peaks, preload, duration); - wavesurfer.play(); - wavesurfer.setFilter(customFilter); - ` + ``` + wavesurfer.load(url | HTMLMediaElement, peaks, preload, duration); + wavesurfer.play(); + wavesurfer.setFilter(customFilter); + ``` - Add `barRadius` option to create waveforms with rounded bars (#953) - Throw error when the url parameter supplied to `wavesurfer.load()` is empty (#1773, #1775) - Specify non-minified wavesurfer.js in `main` entry of `package.json` (#1759) - Add `dblclick` event listener to wavesurfer wrapper (#1764) +- Fix `destroy()` in `MediaElement` backend (#1778) - Cursor plugin: flip position of time text to left of the cursor where needed to improve readability (#1776) - Regions plugin: change region end handler position (#1762) diff --git a/src/mediaelement.js b/src/mediaelement.js index 514ac2403..9254686ee 100755 --- a/src/mediaelement.js +++ b/src/mediaelement.js @@ -15,8 +15,11 @@ export default class MediaElement extends WebAudio { /** @private */ this.params = params; - // Dummy media to catch errors - /** @private */ + /** + * Initially a dummy media element to catch errors. Once `_load` is + * called, this will contain the actual `HTMLMediaElement`. + * @private + */ this.media = { currentTime: 0, duration: 0, @@ -43,6 +46,8 @@ export default class MediaElement extends WebAudio { this.buffer = null; /** @private */ this.onPlayEnd = null; + /** @private */ + this.mediaListeners = {}; } /** @@ -53,9 +58,49 @@ export default class MediaElement extends WebAudio { this.createTimer(); } + /** + * Attach event listeners to media element. + */ + _setupMediaListeners() { + this.mediaListeners.error = () => { + this.fireEvent('error', 'Error loading media element'); + }; + this.mediaListeners.canplay = () => { + this.fireEvent('canplay'); + }; + this.mediaListeners.ended = () => { + this.fireEvent('finish'); + }; + // listen to and relay play, pause and seeked events to enable + // playback control from the external media element + this.mediaListeners.play = () => { + this.fireEvent('play'); + }; + this.mediaListeners.pause = () => { + this.fireEvent('pause'); + }; + this.mediaListeners.seeked = event => { + this.fireEvent('seek'); + }; + this.mediaListeners.volumechange = event => { + this.isMuted = this.media.muted; + if (this.isMuted) { + this.volume = 0; + } else { + this.volume = this.media.volume; + } + this.fireEvent('volume'); + }; + + // reset event listeners + Object.keys(this.mediaListeners).forEach(id => { + this.media.removeEventListener(id, this.mediaListeners[id]); + this.media.addEventListener(id, this.mediaListeners[id]); + }); + } + /** * Create a timer to provide a more precise `audioprocess` event. - * */ createTimer() { const onAudioProcess = () => { @@ -146,43 +191,8 @@ export default class MediaElement extends WebAudio { media.load(); } - media.addEventListener('error', () => { - this.fireEvent('error', 'Error loading media element'); - }); - - media.addEventListener('canplay', () => { - this.fireEvent('canplay'); - }); - - media.addEventListener('ended', () => { - this.fireEvent('finish'); - }); - - // Listen to and relay play, pause and seeked events to enable - // playback control from the external media element - media.addEventListener('play', () => { - this.fireEvent('play'); - }); - - media.addEventListener('pause', () => { - this.fireEvent('pause'); - }); - - media.addEventListener('seeked', event => { - this.fireEvent('seek'); - }); - - media.addEventListener('volumechange', event => { - this.isMuted = media.muted; - if (this.isMuted) { - this.volume = 0; - } else { - this.volume = media.volume; - } - this.fireEvent('volume'); - }); - this.media = media; + this._setupMediaListeners(); this.peaks = peaks; this.onPlayEnd = null; this.buffer = null; @@ -391,6 +401,14 @@ export default class MediaElement extends WebAudio { destroy() { this.pause(); this.unAll(); + this.destroyed = true; + + // cleanup media event listeners + Object.keys(this.mediaListeners).forEach(id => { + if (this.media) { + this.media.removeEventListener(id, this.mediaListeners[id]); + } + }); if ( this.params.removeMediaElementOnDestroy && diff --git a/src/wavesurfer.js b/src/wavesurfer.js index 1e6cae97f..c4baeeccb 100755 --- a/src/wavesurfer.js +++ b/src/wavesurfer.js @@ -1398,9 +1398,12 @@ export default class WaveSurfer extends util.Observer { this.tmpEvents.push( this.backend.once('canplay', () => { - this.drawBuffer(); - this.isReady = true; - this.fireEvent('ready'); + // ignore when backend was already destroyed + if (!this.backend.destroyed) { + this.drawBuffer(); + this.isReady = true; + this.fireEvent('ready'); + } }), this.backend.once('error', err => this.fireEvent('error', err)) ); diff --git a/src/webaudio.js b/src/webaudio.js index 91b8215d1..51fca1654 100755 --- a/src/webaudio.js +++ b/src/webaudio.js @@ -145,6 +145,10 @@ export default class WebAudio extends util.Observer { this.state = null; /** @private */ this.explicitDuration = params.duration; + /** + * Boolean indicating if the backend was destroyed. + */ + this.destroyed = false; } /** @@ -500,6 +504,7 @@ export default class WebAudio extends util.Observer { } this.unAll(); this.buffer = null; + this.destroyed = true; this.disconnectFilters(); this.disconnectSource(); this.gainNode.disconnect();