Skip to content

Commit

Permalink
Web Audio glitches when hardware buffer size is not divisible by 128
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=259899
<radar://112621241>

Reviewed by Chris Dumez.

Whenever the buffer size of the hardware was not a multiple of 128, the
audio would not be in sync with the client leading to audible artifacts.

We can fix this by passing the number of frames from AURemoteIO to the
web process and rendering all of the frames that AURemoteIO asked us to render.

* Source/WebKit/GPUProcess/media/RemoteAudioDestinationManager.cpp:
(WebKit::RemoteAudioDestinationManager::createAudioDestination):
Create a shared memory member variable which we can write the frame count from
AURemoteUI to.

* Source/WebKit/GPUProcess/media/RemoteAudioDestinationManager.h:
Update member function parameters.

* Source/WebKit/GPUProcess/media/RemoteAudioDestinationManager.messages.in:
Pass the shared memory handle to the web process.

* Source/WebKit/WebProcess/GPU/media/RemoteAudioDestinationProxy.cpp:
(WebKit::RemoteAudioDestinationProxy::startRenderingThread):
Get the number of frames to render from the shared memory with the GPU process.

(WebKit::RemoteAudioDestinationProxy::connection):
Create the shared memory handle in the web process and pass it to the GPU process.

We could create it the other way around, but it would require adding another IPC
message, which seems uncessary for this purpose.

* Source/WebKit/WebProcess/GPU/media/RemoteAudioDestinationProxy.h:
Add shared memory member variable.

Canonical link: https://commits.webkit.org/266747@main
  • Loading branch information
mwyrzykowski committed Aug 10, 2023
1 parent 612cb56 commit f35686d
Show file tree
Hide file tree
Showing 5 changed files with 48 additions and 15 deletions.
31 changes: 23 additions & 8 deletions Source/WebKit/GPUProcess/media/RemoteAudioDestinationManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,11 @@ class RemoteAudioDestination final
}

#if PLATFORM(COCOA)
void setSharedMemory(SharedMemory::Handle&& handle)
{
m_frameCount = SharedMemory::map(WTFMove(handle), SharedMemory::Protection::ReadWrite);
}

void audioSamplesStorageChanged(ConsumerSharedCARingBuffer::Handle&& handle)
{
if (m_isPlaying) {
Expand Down Expand Up @@ -132,6 +137,15 @@ class RemoteAudioDestination final

private:
#if PLATFORM(COCOA)
void incrementTotalFrameCount(UInt32 numberOfFrames)
{
static_assert(std::atomic<UInt32>::is_always_lock_free, "Shared memory atomic usage assumes lock free primitives are used");
if (m_frameCount) {
RELEASE_ASSERT(m_frameCount->size() == sizeof(std::atomic<uint32_t>));
WTF::atomicExchangeAdd(static_cast<uint32_t*>(m_frameCount->data()), numberOfFrames);
}
}

OSStatus render(double sampleTime, uint64_t hostTime, UInt32 numberOfFrames, AudioBufferList* ioData)
{
ASSERT(!isMainRunLoop());
Expand All @@ -142,12 +156,8 @@ class RemoteAudioDestination final
status = noErr;
}

unsigned requestedSamplesCount = m_extraRequestedFrames;
for (; requestedSamplesCount < numberOfFrames; requestedSamplesCount += WebCore::AudioUtilities::renderQuantumSize) {
// Ask the audio thread in the WebContent process to render a quantum.
m_renderSemaphore.signal();
}
m_extraRequestedFrames = requestedSamplesCount - numberOfFrames;
incrementTotalFrameCount(numberOfFrames);
m_renderSemaphore.signal();

return status;
}
Expand All @@ -157,10 +167,10 @@ class RemoteAudioDestination final

#if PLATFORM(COCOA)
WebCore::AudioOutputUnitAdaptor m_audioOutputUnitAdaptor;
RefPtr<SharedMemory> m_frameCount;
const uint32_t m_numOutputChannels;
std::unique_ptr<ConsumerSharedCARingBuffer> m_ringBuffer;
uint64_t m_startFrame { 0 };
unsigned m_extraRequestedFrames { 0 };
#endif
};

