Improvements in how WebRTC.Audio.RecordedOnlyZeros is added as histogram

webrtc/modules/audio_device/audio_device_buffer.h

 /*
  *  Copyright (c) 2012 The WebRTC project authors. All Rights Reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
 #ifndef WEBRTC_MODULES_AUDIO_DEVICE_AUDIO_DEVICE_BUFFER_H_
 #define WEBRTC_MODULES_AUDIO_DEVICE_AUDIO_DEVICE_BUFFER_H_
 
 #include "webrtc/base/buffer.h"
 #include "webrtc/base/criticalsection.h"
 #include "webrtc/base/task_queue.h"
 #include "webrtc/base/thread_checker.h"
 #include "webrtc/modules/audio_device/include/audio_device.h"
 #include "webrtc/system_wrappers/include/file_wrapper.h"
 #include "webrtc/typedefs.h"
 
 namespace webrtc {
 // Delta times between two successive playout callbacks are limited to this
 // value before added to an internal array.
 const size_t kMaxDeltaTimeInMs = 500;
 // TODO(henrika): remove when no longer used by external client.
 const size_t kMaxBufferSizeBytes = 3840;  // 10ms in stereo @ 96kHz
 
+enum LogState {
+  kLogStart = 0,

[tommi, 2016/10/27 17:38:24]

LOG_START,
LOG_STOP,
LOG_ACTIVE,

What about moving this enum into the class?  As is, it's webrtc::LOG_START,
which implies a much greater scope than just AudioDeviceBuffer.

[henrika_webrtc, 2016/10/31 12:23:06]

Will fix. It was actually not clear to me that we prefer capital letters. But I
will change that part as well.

+  kLogStop,
+  kLogActive,
+};
+
 class AudioDeviceObserver;
 
 class AudioDeviceBuffer {
  public:
   AudioDeviceBuffer();
   virtual ~AudioDeviceBuffer();
 
   void SetId(uint32_t id) {};
   int32_t RegisterAudioCallback(AudioTransport* audio_callback);
 
-  int32_t InitPlayout();
-  int32_t InitRecording();
+  void StartPlayout();
+  void StartRecording();
+  void StopPlayout();
+  void StopRecording();
 
   int32_t SetRecordingSampleRate(uint32_t fsHz);
   int32_t SetPlayoutSampleRate(uint32_t fsHz);
   int32_t RecordingSampleRate() const;
   int32_t PlayoutSampleRate() const;
 
   int32_t SetRecordingChannels(size_t channels);
   int32_t SetPlayoutChannels(size_t channels);
   size_t RecordingChannels() const;
   size_t PlayoutChannels() const;
@@ skipping 14 matching lines @@
   // 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]);
   int32_t StopOutputFileRecording();
 
   int32_t SetTypingStatus(bool typing_status);
 
  private:
-  // Posts the first delayed task in the task queue and starts the periodic
-  // timer.
-  void StartTimer();
+  // Starts/stops periodic logging of audio stats.
+  void StartPeriodicLogging();
+  void StopPeriodicLogging();
 
   // Called periodically on the internal thread created by the TaskQueue.
-  void LogStats();
-
-  // Clears all members tracking stats for recording and playout.
-  void ResetRecStats();
-  void ResetPlayStats();
+  // Updates some stats but dooes it on the task queue to ensure that access of
+  // members is serialized hence avoiding usage of locks.
+  // state = |kLogStart| => members are initialized and the timer starts.

[tommi, 2016/10/27 17:38:24]

and update documentation to ALL_CAPS style

[henrika_webrtc, 2016/10/31 12:23:06]

Done.

+  // state = |kLogStop| => no logs are printed and the timer stops.
+  // state = |kLogActive| => logs are printed and the timer is kept alive.
+  void LogStats(LogState state);
 
   // Updates counters in each play/record callback but does it on the task
   // queue to ensure that they can be read by LogStats() without any locks since
   // each task is serialized by the task queue.
   void UpdateRecStats(int16_t max_abs, size_t num_samples);
   void UpdatePlayStats(int16_t max_abs, size_t num_samples);
 
+  // Clears all members tracking stats for recording and playout.
+  // These methods both run on the task queue.
+  void ResetRecStats();
+  void ResetPlayStats();
+
   // Ensures that methods are called on the same thread as the thread that
   // 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 lock_;
   rtc::CriticalSection lock_cb_;
 
   // 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.
   rtc::TaskQueue task_queue_;
 
-  // Ensures that the timer is only started once.
-  bool timer_has_started_;
+  // Keeps track of if playout/recording are active or not. A combination
+  // of these states are used to determine when to start and stop the timer.
+  // Only used on the creating thread and not used to control any media flow.
+  bool playing_;
+  bool recording_;
 
   // Sample rate in Hertz.
   uint32_t rec_sample_rate_;
   uint32_t play_sample_rate_;
 
   // Number of audio channels.
   size_t rec_channels_;
   size_t play_channels_;
 
   // Number of bytes per audio sample (2 or 4).
@@ skipping 44 matching lines @@
 
   // Total number of recorded samples stored at the previous timer task.
   uint64_t last_rec_samples_;
 
   // Total number of played audio samples.
   uint64_t play_samples_;
 
   // Total number of played samples stored at the previous timer task.
   uint64_t last_play_samples_;
 
-  // Time stamp of last stat report.
-  uint64_t last_log_stat_time_;
+  // Time stamp of last timer task (drives logging).
+  uint64_t last_timer_task_time_;
 
   // Time stamp of last playout callback.
   uint64_t last_playout_time_;
 
   // An array where the position corresponds to time differences (in
   // milliseconds) between two successive playout callbacks, and the stored
   // value is the number of times a given time difference was found.
   // Writing to the array is done without a lock since it is only read once at
   // destruction when no audio is running.
   uint32_t playout_diff_times_[kMaxDeltaTimeInMs + 1] = {0};
 
   // Contains max level (max(abs(x))) of recorded audio packets over the last
   // 10 seconds where a new measurement is done twice per second. The level
   // is reset to zero at each call to LogStats(). Only modified on the task
   // queue thread.
   int16_t max_rec_level_;
 
   // Contains max level of recorded audio packets over the last 10 seconds
   // where a new measurement is done twice per second.
   int16_t max_play_level_;
 
-  // Counts number of times we detect "no audio" corresponding to a case where
-  // all level measurements since the last log has been exactly zero.
-  // In other words: this counter is incremented only if 20 measurements
-  // (two per second) in a row equals zero. The member is only incremented on
-  // the task queue and max once every 10th second.
-  size_t num_rec_level_is_zero_;
-
   // Counts number of audio callbacks modulo 50 to create a signal when
   // a new storage of audio stats shall be done.
   // Only updated on the OS-specific audio thread that drives audio.
   int16_t rec_stat_count_;
   int16_t play_stat_count_;
+
+  // Time stamps of when playout and recording starts.
+  uint64_t play_start_time_;
+  uint64_t rec_start_time_;
+
+  // Set to true at construction and modified to false as soon as one audio-
+  // level estimate larger than zero is detected.
+  bool only_silence_recorded_;
 };
 
 }  // namespace webrtc
 
 #endif  // WEBRTC_MODULES_AUDIO_DEVICE_AUDIO_DEVICE_BUFFER_H_