175631 2017-08-16 10:44:04 -0700 [Mac/iOS] Increase the audio buffer size when audio capture is on but web audio is not used 2017-08-17 09:11:12 -0700 1 1 1 Unclassified WebKit Media WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 youennf youennf commit-queue eric.carlson oldest_to_newest 1339211 0 youennf 2017-08-16 10:44:04 -0700 Currently the buffer is set to 128 whenever web audio or audio capture is on. 1339213 1 318273 youennf 2017-08-16 10:49:46 -0700 Created attachment 318273 Patch 1339292 2 318273 eric.carlson 2017-08-16 14:23:35 -0700 Comment on attachment 318273 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). 1339299 3 eric.carlson 2017-08-16 14:43:22 -0700 <rdar://problem/33864721> 1339323 4 318295 youennf 2017-08-16 15:48:28 -0700 Created attachment 318295 Patch 1339325 5 youennf 2017-08-16 15:49:54 -0700 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. 1339580 6 318295 commit-queue 2017-08-17 09:11:10 -0700 Comment on attachment 318295 Patch Clearing flags on attachment: 318295 Committed r220859: <http://trac.webkit.org/changeset/220859> 1339581 7 commit-queue 2017-08-17 09:11:12 -0700 All reviewed patches have been landed. Closing bug. 318273 2017-08-16 10:49:46 -0700 2017-08-16 15:48:27 -0700 Patch bug-175631-20170816104946.patch text/plain 2263 youennf U3VidmVyc2lvbiBSZXZpc2lvbjogMjIwNzk1CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggZTA5NzBjMWU1NmExYTMx ZjJhYTMwMTg5NWNmNDFjNjE5NjNlZTZkMy4uNzJjZTQ4YzE3OTE3ODNmMDkzMTBmNjlhYmJmYzA1 ZWE5NWYxOGY1YyAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDEzIEBACisyMDE3LTA4LTE2ICBZb3Vl bm4gRmFibGV0ICA8eW91ZW5uQGFwcGxlLmNvbT4KKworICAgICAgICBbTWFjL2lPU10gSW5jcmVh c2UgdGhlIGF1ZGlvIGJ1ZmZlciBzaXplIHdoZW4gYXVkaW8gY2FwdHVyZSBpcyBvbiBidXQgd2Vi IGF1ZGlvIGlzIG5vdCB1c2VkCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3df YnVnLmNnaT9pZD0xNzU2MzEKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4K KworICAgICAgICAqIHBsYXRmb3JtL2F1ZGlvL2NvY29hL01lZGlhU2Vzc2lvbk1hbmFnZXJDb2Nv YS5jcHA6CisgICAgICAgIChQbGF0Zm9ybU1lZGlhU2Vzc2lvbk1hbmFnZXI6OnVwZGF0ZVNlc3Np b25TdGF0ZSk6IENoYW5nZSB2YWx1ZSBmcm9tIDEyOCB0byB0aGUgZXF1aXZhbGVudCBvZiAyMG1z IHdoZW4gYXVkaW8gY2FwdHVyZSBoYXBwZW5zIGJ1dCBub3Qgd2ViIGF1ZGlvLgorCiAyMDE3LTA4 LTE1ICBZb3Vlbm4gRmFibGV0ICA8eW91ZW5uQGFwcGxlLmNvbT4KIAogICAgICAgICBbQ2FjaGUg QVBJXSBFbnN1cmUgUmVzb3VyY2VSZXNwb25zZSBpcyBub3QgbnVsbCB3aGVuIHJlZGlyZWN0ZWQv dGFpbnRpbmcvdHlwZSBmaWVsZHMgYXJlIHNldApkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUv cGxhdGZvcm0vYXVkaW8vY29jb2EvTWVkaWFTZXNzaW9uTWFuYWdlckNvY29hLmNwcCBiL1NvdXJj ZS9XZWJDb3JlL3BsYXRmb3JtL2F1ZGlvL2NvY29hL01lZGlhU2Vzc2lvbk1hbmFnZXJDb2NvYS5j cHAKaW5kZXggYjUyN2M1NWZiYjQ3YWQxY2Y2MzM0N2FiOTU3NWM3YTAyZmI3NzU3MC4uMjZkZjlk ZWI1N2JiOTI0Yzc3ZWM4ZTU0ZmI2MWU4NjBhMjkzYjdhMSAxMDA2NDQKLS0tIGEvU291cmNlL1dl YkNvcmUvcGxhdGZvcm0vYXVkaW8vY29jb2EvTWVkaWFTZXNzaW9uTWFuYWdlckNvY29hLmNwcAor KysgYi9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9hdWRpby9jb2NvYS9NZWRpYVNlc3Npb25NYW5h Z2VyQ29jb2EuY3BwCkBAIC00Miw4ICs0MiwxMCBAQCB2b2lkIFBsYXRmb3JtTWVkaWFTZXNzaW9u TWFuYWdlcjo6dXBkYXRlU2Vzc2lvblN0YXRlKCkKIHsKICAgICBMT0coTWVkaWEsICJQbGF0Zm9y bU1lZGlhU2Vzc2lvbk1hbmFnZXI6OnVwZGF0ZVNlc3Npb25TdGF0ZSgpIC0gdHlwZXM6IFZpZGVv KCVkKSwgQXVkaW8oJWQpLCBXZWJBdWRpbyglZCkiLCBjb3VudChQbGF0Zm9ybU1lZGlhU2Vzc2lv bjo6VmlkZW8pLCBjb3VudChQbGF0Zm9ybU1lZGlhU2Vzc2lvbjo6QXVkaW8pLCBjb3VudChQbGF0 Zm9ybU1lZGlhU2Vzc2lvbjo6V2ViQXVkaW8pKTsKIAotICAgIGlmIChoYXMoUGxhdGZvcm1NZWRp YVNlc3Npb246OldlYkF1ZGlvKSB8fCBoYXMoUGxhdGZvcm1NZWRpYVNlc3Npb246Ok1lZGlhU3Ry ZWFtQ2FwdHVyaW5nQXVkaW8pKQorICAgIGlmIChoYXMoUGxhdGZvcm1NZWRpYVNlc3Npb246Oldl YkF1ZGlvKSkKICAgICAgICAgQXVkaW9TZXNzaW9uOjpzaGFyZWRTZXNzaW9uKCkuc2V0UHJlZmVy cmVkQnVmZmVyU2l6ZShrV2ViQXVkaW9CdWZmZXJTaXplKTsKKyAgICBlbHNlIGlmIChoYXMoUGxh dGZvcm1NZWRpYVNlc3Npb246Ok1lZGlhU3RyZWFtQ2FwdHVyaW5nQXVkaW8pKQorICAgICAgICBB dWRpb1Nlc3Npb246OnNoYXJlZFNlc3Npb24oKS5zZXRQcmVmZXJyZWRCdWZmZXJTaXplKEF1ZGlv U2Vzc2lvbjo6c2hhcmVkU2Vzc2lvbigpLnNhbXBsZVJhdGUoKSAvIDUwKTsKICAgICBlbHNlIGlm ICgoaGFzKFBsYXRmb3JtTWVkaWFTZXNzaW9uOjpWaWRlbykgfHwgaGFzKFBsYXRmb3JtTWVkaWFT ZXNzaW9uOjpBdWRpbykpICYmIFNldHRpbmdzOjpsb3dQb3dlclZpZGVvQXVkaW9CdWZmZXJTaXpl RW5hYmxlZCgpKSB7CiAgICAgICAgIC8vIEZJWE1FOiA8aHR0cDovL3dlYmtpdC5vcmcvYi8xMTY3 MjU+IEZpZ3VyZSBvdXQgd2h5IGVuYWJsaW5nIHRoZSBjb2RlIGJlbG93CiAgICAgICAgIC8vIGNh dXNlcyBtZWRpYSBMYXlvdXRUZXN0cyB0byBmYWlsIG9uIDEwLjguCg== 318295 2017-08-16 15:48:28 -0700 2017-08-17 09:11:10 -0700 Patch bug-175631-20170816154828.patch text/plain 2512 youennf U3VidmVyc2lvbiBSZXZpc2lvbjogMjIwNzk1CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggZTA5NzBjMWU1NmExYTMx ZjJhYTMwMTg5NWNmNDFjNjE5NjNlZTZkMy4uOTA2MGJiMTJmMzZlMzQzNjBlMTAyMWU5N2E0ODhi Y2Q4NWYxOWE2MyAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDEzIEBACisyMDE3LTA4LTE2ICBZb3Vl bm4gRmFibGV0ICA8eW91ZW5uQGFwcGxlLmNvbT4KKworICAgICAgICBbTWFjL2lPU10gSW5jcmVh c2UgdGhlIGF1ZGlvIGJ1ZmZlciBzaXplIHdoZW4gYXVkaW8gY2FwdHVyZSBpcyBvbiBidXQgd2Vi IGF1ZGlvIGlzIG5vdCB1c2VkCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3df YnVnLmNnaT9pZD0xNzU2MzEKKworICAgICAgICBSZXZpZXdlZCBieSBFcmljIENhcmxzb24uCisK KyAgICAgICAgKiBwbGF0Zm9ybS9hdWRpby9jb2NvYS9NZWRpYVNlc3Npb25NYW5hZ2VyQ29jb2Eu Y3BwOgorICAgICAgICAoUGxhdGZvcm1NZWRpYVNlc3Npb25NYW5hZ2VyOjp1cGRhdGVTZXNzaW9u U3RhdGUpOiBDaGFuZ2UgdmFsdWUgZnJvbSAxMjggdG8gdGhlIGVxdWl2YWxlbnQgb2YgMjBtcyB3 aGVuIGF1ZGlvIGNhcHR1cmUgaGFwcGVucyBidXQgbm90IHdlYiBhdWRpby4KKwogMjAxNy0wOC0x NSAgWW91ZW5uIEZhYmxldCAgPHlvdWVubkBhcHBsZS5jb20+CiAKICAgICAgICAgW0NhY2hlIEFQ SV0gRW5zdXJlIFJlc291cmNlUmVzcG9uc2UgaXMgbm90IG51bGwgd2hlbiByZWRpcmVjdGVkL3Rh aW50aW5nL3R5cGUgZmllbGRzIGFyZSBzZXQKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL3Bs YXRmb3JtL2F1ZGlvL2NvY29hL01lZGlhU2Vzc2lvbk1hbmFnZXJDb2NvYS5jcHAgYi9Tb3VyY2Uv V2ViQ29yZS9wbGF0Zm9ybS9hdWRpby9jb2NvYS9NZWRpYVNlc3Npb25NYW5hZ2VyQ29jb2EuY3Bw CmluZGV4IGI1MjdjNTVmYmI0N2FkMWNmNjMzNDdhYjk1NzVjN2EwMmZiNzc1NzAuLmYyYTNlYWQ4 NmNhMTczM2FkNjFiNWIzZDQyZWM5ZjI4YzhiZDdjYTcgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJD b3JlL3BsYXRmb3JtL2F1ZGlvL2NvY29hL01lZGlhU2Vzc2lvbk1hbmFnZXJDb2NvYS5jcHAKKysr IGIvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vYXVkaW8vY29jb2EvTWVkaWFTZXNzaW9uTWFuYWdl ckNvY29hLmNwcApAQCAtNDIsOCArNDIsMTIgQEAgdm9pZCBQbGF0Zm9ybU1lZGlhU2Vzc2lvbk1h bmFnZXI6OnVwZGF0ZVNlc3Npb25TdGF0ZSgpCiB7CiAgICAgTE9HKE1lZGlhLCAiUGxhdGZvcm1N ZWRpYVNlc3Npb25NYW5hZ2VyOjp1cGRhdGVTZXNzaW9uU3RhdGUoKSAtIHR5cGVzOiBWaWRlbygl ZCksIEF1ZGlvKCVkKSwgV2ViQXVkaW8oJWQpIiwgY291bnQoUGxhdGZvcm1NZWRpYVNlc3Npb246 OlZpZGVvKSwgY291bnQoUGxhdGZvcm1NZWRpYVNlc3Npb246OkF1ZGlvKSwgY291bnQoUGxhdGZv cm1NZWRpYVNlc3Npb246OldlYkF1ZGlvKSk7CiAKLSAgICBpZiAoaGFzKFBsYXRmb3JtTWVkaWFT ZXNzaW9uOjpXZWJBdWRpbykgfHwgaGFzKFBsYXRmb3JtTWVkaWFTZXNzaW9uOjpNZWRpYVN0cmVh bUNhcHR1cmluZ0F1ZGlvKSkKKyAgICBpZiAoaGFzKFBsYXRmb3JtTWVkaWFTZXNzaW9uOjpXZWJB dWRpbykpCiAgICAgICAgIEF1ZGlvU2Vzc2lvbjo6c2hhcmVkU2Vzc2lvbigpLnNldFByZWZlcnJl ZEJ1ZmZlclNpemUoa1dlYkF1ZGlvQnVmZmVyU2l6ZSk7CisgICAgLy8gSW4gY2FzZSBvZiBhdWRp byBjYXB0dXJlLCB3ZSB3YW50IHRvIGdyYWIgMjAgbXMgY2h1bmtzIHRvIGxpbWl0IHRoZSBsYXRl bmN5IHNvIHRoYXQgaXQgaXMgbm90IG5vdGljZWFibGUgYnkgdXNlcnMKKyAgICAvLyB3aGlsZSBo YXZpbmcgYSBsYXJnZSBlbm91Z2ggYnVmZmVyIHNvIHRoYXQgdGhlIGF1ZGlvIHJlbmRlcmluZyBy ZW1haW5zIHN0YWJsZSwgaGVuY2UgYSBjb21wdXRhdGlvbiBiYXNlZCBvbiBzYW1wbGUgcmF0ZS4K KyAgICBlbHNlIGlmIChoYXMoUGxhdGZvcm1NZWRpYVNlc3Npb246Ok1lZGlhU3RyZWFtQ2FwdHVy aW5nQXVkaW8pKQorICAgICAgICBBdWRpb1Nlc3Npb246OnNoYXJlZFNlc3Npb24oKS5zZXRQcmVm ZXJyZWRCdWZmZXJTaXplKEF1ZGlvU2Vzc2lvbjo6c2hhcmVkU2Vzc2lvbigpLnNhbXBsZVJhdGUo KSAvIDUwKTsKICAgICBlbHNlIGlmICgoaGFzKFBsYXRmb3JtTWVkaWFTZXNzaW9uOjpWaWRlbykg fHwgaGFzKFBsYXRmb3JtTWVkaWFTZXNzaW9uOjpBdWRpbykpICYmIFNldHRpbmdzOjpsb3dQb3dl clZpZGVvQXVkaW9CdWZmZXJTaXplRW5hYmxlZCgpKSB7CiAgICAgICAgIC8vIEZJWE1FOiA8aHR0 cDovL3dlYmtpdC5vcmcvYi8xMTY3MjU+IEZpZ3VyZSBvdXQgd2h5IGVuYWJsaW5nIHRoZSBjb2Rl IGJlbG93CiAgICAgICAgIC8vIGNhdXNlcyBtZWRpYSBMYXlvdXRUZXN0cyB0byBmYWlsIG9uIDEw LjguCg==