Fix input_needed_for_output to not return one less than needed, causing draining down the line

.github/workflows/build.yml

@@ -41,6 +41,8 @@ jobs:
       run: |
         brew install switchaudio-osx
         brew install blackhole-2ch
+        sudo kill -9 `pgrep coreaudiod`
+        sleep 10
         SwitchAudioSource -s "BlackHole 2ch" -t input
         SwitchAudioSource -s "BlackHole 2ch" -t output
 

.gitignore

@@ -1,2 +1,3 @@
 .vscode/
+.cache/
 build/

CMakeLists.txt

@@ -42,6 +42,7 @@ else()
 endif()
 
 if(BUILD_TESTS)
+  add_definitions(-DGTEST_ENABLED=1)
   find_package(GTest QUIET)
   if(TARGET GTest::Main)
     add_library(gtest_main ALIAS GTest::Main)
@@ -452,4 +453,3 @@ add_custom_target(clang-format-check
   | xargs -0 ${CLANG_FORMAT_BINARY} -Werror -n
   COMMENT "Check formatting with clang-format"
   VERBATIM)
-

src/cubeb_audio_dump.cpp

@@ -149,6 +149,7 @@ struct cubeb_audio_dump_session {
     stream->close();
     streams.erase(std::remove(streams.begin(), streams.end(), stream),
                   streams.end());
+    delete stream;
     return CUBEB_OK;
   }
   int start()

src/cubeb_resampler.cpp

@@ -371,3 +371,11 @@ cubeb_resampler_latency(cubeb_resampler * resampler)
 {
   return resampler->latency();
 }
+
+#ifdef GTEST_ENABLED
+cubeb_resampler_stats
+cubeb_resampler_stats_get(cubeb_resampler * resampler)
+{
+  return resampler->stats();
+}
+#endif

src/cubeb_resampler.h

@@ -84,6 +84,22 @@ cubeb_resampler_destroy(cubeb_resampler * resampler);
 long
 cubeb_resampler_latency(cubeb_resampler * resampler);
 
+/**
+ * Test-only introspection API to ensure that there is no buffering
+ * buildup when resampling.
+ */
+#ifdef GTEST_ENABLED
+typedef struct {
+  size_t input_input_buffer_size;
+  size_t input_output_buffer_size;
+  size_t output_input_buffer_size;
+  size_t output_output_buffer_size;
+} cubeb_resampler_stats;
+
+cubeb_resampler_stats
+cubeb_resampler_stats_get(cubeb_resampler * resampler);
+#endif
+
 #if defined(__cplusplus)
 }
 #endif

src/cubeb_resampler_internal.h

@@ -56,6 +56,9 @@ struct cubeb_resampler {
   virtual long fill(void * input_buffer, long * input_frames_count,
                     void * output_buffer, long frames_needed) = 0;
   virtual long latency() = 0;
+#ifdef GTEST_ENABLED
+  virtual cubeb_resampler_stats stats() = 0;
+#endif
   virtual ~cubeb_resampler() {}
 };
 
