[Media in GPU Process] Some WebAudio layout tests generate strange noises

https://bugs.webkit.org/show_bug.cgi?id=217921

Reviewed by Eric Carlson.

RemoteAudioDestination::render() should not return `noErr` unless we can provide the requested
samples to the provided AudioBufferList. Otherwise, the audio output unit of CoreAudio will output
the samples in the AudioBufferList, which might be invalid data at the beginning of a rendering.
We have observed that happens in some layout tests and some WebAudio example pages.

Currently, RemoteAudioDestination::render() always returns `noErr` in the render thread (immediately),
but the AudioBufferList (ioData) is updated in the main thread (later). This patch fixes that by only
setting the bounds of CARingBuffer in the completion handler of sendWithAsyncReply() in the main thread,
and fetching AudioBuffer(s) from the CARingBuffer in the render thread. Also, RemoteAudioDestination
tracks the progress of fetching, so RemoteAudioDestinationProxy does not need to send `startFrame`
and `numberOfFramesToRender` to RemoteAudioDestination in response to a buffer request.

* GPUProcess/media/RemoteAudioDestinationManager.cpp:
(WebKit::RemoteAudioDestination::audioSamplesStorageChanged):
(WebKit::RemoteAudioDestination::render): Only return `noErr` if the function renders the requested
sample successfully.

* WebProcess/GPU/media/RemoteAudioDestinationProxy.cpp:
(WebKit::RemoteAudioDestinationProxy::requestBuffer): Remove unused parameters.
(WebKit::RemoteAudioDestinationProxy::renderOnRenderingThead): Ditto.
* WebProcess/GPU/media/RemoteAudioDestinationProxy.h: Ditto.
* WebProcess/GPU/media/RemoteAudioDestinationProxy.messages.in: Ditto.


git-svn-id: http://svn.webkit.org/repository/webkit/trunk@268758 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Author: [email protected]

Source/WebKit/ChangeLog

@@ -1,3 +1,33 @@
+2020-10-20  Peng Liu  <[email protected]>
+
+        [Media in GPU Process] Some WebAudio layout tests generate strange noises
+        https://bugs.webkit.org/show_bug.cgi?id=217921
+
+        Reviewed by Eric Carlson.
+
+        RemoteAudioDestination::render() should not return `noErr` unless we can provide the requested
+        samples to the provided AudioBufferList. Otherwise, the audio output unit of CoreAudio will output
+        the samples in the AudioBufferList, which might be invalid data at the beginning of a rendering.
+        We have observed that happens in some layout tests and some WebAudio example pages.
+
+        Currently, RemoteAudioDestination::render() always returns `noErr` in the render thread (immediately),
+        but the AudioBufferList (ioData) is updated in the main thread (later). This patch fixes that by only
+        setting the bounds of CARingBuffer in the completion handler of sendWithAsyncReply() in the main thread,
+        and fetching AudioBuffer(s) from the CARingBuffer in the render thread. Also, RemoteAudioDestination
+        tracks the progress of fetching, so RemoteAudioDestinationProxy does not need to send `startFrame`
+        and `numberOfFramesToRender` to RemoteAudioDestination in response to a buffer request.
+
+        * GPUProcess/media/RemoteAudioDestinationManager.cpp:
+        (WebKit::RemoteAudioDestination::audioSamplesStorageChanged):
+        (WebKit::RemoteAudioDestination::render): Only return `noErr` if the function renders the requested
+        sample successfully.
+
+        * WebProcess/GPU/media/RemoteAudioDestinationProxy.cpp:
+        (WebKit::RemoteAudioDestinationProxy::requestBuffer): Remove unused parameters.
+        (WebKit::RemoteAudioDestinationProxy::renderOnRenderingThead): Ditto.
+        * WebProcess/GPU/media/RemoteAudioDestinationProxy.h: Ditto.
+        * WebProcess/GPU/media/RemoteAudioDestinationProxy.messages.in: Ditto.
+
 2020-10-20  Chris Dumez  <[email protected]>
 
         Drop legacy code using AssertionServices

Source/WebKit/GPUProcess/media/RemoteAudioDestinationManager.cpp

@@ -82,7 +82,6 @@ class RemoteAudioDestination
         auto memory = SharedMemory::map(ipcHandle.handle, SharedMemory::Protection::ReadOnly);
         storage().setStorage(WTFMove(memory));
         storage().setReadOnly(true);
-
         m_ringBuffer->allocate(description, numberOfFrames);
     }
 #endif
