[webkit-reviews] review granted: [Bug 93510] Implement new synchronized audio I/O render method in chromium : [Attachment 157277] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Aug 13 17:43:11 PDT 2012


Kenneth Russell <kbr at google.com> has granted Chris Rogers
<crogers at google.com>'s request for review:
Bug 93510: Implement new synchronized audio I/O render method in chromium
https://bugs.webkit.org/show_bug.cgi?id=93510

Attachment 157277: Patch
https://bugs.webkit.org/attachment.cgi?id=157277&action=review

------- Additional Comments from Kenneth Russell <kbr at google.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=157277&action=review


Looks generally fine -- r=me -- but a couple of comments which you can feel
free to ignore if they're off base.

> Source/WebKit/chromium/src/AudioDestinationChromium.cpp:92
> +	   AudioBus silence(2, renderBufferSize);

2? Really? Why not numberOfChannels?

> Source/WebKit/chromium/src/AudioDestinationChromium.cpp:139
> +	   AudioBus wrapperBus(2, numberOfFrames, false);

2 again? If this is a temporary hack until this code is generalized then add a
FIXME.

> Source/WebKit/chromium/src/AudioDestinationChromium.cpp:180
> +    // Render using the Web Audio engine.

Does this comment add any value?


More information about the webkit-reviews mailing list