Refactor FakeAudioDevice to have separate methods for starting recording and playout.

webrtc/test/fake_audio_device.cc

Index: webrtc/test/fake_audio_device.cc
diff --git a/webrtc/test/fake_audio_device.cc b/webrtc/test/fake_audio_device.cc
index 9e5e95fb0dbdc229b61dceb7dd182b404c4edc91..f8693821bda87077876b7921ff41cac82278c71f 100644
--- a/webrtc/test/fake_audio_device.cc
+++ b/webrtc/test/fake_audio_device.cc
@@ -11,50 +11,48 @@
 #include "webrtc/test/fake_audio_device.h"
 
 #include <algorithm>
+#include <cmath>
 
 #include "webrtc/base/platform_thread.h"
-#include "webrtc/modules/media_file/media_file_utility.h"
 #include "webrtc/system_wrappers/include/clock.h"
 #include "webrtc/system_wrappers/include/event_wrapper.h"
 #include "webrtc/system_wrappers/include/file_wrapper.h"
-#include "webrtc/test/gtest.h"
 
 namespace webrtc {
 namespace test {
 
-FakeAudioDevice::FakeAudioDevice(Clock* clock,
-                                 const std::string& filename,
-                                 float speed)
+namespace {
+
+const double kPi = std::acos(-1);
+
+}  // namespace
+
+FakeAudioDevice::FakeAudioDevice(Clock* clock, float speed)
     : audio_callback_(NULL),
-      capturing_(false),
+      recording_(false),
+      playing_(false),
+      normalized_frequency_(0),
+      peak_to_peak_(0),
+      n_(0),
       captured_audio_(),
       playout_buffer_(),
       speed_(speed),
       last_playout_ms_(-1),
       clock_(clock, speed),
       tick_(EventTimerWrapper::Create()),
-      thread_(FakeAudioDevice::Run, this, "FakeAudioDevice"),
-      file_utility_(new ModuleFileUtility(0)),
-      input_stream_(FileWrapper::Create()) {
+      thread_(FakeAudioDevice::Run, this, "FakeAudioDevice") {
   memset(captured_audio_, 0, sizeof(captured_audio_));
   memset(playout_buffer_, 0, sizeof(playout_buffer_));
-  // Open audio input file as read-only and looping.
-  EXPECT_TRUE(input_stream_->OpenFile(filename.c_str(), true)) << filename;
 }
 
 FakeAudioDevice::~FakeAudioDevice() {
-  Stop();
-
+  StopPlayout();
+  StopRecording();
   thread_.Stop();
 }
 
 int32_t FakeAudioDevice::Init() {
-  rtc::CritScope cs(&lock_);
-  if (file_utility_->InitPCMReading(*input_stream_.get()) != 0)
-    return -1;
-
-  if (!tick_->StartTimer(true, 10 / speed_))
-    return -1;
+  RTC_CHECK(tick_->StartTimer(true, 10 / speed_));
   thread_.Start();
   thread_.SetPriority(rtc::kHighPriority);
   return 0;
@@ -68,7 +66,7 @@ int32_t FakeAudioDevice::RegisterAudioCallback(AudioTransport* callback) {
 
 bool FakeAudioDevice::Playing() const {
   rtc::CritScope cs(&lock_);
-  return capturing_;
+  return playing_;
 }
 
 int32_t FakeAudioDevice::PlayoutDelay(uint16_t* delay_ms) const {
@@ -78,36 +76,37 @@ int32_t FakeAudioDevice::PlayoutDelay(uint16_t* delay_ms) const {
 
 bool FakeAudioDevice::Recording() const {
   rtc::CritScope cs(&lock_);
-  return capturing_;
+  return recording_;
 }
 
 bool FakeAudioDevice::Run(void* obj) {
-  static_cast<FakeAudioDevice*>(obj)->CaptureAudio();
+  static_cast<FakeAudioDevice*>(obj)->ProcessAudio();
   return true;
 }
 
-void FakeAudioDevice::CaptureAudio() {
+void FakeAudioDevice::ProcessAudio() {
   {
     rtc::CritScope cs(&lock_);
-    if (capturing_) {
-      int bytes_read = file_utility_->ReadPCMData(
-          *input_stream_.get(), captured_audio_, kBufferSizeBytes);
-      if (bytes_read <= 0)
-        return;
-      // 2 bytes per sample.
-      size_t num_samples = static_cast<size_t>(bytes_read / 2);
+    if (recording_) {

[peah-webrtc, 2017/01/24 12:42:21]

Is there a certain reason for changing the capturing_ variable to recording_ and
playing_?
-Why not use capturing_ and rendering_?
-Can both playing and recording_ be active at the same time?

[perkj_webrtc, 2017/01/24 14:38:15]

done and
yes, there are current calltests that starts both recording and playout. I think
it is a loopback test and I did not want to change that.

+      // Get 10ms of audio. 2 bytes per sample.
+      for (uint32_t i = 0; i < kFrequencyHz / 100; i = i + 2) {

[peah-webrtc, 2017/01/24 12:42:21]

uint32_t -> size_t ?

[perkj_webrtc, 2017/01/24 14:38:15]

if you prefer that sure.

+        n_ += normalized_frequency_;

[peah-webrtc, 2017/01/24 12:42:21]

There are a lot of members to this class that only relate to the recording_
call. I'd have preferred if those would be bundled into a class in order to
separate the recording part of the code from the playing part of the code.

+        if (n_ >= 2 * kPi)
+          n_ -= 2 * kPi;
+
+        uint16_t sample =
+            static_cast<uint16_t>((1.0 + std::sin(n_) / 2) * peak_to_peak_);

[peah-webrtc, 2017/01/24 12:42:20]

Since 1.0 + sin()/2 obtains values in the range [0.5:1.5], the cast is
saturating for high values of peak_to_peak. It would make sense to document that
via a DCHECK.

[perkj_webrtc, 2017/01/24 14:38:15]

oops, I wanted it to be between 0 and 1 just to make sure that can not happen.

+        captured_audio_[i] = sample;

[peah-webrtc, 2017/01/24 12:42:21]

What is the sample format for this? Is that a standard format?

[perkj_webrtc, 2017/01/24 14:38:15]

I was hoping you guys know. The old comments for the |captured_audio| just said
it was 2 bytes per sample.
To me it does not matter. I just want to a have a fake device that spits out
something I can use for tests in another project. Current tests does not seem to
care either.

+        captured_audio_[i + 1] = sample >> 8;
+      }
+
+      size_t num_samples = kFrequencyHz / 100;
       uint32_t new_mic_level;
-      EXPECT_EQ(0,
-                audio_callback_->RecordedDataIsAvailable(captured_audio_,
-                                                         num_samples,
-                                                         2,
-                                                         1,
-                                                         kFrequencyHz,
-                                                         0,
-                                                         0,
-                                                         0,
-                                                         false,
-                                                         new_mic_level));
+      RTC_CHECK_EQ(0, audio_callback_->RecordedDataIsAvailable(
+                          captured_audio_, num_samples, 2, 1, kFrequencyHz, 0,
+                          0, 0, false, new_mic_level));
+    }
+    if (playing_) {
       size_t samples_needed = kFrequencyHz / 100;
       int64_t now_ms = clock_.TimeInMilliseconds();
       uint32_t time_since_last_playout_ms = now_ms - last_playout_ms_;
@@ -119,28 +118,39 @@ void FakeAudioDevice::CaptureAudio() {
       size_t samples_out = 0;
       int64_t elapsed_time_ms = -1;
       int64_t ntp_time_ms = -1;
-      EXPECT_EQ(0,
-                audio_callback_->NeedMorePlayData(samples_needed,
-                                                  2,
-                                                  1,
-                                                  kFrequencyHz,
-                                                  playout_buffer_,
-                                                  samples_out,
-                                                  &elapsed_time_ms,
-                                                  &ntp_time_ms));
+      RTC_CHECK_EQ(0, audio_callback_->NeedMorePlayData(
+                          samples_needed, 2, 1, kFrequencyHz, playout_buffer_,
+                          samples_out, &elapsed_time_ms, &ntp_time_ms));
     }
   }
   tick_->Wait(WEBRTC_EVENT_INFINITE);

[peah-webrtc, 2017/01/24 12:42:21]

As it is now, tick controls the rate of calls for both playing and recording. Is
that desired? In practice, I'd imagine that you'd also want to simulate
different rates for this. 

[perkj_webrtc, 2017/01/24 14:38:15]

Maybe, but that is not a change in this cl and nothing I currently care about. 

 }
 
-void FakeAudioDevice::Start() {
+int32_t FakeAudioDevice::StartPlayout() {
   rtc::CritScope cs(&lock_);
-  capturing_ = true;
+  playing_ = true;
+  return 0;
 }
 
-void FakeAudioDevice::Stop() {
+int32_t FakeAudioDevice::StopPlayout() {
   rtc::CritScope cs(&lock_);
-  capturing_ = false;
+  playing_ = false;
+  return 0;
 }
+
+void FakeAudioDevice::StartRecordingSine(int frequency_in_hz,
+                                         uint16_t peak_to_peak) {

[peah-webrtc, 2017/01/24 12:42:21]

Please add DCHECKS for the allowed range of peak_to_peak (see above comment).

[perkj_webrtc, 2017/01/24 14:38:15]

should not happen now.

+  rtc::CritScope cs(&lock_);
+  normalized_frequency_ = (2 * kPi * frequency_in_hz) / kFrequencyHz;
+  peak_to_peak_ = peak_to_peak;
+  recording_ = true;
+}
+
+int32_t FakeAudioDevice::StopRecording() {
+  rtc::CritScope cs(&lock_);
+  recording_ = false;
+  return 0;
+}
+
 }  // namespace test
 }  // namespace webrtc