Skip to content

Commit babb868

Browse files
cdumezcdumez
committed
Unreviewed, reverting r270141.
Caused assertions on bots Reverted changeset: "Poor resampling quality when using AudioContext sampleRate parameter" https://bugs.webkit.org/show_bug.cgi?id=219201 https://trac.webkit.org/changeset/270141 Canonical link: https://commits.webkit.org/231864@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@270155 268f45cc-cd09-0410-ab3c-d52691b4dbfc
1 parent a717593 commit babb868

10 files changed

Lines changed: 70 additions & 126 deletions

‎LayoutTests/ChangeLog‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,16 @@
1+
2020-11-21 Chris Dumez <cdumez@apple.com>
2+
3+
Unreviewed, reverting r270141.
4+
5+
Caused assertions on bots
6+
7+
Reverted changeset:
8+
9+
"Poor resampling quality when using AudioContext sampleRate
10+
parameter"
11+
https://bugs.webkit.org/show_bug.cgi?id=219201
12+
https://trac.webkit.org/changeset/270141
13+
114
2020-11-21 Antti Koivisto <antti@apple.com>
215

316
[LFC][Integration] Remove ensureLineBoxes call from RenderedPosition constructor

‎LayoutTests/webaudio/audiocontext-large-samplerate-expected.txt‎

Lines changed: 0 additions & 11 deletions
This file was deleted.

‎LayoutTests/webaudio/audiocontext-large-samplerate.html‎

Lines changed: 0 additions & 22 deletions
This file was deleted.

‎LayoutTests/webaudio/audiocontext-low-samplerate-expected.txt‎

Lines changed: 0 additions & 11 deletions
This file was deleted.

‎LayoutTests/webaudio/audiocontext-low-samplerate.html‎

Lines changed: 0 additions & 22 deletions
This file was deleted.

‎Source/WebCore/ChangeLog‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,16 @@
1+
2020-11-21 Chris Dumez <cdumez@apple.com>
2+
3+
Unreviewed, reverting r270141.
4+
5+
Caused assertions on bots
6+
7+
Reverted changeset:
8+
9+
"Poor resampling quality when using AudioContext sampleRate
10+
parameter"
11+
https://bugs.webkit.org/show_bug.cgi?id=219201
12+
https://trac.webkit.org/changeset/270141
13+
114
2020-11-21 Andres Gonzalez <andresg_22@apple.com>
215

316
AccessibilityObject::FocusedUIElement should not call AXObjectCache::focusedUIElementForPage that can return an isolated object.

‎Source/WebCore/platform/audio/MultiChannelResampler.cpp‎

Lines changed: 23 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -33,59 +33,62 @@
3333
#include "MultiChannelResampler.h"
3434

3535
#include "AudioBus.h"
36-
#include "AudioUtilities.h"
3736

