From b951404ea74ae432312a83138f5c8945a0d09e1b Mon Sep 17 00:00:00 2001 From: Jean-Yves Avenard Date: Wed, 24 Apr 2024 19:01:06 -0700 Subject: [PATCH] Cherry-pick 272448.960@safari-7618-branch (b7ccdb65258e). https://bugs.webkit.org/show_bug.cgi?id=273176 Always copy all audio channels to the AudioBus to guarantee data lifetime. https://bugs.webkit.org/show_bug.cgi?id=273176 rdar://125166710 Reviewed by Chris Dumez. Following 275262@main, a task is dispatched on the audio render thread. This task dispatch takes a reference to the source and destination AudioBus however when a MultiChannelResampler is in use, the source AudioBus may contain a raw pointer to the resampled's AudioArray and the lifetime of this object may be shorter than the AudioBus. In 232182@main, a speed and memory optimisation was added by passed-in buffer as memory for the first channel in the AudioBus. We revert this change for now and copy all channels' data to the AudioBus. Added test. * LayoutTests/webaudio/crashtest/audioworklet-concurrent-resampler-crash-expected.txt: Added. * LayoutTests/webaudio/crashtest/audioworklet-concurrent-resampler-crash.html: Added. * Source/WebCore/platform/audio/MultiChannelResampler.cpp: (WebCore::MultiChannelResampler::MultiChannelResampler): (WebCore::MultiChannelResampler::provideInputForChannel): * Source/WebCore/platform/audio/MultiChannelResampler.h: Canonical link: https://commits.webkit.org/274313.332@webkitglib/2.44 --- ...et-concurrent-resampler-crash-expected.txt | 1 + ...dioworklet-concurrent-resampler-crash.html | 44 +++++++++++++++++++ .../platform/audio/MultiChannelResampler.cpp | 23 ++-------- .../platform/audio/MultiChannelResampler.h | 2 - 4 files changed, 48 insertions(+), 22 deletions(-) create mode 100644 LayoutTests/webaudio/crashtest/audioworklet-concurrent-resampler-crash-expected.txt create mode 100644 LayoutTests/webaudio/crashtest/audioworklet-concurrent-resampler-crash.html diff --git a/LayoutTests/webaudio/crashtest/audioworklet-concurrent-resampler-crash-expected.txt b/LayoutTests/webaudio/crashtest/audioworklet-concurrent-resampler-crash-expected.txt new file mode 100644 index 000000000000..654ddf7f17ef --- /dev/null +++ b/LayoutTests/webaudio/crashtest/audioworklet-concurrent-resampler-crash-expected.txt @@ -0,0 +1 @@ +This test passes if it does not crash. diff --git a/LayoutTests/webaudio/crashtest/audioworklet-concurrent-resampler-crash.html b/LayoutTests/webaudio/crashtest/audioworklet-concurrent-resampler-crash.html new file mode 100644 index 000000000000..b3ab181d4787 --- /dev/null +++ b/LayoutTests/webaudio/crashtest/audioworklet-concurrent-resampler-crash.html @@ -0,0 +1,44 @@ + + + + + +

This test passes if it does not crash.

+ + + diff --git a/Source/WebCore/platform/audio/MultiChannelResampler.cpp b/Source/WebCore/platform/audio/MultiChannelResampler.cpp index e5a0cfc10caa..c44df274cbbc 100644 --- a/Source/WebCore/platform/audio/MultiChannelResampler.cpp +++ b/Source/WebCore/platform/audio/MultiChannelResampler.cpp @@ -42,19 +42,8 @@ namespace WebCore { MultiChannelResampler::MultiChannelResampler(double scaleFactor, unsigned numberOfChannels, unsigned requestFrames, Function&& provideInput) : m_numberOfChannels(numberOfChannels) , m_provideInput(WTFMove(provideInput)) - , m_multiChannelBus(AudioBus::create(numberOfChannels, requestFrames, false)) + , m_multiChannelBus(AudioBus::create(numberOfChannels, requestFrames)) { - // As an optimization, we will use the buffer passed to provideInputForChannel() as channel memory for the first channel so we - // only need to allocate memory if there is more than one channel. - if (numberOfChannels > 1) { - m_channelsMemory = Vector>(numberOfChannels - 1, [&](size_t i) { - size_t channelIndex = i + 1; - auto floatArray = makeUnique(requestFrames); - m_multiChannelBus->setChannelMemory(channelIndex, floatArray->data(), requestFrames); - return floatArray; - }); - } - // Create each channel's resampler. m_kernels = Vector>(numberOfChannels, [&](size_t channelIndex) { return makeUnique(scaleFactor, requestFrames, std::bind(&MultiChannelResampler::provideInputForChannel, this, std::placeholders::_1, std::placeholders::_2, channelIndex)); @@ -93,16 +82,10 @@ void MultiChannelResampler::process(AudioBus* destination, size_t framesToProces void MultiChannelResampler::provideInputForChannel(std::span buffer, size_t framesToProcess, unsigned channelIndex) { ASSERT(channelIndex < m_multiChannelBus->numberOfChannels()); - ASSERT(framesToProcess == m_multiChannelBus->length()); + ASSERT(framesToProcess <= m_multiChannelBus->length()); - if (!channelIndex) { - // As an optimization, we use the provided buffer as memory for the first channel in the AudioBus. This avoids - // having to memcpy() for the first channel. - RELEASE_ASSERT(framesToProcess <= buffer.size()); - m_multiChannelBus->setChannelMemory(0, buffer.data(), framesToProcess); + if (!channelIndex) m_provideInput(m_multiChannelBus.get(), framesToProcess); - return; - } // Copy the channel data from what we received from m_multiChannelProvider. memcpySpan(buffer.subspan(0, framesToProcess), m_multiChannelBus->channel(channelIndex)->span().subspan(0, framesToProcess)); diff --git a/Source/WebCore/platform/audio/MultiChannelResampler.h b/Source/WebCore/platform/audio/MultiChannelResampler.h index 25d43100b71f..214ee06567ac 100644 --- a/Source/WebCore/platform/audio/MultiChannelResampler.h +++ b/Source/WebCore/platform/audio/MultiChannelResampler.h @@ -29,7 +29,6 @@ #ifndef MultiChannelResampler_h #define MultiChannelResampler_h -#include "AudioArray.h" #include #include #include @@ -62,7 +61,6 @@ private: size_t m_outputFramesReady { 0 }; Function m_provideInput; RefPtr m_multiChannelBus; - Vector> m_channelsMemory; }; } // namespace WebCore -- 2.45.2