[Webkit-unassigned] [Bug 34716] audio engine: audio output classes

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=34716


Adam Barth <abarth at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #48353|review?                     |review-
               Flag|                            |




--- Comment #4 from Adam Barth <abarth at webkit.org>  2010-03-09 09:11:40 PST ---
(From update of attachment 48353)
Sorry that this patch has been up for review for so long.  I'm not that
familiar with audio, so I'm going to give you a mostly syntactic / cultural
review of your patch.  Hopefully you'll find it helpful nonetheless.

+#define CheckError(error, operation)                \

I don't think we should have this macro.  If we decide we should have it, then
it should be defined somewhere more generic so that i can be used beyond the
audio code.

+        fprintf(stderr, "can't find audio output unit\n");

This isn't the way we usually handle errors liek this in WebCore.  If this
can't happen, I'd recommend an ASSERT.  If it can happen, we need to do
something more than printing to stderror.

+     AudioOutputMac* This = static_cast<AudioOutputMac*>(inRefCon);

Please use a different variable name than |This|.  The capitalization is a bit
too subtle.

+ static OSStatus inputProc(void* inRefCon,

WebCore doesn't have an 80 character line limit.  Although I think it's
prettier this way, we usually put all this code on the same line.

+ AudioUnitRenderActionFlags* ioActionFlags

We also omit formal parameter names when the names don't provide any additional
information.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.