Bug 174508 - Increase CoreAudio render audio buffer sizes for WebRTC
Summary: Increase CoreAudio render audio buffer sizes for WebRTC
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-07-14 09:58 PDT by youenn fablet
Modified: 2017-07-14 12:34 PDT (History)
4 users (show)

See Also:


Attachments
Patch (3.17 KB, patch)
2017-07-14 09:59 PDT, youenn fablet
no flags Details | Formatted Diff | Diff
Patch for landing (1.86 KB, patch)
2017-07-14 10:54 PDT, youenn fablet
no flags Details | Formatted Diff | Diff
Patch for landing (1.86 KB, patch)
2017-07-14 11:05 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-07-14 09:58:08 PDT
This will increase a bit latency but will make audio rendering more stable
Comment 1 youenn fablet 2017-07-14 09:59:32 PDT
Created attachment 315437 [details]
Patch
Comment 2 Radar WebKit Bug Importer 2017-07-14 10:00:02 PDT
<rdar://problem/33318418>
Comment 3 Eric Carlson 2017-07-14 10:18:18 PDT
Comment on attachment 315437 [details]
Patch

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

> Source/WebCore/platform/mediastream/mac/CoreAudioCaptureSource.cpp:226
> +    return AudioSession::sharedSession().sampleRate() / 100;

This will result in a buffer size of 80 for 8K which is definitely too small. 50 might be better.

> Source/WebCore/platform/mediastream/mac/CoreAudioCaptureSource.cpp:304
> +    // For the moment we do not need to configure speaker proc as we ar not providing any reference data.

Nit: "we ar not" => "we are not"
Comment 4 youenn fablet 2017-07-14 10:27:51 PDT
Thanks for the review.

(In reply to Eric Carlson from comment #3)
> Comment on attachment 315437 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=315437&action=review
> 
> > Source/WebCore/platform/mediastream/mac/CoreAudioCaptureSource.cpp:226
> > +    return AudioSession::sharedSession().sampleRate() / 100;
> 
> This will result in a buffer size of 80 for 8K which is definitely too
> small. 50 might be better.

How about picking the max of AudioSession::sharedSession().bufferSize() and 10ms?

> 
> > Source/WebCore/platform/mediastream/mac/CoreAudioCaptureSource.cpp:304
> > +    // For the moment we do not need to configure speaker proc as we ar not providing any reference data.
> 
> Nit: "we ar not" => "we are not"

ok
Comment 5 youenn fablet 2017-07-14 10:54:14 PDT
Created attachment 315447 [details]
Patch for landing
Comment 6 youenn fablet 2017-07-14 10:57:15 PDT
(In reply to youenn fablet from comment #5)
> Created attachment 315447 [details]
> Patch for landing

I removed the CoreAudioCaptureSource changes.
I'll upload a separate patch removing speaker configuration
Comment 7 youenn fablet 2017-07-14 11:05:13 PDT
Created attachment 315452 [details]
Patch for landing
Comment 8 WebKit Commit Bot 2017-07-14 11:52:46 PDT
The commit-queue encountered the following flaky tests while processing attachment 315452 [details]:

storage/websql/database-lock-after-reload.html bug 174447 (authors: jsbell@chromium.org and rniwa@webkit.org)
The commit-queue is continuing to process your patch.
Comment 9 WebKit Commit Bot 2017-07-14 11:52:52 PDT
The commit-queue encountered the following flaky tests while processing attachment 315452 [details]:

storage/indexeddb/modern/new-database-after-user-delete.html bug 174520 (author: beidson@apple.com)
The commit-queue is continuing to process your patch.
Comment 10 WebKit Commit Bot 2017-07-14 12:34:36 PDT
Comment on attachment 315452 [details]
Patch for landing

Clearing flags on attachment: 315452

Committed r219517: <http://trac.webkit.org/changeset/219517>
Comment 11 WebKit Commit Bot 2017-07-14 12:34:38 PDT
All reviewed patches have been landed.  Closing bug.