Expand All @@ -171,11 +181,16 @@ RemoteAudioDestinationManager::RemoteAudioDestinationManager(GPUConnectionToWebP

RemoteAudioDestinationManager::~RemoteAudioDestinationManager() = default;

void RemoteAudioDestinationManager::createAudioDestination(RemoteAudioDestinationIdentifier identifier, const String& inputDeviceId, uint32_t numberOfInputChannels, uint32_t numberOfOutputChannels, float sampleRate, float hardwareSampleRate, IPC::Semaphore&& renderSemaphore)
void RemoteAudioDestinationManager::createAudioDestination(RemoteAudioDestinationIdentifier identifier, const String& inputDeviceId, uint32_t numberOfInputChannels, uint32_t numberOfOutputChannels, float sampleRate, float hardwareSampleRate, IPC::Semaphore&& renderSemaphore, SharedMemory::Handle&& handle)
{
MESSAGE_CHECK(!m_gpuConnectionToWebProcess.isLockdownModeEnabled(), "Received a createAudioDestination() message from a webpage in Lockdown mode.");

auto destination = makeUniqueRef<RemoteAudioDestination>(m_gpuConnectionToWebProcess, inputDeviceId, numberOfInputChannels, numberOfOutputChannels, sampleRate, hardwareSampleRate, WTFMove(renderSemaphore));
#if PLATFORM(COCOA)
destination->setSharedMemory(WTFMove(handle));
#else
UNUSED_PARAM(handle);
#endif
m_audioDestinations.add(identifier, WTFMove(destination));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ class RemoteAudioDestinationManager : private IPC::MessageReceiver {
private:
void didReceiveMessage(IPC::Connection&, IPC::Decoder&);

void createAudioDestination(RemoteAudioDestinationIdentifier, const String& inputDeviceId, uint32_t numberOfInputChannels, uint32_t numberOfOutputChannels, float sampleRate, float hardwareSampleRate, IPC::Semaphore&& renderSemaphore);
void createAudioDestination(RemoteAudioDestinationIdentifier, const String& inputDeviceId, uint32_t numberOfInputChannels, uint32_t numberOfOutputChannels, float sampleRate, float hardwareSampleRate, IPC::Semaphore&& renderSemaphore, SharedMemory::Handle&&);
void deleteAudioDestination(RemoteAudioDestinationIdentifier);
void startAudioDestination(RemoteAudioDestinationIdentifier, CompletionHandler<void(bool)>&&);
void stopAudioDestination(RemoteAudioDestinationIdentifier, CompletionHandler<void(bool)>&&);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
#if ENABLE(GPU_PROCESS) && ENABLE(WEB_AUDIO)

messages -> RemoteAudioDestinationManager NotRefCounted {
CreateAudioDestination(WebKit::RemoteAudioDestinationIdentifier identifier, String inputDeviceId, uint32_t numberOfInputChannels, uint32_t numberOfOutputChannels, float sampleRate, float hardwareSampleRate, IPC::Semaphore renderSemaphore)
CreateAudioDestination(WebKit::RemoteAudioDestinationIdentifier identifier, String inputDeviceId, uint32_t numberOfInputChannels, uint32_t numberOfOutputChannels, float sampleRate, float hardwareSampleRate, IPC::Semaphore renderSemaphore, WebKit::SharedMemory::Handle frameCount)
DeleteAudioDestination(WebKit::RemoteAudioDestinationIdentifier identifier)

StartAudioDestination(WebKit::RemoteAudioDestinationIdentifier identifier) -> (bool isPlaying)
Expand Down
24 changes: 19 additions & 5 deletions Source/WebKit/WebProcess/GPU/media/RemoteAudioDestinationProxy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -69,21 +69,28 @@ RemoteAudioDestinationProxy::RemoteAudioDestinationProxy(AudioIOCallback& callba
{
}

uint32_t RemoteAudioDestinationProxy::totalFrameCount() const
{
RELEASE_ASSERT(m_frameCount->size() == sizeof(std::atomic<uint32_t>));
return WTF::atomicLoad(static_cast<uint32_t*>(m_frameCount->data()));
}

void RemoteAudioDestinationProxy::startRenderingThread()
{
ASSERT(!m_renderThread);

auto offThreadRendering = [this]() mutable {
do {
m_renderSemaphore.wait();
if (m_shouldStopThread)
if (m_shouldStopThread || !m_frameCount)
break;

unsigned frameCount = WebCore::AudioUtilities::renderQuantumSize;
while (m_renderSemaphore.waitFor(0_s))
frameCount += WebCore::AudioUtilities::renderQuantumSize;
uint32_t totalFrameCount = this->totalFrameCount();
uint32_t frameCount = (totalFrameCount < m_lastFrameCount) ? (totalFrameCount + (std::numeric_limits<uint32_t>::max() - m_lastFrameCount)) : (totalFrameCount - m_lastFrameCount);

m_lastFrameCount = totalFrameCount;
renderAudio(frameCount);

} while (!m_shouldStopThread);
};
m_renderThread = Thread::create("RemoteAudioDestinationProxy render thread", WTFMove(offThreadRendering), ThreadType::Audio, Thread::QOS::UserInteractive);
Expand All @@ -108,7 +115,14 @@ IPC::Connection* RemoteAudioDestinationProxy::connection()
m_gpuProcessConnection = gpuProcessConnection;
gpuProcessConnection->addClient(*this);
m_destinationID = RemoteAudioDestinationIdentifier::generate();
gpuProcessConnection->connection().send(Messages::RemoteAudioDestinationManager::CreateAudioDestination(m_destinationID, m_inputDeviceId, m_numberOfInputChannels, m_outputBus->numberOfChannels(), sampleRate(), m_remoteSampleRate, m_renderSemaphore), 0);

m_lastFrameCount = 0;
SharedMemory::Handle frameCountHandle;
if ((m_frameCount = SharedMemory::allocate(sizeof(std::atomic<uint32_t>)))) {
if (auto localFrameHandle = m_frameCount->createHandle(SharedMemory::Protection::ReadWrite))
frameCountHandle = WTFMove(*localFrameHandle);
}
gpuProcessConnection->connection().send(Messages::RemoteAudioDestinationManager::CreateAudioDestination(m_destinationID, m_inputDeviceId, m_numberOfInputChannels, m_outputBus->numberOfChannels(), sampleRate(), m_remoteSampleRate, m_renderSemaphore, WTFMove(frameCountHandle)), 0);

#if PLATFORM(COCOA)
m_currentFrame = 0;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,8 @@ class RemoteAudioDestinationProxy final : public WebCore::AudioDestinationResamp
// GPUProcessConnection::Client.
void gpuProcessConnectionDidClose(GPUProcessConnection&) final;

uint32_t totalFrameCount() const;

RemoteAudioDestinationIdentifier m_destinationID; // Call destinationID() getter to make sure the destinationID is valid.

ThreadSafeWeakPtr<GPUProcessConnection> m_gpuProcessConnection;
Expand All @@ -92,6 +94,8 @@ class RemoteAudioDestinationProxy final : public WebCore::AudioDestinationResamp
float m_remoteSampleRate;

RefPtr<Thread> m_renderThread;
RefPtr<SharedMemory> m_frameCount;
uint32_t m_lastFrameCount { 0 };
std::atomic<bool> m_shouldStopThread { false };
};

Expand Down

0 comments on commit f35686d

Please sign in to comment.