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

UMA log for audio_device Init and Start(Playout|Recording). Make Init return a more specific error code, if possible.

BUG=webrtc:5761
R=asapersson@webrtc.org, henrika@webrtc.org

Committed: https://crrev.com/84cab205f51a42e23ed06a4fa6d8bf933b0f140a
Cr-Commit-Position: refs/heads/master@{#13361}

[Max Morin WebRTC, 2016-06-29 11:10:42]

Description was changed from

==========
UMA log for audio_device Init and Start(Playout|Recording). Make Init return a
more specific error code, if possible.

BUG=5761
==========

to

==========
UMA log for audio_device Init and Start(Playout|Recording). Make Init return a
more specific error code, if possible.

Chromium issue: https://codereview.chromium.org/2101373002/

BUG=5761
==========

[Max Morin WebRTC, 2016-06-29 12:48:08]

Patchset #3 (id:40001) has been deleted

[Max Morin WebRTC, 2016-06-29 13:08:30]

maxmorin@webrtc.org changed reviewers:
+ asapersson@webrtc.org, henrika@webrtc.org

[Max Morin WebRTC, 2016-06-29 13:08:31]

Please take a look.

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

Looks good. Some initial comments. Let's ensure that we are in phase before you
continue.

BTW; no XML file?

https://codereview.webrtc.org/2103863004/diff/60001/webrtc/modules/audio_devi...
File webrtc/modules/audio_device/audio_device.gypi (right):

https://codereview.webrtc.org/2103863004/diff/60001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/audio_device.gypi:19:
'<(webrtc_root)/system_wrappers/system_wrappers.gyp:system_wrappers_default',
Why was this needed?

https://codereview.webrtc.org/2103863004/diff/60001/webrtc/modules/audio_devi...
File webrtc/modules/audio_device/audio_device_generic.h (right):

https://codereview.webrtc.org/2103863004/diff/60001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/audio_device_generic.h:21: // For use with UMA
logging. Must be kept in sync with chromiums
..."with histograms.xml in Chrome"

https://codereview.webrtc.org/2103863004/diff/60001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/audio_device_generic.h:24: kOk = 0,
don't we use the Google C++ code style? 
Like: OK, PLAYOUT_ERROR etc.

https://codereview.webrtc.org/2103863004/diff/60001/webrtc/modules/audio_devi...
File webrtc/modules/audio_device/audio_device_impl.cc (right):

https://codereview.webrtc.org/2103863004/diff/60001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/audio_device_impl.cc:271: if (pulseDevice->Init() ==
AudioDeviceGeneric::InitStatus::kOk) {
Can you get rid of the AudioDeviceGeneric:: prefix somehow?

https://codereview.webrtc.org/2103863004/diff/60001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/audio_device_impl.cc:488: LOG(LS_ERROR) <<
__FUNCTION__ << " called with _ptrAudioDevice == nullptr.";
I actually think we should do CHECK() << "short comment" here instead.

https://codereview.webrtc.org/2103863004/diff/60001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/audio_device_impl.cc:496: LOG(LS_ERROR) <<
__FUNCTION__ << " audio device initialization failed.";
Don't need __FUNCTION__. Give shorter comment.

https://codereview.webrtc.org/2103863004/diff/60001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/audio_device_impl.cc:1486: int32_t r =
_ptrAudioDevice->StartPlayout();
Feels a bit unclear converting to an int here. Can you improve?

https://codereview.webrtc.org/2103863004/diff/60001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/audio_device_impl.cc:1487: LOG(INFO) << __FUNCTION__
<< " output: " << r;
Remove _FUNCTION_. THe log will be OK even without it.

https://codereview.webrtc.org/2103863004/diff/60001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/audio_device_impl.cc:1520: int32_t r =
_ptrAudioDevice->StartRecording();
Same here.

https://codereview.webrtc.org/2103863004/diff/60001/webrtc/modules/audio_devi...
File webrtc/modules/audio_device/linux/audio_device_pulse_linux.cc (right):

https://codereview.webrtc.org/2103863004/diff/60001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/linux/audio_device_pulse_linux.cc:174:
WEBRTC_TRACE(kTraceError, kTraceAudioDevice, _id,
Could you fix these as well. I.e., don't use WEBRTC_TRACE

[Max Morin WebRTC, 2016-06-30 08:49:42]

On 2016/06/30 08:44:20, henrika_webrtc wrote:
> Looks good. Some initial comments. Let's ensure that we are in phase before
you
> continue.
> 
> BTW; no XML file?
> 
>
https://codereview.webrtc.org/2103863004/diff/60001/webrtc/modules/audio_devi...
> File webrtc/modules/audio_device/audio_device.gypi (right):
> 
>
https://codereview.webrtc.org/2103863004/diff/60001/webrtc/modules/audio_devi...
> webrtc/modules/audio_device/audio_device.gypi:19:
> '<(webrtc_root)/system_wrappers/system_wrappers.gyp:system_wrappers_default',
> Why was this needed?
> 
>
https://codereview.webrtc.org/2103863004/diff/60001/webrtc/modules/audio_devi...
> File webrtc/modules/audio_device/audio_device_generic.h (right):
> 
>
https://codereview.webrtc.org/2103863004/diff/60001/webrtc/modules/audio_devi...
> webrtc/modules/audio_device/audio_device_generic.h:21: // For use with UMA
> logging. Must be kept in sync with chromiums
> ..."with histograms.xml in Chrome"
> 
>
https://codereview.webrtc.org/2103863004/diff/60001/webrtc/modules/audio_devi...
> webrtc/modules/audio_device/audio_device_generic.h:24: kOk = 0,
> don't we use the Google C++ code style? 
> Like: OK, PLAYOUT_ERROR etc.
> 
>
https://codereview.webrtc.org/2103863004/diff/60001/webrtc/modules/audio_devi...
> File webrtc/modules/audio_device/audio_device_impl.cc (right):
> 
>
https://codereview.webrtc.org/2103863004/diff/60001/webrtc/modules/audio_devi...
> webrtc/modules/audio_device/audio_device_impl.cc:271: if (pulseDevice->Init()
==
> AudioDeviceGeneric::InitStatus::kOk) {
> Can you get rid of the AudioDeviceGeneric:: prefix somehow?
> 
>
https://codereview.webrtc.org/2103863004/diff/60001/webrtc/modules/audio_devi...
> webrtc/modules/audio_device/audio_device_impl.cc:488: LOG(LS_ERROR) <<
> __FUNCTION__ << " called with _ptrAudioDevice == nullptr.";
> I actually think we should do CHECK() << "short comment" here instead.
> 
>
https://codereview.webrtc.org/2103863004/diff/60001/webrtc/modules/audio_devi...
> webrtc/modules/audio_device/audio_device_impl.cc:496: LOG(LS_ERROR) <<
> __FUNCTION__ << " audio device initialization failed.";
> Don't need __FUNCTION__. Give shorter comment.
> 
>
https://codereview.webrtc.org/2103863004/diff/60001/webrtc/modules/audio_devi...
> webrtc/modules/audio_device/audio_device_impl.cc:1486: int32_t r =
> _ptrAudioDevice->StartPlayout();
> Feels a bit unclear converting to an int here. Can you improve?
> 
>
https://codereview.webrtc.org/2103863004/diff/60001/webrtc/modules/audio_devi...
> webrtc/modules/audio_device/audio_device_impl.cc:1487: LOG(INFO) <<
__FUNCTION__
> << " output: " << r;
> Remove _FUNCTION_. THe log will be OK even without it.
> 
>
https://codereview.webrtc.org/2103863004/diff/60001/webrtc/modules/audio_devi...
> webrtc/modules/audio_device/audio_device_impl.cc:1520: int32_t r =
> _ptrAudioDevice->StartRecording();
> Same here.
> 
>
https://codereview.webrtc.org/2103863004/diff/60001/webrtc/modules/audio_devi...
> File webrtc/modules/audio_device/linux/audio_device_pulse_linux.cc (right):
> 
>
https://codereview.webrtc.org/2103863004/diff/60001/webrtc/modules/audio_devi...
> webrtc/modules/audio_device/linux/audio_device_pulse_linux.cc:174:
> WEBRTC_TRACE(kTraceError, kTraceAudioDevice, _id,
> Could you fix these as well. I.e., don't use WEBRTC_TRACE

The xml file is in the Chromium issue (linked in description).

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

Please take another look.

https://codereview.webrtc.org/2103863004/diff/60001/webrtc/modules/audio_devi...
File webrtc/modules/audio_device/audio_device.gypi (right):

https://codereview.webrtc.org/2103863004/diff/60001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/audio_device.gypi:19:
'<(webrtc_root)/system_wrappers/system_wrappers.gyp:system_wrappers_default',
On 2016/06/30 08:44:20, henrika_webrtc wrote:
> Why was this needed?

For linking with metrics. I made the dependencies more precise in the new patch
set.

https://codereview.webrtc.org/2103863004/diff/60001/webrtc/modules/audio_devi...
File webrtc/modules/audio_device/audio_device_generic.h (right):

https://codereview.webrtc.org/2103863004/diff/60001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/audio_device_generic.h:21: // For use with UMA
logging. Must be kept in sync with chromiums
On 2016/06/30 08:44:20, henrika_webrtc wrote:
> ..."with histograms.xml in Chrome"

Done.

https://codereview.webrtc.org/2103863004/diff/60001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/audio_device_generic.h:24: kOk = 0,
On 2016/06/30 08:44:20, henrika_webrtc wrote:
> don't we use the Google C++ code style? 
> Like: OK, PLAYOUT_ERROR etc.

Done.

https://codereview.webrtc.org/2103863004/diff/60001/webrtc/modules/audio_devi...
File webrtc/modules/audio_device/audio_device_impl.cc (right):

https://codereview.webrtc.org/2103863004/diff/60001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/audio_device_impl.cc:271: if (pulseDevice->Init() ==
AudioDeviceGeneric::InitStatus::kOk) {
On 2016/06/30 08:44:20, henrika_webrtc wrote:
> Can you get rid of the AudioDeviceGeneric:: prefix somehow?

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.

https://codereview.webrtc.org/2103863004/diff/60001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/audio_device_impl.cc:488: LOG(LS_ERROR) <<
__FUNCTION__ << " called with _ptrAudioDevice == nullptr.";
On 2016/06/30 08:44:20, henrika_webrtc wrote:
> I actually think we should do CHECK() << "short comment" here instead.

Done.

https://codereview.webrtc.org/2103863004/diff/60001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/audio_device_impl.cc:496: LOG(LS_ERROR) <<
__FUNCTION__ << " audio device initialization failed.";
On 2016/06/30 08:44:20, henrika_webrtc wrote:
> Don't need __FUNCTION__. Give shorter comment.

Done.

https://codereview.webrtc.org/2103863004/diff/60001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/audio_device_impl.cc:1486: int32_t r =
_ptrAudioDevice->StartPlayout();
On 2016/06/30 08:44:20, henrika_webrtc wrote:
> Feels a bit unclear converting to an int here. Can you improve?

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.

https://codereview.webrtc.org/2103863004/diff/60001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/audio_device_impl.cc:1487: LOG(INFO) << __FUNCTION__
<< " output: " << r;
On 2016/06/30 08:44:20, henrika_webrtc wrote:
> Remove _FUNCTION_. THe log will be OK even without it.

Done.

https://codereview.webrtc.org/2103863004/diff/60001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/audio_device_impl.cc:1520: int32_t r =
_ptrAudioDevice->StartRecording();
On 2016/06/30 08:44:20, henrika_webrtc wrote:
> Same here.

Done.

https://codereview.webrtc.org/2103863004/diff/60001/webrtc/modules/audio_devi...
File webrtc/modules/audio_device/linux/audio_device_pulse_linux.cc (right):

https://codereview.webrtc.org/2103863004/diff/60001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/linux/audio_device_pulse_linux.cc:174:
WEBRTC_TRACE(kTraceError, kTraceAudioDevice, _id,
On 2016/06/30 08:44:20, henrika_webrtc wrote:
> Could you fix these as well. I.e., don't use WEBRTC_TRACE

Done.

[Max Morin WebRTC, 2016-06-30 11:22:58]

https://codereview.webrtc.org/2103863004/diff/80001/webrtc/modules/audio_devi...
File webrtc/modules/audio_device/mac/audio_device_mac.cc (right):

https://codereview.webrtc.org/2103863004/diff/80001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/mac/audio_device_mac.cc:10: 
Added missing logging header

https://codereview.webrtc.org/2103863004/diff/80001/webrtc/modules/audio_devi...
File webrtc/modules/audio_device/win/audio_device_wave_win.cc (right):

https://codereview.webrtc.org/2103863004/diff/80001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/win/audio_device_wave_win.cc:10: 
Added missing logging header.

[henrika_webrtc, 2016-06-30 15:19:18]

Started providing feedback but we discussed offline and came to the conclusion
that it might be better to first focus on counting the frequency of OK/ERROR for
the most essential APIs like:

InitRecording/Playout
StartRecording/Playout
StopRecording/Playout

https://codereview.webrtc.org/2103863004/diff/80001/webrtc/modules/audio_devi...
File webrtc/modules/audio_device/android/audio_device_template.h (right):

https://codereview.webrtc.org/2103863004/diff/80001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/android/audio_device_template.h:73: return
InitStatus::OK;
Are you OK with using NO_ERROR instead?

https://codereview.webrtc.org/2103863004/diff/80001/webrtc/modules/audio_devi...
File webrtc/modules/audio_device/audio_device_generic.h (right):

https://codereview.webrtc.org/2103863004/diff/80001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/audio_device_generic.h:22: // Chrome.
Can you add a link to the public file here?

https://codereview.webrtc.org/2103863004/diff/80001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/audio_device_generic.h:24: OK = 0,
I would use NO_ERROR instead

[Max Morin WebRTC, 2016-06-30 16:00:23]

Please take another look.

https://codereview.webrtc.org/2103863004/diff/80001/webrtc/modules/audio_devi...
File webrtc/modules/audio_device/android/audio_device_template.h (right):

https://codereview.webrtc.org/2103863004/diff/80001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/android/audio_device_template.h:73: return
InitStatus::OK;
On 2016/06/30 15:19:18, henrika_webrtc wrote:
> Are you OK with using NO_ERROR instead?

Done.

https://codereview.webrtc.org/2103863004/diff/80001/webrtc/modules/audio_devi...
File webrtc/modules/audio_device/audio_device_generic.h (right):

https://codereview.webrtc.org/2103863004/diff/80001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/audio_device_generic.h:22: // Chrome.
On 2016/06/30 15:19:18, henrika_webrtc wrote:
> Can you add a link to the public file here?

Done.

https://codereview.webrtc.org/2103863004/diff/80001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/audio_device_generic.h:24: OK = 0,
On 2016/06/30 15:19:18, henrika_webrtc wrote:
> I would use NO_ERROR instead

Done.

[henrika_webrtc, 2016-07-01 07:56:29]

Looks much better now. Please double check the dependencies and ensure that
there is no conflict in Chrome.

https://codereview.webrtc.org/2103863004/diff/100001/webrtc/modules/audio_dev...
File webrtc/modules/audio_device/audio_device.gypi (right):

https://codereview.webrtc.org/2103863004/diff/100001/webrtc/modules/audio_dev...
webrtc/modules/audio_device/audio_device.gypi:92: 'dependencies': [
Please check this dependency as discussed off-line.

https://codereview.webrtc.org/2103863004/diff/100001/webrtc/modules/audio_dev...
File webrtc/modules/audio_device/audio_device_impl.cc (right):

https://codereview.webrtc.org/2103863004/diff/100001/webrtc/modules/audio_dev...
webrtc/modules/audio_device/audio_device_impl.cc:487: RTC_CHECK(_ptrAudioDevice)
<< "no _ptrAudioDevice!";
You can skip the comment here. The crash as such is sufficient.

https://codereview.webrtc.org/2103863004/diff/100001/webrtc/modules/audio_dev...
webrtc/modules/audio_device/audio_device_impl.cc:494: LOG(LS_ERROR) << "audio
device initialization failed.";
Capital A

https://codereview.webrtc.org/2103863004/diff/100001/webrtc/modules/audio_dev...
webrtc/modules/audio_device/audio_device_impl.cc:1443: int32_t r =
_ptrAudioDevice->InitPlayout();
Please use res or result instead of r here and below

https://codereview.webrtc.org/2103863004/diff/100001/webrtc/modules/audio_dev...
webrtc/modules/audio_device/audio_device_impl.cc:1445:
RTC_HISTOGRAM_ENUMERATION("WebRTC.Audio.InitPlayoutSuccess",
Possible to avoid the hard coded 2?

[Max Morin WebRTC, 2016-07-01 08:26:46]

ptal

https://codereview.webrtc.org/2103863004/diff/100001/webrtc/modules/audio_dev...
File webrtc/modules/audio_device/audio_device.gypi (right):

https://codereview.webrtc.org/2103863004/diff/100001/webrtc/modules/audio_dev...
webrtc/modules/audio_device/audio_device.gypi:92: 'dependencies': [
On 2016/07/01 07:56:29, henrika_webrtc wrote:
> Please check this dependency as discussed off-line.

Done.

https://codereview.webrtc.org/2103863004/diff/100001/webrtc/modules/audio_dev...
File webrtc/modules/audio_device/audio_device_impl.cc (right):

https://codereview.webrtc.org/2103863004/diff/100001/webrtc/modules/audio_dev...
webrtc/modules/audio_device/audio_device_impl.cc:487: RTC_CHECK(_ptrAudioDevice)
<< "no _ptrAudioDevice!";
On 2016/07/01 07:56:29, henrika_webrtc wrote:
> You can skip the comment here. The crash as such is sufficient.

Done.

https://codereview.webrtc.org/2103863004/diff/100001/webrtc/modules/audio_dev...
webrtc/modules/audio_device/audio_device_impl.cc:494: LOG(LS_ERROR) << "audio
device initialization failed.";
On 2016/07/01 07:56:29, henrika_webrtc wrote:
> Capital A

Done.

https://codereview.webrtc.org/2103863004/diff/100001/webrtc/modules/audio_dev...
webrtc/modules/audio_device/audio_device_impl.cc:1443: int32_t r =
_ptrAudioDevice->InitPlayout();
On 2016/07/01 07:56:29, henrika_webrtc wrote:
> Please use res or result instead of r here and below

Done.

https://codereview.webrtc.org/2103863004/diff/100001/webrtc/modules/audio_dev...
webrtc/modules/audio_device/audio_device_impl.cc:1445:
RTC_HISTOGRAM_ENUMERATION("WebRTC.Audio.InitPlayoutSuccess",
On 2016/07/01 07:56:29, henrika_webrtc wrote:
> Possible to avoid the hard coded 2?

Added RTC_HISTOGRAM_BOOLEAN.

[Max Morin WebRTC, 2016-07-01 08:32:49]

https://codereview.webrtc.org/2103863004/diff/120001/webrtc/modules/audio_dev...
File webrtc/modules/audio_device/win/audio_device_wave_win.cc (right):

https://codereview.webrtc.org/2103863004/diff/120001/webrtc/modules/audio_dev...
webrtc/modules/audio_device/win/audio_device_wave_win.cc:234: LOG(LS_ERROR) <<
"failed to start the timer event");
Fixed.

[henrika_webrtc, 2016-07-01 08:52:39]

LGTM from my side. Please check with Åsa as well.

[åsapersson, 2016-07-01 09:47:08]

https://codereview.webrtc.org/2103863004/diff/120001/webrtc/modules/audio_dev...
File webrtc/modules/audio_device/audio_device_impl.cc (right):

https://codereview.webrtc.org/2103863004/diff/120001/webrtc/modules/audio_dev...
webrtc/modules/audio_device/audio_device_impl.cc:489:
AudioDeviceGeneric::InitStatus err = _ptrAudioDevice->Init();
maybe rename to status or result?

https://codereview.webrtc.org/2103863004/diff/120001/webrtc/modules/audio_dev...
webrtc/modules/audio_device/audio_device_impl.cc:1446: static_cast<int>(result
== 0));
maybe use the macro RTC_LOGGED_HISTOGRAM_BOOLEAN and remove log above

https://codereview.webrtc.org/2103863004/diff/120001/webrtc/modules/audio_dev...
webrtc/modules/audio_device/audio_device_impl.cc:1461: static_cast<int>(result
== 0));
same here and below

[Max Morin WebRTC, 2016-07-01 09:49:43]

Åsa: Please take a final look before upload. Notice the modified metrics.h.

[Max Morin WebRTC, 2016-07-01 10:23:21]

ptal

https://codereview.webrtc.org/2103863004/diff/120001/webrtc/modules/audio_dev...
File webrtc/modules/audio_device/audio_device_impl.cc (right):

https://codereview.webrtc.org/2103863004/diff/120001/webrtc/modules/audio_dev...
webrtc/modules/audio_device/audio_device_impl.cc:489:
AudioDeviceGeneric::InitStatus err = _ptrAudioDevice->Init();
On 2016/07/01 09:47:08, åsapersson wrote:
> maybe rename to status or result?

Done.

https://codereview.webrtc.org/2103863004/diff/120001/webrtc/modules/audio_dev...
webrtc/modules/audio_device/audio_device_impl.cc:1446: static_cast<int>(result
== 0));
On 2016/07/01 09:47:08, åsapersson wrote:
> maybe use the macro RTC_LOGGED_HISTOGRAM_BOOLEAN and remove log above

Unfortunately, the histogram has 1 for success and 0 for failure. I'd rather
keep 0 for success to be consistent with all other logs in this file.

https://codereview.webrtc.org/2103863004/diff/120001/webrtc/modules/audio_dev...
webrtc/modules/audio_device/audio_device_impl.cc:1461: static_cast<int>(result
== 0));
On 2016/07/01 09:47:08, åsapersson wrote:
> same here and below

Acknowledged.

[åsapersson, 2016-07-01 10:44:33]

lgtm

[Max Morin WebRTC, 2016-07-01 11:24:05]

The CQ bit was checked by maxmorin@webrtc.org

[Max Morin WebRTC, 2016-07-01 11:24:05]

The patchset sent to the CQ was uploaded after l-g-t-m from henrika@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2103863004/#ps160001 (title:
"Rename variable.")

[commit-bot: I haz the power, 2016-07-01 11:24:14]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[Max Morin WebRTC, 2016-07-01 11:26:36]

Description was changed from

==========
UMA log for audio_device Init and Start(Playout|Recording). Make Init return a
more specific error code, if possible.

Chromium issue: https://codereview.chromium.org/2101373002/

BUG=5761
==========

to

==========
UMA log for audio_device Init and Start(Playout|Recording). Make Init return a
more specific error code, if possible.

BUG=webrtc:5761
==========

[commit-bot: I haz the power, 2016-07-01 11:31:05]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-01 11:31:07]

Try jobs failed on following builders:
  android_dbg on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/android_dbg/builds/14743)

[Max Morin WebRTC, 2016-07-01 11:35:36]

Description was changed from

==========
UMA log for audio_device Init and Start(Playout|Recording). Make Init return a
more specific error code, if possible.

BUG=webrtc:5761
==========

to

==========
UMA log for audio_device Init and Start(Playout|Recording). Make Init return a
more specific error code, if possible.

BUG=webrtc:5761
R=asapersson@webrtc.org, henrika@webrtc.org

Committed:
https://chromium.googlesource.com/external/webrtc/+/84cab205f51a42e23ed06a4fa...
==========

[Max Morin WebRTC, 2016-07-01 11:35:38]

Committed patchset #8 (id:160001) manually as
84cab205f51a42e23ed06a4fa6d8bf933b0f140a (presubmit successful).

[commit-bot: I haz the power, 2016-07-01 11:35:40]

Description was changed from

==========
UMA log for audio_device Init and Start(Playout|Recording). Make Init return a
more specific error code, if possible.

BUG=webrtc:5761
R=asapersson@webrtc.org, henrika@webrtc.org

Committed:
https://chromium.googlesource.com/external/webrtc/+/84cab205f51a42e23ed06a4fa...
==========

to

==========
UMA log for audio_device Init and Start(Playout|Recording). Make Init return a
more specific error code, if possible.

BUG=webrtc:5761
R=asapersson@webrtc.org, henrika@webrtc.org

Committed: https://crrev.com/84cab205f51a42e23ed06a4fa6d8bf933b0f140a
Cr-Commit-Position: refs/heads/master@{#13361}
==========

[kjellander_webrtc, 2016-09-14 19:47:14]

kjellander@webrtc.org changed reviewers:
+ kjellander@webrtc.org

[kjellander_webrtc, 2016-09-14 19:47:15]

I found a dependency here which I hope can be removed...

https://codereview.webrtc.org/2103863004/diff/160001/webrtc/modules/audio_dev...
File webrtc/modules/audio_device/BUILD.gn (right):

https://codereview.webrtc.org/2103863004/diff/160001/webrtc/modules/audio_dev...
webrtc/modules/audio_device/BUILD.gn:30: "../../base:rtc_base",
We try to avoid this dependency (but we haven't updated the PRESUBMIT check for
GN, it only exists for GYP). Is this dependency really needed here?

If so, it should be added to GYP too, but ideally it wouldn't be needed.

[Max Morin WebRTC, 2016-09-15 07:09:41]

On 2016/09/14 19:47:15, kjellander_webrtc wrote:
> I found a dependency here which I hope can be removed...
> 
>
https://codereview.webrtc.org/2103863004/diff/160001/webrtc/modules/audio_dev...
> File webrtc/modules/audio_device/BUILD.gn (right):
> 
>
https://codereview.webrtc.org/2103863004/diff/160001/webrtc/modules/audio_dev...
> webrtc/modules/audio_device/BUILD.gn:30: "../../base:rtc_base",
> We try to avoid this dependency (but we haven't updated the PRESUBMIT check
for
> GN, it only exists for GYP). Is this dependency really needed here?
> 
> If so, it should be added to GYP too, but ideally it wouldn't be needed.

This is needed to pass gn check:

ERROR at //webrtc/modules/audio_device/audio_device_buffer.cc:15:11: Include not
allowed.
#include "webrtc/base/arraysize.h"
          ^----------------------
It is not in any dependency of
  //webrtc/modules/audio_device:audio_device
The include file is in the target(s):
  //webrtc/base:rtc_base
which should somehow be reachable.

I'm not sure what the preferred solution is. Could this be added to
rtc_base_approved?

[kjellander_webrtc, 2016-09-15 07:55:54]

On 2016/09/15 07:09:41, Max Morin wrote:
> On 2016/09/14 19:47:15, kjellander_webrtc wrote:
> > I found a dependency here which I hope can be removed...
> > 
> >
>
https://codereview.webrtc.org/2103863004/diff/160001/webrtc/modules/audio_dev...
> > File webrtc/modules/audio_device/BUILD.gn (right):
> > 
> >
>
https://codereview.webrtc.org/2103863004/diff/160001/webrtc/modules/audio_dev...
> > webrtc/modules/audio_device/BUILD.gn:30: "../../base:rtc_base",
> > We try to avoid this dependency (but we haven't updated the PRESUBMIT check
> for
> > GN, it only exists for GYP). Is this dependency really needed here?
> > 
> > If so, it should be added to GYP too, but ideally it wouldn't be needed.
> 
> This is needed to pass gn check:
> 
> ERROR at //webrtc/modules/audio_device/audio_device_buffer.cc:15:11: Include
not
> allowed.
> #include "webrtc/base/arraysize.h"
>           ^----------------------
> It is not in any dependency of
>   //webrtc/modules/audio_device:audio_device
> The include file is in the target(s):
>   //webrtc/base:rtc_base
> which should somehow be reachable.
> 
> I'm not sure what the preferred solution is. Could this be added to
> rtc_base_approved?

Yeah, it would be great if you can move webrtc/base/arraysize.h into
rtc_base_approved and remove the dependency from audio_device (in the same CL).
Since webrtc/base/arraysize.h doesn't depend on anything else, that should be
simple (the idea with rtc_base_approved was to move out everything from rtc_base
that depended on things like jsoncpp and expat into another target, then
eventually remove rtc_base entirely at some point).