bug 1429666 cubeb_resampler_speex: don't call data callback while draining r?padenot draft
authorKarl Tomlinson <karlt+@karlt.net>
Thu, 11 Jan 2018 13:30:24 +1300
changeset 718928 2ad7e3f0a414063d072737a1acde2da047b52c6c
parent 718845 4db166f0442dddc5b9011c722d7499501fedf283
child 745639 34888f1bb7e4821bfe14870335735a2bc984d0fe
push id95090
push userktomlinson@mozilla.com
push dateThu, 11 Jan 2018 04:48:00 +0000
reviewerspadenot
bugs1429666
milestone59.0a1
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;
   }