WebAudio / web-audio-api

The Web Audio API v1.0, developed by the W3C Audio WG
https://webaudio.github.io/web-audio-api/
Other
1.05k stars 167 forks source link

Clarify buffer.copyToChannel() must be called before source.buffer = buffer else nothing is played #2227

Closed guest271314 closed 4 years ago

guest271314 commented 4 years ago

Describe the issue

The current language in the specification does not make it clear that buffer.copyToChannel() must be called before source.buffer = buffer else audio will not play.

Nightly 80 appears to implement that algorithm, where the only observable part of the algorithm employed is "nothing is played" even though ended event is dispatched.

At Chromium 86 audio is played when buffer.copyToChannel() is called after source.buffer = buffer. Though since the current language in the specification is not immediately clear if file an issue at Chromium bug tracker cannot now cite specific language relevant to the mandated order of calling copyToChannel() and setting source.buffer = buffer.

Firefox bug https://bugzilla.mozilla.org/show_bug.cgi?id=1655336

Where Is It

1.4.3. Methods https://www.w3.org/TR/webaudio/#AudioBuffer-methods

The acquire the contents of an AudioBuffer operation is invoked in the following cases:

- When AudioBufferSourceNode.start is called, it acquires the contents of the node’s buffer. If the operation fails, nothing is played.

- When the buffer of an AudioBufferSourceNode is set and AudioBufferSourceNode.start has been previously called, the setter acquires the content of the AudioBuffer. If the operation fails, nothing is played.

- When a ConvolverNode's buffer is set to an AudioBuffer it acquires the content of the AudioBuffer.

- When the dispatch of an AudioProcessingEvent completes, it acquires the contents of its outputBuffer.

Note: This means that copyToChannel() cannot be used to change the content of an AudioBuffer currently in use by an AudioNode that has acquired the content of an AudioBuffer since the AudioNode will continue to use the data previously acquired.

Additional Information

The language can be improved to state clearly what is intended to occur based on the necessary order of operations

copyToChannel() MUST be called before source.buffer = buffer else nothing is played

which also means that in the case of source.buffer = buffer being executed before buffer.copyToChannel() "data" at

AudioNode will continue to use the data previously acquired.

can be a Float32Array containing 0s, effectively silence.

guest271314 commented 4 years ago

Re-reading the specification, trying to gather what was intended, Nightly 80 should not begin the "acquire the contents of an AudioBuffer operation" algorithm should not begin until start() is called, at which time in this case copyFromChannel() has already been called and

Note: This means that copyToChannel() cannot be used to change the content of an AudioBuffer currently in use by an AudioNode that has acquired the content of an AudioBuffer since the AudioNode will continue to use the data previously acquired.

cannot apply because start() has not yet been called, nullifying the condition "currently in use" as nothing has been used before start(), when the algorithm is supposed to begin - not when setting source.buffer = buffer alone, without calling start()?

In which case the Note makes no sense.

guest271314 commented 4 years ago

"nothing is played" is still not correct given the context of start() not being called during the evaluation of the procedure, the original contents of AudioBuffer are played.

  • When the buffer of an AudioBufferSourceNode is set and AudioBufferSourceNode.start has been previously called, the setter acquires the content of the AudioBuffer. If the operation fails, nothing is played.
  • When the buffer of an AudioBufferSourceNode is set and AudioBufferSourceNode.start has been previously called, the setter acquires the content of the AudioBuffer. If the operation fails, nothing is played.

includes the term "and" between setting the buffer of AudioBufferSourceNode and start(), meaning both must occur of that list item to be applicable.

The description of the "operation" and the subsequent note inclusive creates a series of circular reference that necessarily includes start() being mandated for the "acquire the contents of an AudioBuffer operation" with the follow-up omitting reference to start(). rather

currently in use by an AudioNode that has acquired the content of an AudioBuffer

yet does not indicate how an AudioNode could have acquired the content of an AudioBuffer without calling start() in the formal "operation" list items, leading the question of how an AudioNode aquired the content of an AudioBuffer without fulfilling the requirements in the "operation", unless AudioBufferSourceNode is implemented as a ConvolverNode?

  • When a ConvolverNode's buffer is set to an AudioBuffer it acquires the content of the AudioBuffer.

What was the original intent of the "operation" and the purpose of the Note, if the two discrete portions of the specification were not comitted at the same time, relevant to precisely when an AudioBufferSourceNode acquires the contents of an AudioBuffer and what is the significance of copyToChannel() reference?

