Bug 174508

Summary: Increase CoreAudio render audio buffer sizes for WebRTC
Product: WebKit Reporter: youenn fablet <youennf>
Component: MediaAssignee: youenn fablet <youennf>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, eric.carlson, jer.noble, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch for landing
none
Patch for landing none

youenn fablet
Reported 2017-07-14 09:58:08 PDT
This will increase a bit latency but will make audio rendering more stable
Attachments
Patch (3.17 KB, patch)
2017-07-14 09:59 PDT, youenn fablet 		no flags
DetailsFormatted DiffDiff
Patch for landing (1.86 KB, patch)
2017-07-14 10:54 PDT, youenn fablet 		no flags
DetailsFormatted DiffDiff
Patch for landing (1.86 KB, patch)
2017-07-14 11:05 PDT, youenn fablet 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
youenn fablet
Comment 1 2017-07-14 09:59:32 PDT
Created attachment 315437 [details] Patch
Radar WebKit Bug Importer
Comment 2 2017-07-14 10:00:02 PDT
<rdar://problem/33318418>
Eric Carlson
Comment 3 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"
youenn fablet
Comment 4 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
youenn fablet
Comment 5 2017-07-14 10:54:14 PDT
Created attachment 315447 [details] Patch for landing
youenn fablet
Comment 6 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
youenn fablet
Comment 7 2017-07-14 11:05:13 PDT
Created attachment 315452 [details] Patch for landing
WebKit Commit Bot
Comment 8 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.
WebKit Commit Bot
Comment 9 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.
WebKit Commit Bot
Comment 10 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>
WebKit Commit Bot
Comment 11 2017-07-14 12:34:38 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.