-
Notifications
You must be signed in to change notification settings - Fork 176
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
base: main
Are you sure you want to change the base?
Changes from 2 commits
28937e1
d483465
c56de3b
9e32a45
991aa95
99cd099
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2480,9 +2480,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(unsigned long chunkSize); | ||
|
gabrielsanbrito marked this conversation as resolved.
Outdated
|
||
| Promise<undefined> resume(); | ||
| Promise<undefined> suspend(double suspendTime); | ||
| Promise<undefined> close(); | ||
| readonly attribute unsigned long length; | ||
| attribute EventHandler oncomplete; | ||
| }; | ||
|
|
@@ -2576,7 +2577,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> | ||
|
|
@@ -2587,9 +2588,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 | ||
| 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 positive infinity. | ||
|
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. IMO - since we have an explicit Probably this is aligned with what @karlt proposed?
Contributor
Author
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. I am not sure if understand this comment :/ Would you mind clarifying? When I wrote this, I intended to have Do you mean that we shouldn't set <li> Otherwise, if {{OfflineAudioContext/length}} is
<code>null</code>, set <var>bufferLength</var> to the
<a>render quantum size</a>. |
||
|
|
||
| : <dfn>oncomplete</dfn> | ||
| :: | ||
| The event type of this event handler is <dfn event>complete</dfn>. The event | ||
|
|
@@ -2601,7 +2604,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. | ||
|
|
@@ -2620,21 +2623,29 @@ 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}}. | ||
|
Contributor
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. We may want renderedFrames to be an internal slot; then we can also specify when and how it is updated. |
||
| <li> If <var>renderedFrames</var> + {{OfflineAudioContext/startRendering(chunkSize)/chunkSize}} <= {{OfflineAudioContext/length}}, set <var>bufferLength</var> to {{OfflineAudioContext/startRendering(chunkSize)/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. nit: Let's explain this logic in descriptive prose rather than using abbreviated math.
Contributor
Author
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. Thanks! Does it look good now? I was not sure if it was needed to make changes to the addition/subtraction. |
||
| <li> Otherwise, set <var>bufferLength</var> to {{OfflineAudioContext/length}} - <var>renderedFrames</var>. | ||
| </ol> | ||
| <li> Otherwise, if {{OfflineAudioContext/length}} is positive infinity, 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}}. | ||
|
|
||
|
|
@@ -2657,9 +2668,12 @@ Methods</h4> | |
| occasion. | ||
|
|
||
| <ol> | ||
| <li>Let <var>nFrames</var> be a number initialized to the value | ||
|
gabrielsanbrito marked this conversation as resolved.
Outdated
|
||
| 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>nFrames</var> sample-frames of audio into | ||
| {{[[rendered buffer]]}}. | ||
|
|
||
| <li>For every <a>render quantum</a>, check and | ||
| {{OfflineAudioContext/suspend()|suspend}} | ||
|
|
@@ -2676,7 +2690,8 @@ 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 | ||
| <li>If {{OfflineAudioContext/length}} is not positive infinity, | ||
| [=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]]}}. | ||
|
|
@@ -2686,9 +2701,9 @@ Methods</h4> | |
| </ol> | ||
| </div> | ||
|
|
||
| <div> | ||
| <em>No parameters.</em> | ||
| </div> | ||
| <pre class=argumentdef for="OfflineAudioContext/startRendering()"> | ||
| chunkSize: The size of the desired {{AudioBuffer}}. | ||
|
gabrielsanbrito marked this conversation as resolved.
Outdated
|
||
| </pre> | ||
| <div> | ||
| <em>Return type:</em> {{Promise}}<{{AudioBuffer}}> | ||
| </div> | ||
|
|
@@ -2783,6 +2798,63 @@ Methods</h4> | |
| <div> | ||
| <em>Return type:</em> {{Promise}}<{{undefined}}> | ||
| </div> | ||
|
|
||
| : <dfn>close()</dfn> | ||
| :: | ||
| Closes the {{AudioContext}}. This will not automatically release | ||
|
gabrielsanbrito marked this conversation as resolved.
Outdated
|
||
| 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"> | ||
|
|
||
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?