Have yet to locate a complete answer to the question at

et al. re copyTo/FromChannel and "nothing is played", which does not happen, though can have that effect due to silence being played.

guest271314 commented 4 years ago

This added some of the language https://github.com/WebAudio/web-audio-api/commit/8e144640af37df011d5f6ffa405faad56762311c. The Note https://github.com/WebAudio/web-audio-api/commit/5d2c203fde2d863b11c884267b45650def4023ba which was committed before the above commit, began by https://github.com/WebAudio/web-audio-api/issues/567.

The language and exact steps are still not clear.

rtoy commented 4 years ago

My understanding is that you MUST call start() to acquire the contents. Before then, you can do anything you want with the buffer.

There is a difference between a ConvolverNode and an AudioBufferSourceNode---convolvers don't have a start method so when you assign the buffer to the convolver, the contents are required then. For an ABSN, the contents should not be acquired until start is called.

Chrome has not implemented this part of the spec for ConvolverNodes or AudioBufferSourceNodes.

guest271314 commented 4 years ago

My understanding is that you MUST call start() to acquire the contents. Before then, you can do anything you want with the buffer.

That appears to be the intent. The Note does not state how contents are acquired re copyToChannel() given that is the method mentioned.

For an ABSN, the contents should not be acquired until start is called.

Contents are acquired before start() is called at Nightly 80 when ABSN.buffer = buffer.

Chrome has not implemented this part of the spec for ConvolverNodes or AudioBufferSourceNodes.

In this case Chromium behaviour is consistent with the specification.

guest271314 commented 4 years ago

What does ABSN.buffer = buffer alone do?

rtoy commented 4 years ago

I think the intent is that once the contents are acquired, copyToChannel doesn't modify the AudioBuffer.

ABSN.buffer = buffer just tells the ABSN to use that buffer as the source of samples to be played. If that's what you're asking.

guest271314 commented 4 years ago

Yes. That is exactly what am asking. Which means Nightly 80 is in compliance with the specification while Chromium is not, which is what you appear to have alluded to earlier. And is the reason for this issue.

Is that restriction clear in the specification?

rtoy commented 4 years ago

Your comment inhttps://github.com/WebAudio/web-audio-api/issues/2227#issuecomment-664730913 says the assigning the buffer to an ABSN acquires the content although start() hasn't been called for Firefox. I don't think that's right. Acquiring the contents can only happen when start() is called.

This means you can assign the AudioBuffer to the source and call copyToChannel and getChannelData as many times as you want to modify the contents of the AudioBuffer. But once you call start(), the contents are acquired and these no longer modify the buffer.

Chrome doesn't acquire the contents in any situation for an AudioBufferSourceNode (or ConvolverNode).

guest271314 commented 4 years ago

Well

Your comment inhttps://github.com//issues/2227#issuecomment-664730913 says the assigning the buffer to an ABSN acquires the content although start() hasn't been called for Firefox. I don't think that's right. Acquiring the contents can only happen when start() is called.

This means you can assign the AudioBuffer to the source and call copyToChannel and getChannelData as many times as you want to modify the contents of the AudioBuffer. But once you call start(), the contents are acquired and these no longer modify the buffer.

is inconsistent with

I think the intent is that once the contents are acquired, copyToChannel doesn't modify the AudioBuffer. ABSN.buffer = buffer just tells the ABSN to use that buffer as the source of samples to be played.

thus this issue.

If your run the code you can apply your analysis to the results, then re-read the specification, where the specification, from perspective here, is not clear exactly what MUST happen when

So far, it is not immediately clear which implementation is behaving in conformance with the specification, whether or not that is due to the specification being clear or not is perhaps a historical question re changes to the specification and implementation that might have implemented one change but not a subsequent change.

What is clear is that the specification is not clear in this regard. If Chromium implementation is correct, then say that. If Firefox implementation is correct, say that. If neither are correct, and the specification does not provide adequate clear guidance to avoid ambiguity, then fix them all - and while doing so be clear, here and now, given the current technology, e.g., writing out a SharedArrayBuffer, growable source, if applicable.

At the minimum, resolve the questions posed by running the code then comparing what the specification states, as did here, and as yet cannot definitively cite the specification as to what MUST occur in the above cases.

rtoy commented 4 years ago

I was only stating what I vaguely remember about the old spec conversation on how this works. The spec should be clarified if needed.

guest271314 commented 4 years ago

