Index: third_party/WebKit/Source/platform/audio/Reverb.cpp |
diff --git a/third_party/WebKit/Source/platform/audio/Reverb.cpp b/third_party/WebKit/Source/platform/audio/Reverb.cpp |
index 32c62acfe1903e73195d6d4b07786e82819630a9..b396f3cab7f36749b927498dd1c3f748448700ae 100644 |
--- a/third_party/WebKit/Source/platform/audio/Reverb.cpp |
+++ b/third_party/WebKit/Source/platform/audio/Reverb.cpp |
@@ -119,15 +119,18 @@ void Reverb::initialize(AudioBus* impulseResponseBuffer, |
size_t numberOfChannels, |
bool useBackgroundThreads) { |
m_impulseResponseLength = impulseResponseBuffer->length(); |
+ m_numberOfChannels = numberOfChannels; |
// The reverb can handle a mono impulse response and still do stereo |
// processing |
hongchan
2017/03/28 16:44:35
nit: a missing period.
|
size_t numResponseChannels = impulseResponseBuffer->numberOfChannels(); |
hongchan
2017/03/28 16:44:35
This can be unsigned.
Raymond Toy
2017/03/28 18:26:50
This requires changing various other bits of code
|
- m_convolvers.reserveCapacity(numberOfChannels); |
+ size_t numConvolvers = std::max(numResponseChannels, static_cast<size_t>(2)); |
+ m_convolvers.reserveCapacity(numConvolvers); |
hongchan
2017/03/28 16:44:35
Is |numConvolvers| properly named? Isn't it more o
Raymond Toy
2017/03/28 18:17:20
numberOfConvolverChannels is confusing too since t
|
int convolverRenderPhase = 0; |
- for (size_t i = 0; i < numResponseChannels; ++i) { |
- AudioChannel* channel = impulseResponseBuffer->channel(i); |
+ for (size_t i = 0; i < numConvolvers; ++i) { |
+ AudioChannel* channel = |
+ impulseResponseBuffer->channel(std::min(i, numResponseChannels - 1)); |
std::unique_ptr<ReverbConvolver> convolver = WTF::wrapUnique( |
new ReverbConvolver(channel, renderSliceSize, maxFFTSize, |
@@ -173,11 +176,42 @@ void Reverb::process(const AudioBus* sourceBus, |
// Handle input -> output matrixing... |
size_t numInputChannels = sourceBus->numberOfChannels(); |
size_t numOutputChannels = destinationBus->numberOfChannels(); |
- size_t numReverbChannels = m_convolvers.size(); |
- |
- if (numInputChannels == 2 && numReverbChannels == 2 && |
+ size_t numReverbChannels = m_numberOfChannels; |
hongchan
2017/03/28 16:44:35
I can't make a clear distinction between numReverb
Raymond Toy
2017/03/28 18:17:20
numReverbChannels is the number of channels in the
hongchan
2017/03/28 19:01:44
Yeah, this name seems to be problematic.
|
+ |
+ DCHECK_LE(numInputChannels, 2ul); |
+ DCHECK_LE(numOutputChannels, 2ul); |
+ DCHECK(numReverbChannels == 1 || numReverbChannels == 2 || |
+ numReverbChannels == 4); |
+ |
+ // These are the possible combinations of number inputs, response |
+ // channels and outputs channels that need to be supported: |
+ // |
+ // numInputChannels: 1 or 2 |
+ // numReverbChannels: 1, 2, or 4 |
+ // numOutputChannels: 1 or 2 |
+ // |
+ // Not all possible combinations are valid. numOutputChannels is |
+ // one only if both numInputChannels and numReverbChannels are 1. |
+ // Otherwise numOutputChannels MUST be 2. |
+ // |
+ // The valid combinations are |
+ // |
+ // Case in -> resp -> out |
+ // 1 1 -> 1 -> 1 |
+ // 2 1 -> 2 -> 2 |
+ // 3 1 -> 4 -> 2 |
+ // 4 2 -> 1 -> 2 |
+ // 5 2 -> 2 -> 2 |
+ // 6 2 -> 4 -> 2 |
+ |
+ if (numInputChannels == 2 && |
+ (numReverbChannels == 1 || numReverbChannels == 2) && |
numOutputChannels == 2) { |
- // 2 -> 2 -> 2 |
+ // Case 4 and 5: 2 -> 2 -> 2 or 2 -> 1 -> 2. |
+ // |
+ // These can be handled in the same way because in the latter |
+ // case, two connvolvers are still created with the second being a |
+ // copy of the first. |
const AudioChannel* sourceChannelR = sourceBus->channel(1); |
AudioChannel* destinationChannelR = destinationBus->channel(1); |
m_convolvers[0]->process(sourceChannelL, destinationChannelL, |
@@ -186,37 +220,20 @@ void Reverb::process(const AudioBus* sourceBus, |
framesToProcess); |
} else if (numInputChannels == 1 && numOutputChannels == 2 && |
numReverbChannels == 2) { |
- // 1 -> 2 -> 2 |
+ // Case 2: 1 -> 2 -> 2 |
for (int i = 0; i < 2; ++i) { |
AudioChannel* destinationChannel = destinationBus->channel(i); |
m_convolvers[i]->process(sourceChannelL, destinationChannel, |
framesToProcess); |
} |
- } else if (numInputChannels == 1 && numReverbChannels == 1 && |
- numOutputChannels == 2) { |
- // 1 -> 1 -> 2 |
- m_convolvers[0]->process(sourceChannelL, destinationChannelL, |
- framesToProcess); |
- |
- // simply copy L -> R |
- AudioChannel* destinationChannelR = destinationBus->channel(1); |
- bool isCopySafe = destinationChannelL->data() && |
- destinationChannelR->data() && |
- destinationChannelL->length() >= framesToProcess && |
- destinationChannelR->length() >= framesToProcess; |
- ASSERT(isCopySafe); |
- if (!isCopySafe) |
- return; |
- memcpy(destinationChannelR->mutableData(), destinationChannelL->data(), |
- sizeof(float) * framesToProcess); |
- } else if (numInputChannels == 1 && numReverbChannels == 1 && |
- numOutputChannels == 1) { |
- // 1 -> 1 -> 1 |
+ } else if (numInputChannels == 1 && numReverbChannels == 1) { |
+ // Case 1: 1 -> 1 -> 1 |
+ DCHECK_EQ(numOutputChannels, 1ul); |
m_convolvers[0]->process(sourceChannelL, destinationChannelL, |
framesToProcess); |
} else if (numInputChannels == 2 && numReverbChannels == 4 && |
numOutputChannels == 2) { |
- // 2 -> 4 -> 2 ("True" stereo) |
+ // Case 6: 2 -> 4 -> 2 ("True" stereo) |
const AudioChannel* sourceChannelR = sourceBus->channel(1); |
AudioChannel* destinationChannelR = destinationBus->channel(1); |
@@ -236,9 +253,9 @@ void Reverb::process(const AudioBus* sourceBus, |
destinationBus->sumFrom(*m_tempBuffer); |
} else if (numInputChannels == 1 && numReverbChannels == 4 && |
numOutputChannels == 2) { |
- // 1 -> 4 -> 2 (Processing mono with "True" stereo impulse response) |
- // This is an inefficient use of a four-channel impulse response, but we |
- // should handle the case. |
+ // Case 3: 1 -> 4 -> 2 (Processing mono with "True" stereo impulse |
+ // response) This is an inefficient use of a four-channel impulse |
+ // response, but we should handle the case. |
AudioChannel* destinationChannelR = destinationBus->channel(1); |
AudioChannel* tempChannelL = m_tempBuffer->channel(0); |
@@ -256,8 +273,7 @@ void Reverb::process(const AudioBus* sourceBus, |
destinationBus->sumFrom(*m_tempBuffer); |
} else { |
- // Handle gracefully any unexpected / unsupported matrixing |
- // FIXME: add code for 5.1 support... |
+ NOTREACHED(); |
destinationBus->zero(); |
} |
} |