Bug 83610 - AudioParam must support fan-in (multiple audio connections)
: AudioParam must support fan-in (multiple audio connections)
Status: RESOLVED FIXED
: WebKit
Web Audio
: 528+ (Nightly build)
: Unspecified Unspecified
: P2 Normal
Assigned To:
:
:
: 86710
:
  Show dependency treegraph
 
Reported: 2012-04-10 13:09 PST by
Modified: 2012-05-23 14:45 PST (History)


Attachments
Patch (15.29 KB, patch)
2012-05-15 18:36 PST, Chris Rogers
no flags Review Patch | Details | Formatted Diff | Diff
Patch (15.41 KB, patch)
2012-05-23 11:47 PST, Chris Rogers
no flags Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2012-04-10 13:09:31 PST
AudioNodes already support multiple connections from diverse sources (with implicit unity-gain summing).
When it becomes possible to connect an AudioNode to an AudioParam, then we should similarly support multiple connections with implicit summing.

This depends on:
https://bugs.webkit.org/show_bug.cgi?id=83524
------- Comment #1 From 2012-05-15 18:36:36 PST -------
Created an attachment (id=142120) [details]
Patch
------- Comment #2 From 2012-05-15 20:41:05 PST -------
(From update of attachment 142120 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=142120&action=review

Looks good to me overall. Couple of minor comments.

> LayoutTests/webaudio/audioparam-connect-audioratesignal.html:92
> +    gainNode.gain.value = 0;

Is this change really related to this patch? If not, I suggest splitting it out. If so, will existing Web Audio apps need to be updated?

> LayoutTests/webaudio/audioparam-summingjunction.html:97
> +    // Create 2st buffer used to control gain (a simple sine wave tone).

2st -> 2nd
------- Comment #3 From 2012-05-16 18:13:34 PST -------
Committed r117372: <http://trac.webkit.org/changeset/117372>
------- Comment #4 From 2012-05-16 18:17:41 PST -------
(In reply to comment #2)
> (From update of attachment 142120 [details] [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=142120&action=review
> 
> Looks good to me overall. Couple of minor comments.
> 
> > LayoutTests/webaudio/audioparam-connect-audioratesignal.html:92
> > +    gainNode.gain.value = 0;
> 
> Is this change really related to this patch? If not, I suggest splitting it out. If so, will existing Web Audio apps need to be updated?

Yes, the behavior needs to be changed to this -- it's the "correct" thing to do.

The audio-rate control of AudioParams is very new (within the last couple of weeks) and is a bit more specialized, so I'm not concerned about breaking existing content here.

> 
> > LayoutTests/webaudio/audioparam-summingjunction.html:97
> > +    // Create 2st buffer used to control gain (a simple sine wave tone).
> 
> 2st -> 2nd

FIXED
------- Comment #5 From 2012-05-16 23:41:38 PST -------
Re-opened since this is blocked by 86710
------- Comment #6 From 2012-05-23 11:47:25 PST -------
Created an attachment (id=143605) [details]
Patch
------- Comment #7 From 2012-05-23 11:49:21 PST -------
Hi Ken, I'm hoping to reland the original patch (which was reverted) now that a fix has been commited in:
http://trac.webkit.org/changeset/118099
------- Comment #8 From 2012-05-23 13:51:58 PST -------
(From update of attachment 143605 [details])
Looks fine as far as I can tell from code inspection.
------- Comment #9 From 2012-05-23 14:45:32 PST -------
(From update of attachment 143605 [details])
Clearing flags on attachment: 143605

Committed r118247: <http://trac.webkit.org/changeset/118247>
------- Comment #10 From 2012-05-23 14:45:37 PST -------
All reviewed patches have been landed.  Closing bug.