Bug 175631

Summary: [Mac/iOS] Increase the audio buffer size when audio capture is on but web audio is not used
Product: WebKit Reporter: youenn fablet <youennf>
Component: MediaAssignee: youenn fablet <youennf>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, eric.carlson
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch none

Description youenn fablet 2017-08-16 10:44:04 PDT
Currently the buffer is set to 128 whenever web audio or audio capture is on.
Comment 1 youenn fablet 2017-08-16 10:49:46 PDT
Created attachment 318273 [details]
Patch
Comment 2 Eric Carlson 2017-08-16 14:23:35 PDT
Comment on attachment 318273 [details]
Patch

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

You should also remove the call to setPreferredBufferSize in AudioDestinationIOS::configure

> Source/WebCore/ChangeLog:9
> +        (PlatformMediaSessionManager::updateSessionState): Change value from 128 to the equivalent of 20ms when audio capture happens but not web audio.

Nit: this line should be wrapped.

> Source/WebCore/platform/audio/cocoa/MediaSessionManagerCocoa.cpp:48
> +        AudioSession::sharedSession().setPreferredBufferSize(AudioSession::sharedSession().sampleRate() / 50);

Why calculate the size based on the sample rate in only this case? I think we should either calculate them all, or set this one to 1024 (the nearest power of 2 above 20ms for 44K).
Comment 3 Eric Carlson 2017-08-16 14:43:22 PDT
<rdar://problem/33864721>
Comment 4 youenn fablet 2017-08-16 15:48:28 PDT
Created attachment 318295 [details]
Patch
Comment 5 youenn fablet 2017-08-16 15:49:54 PDT
Thanks for the review.

(In reply to Eric Carlson from comment #2)
> Comment on attachment 318273 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=318273&action=review
> 
> You should also remove the call to setPreferredBufferSize in
> AudioDestinationIOS::configure
> 
> > Source/WebCore/ChangeLog:9
> > +        (PlatformMediaSessionManager::updateSessionState): Change value from 128 to the equivalent of 20ms when audio capture happens but not web audio.
> 
> Nit: this line should be wrapped.

Done

> 
> > Source/WebCore/platform/audio/cocoa/MediaSessionManagerCocoa.cpp:48
> > +        AudioSession::sharedSession().setPreferredBufferSize(AudioSession::sharedSession().sampleRate() / 50);
> 
> Why calculate the size based on the sample rate in only this case? I think
> we should either calculate them all, or set this one to 1024 (the nearest
> power of 2 above 20ms for 44K).

Added a comment. For web audio we want to be close to the minimum buffer size no matter the sample rate, hence 128.
For gum, we want to be the biggest not noticeable size, which depends on the sample rate.
Comment 6 WebKit Commit Bot 2017-08-17 09:11:10 PDT
Comment on attachment 318295 [details]
Patch

Clearing flags on attachment: 318295

Committed r220859: <http://trac.webkit.org/changeset/220859>
Comment 7 WebKit Commit Bot 2017-08-17 09:11:12 PDT
All reviewed patches have been landed.  Closing bug.