[webkit-reviews] review granted: [Bug 116342] Mac: Set the default audio buffer size to a large value for <video> elements. : [Attachment 202259] Patch

[bugzilla-daemon at webkit.org]

Eric Carlson <eric.carlson at apple.com> has granted Jer Noble
<jer.noble at apple.com>'s request for review:
Bug 116342: Mac: Set the default audio buffer size to a large value for <video>
elements.
https://bugs.webkit.org/show_bug.cgi?id=116342

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

------- Additional Comments from Eric Carlson <eric.carlson at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=202259&action=review


> Source/WebCore/ChangeLog:9
> +	   requested buffer frame size to a large value, such as 4096. Since
this results

"such as 4096" - kPreferredLowPowerBufferSize is set to 4098?

> Source/WebCore/platform/audio/AudioSessionManager.cpp:61
> +    ASSERT(type >= 0);

Is "type >= 0" really useful? Asserting that it is one of the valid values
would be more complete - "ASSERT(type >= None && type <= WebAudio)"

> Source/WebCore/platform/audio/AudioSessionManager.cpp:67
> +    ASSERT(type >= 0);

Ditto.

> Source/WebCore/platform/audio/AudioSessionManager.cpp:74
> +    ASSERT(type >= 0);

Ditto.

> Source/WebCore/platform/audio/ios/AudioDestinationIOS.cpp:47
> +#if USE(AUDIO_SESSION)

Is it possible that we would ever want to build this without "AUDIO_SESSION"?
If not, remove the #ifs and let the build fail if someone tries.

> Source/WebCore/platform/audio/ios/AudioSessionIOS.mm:29
> +#if USE(AUDIO_SESSION) && PLATFORM(IOS)

Ditto.

> Source/WebCore/platform/audio/mac/AudioDestinationMac.cpp:41
> +#if USE(AUDIO_SESSION)

Ditto.

> Source/WebCore/platform/audio/mac/AudioDestinationMac.cpp:74
> +#if USE(AUDIO_SESSION)
> +    return AudioSession::sharedSession().sampleRate();
> +#else
> +    return 0;
> +#endif

OTOH, if it is possible that someone would want to build this without
AUDIO_SESSION shouldn't this include the old code in the #else?