- Revision
- 268758
- Author
- peng.l...@apple.com
- Date
- 2020-10-20 13:42:08 -0700 (Tue, 20 Oct 2020)
Log 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.
Modified Paths
Diff
Modified: trunk/Source/WebKit/ChangeLog (268757 => 268758)
--- trunk/Source/WebKit/ChangeLog 2020-10-20 20:21:07 UTC (rev 268757)
+++ trunk/Source/WebKit/ChangeLog 2020-10-20 20:42:08 UTC (rev 268758)
@@ -1,3 +1,33 @@
+2020-10-20 Peng Liu <peng.l...@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 <cdu...@apple.com>
Drop legacy code using AssertionServices
Modified: trunk/Source/WebKit/GPUProcess/media/RemoteAudioDestinationManager.cpp (268757 => 268758)
--- trunk/Source/WebKit/GPUProcess/media/RemoteAudioDestinationManager.cpp 2020-10-20 20:21:07 UTC (rev 268757)
+++ trunk/Source/WebKit/GPUProcess/media/RemoteAudioDestinationManager.cpp 2020-10-20 20:42:08 UTC (rev 268758)
@@ -82,7 +82,6 @@
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 @@
{
ASSERT(!isMainThread());
- if (m_protectThisDuringGracefulShutdown)
- return noErr;
+ OSStatus status = -1;
- 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 {
+ 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
@@ -161,6 +171,7 @@
WebCore::CAAudioStreamDescription m_description;
UniqueRef<WebCore::CARingBuffer> m_ringBuffer;
+ uint64_t m_startFrame { 0 };
#endif
bool m_isPlaying { false };
Modified: trunk/Source/WebKit/WebProcess/GPU/media/RemoteAudioDestinationProxy.cpp (268757 => 268758)
--- trunk/Source/WebKit/WebProcess/GPU/media/RemoteAudioDestinationProxy.cpp 2020-10-20 20:21:07 UTC (rev 268757)
+++ trunk/Source/WebKit/WebProcess/GPU/media/RemoteAudioDestinationProxy.cpp 2020-10-20 20:42:08 UTC (rev 268758)
@@ -123,12 +123,12 @@
}
#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 @@
uint64_t boundsStartFrame;
uint64_t boundsEndFrame;
m_ringBuffer->getCurrentFrameBounds(boundsStartFrame, boundsEndFrame);
- m_renderCompletionHandler(startFrame, framesToRender, boundsStartFrame, boundsEndFrame);
+ m_renderCompletionHandler(boundsStartFrame, boundsEndFrame);
}
#endif
Modified: trunk/Source/WebKit/WebProcess/GPU/media/RemoteAudioDestinationProxy.h (268757 => 268758)
--- trunk/Source/WebKit/WebProcess/GPU/media/RemoteAudioDestinationProxy.h 2020-10-20 20:21:07 UTC (rev 268757)
+++ trunk/Source/WebKit/WebProcess/GPU/media/RemoteAudioDestinationProxy.h 2020-10-20 20:42:08 UTC (rev 268758)
@@ -71,7 +71,7 @@
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 @@
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;
Modified: trunk/Source/WebKit/WebProcess/GPU/media/RemoteAudioDestinationProxy.messages.in (268757 => 268758)
--- trunk/Source/WebKit/WebProcess/GPU/media/RemoteAudioDestinationProxy.messages.in 2020-10-20 20:21:07 UTC (rev 268757)
+++ trunk/Source/WebKit/WebProcess/GPU/media/RemoteAudioDestinationProxy.messages.in 2020-10-20 20:42:08 UTC (rev 268758)
@@ -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
}
