Bug 46286 – Add AudioGainNode files

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Chris Rogers]

Add AudioGainNode files

[Chris Rogers]

Created attachment 68408 [details]
Patch

[Chris Rogers]

This is the implementation for AudioGainNode as described in the web audio API:
http://chromium.googlecode.com/svn/trunk/samples/audio/specification/specification.html#AudioGainNode-section

[Chris Rogers]

Created attachment 68731 [details]
Patch

[Kenneth Russell]

Comment on attachment 68731 [details]
Patch

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

Basically looks good. Couple of questions. Also, I'm pretty sure m_isInitialized in AudioNode needs to be made volatile, since it's tested out from under the cover of the lock in some places.

> WebCore/webaudio/AudioGainNode.h:52
> +    virtual void process(size_t /*framesToProcess*/);

I'm not sure the commented-out argument name is WebKit style. Since the name is significant it's probably worth just using it.

> WebCore/webaudio/AudioGainNode.h:62
> +    void setGain(PassRefPtr<AudioGain> gain) { m_gain = gain; }

Since the gain attribute in the IDL is readonly, and m_gain is initialized in the constructor, does setGain need to be exposed at all?

> WebCore/webaudio/AudioGainNode.idl:34
> +        readonly attribute AudioGain gain;

The Web Audio API doesn't say this attribute is readonly.

[Chris Rogers]

Created attachment 69121 [details]
Patch

[Chris Rogers]

Comment on attachment 68731 [details]
Patch

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

>> WebCore/webaudio/AudioGainNode.h:52
>> +    virtual void process(size_t /*framesToProcess*/);
> 
> I'm not sure the commented-out argument name is WebKit style. Since the name is significant it's probably worth just using it.

FIXED

>> WebCore/webaudio/AudioGainNode.h:62
>> +    void setGain(PassRefPtr<AudioGain> gain) { m_gain = gain; }
> 
> Since the gain attribute in the IDL is readonly, and m_gain is initialized in the constructor, does setGain need to be exposed at all?

FIXED

>> WebCore/webaudio/AudioGainNode.idl:34
>> +        readonly attribute AudioGain gain;
> 
> The Web Audio API doesn't say this attribute is readonly.

I'ved added a FIXME here to keep it readonly for now, until proper thread safety can be added.  Although there are applications for writing this attribute, they're more advanced use cases.

[Kenneth Russell]

Comment on attachment 69121 [details]
Patch

Looks good.

[WebKit Commit Bot]

Comment on attachment 69121 [details]
Patch

Clearing flags on attachment: 69121

Committed r68734: <http://trac.webkit.org/changeset/68734>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.