Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 13 additions & 8 deletions index.bs
Original file line number Diff line number Diff line change
Expand Up @@ -4650,7 +4650,7 @@ Methods</h4>
<pre class=argumentdef for="AudioBufferSourceNode/start(when, offset, duration)">
when: The <code>when</code> parameter describes at what time (in seconds) the sound should start playing. It is in the same time coordinate system as the {{AudioContext}}'s {{BaseAudioContext/currentTime}} attribute. If 0 is passed in for this value or if the value is less than <b>currentTime</b>, then the sound will start playing immediately. <span class="synchronous">A {{RangeError}} exception MUST be thrown if <code>when</code> is negative.</span>
offset: The <code>offset</code> parameter supplies a <a>playhead position</a> where playback will begin. If 0 is passed in for this value, then playback will start from the beginning of the buffer. <span class="synchronous">A {{RangeError}} exception MUST be thrown if <code>offset</code> is negative.</span> If <code>offset</code> is greater than {{AudioBufferSourceNode/loopEnd}}, playback will begin at {{AudioBufferSourceNode/loopEnd}} (and immediately loop to {{AudioBufferSourceNode/loopStart}}). <code>offset</code> is silently clamped to [0, <code>duration</code>], when <code>startTime</code> is reached, where <code>duration</code> is the value of the <code>duration</code> attribute of the {{AudioBuffer}} set to the {{AudioBufferSourceNode/buffer}} attribute of this <code>AudioBufferSourceNode</code>.
duration: The {{AudioBufferSourceNode/start(when, offset, duration)/duration}} parameter describes the duration of the sound (in seconds) to be played. If this parameter is passed, this method has exactly the same effect as the invocation of <code>start(when, offset)</code> followed by <code>stop(when + duration)</code>. <span class="synchronous">A {{RangeError}} exception MUST be thrown if <code>duration</code> is negative.</span>
duration: The {{AudioBufferSourceNode/start(when, offset, duration)/duration}} parameter describes the duration of sound to be played, expressed as seconds of total buffer content to be output, including any whole or partial loop iterations. The units of {{AudioBufferSourceNode/start(when, offset, duration)/duration}} are independent of the effects of {{AudioBufferSourceNode/playbackRate}}. For example, a {{AudioBufferSourceNode/start(when, offset, duration)/duration}} of 5 seconds with a playback rate of 0.5 will output 5 seconds of buffer content at half speed, producing 10 seconds of audible output. <span class="synchronous">A {{RangeError}} exception MUST be thrown if <code>duration</code> is negative.</span>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think (as mentioned previously), we shouldn't really try to say in words what supposed to happen. Can we just refer to the algorithm?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel strongly that here (as in many other places in the spec) it's valuable to have a short textual description of what a thing is, as a counterpoint to the algorithm.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm only concerned if it's inconsistent with the fine details of the algorithm. A non-normative note is acceptable to me.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's consistent -- if you would like to rephrase it somehow, please propose (or commit) some alternate wording.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As stated, I think a non-normative note would be fine. And I think the duration here also applies to looping. The looping playback will play for duration secs, taking into account the playback rate.

</pre>

<div>
Expand Down Expand Up @@ -4816,21 +4816,22 @@ let loopEnd;
let playbackRate;

// Variables for the node's playback parameters
let start = 0, offset = 0; // Set by start()
let stop = Infinity; // Set by stop(), or by start() with a supplied duration
let start = 0, offset = 0, duration = Infinity; // Set by start()
let stop = Infinity; // Set by stop()


// Variables for tracking node's playback state
let bufferTime = 0, started = false, enteredLoop = false;
let dt = 1 / context.sampleRate;

// Handle invocation of start method call
function handleStart(when, pos, duration) {
function handleStart(when, pos, dur) {
if (arguments.length >= 1) {
start = when;
}
offset = pos;
if (arguments.length >= 3) {
stop = when + duration;
duration = dur;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to define duration?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The duration variable is already defined, on line 4819 (same place as offset).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ack.

}
}

Expand Down Expand Up @@ -4867,6 +4868,7 @@ function playbackSignal(position) {
// of |numberOfFrames| sample frames to be output.
function process(numberOfFrames) {
let currentTime = context.currentTime; // context time of next rendered frame
let bufferDuration = 0; // cumulative duration in "buffer time" (not adjusted for playback rate)
let output = []; // accumulates rendered sample frames

// Combine the two k-rate parameters affecting playback rate
Expand Down Expand Up @@ -4894,16 +4896,16 @@ function process(numberOfFrames) {

// Render each sample frame in the quantum
for (let index = 0; index &lt; numberOfFrames; index++) {
// Check that currentTime is within allowable range for playback
if (currentTime &lt; start || currentTime >= stop) {
// Check that currentTime and bufferDuration are within allowable range for playback
if (currentTime &lt; start || currentTime >= stop || bufferDuration >= duration) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is wrong? Let duration = 129/sampleRate and let the playbackRate = 1. Since bufferDuration is initialized to 0 in process(), and framesToProcess is 128, bufferDuration is at most 128/sampleRate. So bufferDuration >= duration is never true. This means the following lines produce an output. I think that's wrong. Maybe this should be bufferTime instead?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps bufferDuration was intended to be declared outside the function?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @karlt for catching my error here. Now fixed.

output.push(0); // this sample frame is silent
currentTime += dt;
continue;
}

if (!started) {
// Take note that buffer has started playing and get initial playhead position.
bufferTime = offset + ((currentTime - start) * computedPlaybackRate);
bufferTime = offset + bufferDuration;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this right? The initial value for bufferTime is now different between this version and the old version. I think the original is the right value.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're correct about the inconsistency. But another way to go would be to initialize bufferDuration to (currentTime - start) * computedPlaybackRate in this clause. Wouldn't that be an even better fix? It would cause bufferDuration to respect fractional-frame start times.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed this change too, but thought it was fixing a bug.

For the when parameter, "If 0 is passed in for this value or if the value is
less than currentTime, then the sound will start playing immediately." Also,
"The offset parameter supplies a playhead position where playback will begin."
I expect playback to begin with the head at offset regardless of when.

I couldn't make sense of ((currentTime - start) * computedPlaybackRate).
It seems to be aiming to make playback start with the head at some point
after offset if when is less than currentTime.

(The spec text is helpful to indicate the intention of the algorithm.)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I didn't look to see what the original is doing, but I agree with @karlt: playback should start at offset independent of when. I think bufferDuration should actually start at 0. If you choose any other value, then the actual playback duration will be less then the duration arg, even if playback rate is 1.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bufferDuration starts at 0. Simply initializing bufferTime to offset would be more clear.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rtoy could you please change this as per @karlt's last suggestion? I will not be able to work on this for a couple of weeks and this PR is close to done.

started = true;
}

Expand Down Expand Up @@ -4940,6 +4942,7 @@ function process(numberOfFrames) {
}

bufferTime += dt * computedPlaybackRate;
bufferDuration += dt * computedPlaybackRate;
currentTime += dt;
} // End of render quantum loop

Expand Down Expand Up @@ -4970,6 +4973,8 @@ apply:
* linear interpolation is depicted throughout, although a UA
could employ other interpolation techniques.

* the <code>duration</code> values noted in the figures refer to the <code>buffer</code>, not arguments to {{AudioBufferSourceNode/start()}}

This figure illustrates basic playback of a buffer, with a simple
loop that ends after the last sample frame in the buffer:

Expand Down
Loading