- Revision
- 269630
- Author
- [email protected]
- Date
- 2020-11-10 10:51:46 -0800 (Tue, 10 Nov 2020)
Log Message
[GPUProcess] Regression(r268632) Garbage is rendered on speakers when using WebAudio
https://bugs.webkit.org/show_bug.cgi?id=218729
Reviewed by Eric Carlson.
Source/WebCore:
* platform/audio/cocoa/AudioDestinationCocoa.h:
* platform/mock/MockAudioDestinationCocoa.cpp:
(WebCore::MockAudioDestinationCocoa::start):
(WebCore::MockAudioDestinationCocoa::stop):
(WebCore::MockAudioDestinationCocoa::tick):
* platform/mock/MockAudioDestinationCocoa.h:
Make AudioDestinationCocoa::m_dispatchToRenderThread private so that subclasses cannot set it.
Update MockAudioDestinationCocoa to use m_dispatchToRenderThread to dispatch to the
render thread when available, instead of unconditionally dispatching to its own WorkQueue and
then expecting AudioDestinationCocoa::render() to render to the actual rendering thread.
Source/WebKit:
RemoteAudioDestinationProxy::requestBuffer() was calling AudioDestinationCocoa::render()
and expecting RemoteAudioDestinationProxy::renderOnRenderingThead() to get called as
a result. It would take care of writing to the CARingBuffer and sending the IPC back
to the GPU process in renderOnRenderingThead(). The issue was that AudioDestinationCocoa
uses a PushPullFIFO internally for buffering. It first fetches available frames from
the FIFO and then only calls renderOnRenderingThead() with the number of frames that
remain to processed (usually 0). As a result, RemoteAudioDestinationProxy::renderOnRenderingThead()
would often store 0 frames instead of 128 (or sometimes a number of frames less than
128), even though the full 128 frames were actually rendered.
To address the issue, stop overriding renderOnRenderingThead() in
RemoteAudioDestinationProxy. Instead, do the writing to the CARingBuffer and the IPC
response in RemoteAudioDestinationProxy::requestBuffer(), directly after calling
AudioDestinationCocoa::render(). After calling AudioDestinationCocoa::render()
we know that |framesToRender| frames have been rendered / added to the buffer.
* WebProcess/GPU/media/RemoteAudioDestinationProxy.cpp:
(WebKit::RemoteAudioDestinationProxy::requestBuffer):
* WebProcess/GPU/media/RemoteAudioDestinationProxy.h:
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (269629 => 269630)
--- trunk/Source/WebCore/ChangeLog 2020-11-10 17:28:55 UTC (rev 269629)
+++ trunk/Source/WebCore/ChangeLog 2020-11-10 18:51:46 UTC (rev 269630)
@@ -1,3 +1,21 @@
+2020-11-10 Chris Dumez <[email protected]>
+
+ [GPUProcess] Regression(r268632) Garbage is rendered on speakers when using WebAudio
+ https://bugs.webkit.org/show_bug.cgi?id=218729
+
+ Reviewed by Eric Carlson.
+
+ * platform/audio/cocoa/AudioDestinationCocoa.h:
+ * platform/mock/MockAudioDestinationCocoa.cpp:
+ (WebCore::MockAudioDestinationCocoa::start):
+ (WebCore::MockAudioDestinationCocoa::stop):
+ (WebCore::MockAudioDestinationCocoa::tick):
+ * platform/mock/MockAudioDestinationCocoa.h:
+ Make AudioDestinationCocoa::m_dispatchToRenderThread private so that subclasses cannot set it.
+ Update MockAudioDestinationCocoa to use m_dispatchToRenderThread to dispatch to the
+ render thread when available, instead of unconditionally dispatching to its own WorkQueue and
+ then expecting AudioDestinationCocoa::render() to render to the actual rendering thread.
+
2020-11-10 Antti Koivisto <[email protected]>
[LFC][Integration] Allow object-fit
Modified: trunk/Source/WebCore/platform/audio/cocoa/AudioDestinationCocoa.h (269629 => 269630)
--- trunk/Source/WebCore/platform/audio/cocoa/AudioDestinationCocoa.h 2020-11-10 17:28:55 UTC (rev 269629)
+++ trunk/Source/WebCore/platform/audio/cocoa/AudioDestinationCocoa.h 2020-11-10 18:51:46 UTC (rev 269630)
@@ -54,7 +54,6 @@
protected:
WEBCORE_EXPORT bool hasEnoughFrames(UInt32 numberOfFrames) const;
WEBCORE_EXPORT OSStatus render(double sampleTime, uint64_t hostTime, UInt32 numberOfFrames, AudioBufferList* ioData) final;
- WEBCORE_EXPORT virtual void renderOnRenderingThead(size_t framesToRender);
WEBCORE_EXPORT void setIsPlaying(bool);
bool isPlaying() final { return m_isPlaying; }
@@ -65,8 +64,6 @@
WEBCORE_EXPORT void getAudioStreamBasicDescription(AudioStreamBasicDescription&);
- Function<void(Function<void()>&&)> m_dispatchToRenderThread;
-
private:
friend Ref<AudioDestination> AudioDestination::create(AudioIOCallback&, const String&, unsigned, unsigned, float);
@@ -73,6 +70,8 @@
void start(Function<void(Function<void()>&&)>&&, CompletionHandler<void(bool)>&&) override;
void stop(CompletionHandler<void(bool)>&&) override;
+ void renderOnRenderingThead(size_t framesToRender);
+
// AudioSourceProvider.
WEBCORE_EXPORT void provideInput(AudioBus*, size_t framesToProcess) final;
@@ -93,6 +92,8 @@
std::unique_ptr<MultiChannelResampler> m_resampler;
AudioIOPosition m_outputTimestamp;
+ Function<void(Function<void()>&&)> m_dispatchToRenderThread;
+
float m_contextSampleRate;
Lock m_isPlayingLock;
Modified: trunk/Source/WebCore/platform/mock/MockAudioDestinationCocoa.cpp (269629 => 269630)
--- trunk/Source/WebCore/platform/mock/MockAudioDestinationCocoa.cpp 2020-11-10 17:28:55 UTC (rev 269629)
+++ trunk/Source/WebCore/platform/mock/MockAudioDestinationCocoa.cpp 2020-11-10 18:51:46 UTC (rev 269630)
@@ -48,6 +48,12 @@
void MockAudioDestinationCocoa::start(Function<void(Function<void()>&&)>&& dispatchToRenderThread, CompletionHandler<void(bool)>&& completionHandler)
{
m_dispatchToRenderThread = WTFMove(dispatchToRenderThread);
+ if (!m_dispatchToRenderThread) {
+ m_dispatchToRenderThread = [this](Function<void()>&& function) {
+ m_workQueue->dispatch(WTFMove(function));
+ };
+ }
+
m_timer.startRepeating(Seconds { m_numberOfFramesToProcess / sampleRate() });
setIsPlaying(true);
@@ -61,7 +67,7 @@
m_timer.stop();
setIsPlaying(false);
- m_workQueue->dispatch([completionHandler = WTFMove(completionHandler)]() mutable {
+ m_dispatchToRenderThread([completionHandler = WTFMove(completionHandler)]() mutable {
callOnMainThread([completionHandler = WTFMove(completionHandler)]() mutable {
completionHandler(true);
});
@@ -70,7 +76,7 @@
void MockAudioDestinationCocoa::tick()
{
- m_workQueue->dispatch([this, sampleRate = sampleRate(), numberOfFramesToProcess = m_numberOfFramesToProcess] {
+ m_dispatchToRenderThread([this, sampleRate = sampleRate(), numberOfFramesToProcess = m_numberOfFramesToProcess] {
AudioStreamBasicDescription streamFormat;
getAudioStreamBasicDescription(streamFormat);
Modified: trunk/Source/WebCore/platform/mock/MockAudioDestinationCocoa.h (269629 => 269630)
--- trunk/Source/WebCore/platform/mock/MockAudioDestinationCocoa.h 2020-11-10 17:28:55 UTC (rev 269629)
+++ trunk/Source/WebCore/platform/mock/MockAudioDestinationCocoa.h 2020-11-10 18:51:46 UTC (rev 269630)
@@ -54,6 +54,7 @@
Ref<WorkQueue> m_workQueue;
RunLoop::Timer<MockAudioDestinationCocoa> m_timer;
+ Function<void(Function<void()>&&)> m_dispatchToRenderThread;
uint32_t m_numberOfFramesToProcess { 384 };
};
Modified: trunk/Source/WebKit/ChangeLog (269629 => 269630)
--- trunk/Source/WebKit/ChangeLog 2020-11-10 17:28:55 UTC (rev 269629)
+++ trunk/Source/WebKit/ChangeLog 2020-11-10 18:51:46 UTC (rev 269630)
@@ -1,3 +1,30 @@
+2020-11-10 Chris Dumez <[email protected]>
+
+ [GPUProcess] Regression(r268632) Garbage is rendered on speakers when using WebAudio
+ https://bugs.webkit.org/show_bug.cgi?id=218729
+
+ Reviewed by Eric Carlson.
+
+ RemoteAudioDestinationProxy::requestBuffer() was calling AudioDestinationCocoa::render()
+ and expecting RemoteAudioDestinationProxy::renderOnRenderingThead() to get called as
+ a result. It would take care of writing to the CARingBuffer and sending the IPC back
+ to the GPU process in renderOnRenderingThead(). The issue was that AudioDestinationCocoa
+ uses a PushPullFIFO internally for buffering. It first fetches available frames from
+ the FIFO and then only calls renderOnRenderingThead() with the number of frames that
+ remain to processed (usually 0). As a result, RemoteAudioDestinationProxy::renderOnRenderingThead()
+ would often store 0 frames instead of 128 (or sometimes a number of frames less than
+ 128), even though the full 128 frames were actually rendered.
+
+ To address the issue, stop overriding renderOnRenderingThead() in
+ RemoteAudioDestinationProxy. Instead, do the writing to the CARingBuffer and the IPC
+ response in RemoteAudioDestinationProxy::requestBuffer(), directly after calling
+ AudioDestinationCocoa::render(). After calling AudioDestinationCocoa::render()
+ we know that |framesToRender| frames have been rendered / added to the buffer.
+
+ * WebProcess/GPU/media/RemoteAudioDestinationProxy.cpp:
+ (WebKit::RemoteAudioDestinationProxy::requestBuffer):
+ * WebProcess/GPU/media/RemoteAudioDestinationProxy.h:
+
2020-11-10 Carlos Garcia Campos <[email protected]>
[GTK4] WebView is flipped
Modified: trunk/Source/WebKit/WebProcess/GPU/media/RemoteAudioDestinationProxy.cpp (269629 => 269630)
--- trunk/Source/WebKit/WebProcess/GPU/media/RemoteAudioDestinationProxy.cpp 2020-11-10 17:28:55 UTC (rev 269629)
+++ trunk/Source/WebKit/WebProcess/GPU/media/RemoteAudioDestinationProxy.cpp 2020-11-10 18:51:46 UTC (rev 269630)
@@ -132,26 +132,18 @@
}
#if PLATFORM(COCOA)
-void RemoteAudioDestinationProxy::requestBuffer(double sampleTime, uint64_t hostTime, uint64_t numberOfFrames, CompletionHandler<void(uint64_t, uint64_t)>&& completionHandler)
+void RemoteAudioDestinationProxy::requestBuffer(double sampleTime, uint64_t hostTime, uint64_t framesToRender, CompletionHandler<void(uint64_t, uint64_t)>&& completionHandler)
{
ASSERT(!isMainThread());
- if (!hasEnoughFrames(numberOfFrames))
- completionHandler(0, 0);
+ if (!hasEnoughFrames(framesToRender))
+ return completionHandler(0, 0);
- m_renderCompletionHandler = WTFMove(completionHandler);
- m_audioBufferList->setSampleCount(numberOfFrames);
+ auto startFrame = m_currentFrame;
+ m_audioBufferList->setSampleCount(framesToRender);
- AudioDestinationCocoa::render(sampleTime, hostTime, numberOfFrames, m_audioBufferList->list());
-}
+ AudioDestinationCocoa::render(sampleTime, hostTime, framesToRender, m_audioBufferList->list());
-void RemoteAudioDestinationProxy::renderOnRenderingThead(size_t framesToRender)
-{
- ASSERT(m_renderCompletionHandler);
-
- auto startFrame = m_currentFrame;
-
- AudioDestinationCocoa::renderOnRenderingThead(framesToRender);
m_currentFrame += framesToRender;
m_ringBuffer->store(m_audioBufferList->list(), framesToRender, startFrame);
@@ -159,7 +151,7 @@
uint64_t boundsStartFrame;
uint64_t boundsEndFrame;
m_ringBuffer->getCurrentFrameBounds(boundsStartFrame, boundsEndFrame);
- m_renderCompletionHandler(boundsStartFrame, boundsEndFrame);
+ completionHandler(boundsStartFrame, boundsEndFrame);
}
#endif
Modified: trunk/Source/WebKit/WebProcess/GPU/media/RemoteAudioDestinationProxy.h (269629 => 269630)
--- trunk/Source/WebKit/WebProcess/GPU/media/RemoteAudioDestinationProxy.h 2020-11-10 17:28:55 UTC (rev 269629)
+++ trunk/Source/WebKit/WebProcess/GPU/media/RemoteAudioDestinationProxy.h 2020-11-10 18:51:46 UTC (rev 269630)
@@ -87,7 +87,6 @@
#if PLATFORM(COCOA)
void storageChanged(SharedMemory*) final;
- void renderOnRenderingThead(size_t framesToRender) final;
#endif
// IPC::MessageReceiver
@@ -105,7 +104,6 @@
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)> m_renderCompletionHandler;
#endif
Function<void(Function<void()>&&)> m_dispatchToRenderThread;
