Adds support for OpenSL ES based audio capture on Android

Adds support for OpenSL ES based audio capture on Android.

NOTE: the new code is disabled by default in the WebRtcAudioManager to ensure that
OpenSL ES is not accidentally activated in existing clients. There are still some
unresolved issues to sort out before it can be utilized.

Enables possibility to use OpenSL ES based audio in both directions for WebRTC.
All unit tests and demo clients have been tested with the new implementation but
the new support is behind a flag (see above).

More testing is needed before it can be used in the field and additional support for
hardware effects is still missing.

BUG=webrtc:5925
R=tommi@webrtc.org

Committed: https://crrev.com/918b5547897a70ae13053fb0672c5f1a48dd714a
Cr-Commit-Position: refs/heads/master@{#14290}

[henrika_webrtc, 2016-07-01 12:02:12]

Description was changed from

==========
Adds support for OpenSL ES based audio capture on Android

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

to

==========
Adds support for OpenSL ES based audio capture on Android

BUG=webrtc:5925
==========

[henrika_webrtc, 2016-07-01 12:47:34]

Description was changed from

==========
Adds support for OpenSL ES based audio capture on Android

BUG=webrtc:5925
==========

to

==========
Adds support for OpenSL ES based audio capture on Android.

NOTE: the new code is disabled by default in the WebRtcAudioManager to ensure
that
OpenSL ES is not accidentally activated in existing clients. There are still
some
unresolved issues to sort out before it can be utilized. 

Enables possibility to use OpenSL ES based audio in both directions for WebRTC.
All unit tests and demo clients have been tested with the new implementation but
the new support is behind a flag (see above).

More testing is needed before it can be used in the field and additional support
for
hardware effects is still missing. 

BUG=webrtc:5925
==========

[henrika_webrtc, 2016-07-01 12:59:31]

Patchset #3 (id:40001) has been deleted

[henrika_webrtc, 2016-07-01 13:57:23]

henrika@webrtc.org changed reviewers:
+ tommi@webrtc.org

[henrika_webrtc, 2016-07-01 14:04:05]

Lot's of new code here Tommi but it is all behind a flag (can't break anything
as is) and it is inline with what we did for the output side earlier.
Had to do some minor tweaks for output as well since in and out could now share
some parts.

My plan was to land this part first and then do some more refactoring in a
common class called AudioDeviceBuffer to remove locks that are not needed.
And then do more work on adding support for audio effects.

But I would like to get the basic support in place first.

[henrika_webrtc, 2016-09-13 09:51:11]

Rebased and verified that it works well on latest.

PTAL ;-)

[tommi, 2016-09-15 09:34:13]

https://codereview.webrtc.org/2119633004/diff/80001/webrtc/modules/audio_devi...
File webrtc/modules/audio_device/android/audio_device_unittest.cc (right):

https://codereview.webrtc.org/2119633004/diff/80001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/android/audio_device_unittest.cc:729: if
(low_latency_output && low_latency_input)
use {} since lines 732-733 wrap

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

https://codereview.webrtc.org/2119633004/diff/80001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/android/build_info.h:27: SDK_CODE_JELLY_BEAN = 16,  
   //  Android 4.1
nit: fix whitespace

https://codereview.webrtc.org/2119633004/diff/80001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/android/build_info.h:63: int GetSdkVersion();
can this simply return SdkCode?

https://codereview.webrtc.org/2119633004/diff/80001/webrtc/modules/audio_devi...
File
webrtc/modules/audio_device/android/java/src/org/webrtc/voiceengine/BuildInfo.java
(right):

https://codereview.webrtc.org/2119633004/diff/80001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/android/java/src/org/webrtc/voiceengine/BuildInfo.java:49:
return Build.VERSION.SDK_INT;
fix indent

https://codereview.webrtc.org/2119633004/diff/80001/webrtc/modules/audio_devi...
File
webrtc/modules/audio_device/android/java/src/org/webrtc/voiceengine/WebRtcAudioManager.java
(right):

https://codereview.webrtc.org/2119633004/diff/80001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/android/java/src/org/webrtc/voiceengine/WebRtcAudioManager.java:234:
return false;
can you file a bug for this?  Seems like we're changing the behavior for output
now, even though this cl is for input.

https://codereview.webrtc.org/2119633004/diff/80001/webrtc/modules/audio_devi...
File webrtc/modules/audio_device/android/opensles_common.cc (right):

https://codereview.webrtc.org/2119633004/diff/80001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/android/opensles_common.cc:76: default:
I think I've read somewhere that 96kHz is supported (possibly 88.2kHz then as
well?).

