UMA log for audio_device Init and Start(Playout|Recording). Make Init return a more specific error …

webrtc/modules/audio_device/audio_device_impl.cc

Index: webrtc/modules/audio_device/audio_device_impl.cc
diff --git a/webrtc/modules/audio_device/audio_device_impl.cc b/webrtc/modules/audio_device/audio_device_impl.cc
index b2a33ef72f7894e2fd7dc849027e7cb806eb4cb5..bec6653ef3d6b4dce282db5059d9c1f29c1795e4 100644
--- a/webrtc/modules/audio_device/audio_device_impl.cc
+++ b/webrtc/modules/audio_device/audio_device_impl.cc
@@ -13,7 +13,9 @@
 #include "webrtc/base/timeutils.h"
 #include "webrtc/common_audio/signal_processing/include/signal_processing_library.h"
 #include "webrtc/modules/audio_device/audio_device_config.h"
+#include "webrtc/modules/audio_device/audio_device_generic.h"
 #include "webrtc/modules/audio_device/audio_device_impl.h"
+#include "webrtc/system_wrappers/include/metrics.h"
 
 #include <assert.h>
 #include <string.h>
@@ -266,7 +268,7 @@ int32_t AudioDeviceModuleImpl::CreatePlatformSpecificObjects() {
 
     // create *Linux PulseAudio* implementation
     AudioDeviceLinuxPulse* pulseDevice = new AudioDeviceLinuxPulse(Id());
-    if (pulseDevice->Init() != -1) {
+    if (pulseDevice->Init() == AudioDeviceGeneric::InitStatus::kOk) {

[henrika_webrtc, 2016/06/30 08:44:20]

Can you get rid of the AudioDeviceGeneric:: prefix somehow?

[Max Morin WebRTC, 2016/06/30 10:56:04]

I could throw InitStatus into audio_device_config.h, but it's a bit ugly to put
it directly into the webrtc namespace. "using" declarations are forbidden by the
standard. I think this is better than the alternatives.

       ptrAudioDevice = pulseDevice;
       LOG(INFO) << "Linux PulseAudio APIs will be utilized";
     } else {
@@ -482,10 +484,16 @@ int32_t AudioDeviceModuleImpl::Init() {
   if (_initialized)
     return 0;
 
-  if (!_ptrAudioDevice)
+  if (!_ptrAudioDevice) {
+    LOG(LS_ERROR) << __FUNCTION__ << " called with _ptrAudioDevice == nullptr.";

[henrika_webrtc, 2016/06/30 08:44:20]

I actually think we should do CHECK() << "short comment" here instead.

[Max Morin WebRTC, 2016/06/30 10:56:04]

Done.

     return -1;
-
-  if (_ptrAudioDevice->Init() == -1) {
+  }
+  AudioDeviceGeneric::InitStatus err = _ptrAudioDevice->Init();
+  RTC_HISTOGRAM_ENUMERATION(
+      "WebRTC.Audio.InitializationResult", static_cast<int>(err),
+      static_cast<int>(AudioDeviceGeneric::InitStatus::kNumStatuses));
+  if (err != AudioDeviceGeneric::InitStatus::kOk) {
+    LOG(LS_ERROR) << __FUNCTION__ << " audio device initialization failed.";

[henrika_webrtc, 2016/06/30 08:44:20]

Don't need __FUNCTION__. Give shorter comment.

[Max Morin WebRTC, 2016/06/30 10:56:04]

Done.

     return -1;
   }
 
@@ -1475,7 +1483,11 @@ bool AudioDeviceModuleImpl::RecordingIsInitialized() const {
 int32_t AudioDeviceModuleImpl::StartPlayout() {
   LOG(INFO) << __FUNCTION__;
   CHECK_INITIALIZED();
-  return (_ptrAudioDevice->StartPlayout());
+  int32_t r = _ptrAudioDevice->StartPlayout();

[henrika_webrtc, 2016/06/30 08:44:20]

Feels a bit unclear converting to an int here. Can you improve?

[Max Morin WebRTC, 2016/06/30 10:56:04]

There is no conversion, StartPlayout returns an int32_t. We could log
"success"/"not success" instead of zero/nonzero. The uglyness comes from using
an int32_t to denote success. I think the original idea is to use it for error
codes, but the assumption in this file is that these return values are always -1
or 0 anyways.

+  LOG(INFO) << __FUNCTION__ << " output: " << r;

[henrika_webrtc, 2016/06/30 08:44:20]

Remove _FUNCTION_. THe log will be OK even without it.

[Max Morin WebRTC, 2016/06/30 10:56:04]

Done.

+  RTC_HISTOGRAM_ENUMERATION("WebRTC.Audio.StartPlayoutSuccess",
+                            static_cast<int>(r == 0), 2);
+  return r;
 }
 
 // ----------------------------------------------------------------------------
@@ -1505,7 +1517,11 @@ bool AudioDeviceModuleImpl::Playing() const {
 int32_t AudioDeviceModuleImpl::StartRecording() {
   LOG(INFO) << __FUNCTION__;
   CHECK_INITIALIZED();
-  return (_ptrAudioDevice->StartRecording());
+  int32_t r = _ptrAudioDevice->StartRecording();

[henrika_webrtc, 2016/06/30 08:44:20]

Same here.

[Max Morin WebRTC, 2016/06/30 10:56:04]

Done.

+  LOG(INFO) << __FUNCTION__ << " output: " << r;
+  RTC_HISTOGRAM_ENUMERATION("WebRTC.Audio.StartRecordingSuccess",
+                            static_cast<int>(r == 0), 2);
+  return r;
 }
 // ----------------------------------------------------------------------------
 //  StopRecording