CirrusLogic / tinyhal

Configurable audio HAL for Android
Apache License 2.0
31 stars 17 forks source link

Add get_microphones() and get_active_microphone() stub functions #9

Open AntonDehtiarov opened 4 years ago

AntonDehtiarov commented 4 years ago

The Google VTS tests for Android VtsHalAudioV5_0Target(GetMicrophonesTest) needs to use pointer of functions (get_active_micropones)() and (get_active_micropones)() which are not initialized and are not implemented in tinyhal.

Functions in_get_active_microphones() and adev_get_micropones() were implemented as stub functions that will expand in the future. Those functions fill array audio_microphone_characteristic_t with default micriophone infromation.

rsglobal commented 3 years ago

I've just run VTS11r1 on HAL@6 without this PR and got 100% success. I can guess there was some fixes from AOSP side, or this functionality isn't so critical and was excluded from VTS.

================= Results ==================
=============== Consumed Time ==============
    arm64-v8a VtsHalAudioV6_0TargetTest: 23s
    armeabi-v7a VtsHalAudioV6_0TargetTest: 22s
Total aggregated tests run time: 45s
============== TOP 2 Slow Modules ==============
    arm64-v8a VtsHalAudioV6_0TargetTest: 24.99 tests/sec [575 tests / 23009 msec]
    armeabi-v7a VtsHalAudioV6_0TargetTest: 25.33 tests/sec [575 tests / 22697 msec]
============== Modules Preparation Times ==============
    armeabi-v7a VtsHalAudioV6_0TargetTest => prep = 2525 ms || clean = 20104 ms
    arm64-v8a VtsHalAudioV6_0TargetTest => prep = 2190 ms || clean = 20434 ms
Total preparation time: 4s  ||  Total tear down time: 40s
=======================================================
=============== Summary ===============
Total Run time: 1m 50s
2/2 modules completed
Total Tests       : 1150
PASSED            : 1138
FAILED            : 0
IGNORED           : 12
============== End of Results ==============
============================================
rsglobal commented 3 years ago

Test were ignored:

AudioHidlDevice/AudioHidlDeviceTest#GetMicrophonesTest/0_default_primary | IGNORED |   AudioHidlDevice/AudioHidlDeviceTest#GetMicrophonesTest/1_default_r_submix | IGNORED |

rfvirgil commented 3 years ago

According to the Android source the get_microphones() and get_active_microphones() functions were added in API version 4.0. As tinyhal reports that it is API version 2.0 it should not need to implement these functions.

I'd prefer not to submit this large stub function until we have to move up to at least API 4.0 and it becomes mandatory to provide an implementation.

AntonDehtiarov commented 3 years ago

According to the Android source the get_microphones() and get_active_microphones() functions were added in API version 4.0. As tinyhal reports that it is API version 2.0 it should not need to implement these functions.

I'd prefer not to submit this large stub function until we have to move up to at least API 4.0 and it becomes mandatory to provide an implementation.

I noticed that the tinyhal uses preprocessor directives to provide some code lines when using API 3.0:

#ifdef AUDIO_DEVICE_API_VERSION_3_0

Can we use such preprocessor directives to add functions for API version 5.0?

#ifdef AUDIO_DEVICE_API_VERSION_5_0

I have provided the described changes in the PR.

rsglobal commented 3 years ago

@AntonDehtiarov ,

Fallback stub seems to be available in the framework. So there is no reason to duplicate it from HAL side:

Fallback path: https://cs.android.com/android/platform/superproject/+/master:frameworks/base/media/java/android/media/AudioManager.java;l=5788;drc=628590d7ec80e10a3fc24b1c18a1afb55cca10a8

Stub data: https://cs.android.com/android/platform/superproject/+/master:frameworks/base/media/java/android/media/AudioManager.java;l=6222;drc=f39726f2620b152b03e2697fadc5283cd3e002d7

rfvirgil commented 3 years ago

The #ifdef AUDIO_DEVICE_API_VERSION_3_0 in the code is not really correct. It should really be testing for the Android version because it is there to avoid a function that was deprecated in Lollipop.

I have no objection to upgrading the Tinyhal API version, but the public master branch of Android only has defines up to version 3.1: https://android.googlesource.com/platform/hardware/libhardware/+/refs/heads/master/include/hardware/audio.h#60

The Qualcomm code you reference in your commit message reports itself as 2.0: https://android.googlesource.com/platform/hardware/qcom/audio/+/refs/heads/master/hal/audio_hw.c#6579

Where are you getting the define for AUDIO_DEVICE_API_VERSION_5_0?

AntonDehtiarov commented 3 years ago

Where are you getting the define for AUDIO_DEVICE_API_VERSION_5_0?

You are right, there is no define for AUDIO_DEVICE_API_VERSION_5_0. It was my mistake.

rfvirgil commented 3 years ago

If you need a #ifdef to protect this code, but Android doesn't define anything you can use, you could add a tinyhal build define. Maybe call it TINYHAL_REPORT_ACTIVE_MICROPHONES. Then set it in your device.mk.

For an example search for TINYHAL_COMPRESS_PLAYBACK.