Choose modifying startRendering as the preferred approach#1332
Choose modifying startRendering as the preferred approach#1332gabrielsanbrito wants to merge 1 commit into
Conversation
rahulsingh-msft
left a comment
rahulsingh-msft
left a comment
There was a problem hiding this comment.
Thanks for updating this! I made some suggestions to improve readability.
| ### Cons | ||
| - Harder to feature-detect — Unlike a new method (e.g., startRenderingStream), you can't simply check if ("newMethod" in context). Detecting chunkSize support requires a try/catch or similar heuristic. | ||
| - Changes the mental model for startRendering a bit. | ||
|
|
There was a problem hiding this comment.
Should we add an Alternative Considered section here that the Alternatives that follow can live under?
| WebAudio works well in a realtime playback context but it is not suitable for offline context (faster-than-realtime) processing due to a limitation in the design of WebAudio's [OfflineAudioContext API](https://developer.mozilla.org/en-US/docs/Web/API/OfflineAudioContext). The design of the API requires allocating memory to render the whole audio graph's memory up-front which can reach gigabytes of AudioBuffer data. | ||
|
|
||
| This document will propose adding a streaming offline context rendering function so that the audio graph data can be incrementally processed rather than allocating the whole audio buffer up-front. | ||
| This document will propose expanding the functionality of the offline context rendering function so that the audio graph data can be incrementally processed rather than allocating the whole audio buffer up-front. |
There was a problem hiding this comment.
nit: This document proposes.
| ### Goals | ||
|
|
||
| - Allow streaming data out of a WebAudio in an offline context for rendering large audio graphs | ||
| - Allow incrementally rendering data out of a WebAudio offline context for rendering large audio graphs |
There was a problem hiding this comment.
Should we concretely use the API's name i.e. OfflineAudioContext?
| - Change the existing `startRendering()` behavior, this API change is additive | ||
|
|
||
| ## Proposed Approach - Add `startRenderingStream()` function | ||
| ## Proposed Approach - `startRendering` can render in chunks |
There was a problem hiding this comment.
nit: Should Proposed Approach be the Section heading? And then startRendering can render in chunks could be a subheading under that.
| ## Proposed Approach - `startRendering` can render in chunks | ||
|
|
||
| The preferred approach is adding a new method `startRenderingStream()` that yields buffers of interleaved audio samples in a Float32Array, or another format as outlined in Open Questions. In this scenario, the user can read chunks as they arrive and consume them for storage, transcoding via WebCodecs, sending to a server, etc. | ||
| The preferred approach is to modify the behavior of `startRendering()` in a backwards-compatible manner so that it always renders incrementally in chunks. With it, as will be explained ahead, the current one-shot render scenario now becomes a special case of the new behavior where the `chunkSize` is set to `OfflineAudioContextOptions.length`. |
There was a problem hiding this comment.
consider slightly rewording this:
We propose modifying the behavior of startRendering() in a backwards-compatible manner so that it always renders incrementally in chunks. With this, the current one-shot render scenario becomes a special case of the new behavior where the new chunkSize param is set to ...
| ``` | ||
|
|
||
| ### Pros | ||
| - Solution that is also backward-compatible. |
There was a problem hiding this comment.
nit: Maintains backwards compatibility.
|
|
||
| ### Pros | ||
| - Solution that is also backward-compatible. | ||
| - Simple to reason about and implement, since callers just need to request chunks whenever they are ready to process them. |
There was a problem hiding this comment.
Could this be 2 sentences?
Simple to reason about and implement. Callers just need to ...
|
|
||
| ### Cons | ||
| - Harder to feature-detect — Unlike a new method (e.g., startRenderingStream), you can't simply check if ("newMethod" in context). Detecting chunkSize support requires a try/catch or similar heuristic. | ||
| - Changes the mental model for startRendering a bit. |
There was a problem hiding this comment.
Could we say:
Evolves the mental model for startRendering.
| - Doesn't require integration with the Streams API. | ||
|
|
||
| ### Cons | ||
| - Harder to feature-detect — Unlike a new method (e.g., startRenderingStream), you can't simply check if ("newMethod" in context). Detecting chunkSize support requires a try/catch or similar heuristic. |
There was a problem hiding this comment.
Could this be written as:
Harder to feature detect. In contrast, adding a new method would allow checking for its presence in the context...
| - It is not feature detectable, as compared to the Proposed Approach, because it only adds options dictionary to an existing function | ||
| - Less explicit than the proposed approach as it overloads an existing public API function. It is safer and simpler to add a new function and not change the behaviour of an existing function | ||
| - The same cons as Alternative 1 | ||
| - It is not feature detectable, as compared to Alternative 1, because it only adds options dictionary to an existing function |
There was a problem hiding this comment.
Unlike Alternative 1, it is not feature detectable because it modifies an existing function.
2 participants
This PR updates the progressive OfflineAudioContext explainer to reflect the latest discussions that happened in the Audio WG (discussion thread here). The main outcome is that now the preferred approach is to modify the behavior of
OfflineAudioContext.startRenderingin a backward-compatible manner so that it now renders audio in chunks by default.