Cleanup of the AudioDeviceBuffer class

webrtc/modules/audio_device/audio_device_buffer.h

Index: webrtc/modules/audio_device/audio_device_buffer.h
diff --git a/webrtc/modules/audio_device/audio_device_buffer.h b/webrtc/modules/audio_device/audio_device_buffer.h
index f49420c98ea22d67b9bee1a727d112babb9def35..41c2b44df4328bd2a6a3fcc1e0830faebf4d9c56 100644
--- a/webrtc/modules/audio_device/audio_device_buffer.h
+++ b/webrtc/modules/audio_device/audio_device_buffer.h
@@ -22,7 +22,6 @@ namespace webrtc {
 class CriticalSectionWrapper;
 
 const uint32_t kPulsePeriodMs = 1000;
-const size_t kMaxBufferSizeBytes = 3840;  // 10ms in stereo @ 96kHz
 // Delta times between two successive playout callbacks are limited to this
 // value before added to an internal array.
 const size_t kMaxDeltaTimeInMs = 500;
@@ -52,15 +51,19 @@ class AudioDeviceBuffer {
   int32_t SetRecordingChannel(const AudioDeviceModule::ChannelType channel);
   int32_t RecordingChannel(AudioDeviceModule::ChannelType& channel) const;
 
-  virtual int32_t SetRecordedBuffer(const void* audioBuffer, size_t nSamples);
+  virtual int32_t SetRecordedBuffer(const void* audio_buffer,

[magjed_webrtc, 2016/08/18 14:07:01]

Why are many of these functions virtual? I don't find any subclasses of
AudioDeviceBuffer.

Are you planning to remove num_samples as an argument if it must always be 10ms
anyway?

[henrika_webrtc, 2016/08/18 14:22:00]

Virtual? Legacy ;-) Very old code where I assume the intention was to create
platform-specific versions and use this class as base. Removing all references
to virtual.

I would like to keep num_samples to be able to DCHECK that the audio layer is
actually using 10ms on its side. It will lead to a large amount of changes
otherwise.

[magjed_webrtc, 2016/08/19 07:51:15]

Alright.

+                                    size_t num_samples);
   int32_t SetCurrentMicLevel(uint32_t level);
   virtual void SetVQEData(int playDelayMS, int recDelayMS, int clockDrift);
   virtual int32_t DeliverRecordedData();
   uint32_t NewMicLevel() const;
 
-  virtual int32_t RequestPlayoutData(size_t nSamples);
+  virtual int32_t RequestPlayoutData(size_t num_samples);
   virtual int32_t GetPlayoutData(void* audioBuffer);
 
+  // TODO(henrika): these methods should not be used and does not contain any
+  // valid implementation. Investigate the possibility to either remove them
+  // or add a proper implementation if needed.
   int32_t StartInputFileRecording(const char fileName[kAdmMaxFileNameSize]);
   int32_t StopInputFileRecording();
   int32_t StartOutputFileRecording(const char fileName[kAdmMaxFileNameSize]);
@@ -86,11 +89,15 @@ class AudioDeviceBuffer {
   // creates this object.
   rtc::ThreadChecker thread_checker_;
 
+  // Raw pointer to AudioTransport instance. Supplied to RegisterAudioCallback()
+  // and it must outlive this object.
+  AudioTransport* audio_transport_cb_;
+
+  // TODO(henrika): given usage of thread checker, it should be possible to
+  // remove all locks in this class.
   rtc::CriticalSection _critSect;
   rtc::CriticalSection _critSectCb;
 
-  AudioTransport* _ptrCbAudioTransport;
-
   // Task queue used to invoke LogStats() periodically. Tasks are executed on a
   // worker thread but it does not necessarily have to be the same thread for
   // each task.
@@ -99,45 +106,50 @@ class AudioDeviceBuffer {
   // Ensures that the timer is only started once.
   bool timer_has_started_;
 
-  uint32_t _recSampleRate;
-  uint32_t _playSampleRate;
+  // Sample rate in Hertz.
+  uint32_t rec_sample_rate_;
+  uint32_t play_sample_rate_;
 
-  size_t _recChannels;
-  size_t _playChannels;
+  // Number of audio channels.
+  size_t rec_channels_;
+  size_t play_channels_;
 
   // selected recording channel (left/right/both)
-  AudioDeviceModule::ChannelType _recChannel;
-
-  // 2 or 4 depending on mono or stereo
-  size_t _recBytesPerSample;
-  size_t _playBytesPerSample;
+  AudioDeviceModule::ChannelType rec_channel_;
 
-  // 10ms in stereo @ 96kHz
-  int8_t _recBuffer[kMaxBufferSizeBytes];
+  // Number of bytes per audio sample (2 or 4).
+  size_t rec_bytes_per_sample_;
+  size_t play_bytes_per_sample_;
 
-  // one sample <=> 2 or 4 bytes
-  size_t _recSamples;
-  size_t _recSize;  // in bytes
+  // Number of audio samples/bytes per 10ms.
+  size_t rec_samples_per_10ms_;
+  size_t rec_bytes_per_10ms_;
+  size_t play_samples_per_10ms_;
+  size_t play_bytes_per_10ms_;
 
-  // 10ms in stereo @ 96kHz
-  int8_t _playBuffer[kMaxBufferSizeBytes];
+  // Buffer used for recorded audio samples. Size is given by
+  // |rec_bytes_per_10ms_| and the buffer is allocated in InitRecording() on the
+  // main/creating thread.
+  std::unique_ptr<int8_t[]> rec_buffer_;
 
-  // one sample <=> 2 or 4 bytes
-  size_t _playSamples;
-  size_t _playSize;  // in bytes
+  // Buffer used for audio samples to be played out. Size is given by
+  // |play_bytes_per_10ms_| and the buffer is allocated in InitPlayout() on the
+  // main/creating thread.
+  std::unique_ptr<int8_t[]> play_buffer_;
 
-  FileWrapper& _recFile;
-  FileWrapper& _playFile;
+  // AGC parameters.
+  uint32_t current_mic_level_;
+  uint32_t new_mic_level_;
 
-  uint32_t _currentMicLevel;
-  uint32_t _newMicLevel;
+  // Contains true of a key-press has been detected.
+  bool typing_status_;
 
-  bool _typingStatus;
+  // Delay values used by the AEC.
+  int play_delay_ms_;
+  int rec_delay_ms_;
 
-  int _playDelayMS;
-  int _recDelayMS;
-  int _clockDrift;
-  int high_delay_counter_;
+  // Contains a clock-drift measurement.
+  int clock_drift_;
 
   // Counts number of times LogStats() has been called.
   size_t num_stat_reports_;