Refactor AudioUnit code into its own class.

Refactor AudioUnit code into its own class.

BUG=
R=haysc@webrtc.org, henrika@webrtc.org

Committed: https://crrev.com/1300caa3fea840572614772f96755ce7f4080ef6
Cr-Commit-Position: refs/heads/master@{#12056}

[tkchin_webrtc, 2016-03-18 03:16:19]

Patchset #1 (id:1) has been deleted

[tkchin_webrtc, 2016-03-18 03:16:34]

Description was changed from

==========
Refactor AudioUnit code into its own class.

BUG=
==========

to

==========
Refactor AudioUnit code into its own class.

BUG=
==========

[tkchin_webrtc, 2016-03-18 03:16:34]

tkchin@webrtc.org changed reviewers:
+ haysc@webrtc.org, henrika@webrtc.org

[tkchin_webrtc, 2016-03-18 03:22:35]

Patchset #1 (id:20001) has been deleted

[tkchin_webrtc, 2016-03-18 03:27:02]

Part 4 of refactor. I think with next CL it'll be easy to deal with delayed
audio unit initialization now.

[tkchin_webrtc, 2016-03-18 03:31:57]

Patchset #2 (id:60001) has been deleted

[tkchin_webrtc, 2016-03-18 03:33:07]

Patchset #1 (id:40001) has been deleted

[henrika_webrtc, 2016-03-18 11:54:02]

Very nice refactor CL. Much easier to read and maintain the code now. Many
thanks!

LGTM

https://codereview.webrtc.org/1809343002/diff/80001/webrtc/modules/audio_devi...
File webrtc/modules/audio_device/ios/audio_device_ios.mm (right):

https://codereview.webrtc.org/1809343002/diff/80001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/ios/audio_device_ios.mm:197: LOG_F(LS_ERROR) <<
"AudioOutputUnitStart failed for StartPlayout.";
Perhaps the comment can be changed given that the call is not directly
AudioOutputUnitStart

https://codereview.webrtc.org/1809343002/diff/80001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/ios/audio_device_ios.mm:228: LOG_F(LS_ERROR) <<
"AudioOutputUnitStart failed for StartRecording.";
ditto

https://codereview.webrtc.org/1809343002/diff/80001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/ios/audio_device_ios.mm:350: OSStatus
AudioDeviceIOS::OnDeliverRecordedData(
I am assuming that this code is almost identical to the old version. Kind of
hard to compare..

https://codereview.webrtc.org/1809343002/diff/80001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/ios/audio_device_ios.mm:434: RTCLogError(@"Failed to
stop the audio unit.");
Guess you can use the same type of error logs in the other places I've commented
on earlier.

https://codereview.webrtc.org/1809343002/diff/80001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/ios/audio_device_ios.mm:489: RTCLog(@"%@", session);
Nice ;-)

https://codereview.webrtc.org/1809343002/diff/80001/webrtc/modules/audio_devi...
File webrtc/modules/audio_device/ios/voice_processing_audio_unit.h (right):

https://codereview.webrtc.org/1809343002/diff/80001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/ios/voice_processing_audio_unit.h:41: // I/O Audio
Unit. The Voice-Processing I/O unit has the same characteristics
Nit, first Voice Processing and then Voice-Processing ;-)

[Chuck, 2016-03-18 14:44:14]

lgtm

https://codereview.webrtc.org/1809343002/diff/80001/webrtc/modules/audio_devi...
File webrtc/modules/audio_device/ios/audio_device_ios.mm (right):

https://codereview.webrtc.org/1809343002/diff/80001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/ios/audio_device_ios.mm:358: if
(!rtc::AtomicOps::AcquireLoad(&recording_))
Is this allowed in the WebRTC coding style? Referring to the missing braces. I
know this is a move and not newly written code, more for my own education.

The Chromium/Google C++ guide allows it from what I can tell.

[tkchin_webrtc, 2016-03-18 17:34:35]

Pretty happy that we've reduced the size of audio_unit_ios.mm from about 1400
lines to about 650 through these various CLs :)

https://codereview.webrtc.org/1809343002/diff/80001/webrtc/modules/audio_devi...
File webrtc/modules/audio_device/ios/audio_device_ios.mm (right):

https://codereview.webrtc.org/1809343002/diff/80001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/ios/audio_device_ios.mm:197: LOG_F(LS_ERROR) <<
"AudioOutputUnitStart failed for StartPlayout.";
On 2016/03/18 11:54:02, henrika_webrtc wrote:
> Perhaps the comment can be changed given that the call is not directly
> AudioOutputUnitStart

Done.

https://codereview.webrtc.org/1809343002/diff/80001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/ios/audio_device_ios.mm:228: LOG_F(LS_ERROR) <<
"AudioOutputUnitStart failed for StartRecording.";
On 2016/03/18 11:54:02, henrika_webrtc wrote:
> ditto

Done.

