Changeset 268758 in webkit

Timestamp:

Oct 20, 2020, 1:42:08 PM (6 years ago)

Author:

Peng Liu

Message:

[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.

Location:

trunk/Source/WebKit

Files:

• ChangeLog (modified) (1 diff)
• GPUProcess/media/RemoteAudioDestinationManager.cpp (modified) (3 diffs)
• WebProcess/GPU/media/RemoteAudioDestinationProxy.cpp (modified) (2 diffs)
• WebProcess/GPU/media/RemoteAudioDestinationProxy.h (modified) (2 diffs)
• WebProcess/GPU/media/RemoteAudioDestinationProxy.messages.in (modified) (1 diff)

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2020-10-20  Peng Liu  <peng.liu6@apple.com>
+
+        [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  <cdumez@apple.com>
 

trunk/Source/WebKit/GPUProcess/media/RemoteAudioDestinationManager.cpp

@@ -83 +83 @@
         storage().setStorage(WTFMove(memory));
         storage().setReadOnly(true);
-
         m_ringBuffer->allocate(description, numberOfFrames);
     }
@@ -139 +138 @@
         ASSERT(!isMainThread());
 
-        if (m_protectThisDuringGracefulShutdown)
-            return 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 {
+        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)>([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
@@ -162 +172 @@
     WebCore::CAAudioStreamDescription m_description;
     UniqueRef<WebCore::CARingBuffer> m_ringBuffer;
+    uint64_t m_startFrame { 0 };
 #endif
 

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

@@ -124 +124 @@
 
 #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);
@@ -151 +151 @@
     uint64_t boundsEndFrame;
     m_ringBuffer->getCurrentFrameBounds(boundsStartFrame, boundsEndFrame);
-    m_renderCompletionHandler(startFrame, framesToRender, boundsStartFrame, boundsEndFrame);
+    m_renderCompletionHandler(boundsStartFrame, boundsEndFrame);
 }
 #endif

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

@@ -72 +72 @@
 
 #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
 
@@ -106 +106 @@
     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
 

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

@@ -26 +26 @@
 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
 }