Bug 890716. Fix race in Reverb::Reverb. r=padenot
authorRobert O'Callahan <robert@ocallahan.org>
Fri, 26 Jul 2013 13:35:22 +1200
changeset 140130 6b4ff981027f3232bf23cd598fa848d57b064b48
parent 140129 5f7fcbc8fd2777d5767ef1364658455bcacbbcc0
child 140131 366608750a6e9fbd16893250d675f4cfd5266b0c
push id25016
push userryanvm@gmail.com
push dateSat, 27 Jul 2013 02:25:56 +0000
treeherdermozilla-central@fb48c7d58b8b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspadenot
bugs890716
milestone25.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 890716. Fix race in Reverb::Reverb. r=padenot
content/media/AudioNodeEngine.cpp
content/media/AudioNodeEngine.h
content/media/webaudio/blink/Reverb.cpp
content/media/webaudio/blink/Reverb.h
--- a/content/media/AudioNodeEngine.cpp
+++ b/content/media/AudioNodeEngine.cpp
@@ -34,16 +34,30 @@ WriteZeroesToAudioBlock(AudioChunk* aChu
   if (aLength == 0)
     return;
   for (uint32_t i = 0; i < aChunk->mChannelData.Length(); ++i) {
     memset(static_cast<float*>(const_cast<void*>(aChunk->mChannelData[i])) + aStart,
            0, aLength*sizeof(float));
   }
 }
 
