bug 1429666 cubeb_resampler_speex: don't call data callback while draining r=padenot
authorKarl Tomlinson <karlt+@karlt.net>
Thu, 11 Jan 2018 13:30:24 +1300
changeset 398941 1ad55edca07d6706bdec5314724fd2361def43c3
parent 398940 0524b54f7f9dc0568a55b5d70fef4180d4cb1838
child 398942 52100437f9b37345a286f048349f6f3f6d4298a5
push id98867
push usercbrindusan@mozilla.com
push dateFri, 12 Jan 2018 11:35:08 +0000
treeherdermozilla-inbound@b7adfffd7c22 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspadenot
bugs1429666
milestone59.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 1429666 cubeb_resampler_speex: don't call data callback while draining r=padenot MozReview-Commit-ID: 1XEzZjPGai9
media/libcubeb/README_MOZILLA
media/libcubeb/gtest/test_resampler.cpp
media/libcubeb/src/cubeb_resampler.cpp
media/libcubeb/src/cubeb_resampler_internal.h
--- a/media/libcubeb/README_MOZILLA
+++ b/media/libcubeb/README_MOZILLA
@@ -1,8 +1,12 @@
 The source from this directory was copied from the cubeb 
 git repository using the update.sh script.  The only changes
-made were those applied by update.sh and the addition of
-Makefile.in build files for the Mozilla build system.
+made were those applied by update.sh, the addition of
+Makefile.in build files for the Mozilla build system,
+and the following patches, which may be overwritten when
+included upstream.
+https://github.com/kinetiknz/cubeb/pull/398/commits/c8e66dee61a35e6a6d54e3630e1668bdbd6984b4
+https://github.com/kinetiknz/cubeb/pull/398/commits/2ed979bc891cf1a7822799947a357d4d3b625964
 
 The cubeb git repository is: git://github.com/kinetiknz/cubeb.git
 
 The git commit ID used was bda37c26b0ed46c568e35b10728fc498262778f3 (2017-12-28 09:34:08 +1000)
--- a/media/libcubeb/gtest/test_resampler.cpp
+++ b/media/libcubeb/gtest/test_resampler.cpp
@@ -492,52 +492,55 @@ TEST(cubeb, resampler_output_only_noop)
   got = cubeb_resampler_fill(resampler, nullptr, nullptr,
                              out_buffer, out_frames);
 
   ASSERT_EQ(got, out_frames);
 
   cubeb_resampler_destroy(resampler);
 }
 