@@ -86,6 +89,18 @@ class passthrough_resampler : public cubeb_resampler, public processor {
 
   virtual long latency() { return 0; }
 
+#ifdef GTEST_ENABLED
+  virtual cubeb_resampler_stats stats()
+  {
+    cubeb_resampler_stats stats;
+    stats.input_input_buffer_size = internal_input_buffer.length();
+    stats.input_output_buffer_size = 0;
+    stats.output_input_buffer_size = 0;
+    stats.output_output_buffer_size = 0;
+    return stats;
+  }
+#endif
+
   void drop_audio_if_needed()
   {
     uint32_t to_keep = min_buffered_audio_frame(sample_rate);
@@ -122,6 +137,22 @@ class cubeb_resampler_speex : public cubeb_resampler {
   virtual long fill(void * input_buffer, long * input_frames_count,
                     void * output_buffer, long output_frames_needed);
 
+#ifdef GTEST_ENABLED
+  virtual cubeb_resampler_stats stats()
+  {
+    cubeb_resampler_stats stats = {};
+    if (input_processor) {
+      stats.input_input_buffer_size = input_processor->input_buffer_size();
+      stats.input_output_buffer_size = input_processor->output_buffer_size();
+    }
+    if (output_processor) {
+      stats.output_input_buffer_size = output_processor->input_buffer_size();
+      stats.output_output_buffer_size = output_processor->output_buffer_size();
+    }
+    return stats;
+  }
+#endif
+
   virtual long latency()
   {
     if (input_processor && output_processor) {
@@ -280,29 +311,28 @@ template <typename T> class cubeb_resampler_speex_one_way : public processor {
   }
 
   /** 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. */
+   * at least `output_frame_count` resampled frames. */
   uint32_t input_needed_for_output(int32_t output_frame_count) const
   {
     assert(output_frame_count >= 0); // Check overflow
     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;
-    }
-    return (uint32_t)ceilf(input_frames_needed);
+    float input_frames_needed_frac =
+        static_cast<float>(output_frame_count) * resampling_ratio;
+    // speex_resample()` can be irregular in its consumption of input samples.
+    // Provide one more frame than the number that would be required with
+    // regular consumption, to make the speex resampler behave more regularly,
+    // and so predictably.
+    auto input_frame_needed =
+        1 + static_cast<int32_t>(ceilf(input_frames_needed_frac));
+    input_frame_needed -= std::min(unresampled_frames_left, input_frame_needed);
+    return input_frame_needed;
   }
 
   /** Returns a pointer to the input buffer, that contains empty space for at
-   * least `frame_count` elements. This is useful so that consumer can directly
-   * write into the input buffer of the resampler. The pointer returned is
-   * adjusted so that leftover data are not overwritten.
+   * least `frame_count` elements. This is useful so that consumer can
+   * directly write into the input buffer of the resampler. The pointer
+   * returned is adjusted so that leftover data are not overwritten.
    */
   T * input_buffer(size_t frame_count)
   {
@@ -312,8 +342,8 @@ template <typename T> class cubeb_resampler_speex_one_way : public processor {
     return resampling_in_buffer.data() + leftover_samples;
   }
 
-  /** This method works with `input_buffer`, and allows to inform the processor
-      how much frames have been written in the provided buffer. */
+  /** This method works with `input_buffer`, and allows to inform the
+     processor how much frames have been written in the provided buffer. */
   void written(size_t written_frames)
   {
     resampling_in_buffer.set_length(leftover_samples +
@@ -331,6 +361,11 @@ template <typename T> class cubeb_resampler_speex_one_way : public processor {
     }
   }
 
+#ifdef GTEST_ENABLED
+  size_t input_buffer_size() const { return resampling_in_buffer.length(); }
+  size_t output_buffer_size() const { return resampling_out_buffer.length(); }
+#endif
+
 private:
   /** Wrapper for the speex resampling functions to have a typed
    * interface. */
@@ -359,6 +394,7 @@ template <typename T> class cubeb_resampler_speex_one_way : public processor {
             output_frame_count);
     assert(rv == RESAMPLER_ERR_SUCCESS);
   }
+
   /** The state for the speex resampler used internaly. */
   SpeexResamplerState * speex_resampler;
   /** Source rate / target rate. */
@@ -371,8 +407,8 @@ template <typename T> class cubeb_resampler_speex_one_way : public processor {
   auto_array<T> resampling_out_buffer;
   /** Additional latency inserted into the pipeline for synchronisation. */
   uint32_t additional_latency;
-  /** When `input_buffer` is called, this allows tracking the number of samples
-      that were in the buffer. */
+  /** When `input_buffer` is called, this allows tracking the number of
+     samples that were in the buffer. */
   uint32_t leftover_samples;
 };
 
@@ -417,8 +453,8 @@ template <typename T> class delay_line : public processor {
     return delay_output_buffer.data();
   }
   /** Get a pointer to the first writable location in the input buffer>
-   * @parameter frames_needed the number of frames the user needs to write into
-   * the buffer.
+   * @parameter frames_needed the number of frames the user needs to write
+   * into the buffer.
    * @returns a pointer to a location in the input buffer where #frames_needed
    * can be writen. */
   T * input_buffer(uint32_t frames_needed)
@@ -428,8 +464,8 @@ template <typename T> class delay_line : public processor {
                                frames_to_samples(frames_needed));
     return delay_input_buffer.data() + leftover_samples;
   }
-  /** This method works with `input_buffer`, and allows to inform the processor
-      how much frames have been written in the provided buffer. */
+  /** This method works with `input_buffer`, and allows to inform the
+     processor how much frames have been written in the provided buffer. */
   void written(size_t frames_written)
   {
     delay_input_buffer.set_length(leftover_samples +
@@ -450,8 +486,8 @@ template <typename T> class delay_line : public processor {
 
     return to_pop;
   }
-  /** Returns the number of frames one needs to input into the delay line to get
-   * #frames_needed frames back.
+  /** 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. */
@@ -473,15 +509,21 @@ template <typename T> class delay_line : public processor {
     uint32_t to_keep = min_buffered_audio_frame(sample_rate);
     if (available > to_keep) {
       ALOGV("Dropping %u frames", available - to_keep);
+
       delay_input_buffer.pop(nullptr, frames_to_samples(available - to_keep));
     }
   }
 
+#ifdef GTEST_ENABLED
+  size_t input_buffer_size() const { return delay_input_buffer.length(); }
+  size_t output_buffer_size() const { return delay_output_buffer.length(); }
+#endif
+
 private:
   /** The length, in frames, of this delay line */
   uint32_t length;
-  /** When `input_buffer` is called, this allows tracking the number of samples
-      that where in the buffer. */
+  /** When `input_buffer` is called, this allows tracking the number of
+     samples that where in the buffer. */
   uint32_t leftover_samples;
   /** The input buffer, where the delay is applied. */
   auto_array<T> delay_input_buffer;
@@ -511,8 +553,8 @@ cubeb_resampler_create_internal(cubeb_stream * stream,
          "need at least one valid parameter pointer.");
 
   /* All the streams we have have a sample rate that matches the target
-     sample rate, use a no-op resampler, that simply forwards the buffers to the
-     callback. */
+     sample rate, use a no-op resampler, that simply forwards the buffers to
+     the callback. */
   if (((input_params && input_params->rate == target_rate) &&
        (output_params && output_params->rate == target_rate)) ||
       (input_params && !output_params && (input_params->rate == target_rate)) ||

test/common.h

@@ -119,6 +119,7 @@ print_log(const char * msg, ...)
   va_list args;
   va_start(args, msg);
   vprintf(msg, args);
+  printf("\n");
   va_end(args);
 }