As stated earlier, just happended to encounter this issue when attempting to solve or create workarounds for an issue with a different API that does use Web Audio API. One simple way to describe the issue in code is, from the Firefox bug report

Just happened to come across this issue while creating a workaround for an issue with a different API. Am not sure if Nightly behaviour is correct or not per the specification because the specification is not clear from perspective here, https://bugzilla.mozilla.org/show_bug.cgi?id=1655336#c5

Per the Web Audio API specification should content from copyToChannel() be output with this code

        buffer.copyToChannel(result[0], 0, 0);
        buffer.copyToChannel(result[1], 1, 0);
        console.log(result[0].every((f, i) => f === buffer.getChannelData(0)[i]), result[1].every((f, i) => f === 
        buffer.getChannelData(1)[i]));
        source.buffer = buffer;

and conversely not with this code

        source.buffer = buffer;
        buffer.copyToChannel(result[0], 0, 0);
        buffer.copyToChannel(result[1], 1, 0);
        console.log(result[0].every((f, i) => f === buffer.getChannelData(0)[i]), result[1].every((f, i) => f === 
        buffer.getChannelData(1)[i]));

If the specification currently addresses those cases, unambiguously, can you kindly direct to that language?

As an aside, Chromium not infrequently crashes when using AudioWorklet to stream thousands of local files from main thread to AudioWorkletProcessor using Native File System - without a single Uint8Array or SharedArrayBuffer or other fixed-length TypedArray as backing. Saved a couple version of the code that crashes. The one clue that have to that and possibly why Float32Arrays were being set with length set outside of range when cache was diasabled, and a Map was used, is from https://github.com/guest271314/proposal-resizablearraybuffer#motivation-and-use-cases

Growing a new buffer right now requires allocating a new buffer and copying. Not only is this inefficient, it needlessly fragments the address space on 32-bit systems.

It is up to you or other personnel of the repository under the umbrella of the parent corporation to decide what is needed. Have already concluded here the specification is not clear in the specifics delineated above.

So far that conclusion has not been refuted. Even if the conclusion was attempted to be refuted with reference to intent, that might be the case, as committee notes are reviewed when constructing a word or term - in order to reach an unambiguous definition - unless no definition can survive scrutiny, then the term is vague.

padenot commented 4 years ago

This seem to be a bug in Firefox.

padenot commented 4 years ago

The note at the bottom of getChannelData also is imprecise: the buffer can be changed, but after the content has been acquired, on a buffer, then this change of content of the buffer will not be reflected in the output of the AudioBufferSourceNode.

rtoy commented 4 years ago

But, IIUC, the AudioBuffer is actually changed. That change is not reflected in what the ABSN plays.

guest271314 commented 4 years ago

Note, it appears that WebCodecs uses Web Audio API AudioBuffer https://bugs.chromium.org/p/chromium/issues/detail?id=1094087#c6

EncodedAudioConfig: use names from WebAudio

We use WebAudio's AudioBuffer and generally intend to integrate with
that API. So use the same names as they use in AudioBufferOptions.

so the language here relevant to changing contents of an AudioBuffer impacts directly WebCodecs.

rtoy commented 4 years ago

Per today's teleconf, I'm going over this issue again.

The algorithm for setting the buffer says the contents are acquired if you set the buffer and start() has already been called. This makes sense.

The section on start() doesn't anything about acquiring the contents. It probably should. Maybe it should link to acquiring the contents of an AudioBuffer. The steps there clearly say that calling start() acquires the contents of the buffer. It also handles the case mentioned in setting the buffer mentioned above.

In summary, I think the spec says everything it needs to say, except we should have start() explicitly say it acquires the content.

And as @padenot says in https://github.com/WebAudio/web-audio-api/issues/2227#issuecomment-666512458, Firefox has a bug here. While Chrome appears to work, it doesn't acquire the contents and modifying the contents of the buffer may be reflected in playback. That's a bug in Chrome.

guest271314 commented 4 years ago

@rtoy What is the resolution relevant to copyToChannel() being used before and after source.buffer = buffer?

rtoy commented 4 years ago

If you call copyToChannel before start, the AudioBufferSourceNode using that AudioBuffer will play what you copied.

If you call it after calling start, the changes will not be reflected in what is played out because the AudioBufferSourceNode acquired the contents of the buffer. The contents of the AudioBuffer will be changed; but the changes won't be reflected in the audio being played. (I think that's how it's supposed to work.)