Do we have any stats from the field that give us confidence that we will never
see higher than 48kHz in should-be-supported situations?

https://codereview.webrtc.org/2119633004/diff/80001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/android/opensles_common.cc:77: RTC_CHECK(false) <<
"Unsupported sample rate: " << sample_rate;
nit: also include a break; here for consistency.

https://codereview.webrtc.org/2119633004/diff/80001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/android/opensles_common.cc:82: if
(format.numChannels == 1)
{}
(see lines below)

https://codereview.webrtc.org/2119633004/diff/80001/webrtc/modules/audio_devi...
File webrtc/modules/audio_device/android/opensles_player.cc (right):

https://codereview.webrtc.org/2119633004/diff/80001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/android/opensles_player.cc:227: if (engine_)
is ObtainEngineInterface always called on the same thread?  Does it make sense
to start using thread checkers to assert this?

https://codereview.webrtc.org/2119633004/diff/80001/webrtc/modules/audio_devi...
File webrtc/modules/audio_device/android/opensles_recorder.cc (right):

https://codereview.webrtc.org/2119633004/diff/80001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/android/opensles_recorder.cc:30: #define
RETURN_ON_ERROR(op, ...)                          \
I really prefer not to have macros with return statements :-/  also, macros
should in general be avoided.
Can the code be refactored to avoid this?

https://codereview.webrtc.org/2119633004/diff/80001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/android/opensles_recorder.cc:35: return __VA_ARGS__;
                                \
why __VA_ARGS__?

https://codereview.webrtc.org/2119633004/diff/80001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/android/opensles_recorder.cc:60:
thread_checker_opensles_.DetachFromThread();
nit: do this at the top of the ctor

https://codereview.webrtc.org/2119633004/diff/80001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/android/opensles_recorder.cc:130:
(*recorder_)->SetRecordState(recorder_, SL_RECORDSTATE_RECORDING), -1);
it's not clear from reading the code what the -1 value is

https://codereview.webrtc.org/2119633004/diff/80001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/android/opensles_recorder.cc:154:
ALOGD("AttachAudioBuffer");
nit: the function looks like a wall of code. If there are logical parts that
belong together with a log statement, it would make it a little more readable if
there's whitespace that separates those parts.

https://codereview.webrtc.org/2119633004/diff/80001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/android/opensles_recorder.cc:297: OpenSLESRecorder*
stream = reinterpret_cast<OpenSLESRecorder*>(context);
nit: use static_cast

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

https://codereview.webrtc.org/2119633004/diff/80001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/android/opensles_recorder.h:135: AudioManager*
audio_manager_;
AudioManager* const audio_manager_;

[henrika_webrtc, 2016-09-16 13:30:48]

Thanks Tommi!

PTAL

https://codereview.webrtc.org/2119633004/diff/80001/webrtc/modules/audio_devi...
File webrtc/modules/audio_device/android/audio_device_unittest.cc (right):

https://codereview.webrtc.org/2119633004/diff/80001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/android/audio_device_unittest.cc:729: if
(low_latency_output && low_latency_input)
On 2016/09/15 09:34:12, tommi (webrtc) wrote:
> use {} since lines 732-733 wrap

Done.

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

https://codereview.webrtc.org/2119633004/diff/80001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/android/build_info.h:27: SDK_CODE_JELLY_BEAN = 16,  
   //  Android 4.1
On 2016/09/15 09:34:12, tommi (webrtc) wrote:
> nit: fix whitespace

Done.

https://codereview.webrtc.org/2119633004/diff/80001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/android/build_info.h:63: int GetSdkVersion();
On 2016/09/15 09:34:12, tommi (webrtc) wrote:
> can this simply return SdkCode?

Done.

https://codereview.webrtc.org/2119633004/diff/80001/webrtc/modules/audio_devi...
File
webrtc/modules/audio_device/android/java/src/org/webrtc/voiceengine/BuildInfo.java
(right):

https://codereview.webrtc.org/2119633004/diff/80001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/android/java/src/org/webrtc/voiceengine/BuildInfo.java:49:
return Build.VERSION.SDK_INT;
On 2016/09/15 09:34:12, tommi (webrtc) wrote:
> fix indent

Done.

https://codereview.webrtc.org/2119633004/diff/80001/webrtc/modules/audio_devi...
File
webrtc/modules/audio_device/android/java/src/org/webrtc/voiceengine/WebRtcAudioManager.java
(right):

https://codereview.webrtc.org/2119633004/diff/80001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/android/java/src/org/webrtc/voiceengine/WebRtcAudioManager.java:234:
return false;
We are actually not changing anything for output. At least that was my intention
but perhaps you have spotted something that I've missed. AFAIK, all changes in
here only affects input.

