Skip to content

Commit 8e5e312

Browse files
committed
fix memory leak in soundfile
Do not create an event listener every time a soundfile is played. Reference the same event listener and bind to this instance of the soundfile. We use this event listener to keep track of the current time (and may want to consider deprecating that functionality because it creates complications like this!). Same for the "clearOnEnd" event listener, which we use to remove references to the bufferSourceNodes within a single sound file when they are done playing so that they can be garbage collected. This is useful to be able to control many bufferSourceNodes from a single sound file, and do things like soundfile.stopAll(), but it adds complexity and could also be a candidate for deprecation.
1 parent ce44dd4 commit 8e5e312

File tree

1 file changed

+42
-32
lines changed

1 file changed

+42
-32
lines changed

src/soundfile.js

Lines changed: 42 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,9 @@ define(function (require) {
138138
} else {
139139
this._whileLoading = function() {};
140140
}
141+
142+
this._onAudioProcess = _onAudioProcess.bind(this);
143+
this._clearOnEnd = _clearOnEnd.bind(this);
141144
};
142145

143146
// register preload handling of loadSound
@@ -414,25 +417,7 @@ define(function (require) {
414417
this.bufferSourceNodes.push(this.bufferSourceNode);
415418
this.bufferSourceNode._arrayIndex = this.bufferSourceNodes.length - 1;
416419

417-
// delete this.bufferSourceNode from the sources array when it is done playing:
418-
var clearOnEnd = function() {
419-
this._playing = false;
420-
this.removeEventListener('ended', clearOnEnd, false);
421-
// call the onended callback
422-
self._onended(self);
423-
424-
self.bufferSourceNodes.forEach(function(n, i) {
425-
if (n._playing === false) {
426-
self.bufferSourceNodes.splice(i);
427-
}
428-
});
429-
430-
if (self.bufferSourceNodes.length === 0) {
431-
self._playing = false;
432-
}
433-
};
434-
435-
this.bufferSourceNode.onended = clearOnEnd;
420+
this.bufferSourceNode.addEventListener('ended', this._clearOnEnd);
436421
}
437422
// If soundFile hasn't loaded the buffer yet, throw an error
438423
else {
@@ -674,11 +659,11 @@ define(function (require) {
674659
var now = p5sound.audiocontext.currentTime;
675660
var time = _time || 0;
676661
if (this.buffer && this.bufferSourceNode) {
677-
for (var i = 0; i < this.bufferSourceNodes.length; i++) {
678-
if (typeof this.bufferSourceNodes[i] !== undefined) {
662+
for (var i in this.bufferSourceNodes) {
663+
const bufferSourceNode = this.bufferSourceNodes[i];
664+
if (!!bufferSourceNode) {
679665
try {
680-
this.bufferSourceNodes[i].onended = function() {};
681-
this.bufferSourceNodes[i].stop(now + time);
666+
bufferSourceNode.stop(now + time);
682667
} catch(e) {
683668
// this was throwing errors only on Safari
684669
}
@@ -1227,7 +1212,7 @@ define(function (require) {
12271212
// dispose of scope node if it already exists
12281213
if (self._scopeNode) {
12291214
self._scopeNode.disconnect();
1230-
delete self._scopeNode.onaudioprocess;
1215+
self._scopeNode.removeEventListener('audioprocess', self._onAudioProcess);
12311216
delete self._scopeNode;
12321217
}
12331218
self._scopeNode = ac.createScriptProcessor( 256, 1, 1 );
@@ -1240,14 +1225,7 @@ define(function (require) {
12401225
cNode.connect( self._scopeNode );
12411226
self._scopeNode.connect( p5.soundOut._silentNode );
12421227

1243-
self._scopeNode.onaudioprocess = function(processEvent) {
1244-
var inputBuffer = processEvent.inputBuffer.getChannelData( 0 );
1245-
1246-
self._lastPos = inputBuffer[ inputBuffer.length - 1 ] || 0;
1247-
1248-
// do any callbacks that have been scheduled
1249-
self._onTimeUpdate(self._lastPos);
1250-
};
1228+
self._scopeNode.addEventListener('audioprocess', self._onAudioProcess);
12511229

12521230
return cNode;
12531231
};
@@ -1735,4 +1713,36 @@ define(function (require) {
17351713
return new Blob([dataView], { type: 'audio/wav' });
17361714
};
17371715

1716+
// event handler to keep track of current position
1717+
function _onAudioProcess(processEvent) {
1718+
var inputBuffer = processEvent.inputBuffer.getChannelData(0);
1719+
1720+
this._lastPos = inputBuffer[inputBuffer.length - 1] || 0;
1721+
1722+
// do any callbacks that have been scheduled
1723+
this._onTimeUpdate(self._lastPos);
1724+
}
1725+
1726+
// event handler to remove references to the bufferSourceNode when it is done playing
1727+
function _clearOnEnd(e) {
1728+
const thisBufferSourceNode = e.target;
1729+
const soundFile = this;
1730+
1731+
// delete this.bufferSourceNode from the sources array when it is done playing:
1732+
thisBufferSourceNode._playing = false;
1733+
thisBufferSourceNode.removeEventListener('ended', soundFile._clearOnEnd);
1734+
1735+
// call the onended callback
1736+
soundFile._onended(soundFile);
1737+
1738+
soundFile.bufferSourceNodes.forEach(function (n, i) {
1739+
if (n._playing === false) {
1740+
soundFile.bufferSourceNodes.splice(i);
1741+
}
1742+
});
1743+
1744+
if (soundFile.bufferSourceNodes.length === 0) {
1745+
soundFile._playing = false;
1746+
}
1747+
}
17381748
});

0 commit comments

Comments
 (0)