@@ -138,16 +137,27 @@ class RemoteAudioDestination
     {
         ASSERT(!isMainThread());
 
-        if (m_protectThisDuringGracefulShutdown)
-            return noErr;
+        OSStatus status = -1;
+
+        if (m_protectThisDuringGracefulShutdown || !m_isPlaying)
+            return status;
+
+        uint64_t start;
+        uint64_t end;
+        m_ringBuffer->getCurrentFrameBounds(start, end);
+        if (m_startFrame >= start && m_startFrame + numberOfFrames <= end) {
+            m_ringBuffer->fetch(ioData, numberOfFrames, m_startFrame);
+            m_startFrame += numberOfFrames;
+            status = noErr;
+        }
 
-        m_connection.connection().sendWithAsyncReply(Messages::RemoteAudioDestinationProxy::RequestBuffer(sampleTime, hostTime, numberOfFrames), CompletionHandler<void(uint64_t, uint64_t, uint64_t, uint64_t)>([this, protectedThis = makeRef(*this), ioData](auto startFrame, auto numberOfFramesToRender, auto boundsStartFrame, auto boundsEndFrame) mutable {
+        m_connection.connection().sendWithAsyncReply(Messages::RemoteAudioDestinationProxy::RequestBuffer(sampleTime, hostTime, numberOfFrames), CompletionHandler<void(uint64_t, uint64_t)>([this, protectedThis = makeRef(*this)](auto boundsStartFrame, auto boundsEndFrame) mutable {
             ASSERT(isMainThread());
-            m_ringBuffer->setCurrentFrameBounds(boundsStartFrame, boundsEndFrame);
-            m_ringBuffer->fetch(ioData, numberOfFramesToRender, startFrame);
+            if (boundsEndFrame)
+                m_ringBuffer->setCurrentFrameBounds(boundsStartFrame, boundsEndFrame);
         }, CompletionHandlerCallThread::MainThread), m_id.toUInt64());
 
-        return noErr;
+        return status;
     }
 #endif
 
@@ -161,6 +171,7 @@ class RemoteAudioDestination
 
     WebCore::CAAudioStreamDescription m_description;
     UniqueRef<WebCore::CARingBuffer> m_ringBuffer;
+    uint64_t m_startFrame { 0 };
 #endif
 
     bool m_isPlaying { false };

Source/WebKit/WebProcess/GPU/media/RemoteAudioDestinationProxy.cpp

@@ -123,12 +123,12 @@ void RemoteAudioDestinationProxy::stop()
 }
 
 #if PLATFORM(COCOA)
-void RemoteAudioDestinationProxy::requestBuffer(double sampleTime, uint64_t hostTime, uint64_t numberOfFrames, CompletionHandler<void(uint64_t, uint64_t, uint64_t, uint64_t)>&& completionHandler)
+void RemoteAudioDestinationProxy::requestBuffer(double sampleTime, uint64_t hostTime, uint64_t numberOfFrames, CompletionHandler<void(uint64_t, uint64_t)>&& completionHandler)
 {
     ASSERT(!isMainThread());
 
     if (!hasEnoughFrames(numberOfFrames))
-        completionHandler(0, 0, 0, 0);
+        completionHandler(0, 0);
 
     m_renderCompletionHandler = WTFMove(completionHandler);
     m_audioBufferList->setSampleCount(numberOfFrames);
@@ -150,7 +150,7 @@ void RemoteAudioDestinationProxy::renderOnRenderingThead(size_t framesToRender)
     uint64_t boundsStartFrame;
     uint64_t boundsEndFrame;
     m_ringBuffer->getCurrentFrameBounds(boundsStartFrame, boundsEndFrame);
-    m_renderCompletionHandler(startFrame, framesToRender, boundsStartFrame, boundsEndFrame);
+    m_renderCompletionHandler(boundsStartFrame, boundsEndFrame);
 }
 #endif
 

Source/WebKit/WebProcess/GPU/media/RemoteAudioDestinationProxy.h

@@ -71,7 +71,7 @@ class RemoteAudioDestinationProxy
     void didReceiveMessageFromGPUProcess(IPC::Connection& connection, IPC::Decoder& decoder) { didReceiveMessage(connection, decoder); }
 
 #if PLATFORM(COCOA)
-    void requestBuffer(double sampleTime, uint64_t hostTime, uint64_t numberOfFrames, CompletionHandler<void(uint64_t startFrame, uint64_t numberOfFramesToRender, uint64_t boundsStartFrame, uint64_t boundsEndFrame)>&&);
+    void requestBuffer(double sampleTime, uint64_t hostTime, uint64_t numberOfFrames, CompletionHandler<void(uint64_t boundsStartFrame, uint64_t boundsEndFrame)>&&);
 #endif
 
 private:
@@ -105,7 +105,7 @@ class RemoteAudioDestinationProxy
     std::unique_ptr<WebCore::CARingBuffer> m_ringBuffer;
     std::unique_ptr<WebCore::WebAudioBufferList> m_audioBufferList;
     uint64_t m_currentFrame { 0 };
-    WTF::Function<void(uint64_t, uint64_t, uint64_t, uint64_t)> m_renderCompletionHandler;
+    WTF::Function<void(uint64_t, uint64_t)> m_renderCompletionHandler;
 #endif
 
     Function<void(Function<void()>&&)> m_dispatchToRenderThread;

Source/WebKit/WebProcess/GPU/media/RemoteAudioDestinationProxy.messages.in

@@ -25,7 +25,7 @@
 
 messages -> RemoteAudioDestinationProxy NotRefCounted {
 #if PLATFORM(COCOA)
-    RequestBuffer(double sampleTime, uint64_t hostTime, uint64_t numberOfFrames) -> (uint64_t startFrame, uint64_t numberOfFramesToRender, uint64_t boundsStartFrame, uint64_t boundsEndFrame) Async
+    RequestBuffer(double sampleTime, uint64_t hostTime, uint64_t numberOfFrames) -> (uint64_t boundsStartFrame, uint64_t boundsEndFrame) Async
 #endif
 }