https://codereview.webrtc.org/1809343002/diff/80001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/ios/audio_device_ios.mm:350: OSStatus
AudioDeviceIOS::OnDeliverRecordedData(
On 2016/03/18 11:54:02, henrika_webrtc wrote:
> I am assuming that this code is almost identical to the old version. Kind of
> hard to compare..

Yes, should be identical except for some variable renames (e.g. in_bus_number ->
bus_number).

https://codereview.webrtc.org/1809343002/diff/80001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/ios/audio_device_ios.mm:358: if
(!rtc::AtomicOps::AcquireLoad(&recording_))
On 2016/03/18 14:44:14, Chuck wrote:
> Is this allowed in the WebRTC coding style? Referring to the missing braces. I
> know this is a move and not newly written code, more for my own education.
> 
> The Chromium/Google C++ guide allows it from what I can tell.

It's the preferred style now apparently for single line if statements.

https://codereview.webrtc.org/1809343002/diff/80001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/ios/audio_device_ios.mm:434: RTCLogError(@"Failed to
stop the audio unit.");
On 2016/03/18 11:54:02, henrika_webrtc wrote:
> Guess you can use the same type of error logs in the other places I've
commented
> on earlier.

Acknowledged.

https://codereview.webrtc.org/1809343002/diff/80001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/ios/audio_device_ios.mm:489: RTCLog(@"%@", session);
On 2016/03/18 11:54:02, henrika_webrtc wrote:
> Nice ;-)

Acknowledged.

https://codereview.webrtc.org/1809343002/diff/80001/webrtc/modules/audio_devi...
File webrtc/modules/audio_device/ios/voice_processing_audio_unit.h (right):

https://codereview.webrtc.org/1809343002/diff/80001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/ios/voice_processing_audio_unit.h:41: // I/O Audio
Unit. The Voice-Processing I/O unit has the same characteristics
On 2016/03/18 11:54:02, henrika_webrtc wrote:
> Nit, first Voice Processing and then Voice-Processing ;-)

Done.

[tkchin_webrtc, 2016-03-18 17:35:39]

The CQ bit was checked by tkchin@webrtc.org

[tkchin_webrtc, 2016-03-18 17:35:39]

The patchset sent to the CQ was uploaded after l-g-t-m from henrika@webrtc.org,
haysc@webrtc.org
Link to the patchset: https://codereview.webrtc.org/1809343002/#ps100001 (title:
"CR comments")

[commit-bot: I haz the power, 2016-03-18 17:35:46]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1809343002/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1809343002/100001

[commit-bot: I haz the power, 2016-03-18 19:36:13]

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

[commit-bot: I haz the power, 2016-03-18 19:36:14]

Try jobs failed on following builders:
  android_arm64_rel on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  linux_baremetal on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  win_baremetal on tryserver.webrtc (JOB_TIMED_OUT, no build URL)

[tkchin_webrtc, 2016-03-18 19:53:52]

The CQ bit was checked by tkchin@webrtc.org

[commit-bot: I haz the power, 2016-03-18 19:54:02]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1809343002/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1809343002/100001

[tkchin_webrtc, 2016-03-18 20:33:58]

@kjellander: linux_baremetal win_baremetal android_arm64_rel appear dead.
Gonna land manually after they time out.

[tkchin_webrtc, 2016-03-18 21:34:44]

The CQ bit was unchecked by tkchin@webrtc.org

[tkchin_webrtc, 2016-03-18 21:39:26]

Description was changed from

==========
Refactor AudioUnit code into its own class.

BUG=
==========

to

==========
Refactor AudioUnit code into its own class.

BUG=
R=haysc@webrtc.org, henrika@webrtc.org

Committed:
https://chromium.googlesource.com/external/webrtc/+/1300caa3fea840572614772f9...
==========

[tkchin_webrtc, 2016-03-18 21:39:28]

Committed patchset #2 (id:100001) manually as
1300caa3fea840572614772f96755ce7f4080ef6 (presubmit successful).

[commit-bot: I haz the power, 2016-03-18 21:39:29]

Description was changed from

==========
Refactor AudioUnit code into its own class.

BUG=
R=haysc@webrtc.org, henrika@webrtc.org

Committed:
https://chromium.googlesource.com/external/webrtc/+/1300caa3fea840572614772f9...
==========

to

==========
Refactor AudioUnit code into its own class.

BUG=
R=haysc@webrtc.org, henrika@webrtc.org

Committed: https://crrev.com/1300caa3fea840572614772f96755ce7f4080ef6
Cr-Commit-Position: refs/heads/master@{#12056}
==========

[commit-bot: I haz the power, 2016-03-18 21:39:30]

Patchset 2 (id:??) landed as
https://crrev.com/1300caa3fea840572614772f96755ce7f4080ef6
Cr-Commit-Position: refs/heads/master@{#12056}

[kjellander_webrtc, 2016-03-20 08:38:47]

On 2016/03/18 20:33:58, tkchin_webrtc wrote:
> @kjellander: linux_baremetal win_baremetal android_arm64_rel appear dead.
> Gonna land manually after they time out.

They're not dead; they just had long queues. Landing manually is fine. I hope to
be able to find time to fix these issues soon'ish.