[Webkit-unassigned] [Bug 46234] Add HRTFDatabaseLoader files

[bugzilla-daemon at webkit.org]

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


James Robinson <jamesr at chromium.org> changed:

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




--- Comment #9 from James Robinson <jamesr at chromium.org>  2010-09-27 16:50:05 PST ---
(From update of attachment 68981)
View in context: https://bugs.webkit.org/attachment.cgi?id=68981&action=review

Looking pretty good!  Thanks for the threading ASSERT()s, they make it much easier to reason about.

r- for nits

> WebCore/platform/audio/HRTFDatabaseLoader.cpp:65
> +    , m_databaseSampleRate(44100.0) // proper value gets set later

the constructor should just take the sample rate as a parameter since it's always set immediately after construction

> WebCore/platform/audio/HRTFDatabaseLoader.h:60
> +    static HRTFDatabaseLoader* s_loader; // singleton

should this be public?

> WebCore/platform/audio/HRTFDatabaseLoader.h:72
> +    mutable OwnPtr<HRTFDatabase> m_hrtfDatabase;

nit: it doesn't appear mutable is needed here

> WebCore/platform/audio/HRTFDatabaseLoader.h:81
> +// defaultHRTFDatabase() gives access to the loaded database.
> +// This can be called from any thread, but it is the callers responsibilty to call this while the context (and thus HRTFDatabaseLoader)
> +// is still alive.  Otherwise this will return 0.
> +extern HRTFDatabase* defaultHRTFDatabase();

This should be a static function on HRTFDatabaseLoader, no reason to put this directly in the WebCore namespace.

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