Make OS audio buffer size limits visible.

media/audio/audio_manager_unittest.cc

Index: media/audio/audio_manager_unittest.cc
diff --git a/media/audio/audio_manager_unittest.cc b/media/audio/audio_manager_unittest.cc
index 904ee12fd9847dffdbf3527025e3a8dd1cb09a87..93a6594e349c6e92364754999bff06abcf43b012 100644
--- a/media/audio/audio_manager_unittest.cc
+++ b/media/audio/audio_manager_unittest.cc
@@ -25,7 +25,9 @@
 #include "media/audio/audio_unittest_util.h"
 #include "media/audio/fake_audio_log_factory.h"
 #include "media/audio/fake_audio_manager.h"
+#include "media/audio/mock_audio_source_callback.h"
 #include "media/audio/test_audio_thread.h"
+#include "media/base/limits.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -33,6 +35,11 @@
 #include "media/audio/alsa/audio_manager_alsa.h"
 #endif  // defined(USE_ALSA)
 
+#if defined(OS_MACOSX)
+#include "media/audio/mac/audio_manager_mac.h"
+#include "media/base/mac/audio_util_mac.h"
+#endif
+
 #if defined(OS_WIN)
 #include "base/win/scoped_com_initializer.h"
 #include "media/audio/win/audio_manager_win.h"
@@ -677,4 +684,109 @@ TEST_F(AudioManagerTest, AudioDebugRecording) {
   audio_manager_->DisableOutputDebugRecording();
 }
 
