Bug 76394 – WebAudio: Optimize AudioBus::createByMixingToMono()

• Status: NEW
• Component: Web Audio

[Jer Noble]

WebAudio: Optimize AudioBus::createByMixingToMono()

[Jer Noble]

Created attachment 122664 [details]
Patch

[Chris Rogers]

Comment on attachment 122664 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=122664&action=review

> Source/WebCore/platform/audio/VectorMath.cpp:174
> +        *destP = **source1P + *source2P) * *scale;

** should be (*

[Chris Rogers]

Comment on attachment 122664 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=122664&action=review

> Source/WebCore/platform/audio/VectorMath.cpp:172
> +    int n = framesToProcess;

I would pull scale into a local here so you don't have to have a pointer de-ref inside the loop

float k = *scale;

and then use k below

[Jer Noble]

Created attachment 122675 [details]
Patch

[Chris Rogers]

Seems fine to me.

[Jer Noble]

Created attachment 132887 [details]
Incorrect patch, please ignore.

[Jer Noble]

Comment on attachment 132887 [details]
Incorrect patch, please ignore.

Uploaded patch to incorrect bug.  Please ignore.

[Jer Noble]

(In reply to comment #5)
> Seems fine to me.

Chris, could you give an official r+ now that you have reviewer powers?

[Chris Rogers]

Hi Jer, I'm confused.  Isn't this just an earlier version of:
https://bugs.webkit.org/show_bug.cgi?id=74553

which you just landed today?

[Jer Noble]

(In reply to comment #9)
> Hi Jer, I'm confused.  Isn't this just an earlier version of:
> https://bugs.webkit.org/show_bug.cgi?id=74553
> 
> which you just landed today?

No, this optimizes one path of AudioBus to use a vector operation instead of a for-loop.  It's orthogonal to the silence patch.

[Chris Rogers]

(In reply to comment #10)
> (In reply to comment #9)
> > Hi Jer, I'm confused.  Isn't this just an earlier version of:
> > https://bugs.webkit.org/show_bug.cgi?id=74553
> > 
> > which you just landed today?
> 
> No, this optimizes one path of AudioBus to use a vector operation instead of a for-loop.  It's orthogonal to the silence patch.

Sorry, still confused :)

The latest patch/attachment is dated 2012-03-20 and seems to be an earlier version of the silence-hint patch.  For, example the ChangeLog says:


12011-12-14  Jer Noble  <[e-mail]>

        WebAudio: propagate a silence hint through the AudioNode graph.
        https://bugs.webkit.org/show_bug.cgi?id=74553

Do you have a more recent patch I'm not seeing?

[Jer Noble]

(In reply to comment #11)
> Do you have a more recent patch I'm not seeing?

Ah yes.  Webkit-patch upload uploaded to the wrong bug.  See comment #7:  

"(From update of attachment 132887 [details])
Uploaded patch to incorrect bug.  Please ignore."

I've marked that incorrect patch as obsolete.  The patch to which I refer is this one: <https://bugs.webkit.org/attachment.cgi?id=122675&action=review>

[Chris Rogers]

(In reply to comment #12)
> (In reply to comment #11)
> > Do you have a more recent patch I'm not seeing?
> 
> Ah yes.  Webkit-patch upload uploaded to the wrong bug.  See comment #7:  
> 
> "(From update of attachment 132887 [details] [details])
> Uploaded patch to incorrect bug.  Please ignore."
> 
> I've marked that incorrect patch as obsolete.  The patch to which I refer is this one: <https://bugs.webkit.org/attachment.cgi?id=122675&action=review>

Could you please re-upload it to this bug?