Re: New proposal for fixing race conditions

On Fri, Jul 19, 2013 at 2:54 PM, Chris Rogers <crogers@google.com> wrote:

> Jer, thanks for sharing the proposal.
>
> I can't support a solution like this because it:
>
> 1. Forces the need for copies both for setting and reading data.  PCM data
> is large and it's inefficient to be required to copy.
>

Can you please be more specific?  What common operations do you think would
require a copy with this proposal?


> 2. It needlessly complicates the API, where we now have a simple solution.
>

Hmm, again can you please be more specific about the complications you're
mentioning?  To my eyes, this API uses more webby structures such as a
constructor, and uses better names for its methods.  It eliminates the
|node.getChannelData(i)[j]| pattern which is quite strange compared to
other Web APIs.


> 3. The "problem" it is supposed to solve is effectively a non-issue.
>

I'm starting to doubt that we are going to be able to convince you
otherwise on this issue, unfortunately.  We have demonstrated test cases,
and explained over and over how all other Web APIs have protect against
this problem.  I will stop trying to repeat the same arguments at this
point.  But please let's continue the discussion on the other two points.

Thanks!
--
Ehsan
<http://ehsanakhgari.org/>




> Chris
>
>
>
> On Thu, Jul 18, 2013 at 1:02 PM, Jer Noble <jer.noble@apple.com> wrote:
>
>> Overview
>>
>> In order to solve the "data race" issue in WebAudio (bug 22725<https://www.w3.org/Bugs/Public/show_bug.cgi?id=22725>),
>> this proposal restricts access to the internal PCM data of an AudioBuffer.
>> This does not require AudioBuffers to be immutable, but that modifying
>> their contents occurs through an API which enforces no-data-race semantics.
>>
>> Since this will involve breaking API changes, this proposal incorporates
>> suggestions from Issue 48<https://www.w3.org/Bugs/Public/show_bug.cgi?id=17341> to
>> simplify the AudioBuffer interface, and will also adopt a constructor, as
>> opposed to a factory method.
>>
>> The new constructor provides an upgrade path for existing pages while not
>> breaking those pages. The factory method can continue to create the
>> "legacy" AudioBuffer in prefixed implementations.
>>
>> *Note:* I still prefer the "Immutable ArrayBuffer" proposal.
>> New interfacesAudioBufferChannel
>>
>> This interface represents a single channel of an AudioBuffer.
>>
>> interface AudioBufferChannel {
>>     readonly attribute unsigned long length;
>>
>>     void set(Float32Array array, optional unsigned long offset);
>>     Float32Array slice(long begin, optional long end);
>> };
>>
>> Attributes
>>
>> length
>>
>>    - Length of the PCM audio data in sample-frames.
>>
>> Methods
>>
>> set
>>
>>    -
>>
>>    Copies the Float32Array representing the PCM audio data.
>>    -
>>
>>    An INDEX_SIZE_ERR exception must be thrown if the length ofarray plus
>>    offset exceeds the length of the available audio data.
>>    -
>>
>>    An NO_MODIFICATION_ALLOWED_ERR exception must be thrown if the
>>    AudioBufferChannel is currently is use by a live AudioNode.
>>
>> slice
>>
>>    -
>>
>>    Returns a copy of the underlying PCM audio data. If eitherbegin or end is
>>    negative, it refers to an index from the end of the audio data.
>>    -
>>
>>    If end is unspecified, the copy will include all the data frombegin to
>>    the end of the audio data.
>>    -
>>
>>    The range specified by begin and end is clamped to the valid range
>>    for the audio data. If the computed length of the copied data would be
>>    negative, it is clamped to zero.
>>
>> AudioBuffer
>>
>> [Constructor(unsigned long numberOfChannels, unsigned long length, float sampleRate)]
>> interface AudioBuffer {
>>
>>     readonly attribute float sampleRate;
>>
>>     // in seconds
>>     readonly attribute double duration;
>>
>>     readonly attribute AudioBufferChannel[] channels;
>> };
>>
>> Attributes
>>
>> sampleRate
>>
>>    - The sample-rate for the PCM audio data in samples per second.
>>
>> duration
>>
>>    - Duration of the PCM audio data in seconds.
>>
>> channels
>>
>>    -
>>
>>    An array of AudioBufferChannel objects representing the PCM audio
>>    data for each audio channel.
>>    -
>>
>>    This array is of fixed length and is read-only.
>>
>> AlternativesImmutable ArrayBuffer
>>
>> To make ArrayBuffer immutable, the set() method would be removed from
>> ArrayBufferChannel.
>>
>> interface ArrayBufferChannel {
>>     readonly attribute unsigned long length;
>>     Float32Array slice(long begin, optional long end);
>> };
>>
>> This would require a different constructor method on ArrayBuffer.
>>
>> [Constructor(Float32Array[] arrays, float sampleRate)]
>>
>> This would also require the outputBuffer attribute on
>> AudioProcessingEvent to drop the 'readonly' declaration.
>>
>> partial interface AudioProcessingEvent {
>>     attribute AudioBuffer outputBuffer;
>> };
>>
>> Backwards Compatibility Not normative WebKitAudioBuffer
>>
>> interface WebKitAudioBuffer : AudioBuffer {
>>     readonly attribute length;
>>
>>     readonly attribute long numberOfChannels;
>>
>>     Float32Array getChannelData(unsigned long channel);
>> };
>>
>> WebKitAudioContext
>>
>> partial interface WebKitAudioContext {
>>     WebKitAudioBuffer createBuffer(unsigned long numberOfChannels, unsigned long length, float sampleRate);
>> };
>>
>> WebKit will continue to support WebKitAudioBuffer objects in the prefixed
>> version of the API for every attribute or parameter which currently
>> specifies AudioBuffer. WebKit will drop WebKitAudioBuffer support in the
>> unprefixed API.
>>
>
>