+void AudioBufferCopyWithScale(const float* aInput,
+                              float aScale,
+                              float* aOutput,
+                              uint32_t aSize)
+{
+  if (aScale == 1.0f) {
+    PodCopy(aOutput, aInput, aSize);
+  } else {
+    for (uint32_t i = 0; i < aSize; ++i) {
+      aOutput[i] = aInput[i]*aScale;
+    }
+  }
+}
+
 void AudioBufferAddWithScale(const float* aInput,
                              float aScale,
                              float* aOutput,
                              uint32_t aSize)
 {
   if (aScale == 1.0f) {
     for (uint32_t i = 0; i < aSize; ++i) {
       aOutput[i] += aInput[i];
--- a/content/media/AudioNodeEngine.h
+++ b/content/media/AudioNodeEngine.h
@@ -83,16 +83,24 @@ private:
 void AllocateAudioBlock(uint32_t aChannelCount, AudioChunk* aChunk);
 
 /**
  * aChunk must have been allocated by AllocateAudioBlock.
  */
 void WriteZeroesToAudioBlock(AudioChunk* aChunk, uint32_t aStart, uint32_t aLength);
 
 /**
+ * Copy with scale. aScale == 1.0f should be optimized.
+ */
+void AudioBufferCopyWithScale(const float* aInput,
+                              float aScale,
+                              float* aOutput,
+                              uint32_t aSize);
+
+/**
  * Pointwise multiply-add operation. aScale == 1.0f should be optimized.
  */
 void AudioBufferAddWithScale(const float* aInput,
                              float aScale,
                              float* aOutput,
                              uint32_t aSize);
 
 /**
--- a/content/media/webaudio/blink/Reverb.cpp
+++ b/content/media/webaudio/blink/Reverb.cpp
@@ -75,51 +75,53 @@ static float calculateNormalizationScale
 
     return scale;
 }
 
 Reverb::Reverb(ThreadSharedFloatArrayBufferList* impulseResponse, size_t impulseResponseBufferLength, size_t renderSliceSize, size_t maxFFTSize, size_t numberOfChannels, bool useBackgroundThreads, bool normalize, float sampleRate)
 {
     float scale = 1;
 
+    nsAutoTArray<const float*,4> irChannels;
+    for (size_t i = 0; i < impulseResponse->GetChannels(); ++i) {
+        irChannels.AppendElement(impulseResponse->GetData(i));
+    }
+    nsAutoTArray<float,1024> tempBuf;
+
     if (normalize) {
         scale = calculateNormalizationScale(impulseResponse, impulseResponseBufferLength, sampleRate);
 
         if (scale) {
-            for (uint32_t i = 0; i < impulseResponse->GetChannels(); ++i) {
-                AudioBufferInPlaceScale(const_cast<float*>(impulseResponse->GetData(i)),
-                                        1, scale, impulseResponseBufferLength);
+            tempBuf.SetLength(irChannels.Length()*impulseResponseBufferLength);
+            for (uint32_t i = 0; i < irChannels.Length(); ++i) {
+                float* buf = &tempBuf[i*impulseResponseBufferLength];
+                AudioBufferCopyWithScale(irChannels[i], scale, buf,
+                                         impulseResponseBufferLength);
+                irChannels[i] = buf;
             }
         }
     }
 
-    initialize(impulseResponse, impulseResponseBufferLength, renderSliceSize, maxFFTSize, numberOfChannels, useBackgroundThreads);
-
-    // Undo scaling since this shouldn't be a destructive operation on impulseResponse.
-    // FIXME: What about roundoff? Perhaps consider making a temporary scaled copy
-    // instead of scaling and unscaling in place.
-    if (normalize && scale) {
-        for (uint32_t i = 0; i < impulseResponse->GetChannels(); ++i) {
-            AudioBufferInPlaceScale(const_cast<float*>(impulseResponse->GetData(i)),
-                                    1, 1 / scale, impulseResponseBufferLength);
-        }
-    }
+    initialize(irChannels, impulseResponseBufferLength, renderSliceSize,
+               maxFFTSize, numberOfChannels, useBackgroundThreads);
 }
 
-void Reverb::initialize(ThreadSharedFloatArrayBufferList* impulseResponseBuffer, size_t impulseResponseBufferLength, size_t renderSliceSize, size_t maxFFTSize, size_t numberOfChannels, bool useBackgroundThreads)
+void Reverb::initialize(const nsTArray<const float*>& impulseResponseBuffer,
+                        size_t impulseResponseBufferLength, size_t renderSliceSize,
+                        size_t maxFFTSize, size_t numberOfChannels, bool useBackgroundThreads)
 {
     m_impulseResponseLength = impulseResponseBufferLength;
 
     // The reverb can handle a mono impulse response and still do stereo processing
-    size_t numResponseChannels = impulseResponseBuffer->GetChannels();
+    size_t numResponseChannels = impulseResponseBuffer.Length();
     m_convolvers.SetCapacity(numberOfChannels);
 
     int convolverRenderPhase = 0;
     for (size_t i = 0; i < numResponseChannels; ++i) {
-        const float* channel = impulseResponseBuffer->GetData(i);
+        const float* channel = impulseResponseBuffer[i];
         size_t length = impulseResponseBufferLength;
 
         nsAutoPtr<ReverbConvolver> convolver(new ReverbConvolver(channel, length, renderSliceSize, maxFFTSize, convolverRenderPhase, useBackgroundThreads));
         m_convolvers.AppendElement(convolver.forget());
 
         convolverRenderPhase += renderSliceSize;
     }
 
--- a/content/media/webaudio/blink/Reverb.h
+++ b/content/media/webaudio/blink/Reverb.h
@@ -50,17 +50,17 @@ public:
 
     void process(const mozilla::AudioChunk* sourceBus, mozilla::AudioChunk* destinationBus, size_t framesToProcess);
     void reset();
 
     size_t impulseResponseLength() const { return m_impulseResponseLength; }
     size_t latencyFrames() const;
 
 private:
-    void initialize(mozilla::ThreadSharedFloatArrayBufferList* impulseResponseBuffer, size_t impulseResponseBufferLength, size_t renderSliceSize, size_t maxFFTSize, size_t numberOfChannels, bool useBackgroundThreads);
+    void initialize(const nsTArray<const float*>& impulseResponseBuffer, size_t impulseResponseBufferLength, size_t renderSliceSize, size_t maxFFTSize, size_t numberOfChannels, bool useBackgroundThreads);
 
     size_t m_impulseResponseLength;
 
     nsTArray<nsAutoPtr<ReverbConvolver> > m_convolvers;
 
     // For "True" stereo processing
     mozilla::AudioChunk m_tempBuffer;
 };