Moves ownership of OpenSL engine object to Android audio manager

Moves ownership of OpenSL engine object to Android audio manager with the goal of adding support for OpenSL ES based audio capture.

BUG=webrtc:5925
Committed: https://crrev.com/521f7a8db7dc4125e2eda72801188a1be2dce436
Cr-Commit-Position: refs/heads/master@{#12975}

[henrika_webrtc, 2016-05-31 08:35:14]

Description was changed from

==========
Moves ownership of OpenSL engine object to Android audio manager

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

to

==========
Moves ownership of OpenSL engine object to Android audio manager with the goal
of adding support for OpenSL ES based audio capture.

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

[henrika_webrtc, 2016-05-31 09:58:17]

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

[henrika_webrtc, 2016-05-31 09:58:29]

PTAL

[magjed_webrtc, 2016-05-31 11:24:07]

https://codereview.webrtc.org/2019223004/diff/40001/webrtc/modules/audio_devi...
File webrtc/modules/audio_device/android/audio_manager_unittest.cc (right):

https://codereview.webrtc.org/2019223004/diff/40001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/android/audio_manager_unittest.cc:55: bool
SLEngineIsValid(SLObjectItf engine_object) {
If you return a bool, you will not know why SLEngineIsValid failed when used in
EXPECT_TRUE. You can return a ::testing::AssertionResult instead and replace
'return false;'
with e.g.
'return ::testing::AssertionFailure() << "SLEngine::GetInterface() failed";'
and replace 'return true' with 'return '::testing::AssertionSuccess()'.

Or simpler, rename this method to 'void ValidateSLEngine' and use EXPECT*
instead of returning anything, and replace
'EXPECT_TRUE(SLEngineIsValid(engine_object))' with
'ValidateSLEngine(engine_object)'.

https://codereview.webrtc.org/2019223004/diff/40001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/android/audio_manager_unittest.cc:97: for (size_t i
= 0; i < arraysize(opensles_audio); ++i) {
nit: Use for-each loop instead, i.e.
for (const AudioDeviceModule::AudioLayer audioLayer : opensles_audio) {
  ...
}

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

https://codereview.webrtc.org/2019223004/diff/40001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/android/opensles_common.cc:26: const char*
sl_error_strings[] = {
Make this variable static.

https://codereview.webrtc.org/2019223004/diff/40001/webrtc/modules/audio_devi...
File webrtc/modules/audio_device/android/opensles_player.h (right):

https://codereview.webrtc.org/2019223004/diff/40001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/android/opensles_player.h:102: bool
GetEngineInterface();
I don't think calling this function 'GetEngineInterface' is appropriate when you
don't return any engine. I think CreateEngine or InitEngine is better.

https://codereview.webrtc.org/2019223004/diff/40001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/android/opensles_player.h:127: AudioManager*
audio_manager_;
Can you comment on the lifetime of AudioManager? It looks like it's created in
AudioDeviceModuleImpl::CreatePlatformSpecificObjects().

[henrika_webrtc, 2016-05-31 12:04:03]

Thanks, PTAL ;-)

https://codereview.webrtc.org/2019223004/diff/40001/webrtc/modules/audio_devi...
File webrtc/modules/audio_device/android/audio_manager_unittest.cc (right):

https://codereview.webrtc.org/2019223004/diff/40001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/android/audio_manager_unittest.cc:55: bool
SLEngineIsValid(SLObjectItf engine_object) {
Thanks. Used the second approach ;-)

https://codereview.webrtc.org/2019223004/diff/40001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/android/audio_manager_unittest.cc:97: for (size_t i
= 0; i < arraysize(opensles_audio); ++i) {
Thanks, done ;-)

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

https://codereview.webrtc.org/2019223004/diff/40001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/android/opensles_common.cc:26: const char*
sl_error_strings[] = {
On 2016/05/31 11:24:07, magjed_webrtc wrote:
> Make this variable static.

Done.

https://codereview.webrtc.org/2019223004/diff/40001/webrtc/modules/audio_devi...
File webrtc/modules/audio_device/android/opensles_player.h (right):

https://codereview.webrtc.org/2019223004/diff/40001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/android/opensles_player.h:102: bool
GetEngineInterface();
Good point. However, the OpenSL ES API is a bit messy it would be complicated to
return the actual interface and the interface is fact not created. The existing
engine object has a set if implicit interface (already created) and we simply
get (and stores) it here. I changed the name to Obtain instead. Hope that is OK.

https://codereview.webrtc.org/2019223004/diff/40001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/android/opensles_player.h:127: AudioManager*
audio_manager_;
On 2016/05/31 11:24:07, magjed_webrtc wrote:
> Can you comment on the lifetime of AudioManager? It looks like it's created in
> AudioDeviceModuleImpl::CreatePlatformSpecificObjects().

Done.

[magjed_webrtc, 2016-05-31 12:23:58]

lgtm

[henrika_webrtc, 2016-05-31 12:25:47]

The CQ bit was checked by henrika@webrtc.org

[commit-bot: I haz the power, 2016-05-31 12:25:58]

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

[commit-bot: I haz the power, 2016-05-31 14:03:22]

Description was changed from

==========
Moves ownership of OpenSL engine object to Android audio manager with the goal
of adding support for OpenSL ES based audio capture.

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

to

==========
Moves ownership of OpenSL engine object to Android audio manager with the goal
of adding support for OpenSL ES based audio capture.

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

[commit-bot: I haz the power, 2016-05-31 14:03:23]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-05-31 14:03:34]

Description was changed from

==========
Moves ownership of OpenSL engine object to Android audio manager with the goal
of adding support for OpenSL ES based audio capture.

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

to

==========
Moves ownership of OpenSL engine object to Android audio manager with the goal
of adding support for OpenSL ES based audio capture.

BUG=webrtc:5925

Committed: https://crrev.com/521f7a8db7dc4125e2eda72801188a1be2dce436
Cr-Commit-Position: refs/heads/master@{#12975}
==========

[commit-bot: I haz the power, 2016-05-31 14:03:35]

Patchset 4 (id:??) landed as
https://crrev.com/521f7a8db7dc4125e2eda72801188a1be2dce436
Cr-Commit-Position: refs/heads/master@{#12975}