Bug 175631 - [Mac/iOS] Increase the audio buffer size when audio capture is on but web audio is not used
Summary: [Mac/iOS] Increase the audio buffer size when audio capture is on but web aud...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: youenn fablet
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-08-16 10:44 PDT by youenn fablet
Modified: 2017-08-17 09:11 PDT (History)
2 users (show)

See Also:


Attachments
Patch (2.21 KB, patch)
2017-08-16 10:49 PDT, youenn fablet
no flags Details | Formatted Diff | Diff
Patch (2.45 KB, patch)
2017-08-16 15:48 PDT, youenn fablet
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.