+#if defined(OS_MACOSX) || defined(USE_CRAS) || defined(USE_PULSEAUDIO)
+#if defined(OS_MACOSX)
+class TestAudioManagerBase : public AudioManagerMac {
+#elif defined(USE_CRAS)
+class TestAudioManagerBase : public AudioManagerCras {
+#elif defined(USE_PULSEAUDIO)
+class TestAudioManagerBase : public AudioManagerPulse {
+#endif
+ public:
+  TestAudioManagerBase(std::unique_ptr<AudioThread> audio_thread,
+                       AudioLogFactory* audio_log_factory)
+      : AudioManagerMac(std::move(audio_thread), audio_log_factory) {}
+
+  AudioParameters GetPreferredOutputStreamParametersForTesting(
+      const std::string& output_device_id,
+      const AudioParameters& input_params) {
+    return GetPreferredOutputStreamParameters(output_device_id, input_params);
+  }
+};  // namespace media
+
+ACTION(ZeroBuffer) {
+  arg3->Zero();
+}
+
+ACTION_P3(MaybeSignalEvent, counter, signal_at_count, event) {
+  if (++(*counter) == signal_at_count)
+    event->Signal();
+}
+
+TEST_F(AudioManagerTest, CheckMinimumAudioBufferSizes) {
+  ABORT_AUDIO_TEST_IF_NOT(OutputDevicesAvailable());
+
+  CreateAudioManagerForTesting<TestAudioManagerBase>();
+  DCHECK(audio_manager_);
+
+  TestAudioManagerBase* test_audio_manager_base =
+      static_cast<TestAudioManagerBase*>(audio_manager_.get());
+
+  AudioParameters default_params =
+      test_audio_manager_base->GetPreferredOutputStreamParametersForTesting(
+          "", AudioParameters());
+  ASSERT_LT(default_params.frames_per_buffer(),
+            media::limits::kMaxAudioBufferSize);
+
+#if defined(OS_MACOSX)
+  // On OSX the preferred output buffer size is higher than the minimum
+  // but users may request the minimum size explicitly.
+  ASSERT_GT(
+      default_params.frames_per_buffer(),
+      audio_util_mac::GetMinAudioBufferSizeForSampleRate(
+          media::limits::kMinAudioBufferSize, default_params.sample_rate()));
+#elif defined(USE_CRAS)
+  // On CRAS the preferred output buffer size varies per board so may be as low
+  // as the minimum in some cases. The minimum can always be requested.

[o1ka, 2017/06/27 16:31:19]

"The minimum can always be requested." - what does it actually mean here?

[Andrew MacPherson, 2017/06/27 17:03:40]

I've updated the comment, I meant that the "preferred buffer size" on CRAS has
different values depending on the board but the kMinAudioBufferSize can always
be requested explicitly (via the latencyHint) regardless of per-board default
values.

+  ASSERT_GE(params.frames_per_buffer(), media::limits::kMinAudioBufferSize);
+#elif defined(USE_PULSEAUDIO)
+  ASSERT_EQ(params.frames_per_buffer(), media::limits::kMinAudioBufferSize);
+#else
+  NOTREACHED();
+#endif
+
+  AudioParameters params;

[o1ka, 2017/06/27 16:31:19]

Make it a separate test maybe?

[Andrew MacPherson, 2017/06/27 17:03:40]

Done.

+
+  AudioParameters min_params = default_params;
+  min_params.set_frames_per_buffer(media::limits::kMinAudioBufferSize / 2);
+  params =
+      test_audio_manager_base->GetPreferredOutputStreamParametersForTesting(
+          "", min_params);
+  ASSERT_EQ(params.frames_per_buffer(), media::limits::kMinAudioBufferSize);
+
+  AudioParameters max_params = default_params;
+  max_params.set_frames_per_buffer(media::limits::kMaxAudioBufferSize * 2);
+  params =
+      test_audio_manager_base->GetPreferredOutputStreamParametersForTesting(
+          "", max_params);
+  ASSERT_EQ(params.frames_per_buffer(), media::limits::kMaxAudioBufferSize);
+
+  // Create an output stream with the minimum buffer size parameters and ensure
+  // that no errors are returned.

[o1ka, 2017/06/27 16:31:19]

I'm not sure I'm following. min_params have (media::limits::kMinAudioBufferSize
/ 2) as a buffer size. Why would we want to create such a stream?

[Andrew MacPherson, 2017/06/27 17:03:40]

I've added a comment here now, I wanted to test that requesting values below the
minimum will be clamped to the minimum value.

+  AudioOutputStream* stream =
+      audio_manager_->MakeAudioOutputStreamProxy(min_params, "");
+  ASSERT_TRUE(stream);
+  EXPECT_TRUE(stream->Open());
+
+  base::WaitableEvent event(base::WaitableEvent::ResetPolicy::AUTOMATIC,
+                            base::WaitableEvent::InitialState::NOT_SIGNALED);
+  int callback_counter = 0;
+  MockAudioSourceCallback source;
+  const int number_of_callbacks = 2;
+  EXPECT_CALL(source,
+              OnMoreData(testing::_, testing::_, testing::_, testing::_))

[o1ka, 2017/06/27 16:31:19]

Do we want to test that AudioOutputStream callbacks request exactly the buffer
size we asked for when we got it from AudioLatency::GetExactBufferSize()?

[Andrew MacPherson, 2017/06/27 17:03:40]

Yes that would be ideal, I wasn't totally clear on how to accomplish that
though, is there another test doing anything similar that I could look at as an
example?

[o1ka, 2017/06/28 17:38:22]

Are there some problems with checking AudioBus size passed to OnMoreData? 

[Andrew MacPherson, 2017/06/28 18:19:04]

That works great, I've updated the test now to do this, does that make more
sense?

+      .Times(number_of_callbacks)
+      .WillRepeatedly(testing::DoAll(
+          ZeroBuffer(),
+          MaybeSignalEvent(&callback_counter, number_of_callbacks, &event),
+          testing::Return(0)));
+  EXPECT_CALL(source, OnError()).Times(0);
+  stream->Start(&source);
+  event.Wait();
+
+  stream->Stop();
+  stream->Close();
+}
+#endif
+
 }  // namespace media