-long test_drain_data_cb(cubeb_stream * /*stm*/, void * /*user_ptr*/,
+long test_drain_data_cb(cubeb_stream * /*stm*/, void * user_ptr,
                         const void * input_buffer,
                         void * output_buffer, long frame_count)
 {
   EXPECT_TRUE(output_buffer);
   EXPECT_TRUE(!input_buffer);
-  return frame_count - 10;
+  auto cb_count = static_cast<int *>(user_ptr);
+  (*cb_count)++;
+  return frame_count - 1;
 }
 
 TEST(cubeb, resampler_drain)
 {
   cubeb_stream_params output_params;
   int target_rate;
 
   output_params.rate = 44100;
   output_params.channels = 1;
   output_params.format = CUBEB_SAMPLE_FLOAT32NE;
   target_rate = 48000;
+  int cb_count = 0;
 
   cubeb_resampler * resampler =
     cubeb_resampler_create((cubeb_stream*)nullptr, nullptr, &output_params, target_rate,
-                           test_drain_data_cb, nullptr,
+                           test_drain_data_cb, &cb_count,
                            CUBEB_RESAMPLER_QUALITY_VOIP);
 
   const long out_frames = 128;
   float out_buffer[out_frames];
   long got;
 
   do {
     got = cubeb_resampler_fill(resampler, nullptr, nullptr,
                                out_buffer, out_frames);
   } while (got == out_frames);
 
-  /* If the above is not an infinite loop, the drain was a success, just mark
-   * this test as such. */
-  ASSERT_TRUE(true);
+  /* The callback should be called once but not again after returning <
+   * frame_count. */
+  ASSERT_EQ(cb_count, 1);
 
   cubeb_resampler_destroy(resampler);
 }
 
 // gtest does not support using ASSERT_EQ and friend in a function that returns
 // a value.
 void check_output(const void * input_buffer, void * output_buffer, long frame_count)
 {
--- a/media/libcubeb/src/cubeb_resampler.cpp
+++ b/media/libcubeb/src/cubeb_resampler.cpp
@@ -140,37 +140,43 @@ template<typename T, typename InputProce
 long
 cubeb_resampler_speex<T, InputProcessor, OutputProcessor>
 ::fill_internal_output(T * input_buffer, long * input_frames_count,
                        T * output_buffer, long output_frames_needed)
 {
   assert(!input_buffer && (!input_frames_count || *input_frames_count == 0) &&
          output_buffer && output_frames_needed);
 
-  long got = 0;
-  T * out_unprocessed = nullptr;
-  long output_frames_before_processing = 0;
+  if (!draining) {
+    long got = 0;
+    T * out_unprocessed = nullptr;
+    long output_frames_before_processing = 0;
 
-  /* fill directly the input buffer of the output processor to save a copy */
-  output_frames_before_processing =
-    output_processor->input_needed_for_output(output_frames_needed);
+    /* fill directly the input buffer of the output processor to save a copy */
+    output_frames_before_processing =
+      output_processor->input_needed_for_output(output_frames_needed);
+
+    out_unprocessed =
+      output_processor->input_buffer(output_frames_before_processing);
 
-  out_unprocessed =
-    output_processor->input_buffer(output_frames_before_processing);
+    got = data_callback(stream, user_ptr,
+                        nullptr, out_unprocessed,
+                        output_frames_before_processing);
+
+    if (got < output_frames_before_processing) {
+      draining = true;
 
-  got = data_callback(stream, user_ptr,
-                      nullptr, out_unprocessed,
-                      output_frames_before_processing);
+      if (got < 0) {
+        return got;
+      }
+    }
 
-  if (got < 0) {
-    return got;
+    output_processor->written(got);
   }
 
-  output_processor->written(got);
-
   /* Process the output. If not enough frames have been returned from the
   * callback, drain the processors. */
   return output_processor->output(output_buffer, output_frames_needed);
 }
 
 template<typename T, typename InputProcessor, typename OutputProcessor>
 long
 cubeb_resampler_speex<T, InputProcessor, OutputProcessor>
@@ -199,21 +205,26 @@ cubeb_resampler_speex<T, InputProcessor,
 }
 
 template<typename T, typename InputProcessor, typename OutputProcessor>
 long
 cubeb_resampler_speex<T, InputProcessor, OutputProcessor>
 ::fill_internal_duplex(T * in_buffer, long * input_frames_count,
                        T * out_buffer, long output_frames_needed)
 {
+  if (draining) {
+    // discard input and drain any signal remaining in the resampler.
+    return output_processor->output(out_buffer, output_frames_needed);
+  }
+
   /* The input data, after eventual resampling. This is passed to the callback. */
   T * resampled_input = nullptr;
   /* The output buffer passed down in the callback, that might be resampled. */
   T * out_unprocessed = nullptr;
-  size_t output_frames_before_processing = 0;
+  long output_frames_before_processing = 0;
   /* The number of frames returned from the callback. */
   long got = 0;
 
   /* We need to determine how much frames to present to the consumer.
    * - If we have a two way stream, but we're only resampling input, we resample
    * the input to the number of output frames.
    * - If we have a two way stream, but we're only resampling the output, we
    * resize the input buffer of the output resampler to the number of input
@@ -238,18 +249,22 @@ cubeb_resampler_speex<T, InputProcessor,
   } else {
     resampled_input = nullptr;
   }
 
   got = data_callback(stream, user_ptr,
                       resampled_input, out_unprocessed,
                       output_frames_before_processing);
 
-  if (got < 0) {
-    return got;
+  if (got < output_frames_before_processing) {
+    draining = true;
+
+    if (got < 0) {
+      return got;
+    }
   }
 
   output_processor->written(got);
 
   input_processor->drop_audio_if_needed();
 
   /* Process the output. If not enough frames have been returned from the
    * callback, drain the processors. */
--- a/media/libcubeb/src/cubeb_resampler_internal.h
+++ b/media/libcubeb/src/cubeb_resampler_internal.h
@@ -57,21 +57,21 @@ struct cubeb_resampler {
 
 /** Base class for processors. This is just used to share methods for now. */
 class processor {
 public:
   explicit processor(uint32_t channels)
     : channels(channels)
   {}
 protected:
-  size_t frames_to_samples(size_t frames)
+  size_t frames_to_samples(size_t frames) const
   {
     return frames * channels;
   }
-  size_t samples_to_frames(size_t samples)
+  size_t samples_to_frames(size_t samples) const
   {
     assert(!(samples % channels));
     return samples / channels;
   }
   /** The number of channel of the audio buffers to be resampled. */
   const uint32_t channels;
 };
 
@@ -152,16 +152,17 @@ private:
                             T * output_buffer, long output_frames_needed);
 
   std::unique_ptr<InputProcessing> input_processor;
   std::unique_ptr<OutputProcessing> output_processor;
   processing_callback fill_internal;
   cubeb_stream * const stream;
   const cubeb_data_callback data_callback;
   void * const user_ptr;
+  bool draining = false;
 };
 
 /** Handles one way of a (possibly) duplex resampler, working on interleaved
  * audio buffers of type T. This class is designed so that the number of frames
  * coming out of the resampler can be precisely controled. It manages its own
  * input buffer, and can use the caller's output buffer, or allocate its own. */
 template<typename T>
 class cubeb_resampler_speex_one_way : public processor {
@@ -277,17 +278,17 @@ public:
 
     return latency;
   }
 
   /** Returns the number of frames to pass in the input of the resampler to have
    * exactly `output_frame_count` resampled frames. This can return a number
    * slightly bigger than what is strictly necessary, but it guaranteed that the
    * number of output frames will be exactly equal. */
-  uint32_t input_needed_for_output(uint32_t output_frame_count)
+  uint32_t input_needed_for_output(uint32_t output_frame_count) const
   {
     int32_t unresampled_frames_left = samples_to_frames(resampling_in_buffer.length());
     int32_t resampled_frames_left = samples_to_frames(resampling_out_buffer.length());
     float input_frames_needed =
       (output_frame_count - unresampled_frames_left) * resampling_ratio
         - resampled_frames_left;
     if (input_frames_needed < 0) {
       return 0;
@@ -456,17 +457,17 @@ public:
 
     return to_pop;
   }
   /** Returns the number of frames one needs to input into the delay line to get
    * #frames_needed frames back.
    * @parameter frames_needed the number of frames one want to write into the
    * delay_line
    * @returns the number of frames one will get. */
-  size_t input_needed_for_output(uint32_t frames_needed)
+  size_t input_needed_for_output(uint32_t frames_needed) const
   {
     return frames_needed;
   }
   /** Returns the number of frames produces for `input_frames` frames in input */
   size_t output_for_input(uint32_t input_frames)
   {
     return input_frames;
   }