Update AudioSession buffer size handling
rdar://76920375
Created attachment 428959 [details] Patch
Created attachment 428973 [details] Patch
Comment on attachment 428973 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=428973&action=review > Source/WebCore/platform/audio/cocoa/MediaSessionManagerCocoa.mm:96 > + ++webAudioCount; I am thinking whether we need to change ``` PlatformMediaSession::MediaType mediaType() const final { return isSuspended() ? PlatformMediaSession::MediaType::None : PlatformMediaSession::MediaType::WebAudio; } ``` to ``` PlatformMediaSession::MediaType mediaType() const final { return isStopped() ? PlatformMediaSession::MediaType::None : PlatformMediaSession::MediaType::WebAudio; } ``` If we make this change, then we don't need to change `MediaSessionManagerCocoa::updateSessionState()` here.
Comment on attachment 428973 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=428973&action=review >> Source/WebCore/platform/audio/cocoa/MediaSessionManagerCocoa.mm:96 >> + ++webAudioCount; > > I am thinking whether we need to change > ``` > PlatformMediaSession::MediaType mediaType() const final { return isSuspended() ? PlatformMediaSession::MediaType::None : PlatformMediaSession::MediaType::WebAudio; } > ``` > to > ``` > PlatformMediaSession::MediaType mediaType() const final { return isStopped() ? PlatformMediaSession::MediaType::None : PlatformMediaSession::MediaType::WebAudio; } > ``` > > If we make this change, then we don't need to change `MediaSessionManagerCocoa::updateSessionState()` here. I don't think that is sufficient because `AudioContext.isStopped()` returns true only when the context is stopped, and we should not set the buffer size to the render quantum size when an AudioContext is reachable but not currently playing. It also isn't sufficient because we want to reset the buffer size to the "low power" size when nothing at all is playing.
Comment on attachment 428973 [details] Patch Some API test failures seem related. View in context: https://bugs.webkit.org/attachment.cgi?id=428973&action=review > Source/WebCore/platform/audio/cocoa/MediaSessionManagerCocoa.mm:129 > + if (bufferSize) There will be cases where bufferSize will be 0, for instance when lowPowerVideoAudioBufferSizeEnabled and there are some sessions. Is it what we want?
Comment on attachment 428973 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=428973&action=review >> Source/WebCore/platform/audio/cocoa/MediaSessionManagerCocoa.mm:129 >> + if (bufferSize) > > There will be cases where bufferSize will be 0, for instance when lowPowerVideoAudioBufferSizeEnabled and there are some sessions. > Is it what we want? I'll update the patch to use the default size in this case.
Created attachment 429096 [details] Update patch
Comment on attachment 429096 [details] Update patch View in context: https://bugs.webkit.org/attachment.cgi?id=429096&action=review > Source/WebCore/platform/audio/cocoa/MediaSessionManagerCocoa.mm:97 > + ++webAudioCount; It seems some api tests are breaking, maybe due to that change? Worth checking whether we want to update the tests or keep status quo.
Created attachment 429233 [details] Patch
Committed r277876 (238014@main): <https://commits.webkit.org/238014@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 429233 [details].
Comment on attachment 429233 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=429233&action=review > Source/WebCore/platform/audio/cocoa/MediaSessionManagerCocoa.mm:202 > + m_supportedAudioHardwareBufferSizes = m_audioHardwareListener->supportedBufferSizes(); Kind of wish this and the one line in audioOutputDeviceChanged was the same. > Source/WebCore/testing/Internals.cpp:5447 > +#if USE(AUDIO_SESSION) > + return AudioSession::sharedSession().bufferSize(); > +#endif > + return 0; Please use #else here instead of two returns in a row.