Re: WaveShaperNode.curve

On Thu, May 16, 2013 at 10:16 AM, Srikumar Karaikudi Subramanian <
srikumarks@gmail.com> wrote:

> To recap, here are what I understand to be the core issues with passing
> buffers to nodes of various types, including buffer source nodes, wave
> shaper nodes, convolver nodes, etc. - (please fix if you see any
> misunderstanding or omissions) -
>
>    1. Race conditions should not be possible. Setting a buffer parameter
>    of a node and subsequent modification of that buffer should not affect the
>    behaviour of the node in order for the system to not be racy (note: whether
>    to allow that modification is an orthogonal issue). Current opinion (Robert
>    O' Callahan and Ehsan Akhgari) is that the ideal behaviour in this case is
>    that subsequent modification of the buffer should fail dramatically instead
>    of subtly.
>    2. Backward compatibility needs to be maintained as much as possible -
>    current styles of usage shouldn't break.
>
> The current standing suggestion is to neuter ArrayBuffer and Float32Arrays
> passed to nodes, to prevent them from being modified further. The advantage
> of this is that race conditions are avoided, and it can be arranged such
> that current interfaces aren't broken (by, for example, having getters
> return copies). However, buffers can no longer be shared between nodes. The
> disadvantage is that different nodes now MUST use different copies of a
> buffer, which increases demand on memory and decreases programming
> convenience.
>
> Another possibility -
>
> The proper solution to resolving race conditions due to data structures
> shared between threads is to use immutable data structures. The only binary
> data structure in Javascript that is immutable is the Blob (afaik). If the
> various nodes that currently use Float32Array buffer attributes are spec'd
> to accept Blob objects instead, perhaps with mime type
> "audio/x-raw;format=float32", we solve all race conditions. Data stored in
> a Blob object can now be safely shared between nodes. Implementations are
> free to choose whether to keep internal copies in nodes or not.
>
> However, current code would break. A Blob object "b" can be created from a
> Float32Array "arr" like this -
>
> var b = new Blob([arr], {type: "audio/x-raw;format=float32"});
>
> If a Float32Array attribute -- ex: wave shaper node's 'curve' attribute --
> is implemented to a) accept Blob buffers and b) when a Float32Array is
> assigned to it, it is first converted into a Blob, which is subsequently
> returned by the attribute's getter, we get compatibility with current code
> base.
>


Would that not break code which expects to get a Float32Array out of the
getter?  Also, there are two other complications with that solution:

* I'm not sure if it's current possible to express attributes with one type
in the setter and another type in the getter.
* I don't see how that avoid copying in the common case of just setting the
property to a Float32Array.  Blobs are not magically immutable, they store
a copy of the array that you pass them.


> The following kinds of code will work correctly without race conditions -
>
> var shaper1 = context.createWaveShaper(...);
> var shaper2 = context.createWaveShaper(...);
> var buffer = new Float32Buffer(..); // fill up with something.
>
> shaper1.curve = buffer; // First internal copy made.
> shaper2.curve = buffer; // Second internal copy made.
> shaper1.curve = shaper2.curve = buffer; // One internal copy made in
> shaper2 which is shared with shaper1.
>
> var b = new Blob([buffer], {type: "audio/x-raw;format=float32"}); // Copy
> of buffer made. buffer is free to be GC'd if no other use for it.
> shaper1.curve = shaper2.curve = b; // Shared immutable buffer. No race
> conditions.
> shaper1.curve = b; // Shared immutable buffer.
> shaper2.curve = b; // Shared immutable buffer.
> shaper3.curve = b.slice(64, 128, b.type); // Shared slice of immutable
> buffer. Also no race condition.
>
>
> The following kind of code will fail dramatically -
>
> var shaper = context.createWaveShaper(...);
> var buf = new Float32Buffer(100);
> shaper.curve = buf; // Internal copy made.
> for (var i = 0; i < 100; ++i) {
> shaper.curve[i] = something; // Array-style indexing into Blob will fail.
> }
>
>
Well, it seems like you're breaking backwards compatibility in a much worse
way.  As far as I know, there is no other API on the web platform which
returns a different type of object from the setter.


> The following kind of code is probably irrelevant since I don't expect
> anyone (yet) to have set a buffer without filling it first -
>
> var shaper = context.createWaveShaper(...);
> var buf = new Float32Buffer(100);
> shaper.curve = buf; // Internal copy made.
> for (var i = 0; i < 100; ++i) {
> buff[i] = something; // Modifications to buff have no effect.
> }
>
>
Well, you would be surprised to see the types of mistakes like the above
that browser vendors need to deal with.  :-)  The fact that this code will
work without throwing and does not do what the author intends is a
problem.  Basically, if shaper was a regular JS object, then this code
_would_ do what the author intended...


I'd like to wait for crogers to chime in at this point, I think we've
enumerated the possible alternatives here pretty well!



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