Skip to content
Permalink
Browse files
Web audio rendering becomes garbled with switching from speakers to h…
…eadphones (and vice-versa)

https://bugs.webkit.org/show_bug.cgi?id=241223
rdar://94721398

Reviewed by Jer Noble.

Web Audio requires a rendering quantum of 128. As a result,
MediaSessionManagerCocoa::updateSessionState() always requests a buffer size of
128 if WebAudio is in use, by calling AudioSession::setPreferredBufferSize().

When plugging in the headphones, I noticed that the rendering quantum became
480 instead of 128, which was the cause of the garbled Web Audio rendering.
updateSessionState() was however called when plugging in the headphones and
did try to set the buffer size of 128. However,
AudioSessionMac::setPreferredBufferSize() would return early because
m_bufferSize was already 128. The issue was that m_bufferSize was stale
and causing the call to setPreferredBufferSize(128) to be incorrectly ignored.

Normally, when we initialize m_bufferSize, we set a property listener so that
we can update our cached m_bufferSize whenever the corresponding property on
the audio device changes. However, our listener was set on a particular audio
device. When plugging in the headphone, the default audio device would change
and our listener would still listen for changes on the old device. As a result,
m_bufferSize would not get updated even though the audio device (and thus the
buffer size) would change.

To address the issue, I now register a default audio device listener so that
we get notified whenever the default audio device changes. When it changes,
we do the following:
1. Unregister the property listeners we had on the previous audio device
2. Register the property listeners (the one we had) on the new device
3. If we have cached property values, call the change handler for these
   properties so that our cached values get updated.

Even though the bug was caused by the buffer size not being updated, the
same issue applied to the sample rate and the muted state. I fixed all
3.

* Source/WebCore/platform/audio/mac/AudioSessionMac.h:
* Source/WebCore/platform/audio/mac/AudioSessionMac.mm:
(WebCore::defaultDeviceWithoutCaching):
(WebCore::defaultDeviceTransportIsBluetooth):
(WebCore::AudioSessionMac::removePropertyListenersForDefaultDevice const):
(WebCore::AudioSessionMac::handleDefaultDeviceChange):
(WebCore::AudioSessionMac::defaultOutputDeviceAddress):
(WebCore::AudioSessionMac::addDefaultDeviceObserverIfNeeded const):
(WebCore::AudioSessionMac::nominalSampleRateAddress):
(WebCore::AudioSessionMac::addSampleRateObserverIfNeeded const):
(WebCore::AudioSessionMac::handleSampleRateChange):
(WebCore::AudioSessionMac::handleSampleRateChange const):
(WebCore::AudioSessionMac::bufferSizeAddress):
(WebCore::AudioSessionMac::addBufferSizeObserverIfNeeded const):
(WebCore::AudioSessionMac::handleBufferSizeChange):
(WebCore::AudioSessionMac::handleBufferSizeChange const):
(WebCore::AudioSessionMac::sampleRate const):
(WebCore::AudioSessionMac::sampleRateWithoutCaching const):
(WebCore::AudioSessionMac::bufferSize const):
(WebCore::AudioSessionMac::bufferSizeWithoutCaching const):
(WebCore::AudioSessionMac::defaultDevice const):
(WebCore::AudioSessionMac::setPreferredBufferSize):
(WebCore::AudioSessionMac::muteAddress):
(WebCore::AudioSessionMac::addConfigurationChangeObserver):
(WebCore::AudioSessionMac::removeConfigurationChangeObserver):
(WebCore::AudioSessionMac::addMuteChangeObserverIfNeeded const):
(WebCore::AudioSessionMac::removeMuteChangeObserverIfNeeded const):
(WebCore::defaultDevice): Deleted.
* Source/WebKit/WebProcess/GPU/media/RemoteAudioDestinationProxy.cpp:
(WebKit::RemoteAudioDestinationProxy::connection):

Canonical link: https://commits.webkit.org/256712@main
  • Loading branch information
cdumez committed Nov 15, 2022
1 parent 2dc9f3a commit 91ac45e95dc389df2d0ee9c5b3da6ac4f84d8d88
Show file tree
Hide file tree
Showing 2 changed files with 201 additions and 115 deletions.
@@ -28,6 +28,7 @@
#if USE(AUDIO_SESSION) && PLATFORM(MAC)

#include "AudioSession.h"
#include <pal/spi/cf/CoreAudioSPI.h>

typedef UInt32 AudioObjectID;
typedef struct AudioObjectPropertyAddress AudioObjectPropertyAddress;
@@ -42,11 +43,25 @@ class AudioSessionMac final : public AudioSession {
private:
void addSampleRateObserverIfNeeded() const;
void addBufferSizeObserverIfNeeded() const;
void addDefaultDeviceObserverIfNeeded() const;
void addMuteChangeObserverIfNeeded() const;
void removeMuteChangeObserverIfNeeded() const;

float sampleRateWithoutCaching() const;
std::optional<size_t> bufferSizeWithoutCaching() const;
void removePropertyListenersForDefaultDevice() const;

static OSStatus handleSampleRateChange(AudioObjectID, UInt32, const AudioObjectPropertyAddress*, void* inClientData);
void handleSampleRateChange() const;
static OSStatus handleBufferSizeChange(AudioObjectID, UInt32, const AudioObjectPropertyAddress*, void* inClientData);
void handleBufferSizeChange() const;
static OSStatus handleDefaultDeviceChange(AudioObjectID, UInt32, const AudioObjectPropertyAddress*, void* inClientData);

AudioDeviceID defaultDevice() const;
static const AudioObjectPropertyAddress& defaultOutputDeviceAddress();
static const AudioObjectPropertyAddress& nominalSampleRateAddress();
static const AudioObjectPropertyAddress& bufferSizeAddress();
static const AudioObjectPropertyAddress& muteAddress();

// AudioSession
CategoryType category() const final { return m_category; }
@@ -79,8 +94,11 @@ class AudioSessionMac final : public AudioSession {
#endif
mutable bool m_hasSampleRateObserver { false };
mutable bool m_hasBufferSizeObserver { false };
mutable bool m_hasDefaultDeviceObserver { false };
mutable bool m_hasMuteChangeObserver { false };
mutable std::optional<double> m_sampleRate;
mutable std::optional<size_t> m_bufferSize;
mutable std::optional<AudioDeviceID> m_defaultDevice;
};

}

0 comments on commit 91ac45e

Please sign in to comment.