-
-
Notifications
You must be signed in to change notification settings - Fork 682
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
Add ring buffers to AudioWorklet processors to support variable buffer sizes #376
Changes from 1 commit
49ada2a
515b366
fb5e96f
226a237
d423aec
2014fbc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1238,13 +1238,15 @@ define(function (require) { | |
var now = ac.currentTime; | ||
var cNode = ac.createBufferSource(); | ||
|
||
const workletBufferSize = 256; | ||
|
||
// dispose of worklet node if it already exists | ||
if (self._workletNode) { | ||
self._workletNode.disconnect(); | ||
delete self._workletNode; | ||
} | ||
self._workletNode = new AudioWorkletNode(ac, processorNames.soundFileProcessor, { | ||
processorOptions: { bufferSize: 256 } | ||
processorOptions: { bufferSize: workletBufferSize } | ||
}); | ||
self._workletNode.port.onmessage = event => { | ||
if (event.data.name === 'position') { | ||
|
@@ -1259,6 +1261,17 @@ define(function (require) { | |
} | ||
}; | ||
|
||
// if the AudioWorkletNode is actually a ScriptProcessorNode created via polyfill, | ||
// make sure that our chosen buffer size isn't smaller than the buffer size automatically | ||
// selected by the polyfill | ||
// reference: https://github.com/GoogleChromeLabs/audioworklet-polyfill/issues/13#issuecomment-425014930 | ||
if (self._workletNode instanceof ScriptProcessorNode) { | ||
self._workletNode.port.postMessage({ | ||
name: 'bufferSize', | ||
bufferSize: Math.max(workletBufferSize, self._workletNode.bufferSize) | ||
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. is it better to pick the max, or—in cases where AudioWorklet is not supported—to always go with the script processor node buffer size? 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. yeah, I agree that it's probably better to always go with the script processor node buffer size - I've used that approach in my latest commit |
||
}); | ||
} | ||
|
||
// create counter buffer of the same length as self.buffer | ||
cNode.buffer = _createCounterBuffer( self.buffer ); | ||
|
||
|
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.
I think we could determine the proper buffer size here, and abstract the code to do it into a shared method. Maybe something like
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.
@therewasaguy I've extracted this logic into a shared method in
helpers.js
: 2014fbc#diff-b38f7254be1a9f817fd2e33c294481a1R305