3837
namespace WebCore {
3938

4039
// ChannelProvider provides a single channel of audio data (one channel at a time) for each channel
4140
// of data provided to us in a multi-channel provider.
42-
class MultiChannelResampler::ChannelProvider {
41+
class MultiChannelResampler::ChannelProvider : public AudioSourceProvider {
4342
WTF_MAKE_FAST_ALLOCATED;
4443
public:
4544
explicit ChannelProvider(unsigned numberOfChannels)
46-
: m_multiChannelBus(AudioBus::create(numberOfChannels, AudioUtilities::renderQuantumSize))
45+
: m_numberOfChannels(numberOfChannels)
4746
{
4847
}
4948

5049
void setProvider(AudioSourceProvider* multiChannelProvider)
5150
{
51+
m_currentChannel = 0;
52+
m_framesToProcess = 0;
5253
m_multiChannelProvider = multiChannelProvider;
5354
}
5455

5556
// provideInput() will be called once for each channel, starting with the first channel.
5657
// Each time it's called, it will provide the next channel of data.
57-
void provideInputForChannel(AudioBus* bus, size_t framesToProcess, unsigned channelIndex)
58+
void provideInput(AudioBus* bus, size_t framesToProcess) override
5859
{
59-
ASSERT(channelIndex < m_multiChannelBus->numberOfChannels());
60-
ASSERT(framesToProcess <= AudioUtilities::renderQuantumSize);
61-
if (framesToProcess > AudioUtilities::renderQuantumSize)
62-
return;
63-
6460
bool isBusGood = bus && bus->numberOfChannels() == 1;
6561
ASSERT(isBusGood);
6662
if (!isBusGood)
6763
return;
6864

6965
// Get the data from the multi-channel provider when the first channel asks for it.
7066
// For subsequent channels, we can just dish out the channel data from that (stored in m_multiChannelBus).
71-
if (!channelIndex) {
67+
if (!m_currentChannel) {
7268
m_framesToProcess = framesToProcess;
69+
m_multiChannelBus = AudioBus::create(m_numberOfChannels, framesToProcess);
7370
m_multiChannelProvider->provideInput(m_multiChannelBus.get(), framesToProcess);
7471
}
7572

7673
// All channels must ask for the same amount. This should always be the case, but let's just make sure.
77-
bool isGood = framesToProcess == m_framesToProcess;
74+
bool isGood = m_multiChannelBus.get() && framesToProcess == m_framesToProcess;
7875
ASSERT(isGood);
7976
if (!isGood)
8077
return;
8178

8279
// Copy the channel data from what we received from m_multiChannelProvider.
83-
memcpy(bus->channel(0)->mutableData(), m_multiChannelBus->channel(channelIndex)->data(), sizeof(float) * framesToProcess);
80+
ASSERT(m_currentChannel <= m_numberOfChannels);
81+
if (m_currentChannel < m_numberOfChannels) {
82+
memcpy(bus->channel(0)->mutableData(), m_multiChannelBus->channel(m_currentChannel)->data(), sizeof(float) * framesToProcess);
83+
++m_currentChannel;
84+
}
8485
}
8586

8687
private:
8788
AudioSourceProvider* m_multiChannelProvider { nullptr };
8889
RefPtr<AudioBus> m_multiChannelBus;
90+
unsigned m_numberOfChannels { 0 };
91+
unsigned m_currentChannel { 0 };
8992
size_t m_framesToProcess { 0 }; // Used to verify that all channels ask for the same amount.
9093
};
9194

@@ -109,25 +112,14 @@ void MultiChannelResampler::process(AudioSourceProvider* provider, AudioBus* des
109112
// channelProvider wraps the original multi-channel provider and dishes out one channel at a time.
110113
m_channelProvider->setProvider(provider);
111114

112-
// We need to ensure that SincResampler only calls provideInput() once for each channel or it will confuse the logic
113-
// inside ChannelProvider. To ensure this, we chunk the number of requested frames into SincResampler::chunkSize()
114-
// sized chunks. SincResampler guarantees it will only call provideInput() once once we resample this way.
115-
m_outputFramesReady = 0;
116-
while (m_outputFramesReady < framesToProcess) {
117-
size_t chunkSize = m_kernels[0]->chunkSize();
118-
size_t framesThisTime = std::min(framesToProcess - m_outputFramesReady, chunkSize);
119-
120-
for (unsigned channelIndex = 0; channelIndex < m_numberOfChannels; ++channelIndex) {
121-
ASSERT(chunkSize == m_kernels[channelIndex]->chunkSize());
122-
bool wasProvideInputCalled = false;
123-
m_kernels[channelIndex]->process(destination->channel(channelIndex)->mutableData() + m_outputFramesReady, framesThisTime, [this, channelIndex, &wasProvideInputCalled](AudioBus* bus, size_t framesToProcess) {
124-
ASSERT_WITH_MESSAGE(!wasProvideInputCalled, "provideInputForChannel should only be called once");
125-
wasProvideInputCalled = true;
126-
m_channelProvider->provideInputForChannel(bus, framesToProcess, channelIndex);
127-
});
128-
}
129-
130-
m_outputFramesReady += framesThisTime;
115+
for (unsigned channelIndex = 0; channelIndex < m_numberOfChannels; ++channelIndex) {
116+
// Depending on the sample-rate scale factor, and the internal buffering used in a SincResampler
117+
// kernel, this call to process() will only sometimes call provideInput() on the channelProvider.
118+
// However, if it calls provideInput() for the first channel, then it will call it for the remaining
119+
// channels, since they all buffer in the same way and are processing the same number of frames.
120+
m_kernels[channelIndex]->process(m_channelProvider.get(),
121+
destination->channel(channelIndex)->mutableData(),
122+
framesToProcess);
131123
}
132124

133125
m_channelProvider->setProvider(nullptr);

‎Source/WebCore/platform/audio/MultiChannelResampler.h‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,6 @@ class MultiChannelResampler final {
5959

6060
class ChannelProvider;
6161
std::unique_ptr<ChannelProvider> m_channelProvider;
62-
size_t m_outputFramesReady { 0 };
6362
};
6463

6564
} // namespace WebCore

‎Source/WebCore/platform/audio/SincResampler.cpp‎

Lines changed: 15 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -115,11 +115,6 @@ constexpr unsigned defaultRequestFrames { 512 };
115115
constexpr unsigned kernelSize { 32 };
116116
constexpr unsigned numberOfKernelOffsets { 32 };
117117

118-
static size_t calculateChunkSize(unsigned blockSize, double scaleFactor)
119-
{
120-
return blockSize / scaleFactor;
121-
}
122-
123118
SincResampler::SincResampler(double scaleFactor, Optional<unsigned> requestFrames)
124119
: m_scaleFactor(scaleFactor)
125120
, m_kernelStorage(kernelSize * (numberOfKernelOffsets + 1))
@@ -142,7 +137,6 @@ void SincResampler::updateRegions(bool isSecondLoad)
142137
m_r3 = m_r0 + m_requestFrames - kernelSize;
143138
m_r4 = m_r0 + m_requestFrames - kernelSize / 2;
144139
m_blockSize = m_r4 - m_r2;
145-
m_chunkSize = calculateChunkSize(m_blockSize, m_scaleFactor);
146140

147141
// m_r1 at the beginning of the buffer.
148142
ASSERT(m_r1 == m_inputBuffer.data());
@@ -193,10 +187,10 @@ void SincResampler::initializeKernel()
193187
}
194188
}
195189

196-
void SincResampler::consumeSource(float* buffer, unsigned numberOfSourceFrames, const Function<void(AudioBus* bus, size_t framesToProcess)>& provideInput)
190+
void SincResampler::consumeSource(float* buffer, unsigned numberOfSourceFrames)
197191
{
198-
ASSERT(provideInput);
199-
if (!provideInput)
192+
ASSERT(m_sourceProvider);
193+
if (!m_sourceProvider)
200194
return;
201195

202196
// Wrap the provided buffer by an AudioBus for use by the source provider.
@@ -206,14 +200,14 @@ void SincResampler::consumeSource(float* buffer, unsigned numberOfSourceFrames,
206200
// FIXME: Find a way to make the following const-correct:
207201
m_internalBus->setChannelMemory(0, buffer, numberOfSourceFrames);
208202

209-
provideInput(m_internalBus.get(), numberOfSourceFrames);
203+
m_sourceProvider->provideInput(m_internalBus.get(), numberOfSourceFrames);
210204
}
211205

212206
namespace {
213207

214-
// BufferSourceProvider is an audio source provider wrapping an in-memory buffer.
208+
// BufferSourceProvider is an AudioSourceProvider wrapping an in-memory buffer.
215209

216-
class BufferSourceProvider {
210+
class BufferSourceProvider : public AudioSourceProvider {
217211
public:
218212
explicit BufferSourceProvider(const float* source, size_t numberOfSourceFrames)
219213
: m_source(source)
@@ -222,7 +216,7 @@ class BufferSourceProvider {
222216
}
223217

224218
// Consumes samples from the in-memory buffer.
225-
void provideInput(AudioBus* bus, size_t framesToProcess)
219+
void provideInput(AudioBus* bus, size_t framesToProcess) override
226220
{
227221
ASSERT(m_source && bus);
228222
if (!m_source || !bus)
@@ -259,27 +253,27 @@ void SincResampler::process(const float* source, float* destination, unsigned nu
259253

260254
while (remaining) {
261255
unsigned framesThisTime = std::min(remaining, m_requestFrames);
262-
process(destination, framesThisTime, [&sourceProvider](AudioBus* bus, size_t framesToProcess) {
263-
sourceProvider.provideInput(bus, framesToProcess);
264-
});
256+
process(&sourceProvider, destination, framesThisTime);
265257

266258
destination += framesThisTime;
267259
remaining -= framesThisTime;
268260
}
269261
}
270262

271-
void SincResampler::process(float* destination, size_t framesToProcess, const Function<void(AudioBus* bus, size_t framesToProcess)>& provideInput)
263+
void SincResampler::process(AudioSourceProvider* sourceProvider, float* destination, size_t framesToProcess)
272264
{
273-
ASSERT(provideInput);
274-
if (!provideInput)
265+
ASSERT(sourceProvider);
266+
if (!sourceProvider)
275267
return;
268+
269+
m_sourceProvider = sourceProvider;
276270

277271
unsigned numberOfDestinationFrames = framesToProcess;
278272

279273
// Step (1)
280274
// Prime the input buffer at the start of the input stream.
281275
if (!m_isBufferPrimed) {
282-
consumeSource(m_r0, m_requestFrames, provideInput);
276+
consumeSource(m_r0, m_requestFrames);
283277
m_isBufferPrimed = true;
284278
}
285279

@@ -527,7 +521,7 @@ void SincResampler::process(float* destination, size_t framesToProcess, const Fu
527521

528522
// Step (5)
529523
// Refresh the buffer with more input.
530-
consumeSource(m_r0, m_requestFrames, provideInput);
524+
consumeSource(m_r0, m_requestFrames);
531525
}
532526
}
533527

‎Source/WebCore/platform/audio/SincResampler.h‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -45,17 +45,15 @@ class SincResampler final {
4545
// requestFrames controls the size in frames of the buffer requested by each provideInput() call.
4646
SincResampler(double scaleFactor, Optional<unsigned> requestFrames = WTF::nullopt);
4747

48-
size_t chunkSize() const { return m_chunkSize; }
49-
5048
// Processes numberOfSourceFrames from source to produce numberOfSourceFrames / scaleFactor frames in destination.
5149
void process(const float* source, float* destination, unsigned numberOfSourceFrames);
5250

53-
// Process with provideInput callback function for streaming applications.
54-
void process(float* destination, size_t framesToProcess, const Function<void(AudioBus* bus, size_t framesToProcess)>& provideInput);
51+
// Process with input source callback function for streaming applications.
52+
void process(AudioSourceProvider*, float* destination, size_t framesToProcess);
5553

5654
protected:
5755
void initializeKernel();
58-
void consumeSource(float* buffer, unsigned numberOfSourceFrames, const Function<void(AudioBus* bus, size_t framesToProcess)>& provideInput);
56+
void consumeSource(float* buffer, unsigned numberOfSourceFrames);
5957
void updateRegions(bool isSecondLoad);
6058

6159
double m_scaleFactor;
@@ -74,8 +72,6 @@ class SincResampler final {
7472
// The number of source frames processed per pass.
7573
unsigned m_blockSize { 0 };
7674

77-
size_t m_chunkSize { 0 };
78-
7975
// Source is copied into this buffer for each processing pass.
8076
AudioFloatArray m_inputBuffer;
8177

@@ -89,6 +85,9 @@ class SincResampler final {
8985

9086
const float* m_source { nullptr };
9187
unsigned m_sourceFramesAvailable { 0 };
88+
89+
// m_sourceProvider is used to provide the audio input stream to the resampler.
90+
AudioSourceProvider* m_sourceProvider { nullptr };
9291

9392
// The buffer is primed once at the very beginning of processing.
9493
bool m_isBufferPrimed { false };

0 commit comments

Comments
 (0)