Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

AntonDehtiarov
Copy link

@AntonDehtiarov AntonDehtiarov commented May 26, 2020

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
Copy link
Contributor

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
Copy link
Contributor

Test were ignored:

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

@rfvirgil
Copy link
Contributor

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.

The Google VTS tests for Android VtsHalAudioV5_0Target(GetMicrophonesTest)
needs to use pointer of function (*get_micropones)() which is not initialized
and function adev_get_microphones() required for initialization witch is
not implemented in tinyhal.

The analysis of the implementation of the audio hal for Qualcomm
(https://android.googlesource.com/platform/hardware/qcom/audio/+/refs/heads/
master/hal/audio_hw.c) and Goldfish
(https://android.googlesource.com/device/generic/goldfish/+/dc18a59%5E%21/)
have been made. The implementation similar to Goldfish(stub) has been chosen as
basic.

Function adev_get_microphones() was implemented as stub function that will
expand in the future. This function fill array audio_microphone_characteristic_t
with default microphone information.

Signed-off-by: Anton Dehtiarov <anton.dehtiarov@globallogic.com>
The Google VTS tests for Android VtsHalAudioV5_0Target(GetMicrophonesTest)
needs to use pointer of function (*get_active_micropones)() which is not
initialized and function in_get_active_microphones() required for initialization
witch is not implemented in tinyhal.

The analysis of the implementation of the audio hal for Qualcomm
(https://android.googlesource.com/platform/hardware/qcom/audio/+/refs/heads/
master/hal/audio_hw.c) and Goldfish
(https://android.googlesource.com/device/generic/goldfish/+/dc18a59%5E%21/)
have been made. The implementation similar to Goldfish(stub) has been chosen as
basic.

Function in_get_active_microphones() was implemented as stub function that will
expand in the future. This function fill array audio_microphone_characteristic_t
with default microphone information.

Signed-off-by: Anton Dehtiarov <anton.dehtiarov@globallogic.com>
@AntonDehtiarov
Copy link
Author

AntonDehtiarov commented Sep 29, 2020

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.

@rfvirgil
Copy link
Contributor

rfvirgil commented Oct 2, 2020

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
Copy link
Author

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
Copy link
Contributor

rfvirgil commented Jan 8, 2021

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants