-
Notifications
You must be signed in to change notification settings - Fork 177
OfflineAudioContext Incremental Rendering #2675
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
Open
gabrielsanbrito
wants to merge
6
commits into
WebAudio:main
Choose a base branch
from
gabrielsanbrito:offlineaudiocontext-progressive-rendering
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
28937e1
Initial draft
gabrielsanbrito d483465
Attempt to fix compile error
gabrielsanbrito c56de3b
Merge branch 'WebAudio:main' into offlineaudiocontext-progressive-ren…
gabrielsanbrito 9e32a45
Merge branch 'WebAudio:main' into offlineaudiocontext-progressive-ren…
gabrielsanbrito 991aa95
Addressing some feedback
gabrielsanbrito 99cd099
Wrap text at column 80
gabrielsanbrito File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2500,9 +2500,10 @@ returned promise with the rendered result as an | |
| interface OfflineAudioContext : BaseAudioContext { | ||
| constructor(OfflineAudioContextOptions contextOptions); | ||
| constructor(unsigned long numberOfChannels, unsigned long length, float sampleRate); | ||
| Promise<AudioBuffer> startRendering(); | ||
| Promise<AudioBuffer> startRendering(optional unsigned long chunkSize); | ||
|
Member
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. |
||
| Promise<undefined> resume(); | ||
| Promise<undefined> suspend(double suspendTime); | ||
| Promise<undefined> close(); | ||
| readonly attribute unsigned long length; | ||
| attribute EventHandler oncomplete; | ||
| }; | ||
|
|
@@ -2596,7 +2597,7 @@ Constructors</h4> | |
|
|
||
| <pre class=argumentdef for="OfflineAudioContext/constructor(numberOfChannels, length, sampleRate)"> | ||
| numberOfChannels: Determines how many channels the buffer will have. See {{BaseAudioContext/createBuffer()}} for the supported number of channels. | ||
| length: Determines the size of the buffer in sample-frames. | ||
| length: Determines the total size of the audio render in sample-frames. | ||
| sampleRate: Describes the sample-rate of the [=linear PCM=] audio data in the buffer in sample-frames per second. See [[#sample-rates]] for the required supported range. | ||
| </pre> | ||
| </dl> | ||
|
|
@@ -2607,8 +2608,11 @@ Attributes</h4> | |
| <dl dfn-type=attribute dfn-for="OfflineAudioContext"> | ||
| : <dfn>length</dfn> | ||
| :: | ||
| The size of the buffer in sample-frames. This is the same as the | ||
| value of the <code>length</code> parameter for the constructor. | ||
| The total size of the audio render in sample-frames. This is the same | ||
| as the value of the <code>length</code> parameter for the constructor. | ||
|
|
||
| For undefined-length rendering, this attribute SHOULD be set to | ||
| <code>null</code>. | ||
|
|
||
| : <dfn>oncomplete</dfn> | ||
| :: | ||
|
|
@@ -2621,7 +2625,7 @@ Attributes</h4> | |
| Methods</h4> | ||
|
|
||
| <dl dfn-type=method dfn-for="OfflineAudioContext"> | ||
| : <dfn>startRendering()</dfn> | ||
| : <dfn>startRendering(chunkSize)</dfn> | ||
| :: | ||
| Given the current connections and scheduled changes, starts | ||
| rendering audio. | ||
|
|
@@ -2640,21 +2644,41 @@ Methods</h4> | |
| <ol> | ||
| <li>If [=this=]'s [=relevant global object=]'s [=associated Document=] is not [=fully active=] then return [=a promise rejected with=] "{{InvalidStateError}}" {{DOMException}}. | ||
|
|
||
| <li>If the {{[[rendering started]]}} slot on the | ||
| {{OfflineAudioContext}} is <em>true</em>, return a rejected | ||
| promise with {{InvalidStateError}}, and abort these | ||
| steps. | ||
|
|
||
| <li>Set the {{[[rendering started]]}} slot of the | ||
| {{OfflineAudioContext}} to <em>true</em>. | ||
|
|
||
| <li>Let <var>promise</var> be a new promise. | ||
|
|
||
| <lI>Let <var>bufferLength</var> be a number initialized to the | ||
| value of {{OfflineAudioContext/length}}. | ||
|
|
||
| <ol> | ||
| <li> If {{OfflineAudioContext/startRendering(chunkSize)/chunkSize}} | ||
| is provided: | ||
| <ol> | ||
| <li> Let <var>renderedFrames</var> be the number of | ||
| sample-frames already rendered by the | ||
| {{OfflineAudioContext}}. | ||
| <li> If <var>renderedFrames</var> + | ||
| {{OfflineAudioContext/startRendering(chunkSize)/chunkSize}} | ||
| is less than or equal to {{OfflineAudioContext/length}}, | ||
| set <var>bufferLength</var> to | ||
| {{OfflineAudioContext/startRendering(chunkSize)/chunkSize}}. | ||
| <li> Otherwise, set <var>bufferLength</var> to | ||
| {{OfflineAudioContext/length}} - | ||
| <var>renderedFrames</var>. | ||
| </ol> | ||
| <li> Otherwise, if {{OfflineAudioContext/length}} is | ||
| <code>null</code>, set <var>bufferLength</var> to the | ||
| <a>render quantum size</a>. | ||
| </ol> | ||
|
|
||
| <li>Create a new {{AudioBuffer}}, with a number of | ||
| channels, length and sample rate equal respectively to the | ||
| <code>numberOfChannels</code>, <code>length</code> and | ||
| channels and sample rate equal respectively to the | ||
| <code>numberOfChannels</code> and | ||
| <code>sampleRate</code> values passed to this instance's | ||
| constructor in the <code>contextOptions</code> parameter. | ||
| constructor in the <code>contextOptions</code> parameter; | ||
| and length equal to <var>bufferLength</var>. | ||
| Assign this buffer to an internal slot | ||
| <dfn attribute for="OfflineAudioContext">[[rendered buffer]]</dfn> in the {{OfflineAudioContext}}. | ||
|
|
||
|
|
@@ -2677,9 +2701,12 @@ Methods</h4> | |
| occasion. | ||
|
|
||
| <ol> | ||
| <li>Let <var>numberOfFrames</var> be a number initialized to | ||
| the value of the length of {{[[rendered buffer]]}}. | ||
|
|
||
| <li>Given the current connections and scheduled changes, start | ||
| rendering <code>length</code> sample-frames of audio into | ||
| {{[[rendered buffer]]}} | ||
| rendering <var>numberOfFrames</var> sample-frames of audio into | ||
| {{[[rendered buffer]]}}. | ||
|
|
||
| <li>For every <a>render quantum</a>, check and | ||
| {{OfflineAudioContext/suspend()|suspend}} | ||
|
|
@@ -2696,19 +2723,21 @@ Methods</h4> | |
| <li>Resolve the <var ignore>promise</var> created by {{startRendering()}} | ||
| with {{[[rendered buffer]]}}. | ||
|
|
||
| <li>[=Queue a media element task=] to [=fire an event=] named | ||
| {{OfflineAudioContext/complete}} at the {{OfflineAudioContext}} using | ||
| {{OfflineAudioCompletionEvent}} whose `renderedBuffer` property is set to | ||
| {{[[rendered buffer]]}}. | ||
| <li>If {{OfflineAudioContext/length}} is not | ||
| <code>null</code>, [=Queue a media element task=] to | ||
| [=fire an event=] named {{OfflineAudioContext/complete}} | ||
| at the {{OfflineAudioContext}} using | ||
| {{OfflineAudioCompletionEvent}} whose `renderedBuffer` | ||
| property is set to {{[[rendered buffer]]}}. | ||
|
|
||
| </ol> | ||
|
|
||
| </ol> | ||
| </div> | ||
|
|
||
| <div> | ||
| <em>No parameters.</em> | ||
| </div> | ||
| <pre class=argumentdef for="OfflineAudioContext/startRendering()"> | ||
| chunkSize: The number of sample-frames to render during this call. | ||
| </pre> | ||
| <div> | ||
| <em>Return type:</em> {{Promise}}<{{AudioBuffer}}> | ||
| </div> | ||
|
|
@@ -2803,6 +2832,71 @@ Methods</h4> | |
| <div> | ||
| <em>Return type:</em> {{Promise}}<{{undefined}}> | ||
| </div> | ||
|
|
||
| : <dfn>close()</dfn> | ||
| :: | ||
| Closes the {{OfflineAudioContext}}. This will not automatically release | ||
| all {{AudioContext}}-created objects, but will suspend the progression | ||
| of the {{AudioContext}}'s {{BaseAudioContext/currentTime}}, and stop | ||
| processing audio data. | ||
|
|
||
| <div algorithm="OfflineAudioContext.close()"> | ||
| <span class="synchronous">When close is called, execute these steps:</span> | ||
|
|
||
| 1. If [=this=]'s [=relevant global object=]'s | ||
| [=associated Document=] is not [=fully active=] then return | ||
| [=a promise rejected with=] "{{InvalidStateError}}" | ||
| {{DOMException}}. | ||
|
|
||
| 1. Let <var>promise</var> be a new Promise. | ||
|
|
||
| 1. If the {{[[control thread state]]}} flag on the | ||
| {{OfflineAudioContext}} is <code>closed</code> reject the | ||
| promise with {{InvalidStateError}}, abort these steps, | ||
| returning <var>promise</var>. | ||
|
|
||
| 1. Set the {{[[control thread state]]}} flag on the | ||
| {{OfflineAudioContext}} to <code>closed</code>. | ||
|
|
||
| 1. <a>Queue a control message</a> to close the | ||
| {{OfflineAudioContext}}. | ||
|
|
||
| 1. Return <em>promise</em>. | ||
| </div> | ||
|
|
||
| <div algorithm="run a control message to close an OfflineAudioContext"> | ||
| Running a <a>control message</a> to close an {{OfflineAudioContext}} | ||
| means running these steps on the <a>rendering thread</a>: | ||
|
|
||
| 1. Set the {{[[rendering thread state]]}} to <code>suspended</code>. | ||
| <div class="note"> | ||
| This will stop rendering. | ||
| </div> | ||
|
|
||
| 1. If this <a>control message</a> is being run in a reaction to the | ||
| document being unloaded, abort this algorithm. | ||
| <div class="note"> | ||
| There is no need to notify the control thread in this case. | ||
| </div> | ||
|
|
||
| 1. <a href="https://html.spec.whatwg.org/multipage/media.html#queue-a-media-element-task"> | ||
| queue a media element task</a> to execute the following steps: | ||
|
|
||
| 1. Resolve <em>promise</em>. | ||
| 1. If the {{BaseAudioContext/state}} attribute of the | ||
| {{OfflineAudioContext}} is not already | ||
| "{{AudioContextState/closed}}": | ||
| 1. Set the {{BaseAudioContext/state}} attribute of the | ||
| {{OfflineAudioContext}} to | ||
| "{{AudioContextState/closed}}". | ||
|
|
||
| 1. <a href="https://html.spec.whatwg.org/multipage/media.html#queue-a-media-element-task"> | ||
| queue a media element task</a> to | ||
| <a spec="dom" lt="fire an event">fire an event</a> named | ||
| {{BaseAudioContext/statechange}} at the | ||
| {{OfflineAudioContext}}. | ||
| </div> | ||
|
|
||
| </dl> | ||
|
|
||
| <h4 dictionary id="OfflineAudioContextOptions"> | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
If we make
lengthnullable I think it would becomelength?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.
Based on the discussion in the spec issue, was using
lengththe final resolution?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.
The discussion has stalled since my last comment. AFAIU, it seems that everyone agreed on using
length, but there is some divergence on how it should be used. Should it beoptionalor nullable?I'd rather have
lengthbe nullable instead ofoptional. This way callers need to be a little more intentional when creating indefinite-lengthOfflineAudioContexts. What do you think?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.
Then let's clarify the resolution first. I'd really liked to have a WG decision on this:
IIUC your design is to make
lengthrequired, and the proposal from @karlt is to make it optional?@karlt @padenot Can you all chime in here so we can make a decision?