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

 /*
  *  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.
  */
 
 #include "webrtc/base/logging.h"
 #include "webrtc/base/refcount.h"
 #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>
 
 #if defined(_WIN32)
 #include "audio_device_wave_win.h"
 #if defined(WEBRTC_WINDOWS_CORE_AUDIO_BUILD)
 #include "audio_device_core_win.h"
 #endif
 #elif defined(WEBRTC_ANDROID)
@@ skipping 232 matching lines @@
 // Create the *Linux* implementation of the Audio Device
 //
 #elif defined(WEBRTC_LINUX)
   if ((audioLayer == kLinuxPulseAudio) ||
       (audioLayer == kPlatformDefaultAudio)) {
 #if defined(LINUX_PULSE)
     LOG(INFO) << "attempting to use the Linux PulseAudio APIs...";
 
     // 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 {
       delete pulseDevice;
 #endif
 #if defined(LINUX_ALSA)
       // create *Linux ALSA Audio* implementation
       ptrAudioDevice = new AudioDeviceLinuxALSA(Id());
       if (ptrAudioDevice != NULL) {
         // Pulse Audio was not supported => revert to ALSA instead
@@ skipping 195 matching lines @@
 
 // ----------------------------------------------------------------------------
 //  Init
 // ----------------------------------------------------------------------------
 
 int32_t AudioDeviceModuleImpl::Init() {
   LOG(INFO) << __FUNCTION__;
   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;
   }
 
   _initialized = true;
   return 0;
 }
 
 // ----------------------------------------------------------------------------
 //  Terminate
 // ----------------------------------------------------------------------------
@@ skipping 969 matching lines @@
   return (_ptrAudioDevice->RecordingIsInitialized());
 }
 
 // ----------------------------------------------------------------------------
 //  StartPlayout
 // ----------------------------------------------------------------------------
 
 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;
 }
 
 // ----------------------------------------------------------------------------
 //  StopPlayout
 // ----------------------------------------------------------------------------
 
 int32_t AudioDeviceModuleImpl::StopPlayout() {
   LOG(INFO) << __FUNCTION__;
   CHECK_INITIALIZED();
   return (_ptrAudioDevice->StopPlayout());
 }
 
 // ----------------------------------------------------------------------------
 //  Playing
 // ----------------------------------------------------------------------------
 
 bool AudioDeviceModuleImpl::Playing() const {
   LOG(INFO) << __FUNCTION__;
   CHECK_INITIALIZED_BOOL();
   return (_ptrAudioDevice->Playing());
 }
 
 // ----------------------------------------------------------------------------
 //  StartRecording
 // ----------------------------------------------------------------------------
 
 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
 // ----------------------------------------------------------------------------
 
 int32_t AudioDeviceModuleImpl::StopRecording() {
   LOG(INFO) << __FUNCTION__;
   CHECK_INITIALIZED();
   return (_ptrAudioDevice->StopRecording());
 }
@@ skipping 447 matching lines @@
 //  PlatformAudioLayer
 // ----------------------------------------------------------------------------
 
 AudioDeviceModule::AudioLayer AudioDeviceModuleImpl::PlatformAudioLayer()
     const {
   LOG(INFO) << __FUNCTION__;
   return _platformAudioLayer;
 }
 
 }  // namespace webrtc