Bug 1344785 - Clamp the resampling ratio sent to speex's resampler to something reasonnable, and drop the associated patch on upstream. r?karlt draft
authorPaul Adenot <paul@paul.cx>
Tue, 14 Mar 2017 11:58:40 +0100
changeset 498119 72d09b1f18f1ddbbb782e526129521cf4c0ed937
parent 497520 a035c2b6ae72b394debf576a29a97a1274232597
child 549095 636988e7b3b665449ccdb3e5700f69c63b9dc262
push id49122
push userpaul@paul.cx
push dateTue, 14 Mar 2017 10:59:37 +0000
reviewerskarlt
bugs1344785
milestone55.0a1
Bug 1344785 - Clamp the resampling ratio sent to speex's resampler to something reasonnable, and drop the associated patch on upstream. r?karlt MozReview-Commit-ID: 2Ebop7UyFHb
dom/media/webaudio/AudioBufferSourceNode.cpp
media/libspeex_resampler/set-rate-overflow-no-return.patch
media/libspeex_resampler/src/resample.c
media/libspeex_resampler/update.sh
--- a/dom/media/webaudio/AudioBufferSourceNode.cpp
+++ b/dom/media/webaudio/AudioBufferSourceNode.cpp
@@ -27,16 +27,22 @@ NS_IMPL_CYCLE_COLLECTION_INHERITED(Audio
                                    mPlaybackRate, mDetune)
 
 NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION_INHERITED(AudioBufferSourceNode)
 NS_INTERFACE_MAP_END_INHERITING(AudioScheduledSourceNode)
 
 NS_IMPL_ADDREF_INHERITED(AudioBufferSourceNode, AudioScheduledSourceNode)
 NS_IMPL_RELEASE_INHERITED(AudioBufferSourceNode, AudioScheduledSourceNode)
 
+// We clamp the resampling ratio to [-1000, 1000]. This limit is arbitrary
+// but high, to allow creative uses of the playback-rate. This is
+// guaranteed to not overflow during the various calculations performed
+// inside the resampler, which are performed using 32bit unsigned integers.
+const uint32_t MaxPlaybackRate = 1000;
+
 /**
  * Media-thread playback engine for AudioBufferSourceNode.
  * Nothing is played until a non-null buffer has been set (via
  * AudioNodeStream::SetBuffer) and a non-zero mBufferEnd has been set (via
  * AudioNodeStream::SetInt32Parameter).
  */
 class AudioBufferSourceNodeEngine final : public AudioNodeEngine
 {
@@ -179,19 +185,30 @@ public:
       mChannels = aChannels;
       mResampler = speex_resampler_init(mChannels, mBufferSampleRate, aOutRate,
                                         SPEEX_RESAMPLER_QUALITY_MIN,
                                         nullptr);
     } else {
       if (mResamplerOutRate == aOutRate) {
         return;
       }
-      if (speex_resampler_set_rate(mResampler, mBufferSampleRate, aOutRate) != RESAMPLER_ERR_SUCCESS) {
-        NS_ASSERTION(false, "speex_resampler_set_rate failed");
-        return;
+      MOZ_ASSERT(mBufferSampleRate >= WebAudioUtils::MinSampleRate);
+      float resamplingRatio = static_cast<float>(aOutRate) / mBufferSampleRate;
+      if (resamplingRatio > MaxPlaybackRate) {
+        resamplingRatio = MaxPlaybackRate;
+
+        aOutRate = resamplingRatio * mBufferSampleRate;
+      }
+      int rv = speex_resampler_set_rate(mResampler, mBufferSampleRate, aOutRate);
+      MOZ_ASSERT(rv == RESAMPLER_ERR_SUCCESS);
+      if (rv != RESAMPLER_ERR_SUCCESS) {
+        // This should not happen: it can fail in case of allocation, and we use
+        // an infallible allocator, and if the resampling rate calculation
+        // overflows, but we've taken care of this just above.
+        aOutRate = mBufferSampleRate;
       }
     }
 
     mResamplerOutRate = aOutRate;
 
     if (!BegunResampling()) {
       // Low pass filter effects from the resampler mean that samples before
       // the start time are influenced by resampling the buffer.  The input
deleted file mode 100644
--- a/media/libspeex_resampler/set-rate-overflow-no-return.patch
+++ /dev/null
@@ -1,25 +0,0 @@
-diff --git a/media/libspeex_resampler/src/resample.c b/media/libspeex_resampler/src/resample.c
---- a/media/libspeex_resampler/src/resample.c
-+++ b/media/libspeex_resampler/src/resample.c
-@@ -1141,18 +1141,19 @@ EXPORT int speex_resampler_set_rate_frac
- 
-    st->num_rate /= fact;
-    st->den_rate /= fact;
- 
-    if (old_den > 0)
-    {
-       for (i=0;i<st->nb_channels;i++)
-       {
--         if (_muldiv(&st->samp_frac_num[i],st->samp_frac_num[i],st->den_rate,old_den) != RESAMPLER_ERR_SUCCESS)
--            return RESAMPLER_ERR_OVERFLOW;
-+         if (_muldiv(&st->samp_frac_num[i],st->samp_frac_num[i],st->den_rate,old_den) != RESAMPLER_ERR_SUCCESS) {
-+           st->samp_frac_num[i] = st->den_rate-1;
-+         }
-          /* Safety net */
-          if (st->samp_frac_num[i] >= st->den_rate)
-             st->samp_frac_num[i] = st->den_rate-1;
-       }
-    }
- 
-    if (st->initialised)
-       return update_filter(st);
--- a/media/libspeex_resampler/src/resample.c
+++ b/media/libspeex_resampler/src/resample.c
@@ -1141,19 +1141,18 @@ EXPORT int speex_resampler_set_rate_frac
 
    st->num_rate /= fact;
    st->den_rate /= fact;
 
    if (old_den > 0)
    {
       for (i=0;i<st->nb_channels;i++)
       {
-         if (_muldiv(&st->samp_frac_num[i],st->samp_frac_num[i],st->den_rate,old_den) != RESAMPLER_ERR_SUCCESS) {
-           st->samp_frac_num[i] = st->den_rate-1;
-         }
+         if (_muldiv(&st->samp_frac_num[i],st->samp_frac_num[i],st->den_rate,old_den) != RESAMPLER_ERR_SUCCESS)
+            return RESAMPLER_ERR_OVERFLOW;
          /* Safety net */
          if (st->samp_frac_num[i] >= st->den_rate)
             st->samp_frac_num[i] = st->den_rate-1;
       }
    }
 
    if (st->initialised)
       return update_filter(st);
--- a/media/libspeex_resampler/update.sh
+++ b/media/libspeex_resampler/update.sh
@@ -20,9 +20,8 @@ cp $1/AUTHORS .
 cp $1/COPYING .
 
 # apply outstanding local patches
 patch -p3 < outside-speex.patch
 patch -p3 < simd-detect-runtime.patch
 patch -p3 < set-skip-frac.patch
 patch -p3 < hugemem.patch
 patch -p3 < remove-empty-asm-clobber.patch
-patch -p3 < set-rate-overflow-no-return.patch