https://codereview.webrtc.org/2119633004/diff/80001/webrtc/modules/audio_devi...
File webrtc/modules/audio_device/android/opensles_common.cc (right):

https://codereview.webrtc.org/2119633004/diff/80001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/android/opensles_common.cc:76: default:
Great feedback. You are correct. I have never seen such a device but I am adding
it just in case.

https://codereview.webrtc.org/2119633004/diff/80001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/android/opensles_common.cc:77: RTC_CHECK(false) <<
"Unsupported sample rate: " << sample_rate;
On 2016/09/15 09:34:12, tommi (webrtc) wrote:
> nit: also include a break; here for consistency.

Done.

https://codereview.webrtc.org/2119633004/diff/80001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/android/opensles_common.cc:82: if
(format.numChannels == 1)
On 2016/09/15 09:34:13, tommi (webrtc) wrote:
> {}
> (see lines below)

Done.

https://codereview.webrtc.org/2119633004/diff/80001/webrtc/modules/audio_devi...
File webrtc/modules/audio_device/android/opensles_player.cc (right):

https://codereview.webrtc.org/2119633004/diff/80001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/android/opensles_player.cc:227: if (engine_)
Yes Sir, it is...and I do have a thread checker to ensure it :-)

https://codereview.webrtc.org/2119633004/diff/80001/webrtc/modules/audio_devi...
File webrtc/modules/audio_device/android/opensles_recorder.cc (right):

https://codereview.webrtc.org/2119633004/diff/80001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/android/opensles_recorder.cc:30: #define
RETURN_ON_ERROR(op, ...)                          \
Fixed and discussed offline.

https://codereview.webrtc.org/2119633004/diff/80001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/android/opensles_recorder.cc:35: return __VA_ARGS__;
                                \
Now removed.

https://codereview.webrtc.org/2119633004/diff/80001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/android/opensles_recorder.cc:60:
thread_checker_opensles_.DetachFromThread();
On 2016/09/15 09:34:13, tommi (webrtc) wrote:
> nit: do this at the top of the ctor

Done.

https://codereview.webrtc.org/2119633004/diff/80001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/android/opensles_recorder.cc:130:
(*recorder_)->SetRecordState(recorder_, SL_RECORDSTATE_RECORDING), -1);
It is the returned value and we use 0/-1 to signal OK/ERROR. Not sure how to
make that more clear. Adding a comment as well.

https://codereview.webrtc.org/2119633004/diff/80001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/android/opensles_recorder.cc:154:
ALOGD("AttachAudioBuffer");
Thanks. Will improve.

https://codereview.webrtc.org/2119633004/diff/80001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/android/opensles_recorder.cc:297: OpenSLESRecorder*
stream = reinterpret_cast<OpenSLESRecorder*>(context);
On 2016/09/15 09:34:13, tommi (webrtc) wrote:
> nit: use static_cast

Done.

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

https://codereview.webrtc.org/2119633004/diff/80001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/android/opensles_recorder.h:135: AudioManager*
audio_manager_;
But of course.

[tommi, 2016-09-17 09:27:08]

lgtm

https://codereview.webrtc.org/2119633004/diff/80001/webrtc/modules/audio_devi...
File
webrtc/modules/audio_device/android/java/src/org/webrtc/voiceengine/WebRtcAudioManager.java
(right):

https://codereview.webrtc.org/2119633004/diff/80001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/android/java/src/org/webrtc/voiceengine/WebRtcAudioManager.java:234:
return false;
On 2016/09/16 13:30:47, henrika_webrtc wrote:
> We are actually not changing anything for output. At least that was my
intention
> but perhaps you have spotted something that I've missed. AFAIK, all changes in
> here only affects input.

sorry, you're right, I must have missed the 'Input' part of the function name.

[henrika_webrtc, 2016-09-19 11:16:13]

https://codereview.webrtc.org/2119633004/diff/80001/webrtc/modules/audio_devi...
File
webrtc/modules/audio_device/android/java/src/org/webrtc/voiceengine/WebRtcAudioManager.java
(right):

https://codereview.webrtc.org/2119633004/diff/80001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/android/java/src/org/webrtc/voiceengine/WebRtcAudioManager.java:234:
return false;
On 2016/09/17 09:27:08, tommi (webrtc) wrote:
> On 2016/09/16 13:30:47, henrika_webrtc wrote:
> > We are actually not changing anything for output. At least that was my
> intention
> > but perhaps you have spotted something that I've missed. AFAIK, all changes
in
> > here only affects input.
> 
> sorry, you're right, I must have missed the 'Input' part of the function name.

Acknowledged.

[henrika_webrtc, 2016-09-19 11:18:07]

The CQ bit was checked by henrika@webrtc.org

[henrika_webrtc, 2016-09-19 11:18:08]

The patchset sent to the CQ was uploaded after l-g-t-m from tommi@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2119633004/#ps120001 (title:
"Restoring the hardcoded disable flag")

[commit-bot: I haz the power, 2016-09-19 11:18:14]

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

[commit-bot: I haz the power, 2016-09-19 11:31:03]

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

[commit-bot: I haz the power, 2016-09-19 11:31:04]

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

[henrika_webrtc, 2016-09-19 13:36:50]

The CQ bit was checked by henrika@webrtc.org

[henrika_webrtc, 2016-09-19 13:36:51]

The patchset sent to the CQ was uploaded after l-g-t-m from tommi@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2119633004/#ps140001 (title:
"Fixing presubmit warnings")

[commit-bot: I haz the power, 2016-09-19 13:36:54]

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

[commit-bot: I haz the power, 2016-09-19 13:42:04]

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

[commit-bot: I haz the power, 2016-09-19 13:42:05]

Try jobs failed on following builders:
  android_dbg on master.tryserver.webrtc (JOB_TIMED_OUT, no build URL)

[henrika_webrtc, 2016-09-19 13:44:26]

Description was changed from

==========
Adds support for OpenSL ES based audio capture on Android.

NOTE: the new code is disabled by default in the WebRtcAudioManager to ensure
that
OpenSL ES is not accidentally activated in existing clients. There are still
some
unresolved issues to sort out before it can be utilized. 

Enables possibility to use OpenSL ES based audio in both directions for WebRTC.
All unit tests and demo clients have been tested with the new implementation but
the new support is behind a flag (see above).

More testing is needed before it can be used in the field and additional support
for
hardware effects is still missing. 

BUG=webrtc:5925
==========

to

==========
Adds support for OpenSL ES based audio capture on Android.

NOTE: the new code is disabled by default in the WebRtcAudioManager to ensure
that
OpenSL ES is not accidentally activated in existing clients. There are still
some
unresolved issues to sort out before it can be utilized.

Enables possibility to use OpenSL ES based audio in both directions for WebRTC.
All unit tests and demo clients have been tested with the new implementation but
the new support is behind a flag (see above).

More testing is needed before it can be used in the field and additional support
for
hardware effects is still missing.

BUG=webrtc:5925
R=tommi@webrtc.org

Committed:
https://chromium.googlesource.com/external/webrtc/+/918b5547897a70ae13053fb06...
==========

[commit-bot: I haz the power, 2016-09-19 13:44:27]

Description was changed from

==========
Adds support for OpenSL ES based audio capture on Android.

NOTE: the new code is disabled by default in the WebRtcAudioManager to ensure
that
OpenSL ES is not accidentally activated in existing clients. There are still
some
unresolved issues to sort out before it can be utilized.

Enables possibility to use OpenSL ES based audio in both directions for WebRTC.
All unit tests and demo clients have been tested with the new implementation but
the new support is behind a flag (see above).

More testing is needed before it can be used in the field and additional support
for
hardware effects is still missing.

BUG=webrtc:5925
R=tommi@webrtc.org

Committed:
https://chromium.googlesource.com/external/webrtc/+/918b5547897a70ae13053fb06...
==========

to

==========
Adds support for OpenSL ES based audio capture on Android.

NOTE: the new code is disabled by default in the WebRtcAudioManager to ensure
that
OpenSL ES is not accidentally activated in existing clients. There are still
some
unresolved issues to sort out before it can be utilized.

Enables possibility to use OpenSL ES based audio in both directions for WebRTC.
All unit tests and demo clients have been tested with the new implementation but
the new support is behind a flag (see above).

More testing is needed before it can be used in the field and additional support
for
hardware effects is still missing.

BUG=webrtc:5925
R=tommi@webrtc.org

Committed: https://crrev.com/918b5547897a70ae13053fb0672c5f1a48dd714a
Cr-Commit-Position: refs/heads/master@{#14290}
==========

[henrika_webrtc, 2016-09-19 13:44:28]

Committed patchset #7 (id:140001) manually as
918b5547897a70ae13053fb0672c5f1a48dd714a (presubmit successful).

[commit-bot: I haz the power, 2016-09-19 13:44:28]

Patchset 7 (id:??) landed as
https://crrev.com/918b5547897a70ae13053fb0672c5f1a48dd714a
Cr-Commit-Position: refs/heads/master@{#14290}