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

[Seine][pdx201][AOSP10] Echo cancellation doesn't work in loud speaker mode #684

Closed
AngryGami opened this issue Mar 29, 2021 · 31 comments
Closed
Labels

Comments

@AngryGami
Copy link

AngryGami commented Mar 29, 2021

Platform: Seine
Device: pdx201
Kernel version: 4.14 (be667ecc267b81)
Android version: 10
Software binaries version: SW_binaries_for_Xperia_Android_10.0.7.1_r1_v12b_seine.img

Previously working on
Stock image doesn't have this issue.

Description
When in regular (GSM/LTE) phone call and enable loud speaker microphone on the device starts picking sound from the speaker creating annoying echo on the other side of the call.

How to reproduce
Make a call, enable loud speaker and hear echo on the other end of the call.

Additional context

I made some investigation of this problem but eventually get out of ideas what might be the reason for this. So here what I've found so far:

  1. I've looked through mixer_paths.xml file and noticed paths like these:
    <path name="echo-reference">
        <ctl name="AUDIO_REF_EC_UL1 MUX" value="PRI_MI2S_TX" />
    </path>

    <path name="echo-reference headphones">
        <ctl name="AUDIO_REF_EC_UL1 MUX" value="RX_CDC_DMA_RX_0" />
    </path>

These paths looks related to echo cancellation :)

  1. After searching codebase for usages of these paths I've found that only place where these are touched is in platform_set_echo_reference function in vendor/qcom/opensource/audio-hal/primary-hal/hal/msm8974/platform.c file (well one that is relevant for this platform at least). This method is called from several places, but it looks like one that is relevant to loud speaker mode is this:
        } else if (out_device & AUDIO_DEVICE_OUT_SPEAKER ||
                   out_device & AUDIO_DEVICE_OUT_SPEAKER_SAFE ||
                   out_device & AUDIO_DEVICE_OUT_WIRED_HEADPHONE ||
                   out_device & AUDIO_DEVICE_OUT_LINE ||
                   out_device & AUDIO_DEVICE_OUT_BUS) {
            if (my_data->fluence_type != FLUENCE_NONE &&
                (my_data->fluence_in_voice_call ||
                 my_data->fluence_in_hfp_call) &&
                my_data->fluence_in_spkr_mode) {
...
                if (audio_extn_hfp_is_active(adev)){
                    ALOGV("%s: audio_extn_hfp_is_active returned true! ", __func__);
                    platform_set_echo_reference(adev, true, out_device);
                } else {
                    ALOGV("%s: audio_extn_hfp_is_active returned FALSE! ", __func__);
                }

These are additional log statements I've added to track it down.

  1. Function audio_extn_hfp_is_active is defined in vendor/qcom/opensource/audio-hal/primary-hal/hal/audio_extn/audio_extn.c and it checks if hfp_is_active function is defined and call it for answer. This function only get defined if vendor.audio.feature.hfp.enable property is true and it is set to true in device/sony/common/common-prop.mk. I see followint log messages in both stock and aosp logs:
03-22 15:30:36.579   776   776 D audio_hw_extn: hfp_feature_init: Called with feature Enabled

But but aosp version cannot enable this properly because it can't find libhfp.so library, so I have to include it into my mk file like this:

PRODUCT_PACKAGES += \
    libhfp 

After had done that I got following messages in aosp image log:

07-31 03:39:19.728   686   766 D audio_hw_extn: hfp_feature_init: Called with feature Enabled
07-31 03:39:19.728   818   818 E QMI_FW  : QMUXD: WARNING qmi_qmux_if_pwr_up_init failed! rc=-6
07-31 03:39:19.729     0     0 I ueventd : firmware: loading 'cdsp.b10' for '/devices/platform/soc/b300000.qcom,turing/firmware/cdsp.b10'
07-31 03:39:19.730     0     0 E CP      : charge_pump missing
07-31 03:39:19.732   688   688 I mm-camera: <SENSOR>< INFO> 311: sensor_sdk_util_get_i2c_freq_mode: Invalid i2c_freq_mode = 4
07-31 03:39:19.732     0     0 E msm_sensor_fill_eeprom_subdevid_by_name: 222 Eeprom userspace probe for s5k4h7yx_front
07-31 03:39:19.733   686   766 D audio_hw_extn: hfp_feature_init:: ---- Feature HFP is Enabled ----

Though this was not enough to make hfp_is_active to return true during call because even feature is enabled it was not initialized properly.

  1. hfp_is_active is defined in vendor/qcom/opensource/audio-hal/primary-hal/hal/audio_extn/hfp.c" file and call into fp_get_usecase_from_list`:
bool hfp_is_active(struct audio_device *adev)
{
    struct audio_usecase *hfp_usecase = NULL;
    hfp_usecase = fp_get_usecase_from_list(adev, hfpmod.ucid);

    if (hfp_usecase != NULL){
        ALOGI("%s: usecase for hfp found", __func__);
        return true;
    } else {
        ALOGI("%s: usecase for hfp not found", __func__);
        return false;
    }
}

Again, additional logging added by me. This fp_get_usecase_from_list function comes from audio_extn.c file in form of reference to get_usecase_from_list from vendor/qcom/opensource/audio-hal/primary-hal/hal/audio_hw.c. This function just goes through list of all usecases that are assigned to adev struct and looking for USECASE_AUDIO_HFP_SCO, which is added by start_hfp function like this:

    list_add_tail(&adev->usecase_list, &uc_info->list);

I'm not very familiar with C and this took me a while to understand that this is basically "append" operation, especially confusing is uc_info->list since this is not actually "list" but rather a "listnode".
Anyway I started looking what is calling this start_hfp function.

  1. Apparently it is called from very same hfp.c file from hfp_set_parameters function, but there is a condition around it:
    ret = str_parms_get_str(parms, AUDIO_PARAMETER_HFP_ENABLE, value,
                            sizeof(value));
    if (ret >= 0) {
...
    if (!strncmp(value, "true", sizeof(value))) {
                       ret = start_hfp(adev,parms);
...

I.e. it will only get invoked if AUDIO_PARAMETER_HFP_ENABLE configuration parameter is set to "true". This parameter could be set via audio_platform_info.xml <config_params> section, so I did that and now my device refused to boot because of following error:

07-31 03:39:23.060  1463  1463 F DEBUG   : *** *** *** *** *** *** *** *** *** *** *** *** *** *** *** ***
07-31 03:39:23.060  1463  1463 F DEBUG   : Build fingerprint: 'Acme/aosp_xqau52/pdx201:10/QQ3A.200805.001/eng.acme.20210325.112824:userdebug/dev-keys'
07-31 03:39:23.060  1463  1463 F DEBUG   : Revision: '0'
07-31 03:39:23.060  1463  1463 F DEBUG   : ABI: 'arm'
07-31 03:39:23.060  1463  1463 F DEBUG   : Timestamp: 1970-07-31 03:39:23+0000
07-31 03:39:23.060  1463  1463 F DEBUG   : pid: 686, tid: 766, name: HwBinder:686_2  >>> /vendor/bin/hw/android.hardware.audio@2.0-service <<<
07-31 03:39:23.060  1463  1463 F DEBUG   : uid: 1041
07-31 03:39:23.060  1463  1463 F DEBUG   : signal 11 (SIGSEGV), code 1 (SEGV_MAPERR), fault addr 0x0
07-31 03:39:23.060  1463  1463 F DEBUG   : Cause: null pointer dereference
07-31 03:39:23.060  1463  1463 F DEBUG   :     r0  f00a202a  r1  00000000  r2  eea6b250  r3  00000003
07-31 03:39:23.061  1463  1463 F DEBUG   :     r4  00000000  r5  00000000  r6  00000000  r7  ee9b081e
07-31 03:39:23.061  1463  1463 F DEBUG   :     r8  f0a9e260  r9  f0c5d150  r10 ee6ef9c0  r11 f0bdd300
07-31 03:39:23.061  1463  1463 F DEBUG   :     ip  ef1b1148  sp  ef1b15f8  lr  ee9b18e1  pc  eea6b274
07-31 03:39:23.085   818   818 I QC-NETMGR-LIB: NetmgrNetdClientInit(): Looking for Netd service
07-31 03:39:23.089   818   818 D QC-NETMGR-LIB: registerServerNotification(): Successfully registered for Netd HAL service
07-31 03:39:23.089  1463  1463 F DEBUG   : 
07-31 03:39:23.089  1463  1463 F DEBUG   : backtrace:
07-31 03:39:23.089  1463  1463 F DEBUG   :       #00 pc 0004d274  /vendor/lib/hw/audio.primary.sm6125.so (platform_set_mic_mute+36) (BuildId: 8bbfcc7a41151050b88551f6b1e0557d)
07-31 03:39:23.089  1463  1463 F DEBUG   :       #01 pc 000028df  /vendor/lib/libhfp.so (hfp_set_parameters+930) (BuildId: 4e5e1bb0853b5b7a08c642f7d2bbbb2b)
07-31 03:39:23.089  1463  1463 F DEBUG   :       #02 pc 0005112c  /vendor/lib/hw/audio.primary.sm6125.so (platform_set_parameters+3508) (BuildId: 8bbfcc7a41151050b88551f6b1e0557d)
07-31 03:39:23.089  1463  1463 F DEBUG   :       #03 pc 00008843  /system/lib/vndk-29/libexpat.so (doContent+1258) (BuildId: b95967a5fb4e2dbc8e0650ab6b5a4c6e)
07-31 03:39:23.089  1463  1463 F DEBUG   :       #04 pc 000070cb  /system/lib/vndk-29/libexpat.so (contentProcessor+36) (BuildId: b95967a5fb4e2dbc8e0650ab6b5a4c6e)
07-31 03:39:23.089  1463  1463 F DEBUG   :       #05 pc 00004673  /system/lib/vndk-29/libexpat.so (XML_ParseBuffer+86) (BuildId: b95967a5fb4e2dbc8e0650ab6b5a4c6e)
07-31 03:39:23.089  1463  1463 F DEBUG   :       #06 pc 0003fa38  /vendor/lib/hw/audio.primary.sm6125.so (platform_info_init+444) (BuildId: 8bbfcc7a41151050b88551f6b1e0557d)
07-31 03:39:23.089  1463  1463 F DEBUG   :       #07 pc 00046b98  /vendor/lib/hw/audio.primary.sm6125.so (platform_init+9200) (BuildId: 8bbfcc7a41151050b88551f6b1e0557d)
07-31 03:39:23.089  1463  1463 F DEBUG   :       #08 pc 00038658  /vendor/lib/hw/audio.primary.sm6125.so (adev_open+1020) (BuildId: 8bbfcc7a41151050b88551f6b1e0557d)
07-31 03:39:23.089  1463  1463 F DEBUG   :       #09 pc 0001b681  /vendor/lib/hw/android.hardware.audio@5.0-impl.so (android::hardware::audio::V5_0::implementation::DevicesFactory::loadAudioInterface(char const*, audio_hw_device**)+72) (BuildId: a3b7cd5320c57fde9b203c116c2f2de5)
07-31 03:39:23.089  1463  1463 F DEBUG   :       #10 pc 0001b2bd  /vendor/lib/hw/android.hardware.audio@5.0-impl.so (android::hardware::Return<void> android::hardware::audio::V5_0::implementation::DevicesFactory::openDevice<android::hardware::audio::V5_0::implementation::PrimaryDevice, std::__1::function<void (android::hardware::audio::V5_0::Result, android::sp<android::hardware::audio::V5_0::IDevice> const&)>>(char const*, std::__1::function<void (android::hardware::audio::V5_0::Result, android::sp<android::hardware::audio::V5_0::IDevice> const&)>)+28) (BuildId: a3b7cd5320c57fde9b203c116c2f2de5)
07-31 03:39:23.089  1463  1463 F DEBUG   :       #11 pc 0001b275  /vendor/lib/hw/android.hardware.audio@5.0-impl.so (android::hardware::audio::V5_0::implementation::DevicesFactory::openDevice(android::hardware::hidl_string const&, std::__1::function<void (android::hardware::audio::V5_0::Result, android::sp<android::hardware::audio::V5_0::IDevice> const&)>)+216) (BuildId: a3b7cd5320c57fde9b203c116c2f2de5)
07-31 03:39:23.090  1463  1463 F DEBUG   :       #12 pc 0003f041  /system/lib/vndk-29/android.hardware.audio@5.0.so (android::hardware::audio::V5_0::BnHwDevicesFactory::_hidl_openDevice(android::hidl::base::V1_0::BnHwBase*, android::hardware::Parcel const&, android::hardware::Parcel*, std::__1::function<void (android::hardware::Parcel&)>)+220) (BuildId: 97f44903b2ff203f68a0d8f27ce4d390)
07-31 03:39:23.090  1463  1463 F DEBUG   :       #13 pc 0003f4a3  /system/lib/vndk-29/android.hardware.audio@5.0.so (android::hardware::audio::V5_0::BnHwDevicesFactory::onTransact(unsigned int, android::hardware::Parcel const&, android::hardware::Parcel*, unsigned int, std::__1::function<void (android::hardware::Parcel&)>)+194) (BuildId: 97f44903b2ff203f68a0d8f27ce4d390)
07-31 03:39:23.090  1463  1463 F DEBUG   :       #14 pc 00068bb7  /system/lib/vndk-sp-29/libhidlbase.so (android::hardware::BHwBinder::transact(unsigned int, android::hardware::Parcel const&, android::hardware::Parcel*, unsigned int, std::__1::function<void (android::hardware::Parcel&)>)+46) (BuildId: a1f85c0ad927d6cf45a73be8f70a94ff)
07-31 03:39:23.090  1463  1463 F DEBUG   :       #15 pc 0006b0ed  /system/lib/vndk-sp-29/libhidlbase.so (android::hardware::IPCThreadState::getAndExecuteCommand()+924) (BuildId: a1f85c0ad927d6cf45a73be8f70a94ff)
07-31 03:39:23.090  1463  1463 F DEBUG   :       #16 pc 0006c277  /system/lib/vndk-sp-29/libhidlbase.so (android::hardware::IPCThreadState::joinThreadPool(bool)+110) (BuildId: a1f85c0ad927d6cf45a73be8f70a94ff)
07-31 03:39:23.090  1463  1463 F DEBUG   :       #17 pc 00076c91  /system/lib/vndk-sp-29/libhidlbase.so (android::hardware::PoolThread::threadLoop()+12) (BuildId: a1f85c0ad927d6cf45a73be8f70a94ff)
07-31 03:39:23.090  1463  1463 F DEBUG   :       #18 pc 0000d853  /system/lib/vndk-sp-29/libutils.so (android::Thread::_threadLoop(void*)+182) (BuildId: 081b7de3d6307c4ac2d946b2280c98eb)
07-31 03:39:23.090  1463  1463 F DEBUG   :       #19 pc 000a5ea3  /apex/com.android.runtime/lib/bionic/libc.so (__pthread_start(void*)+20) (BuildId: 7b0c9b68af6fcfcb1364f04b4b069424)
07-31 03:39:23.090  1463  1463 F DEBUG   :       #20 pc 00060773  /apex/com.android.runtime/lib/bionic/libc.so (__start_thread+30) (BuildId: 7b0c9b68af6fcfcb1364f04b4b069424)

Looking at this error closely I saw that this is merely an NPE (Null pointer exception) somewhere in platform_set_mic_mute function, so I've looked into it and eventually found that it was invoked with platform parameter being null and this happens because at the moment when start_hfp get called adev->platform is not yet initialized. Actually it is called from inside function that right now initializes this field in audio_hw.c:

    adev->platform = platform_init(adev);

I.e. it calls it prematurely and this start_hfp have to be delayed. I've fixed this error by adding is_hfp_enabled_by_config into hfp_module struct and by adding some sanity checks in hfp_set_parameters function.
hfp_set_parameters get called multiple times and if platform is not initialized at first call - I just store flag that it need to be initialized next time.

  1. Finally hfp starts and I got usecase for hfp found message in the logs and I assumed that this is enough to enable echo cancellation.... though it isn't. Even that I got start_hfp invoked it immediately stops because devices it tries to open/use are either do not exist or do not respond properly. This happens here in start_hfp function:
    hfpmod.hfp_sco_rx = pcm_open(adev->snd_card,
                                  pcm_dev_asm_rx_id,
                                  PCM_OUT, &pcm_config_hfp);
    if (hfpmod.hfp_sco_rx && !pcm_is_ready(hfpmod.hfp_sco_rx)) {
        ALOGE("%s: %s", __func__, pcm_get_error(hfpmod.hfp_sco_rx));
        ret = -EIO;
        goto exit;
    }

    hfpmod.hfp_pcm_rx = pcm_open(adev->snd_card,
                                 pcm_dev_rx_id,
                                 PCM_OUT, &pcm_config_hfp);
    if (hfpmod.hfp_pcm_rx && !pcm_is_ready(hfpmod.hfp_pcm_rx)) {
        ALOGE("%s: %s", __func__, pcm_get_error(hfpmod.hfp_pcm_rx));
        ret = -EIO;
        goto exit;
    }

    hfpmod.hfp_sco_tx = pcm_open(adev->snd_card,
                                  pcm_dev_asm_tx_id,
                                  PCM_IN, &pcm_config_hfp);
    if (hfpmod.hfp_sco_tx && !pcm_is_ready(hfpmod.hfp_sco_tx)) {
        ALOGE("%s: %s", __func__, pcm_get_error(hfpmod.hfp_sco_tx));
        ret = -EIO;
        goto exit;
    }

    hfpmod.hfp_pcm_tx = pcm_open(adev->snd_card,
                                 pcm_dev_tx_id,
                                 PCM_IN, &pcm_config_hfp);
    if (hfpmod.hfp_pcm_tx && !pcm_is_ready(hfpmod.hfp_pcm_tx)) {
        ALOGE("%s: %s", __func__, pcm_get_error(hfpmod.hfp_pcm_tx));
        ret = -EIO;
        goto exit;
    }

    if (pcm_start(hfpmod.hfp_sco_rx) < 0) {
        ALOGE("%s: pcm start for hfp sco rx failed", __func__);
        ret = -EINVAL;
        goto exit;
    }
    if (pcm_start(hfpmod.hfp_sco_tx) < 0) {
        ALOGE("%s: pcm start for hfp sco tx failed", __func__);
        ret = -EINVAL;
        goto exit;
    }

    if (pcm_start(hfpmod.hfp_pcm_rx) < 0) {
        ALOGE("%s: pcm start for hfp pcm rx failed", __func__);
        ret = -EINVAL;
        goto exit;
    }
    if (pcm_start(hfpmod.hfp_pcm_tx) < 0) {
        ALOGE("%s: pcm start for hfp pcm tx failed", __func__);
        ret = -EINVAL;
        goto exit;
    }

    hfpmod.is_hfp_running = true;
    hfp_set_volume(adev, hfpmod.hfp_volume);

    /* Set mic volume by mute status, we don't provide set mic volume in phone app, only
    provide mute and unmute. */
    hfp_set_mic_mute(adev, adev->mic_muted);

    ALOGD("%s: exit: status(%d)", __func__, ret);
    return 0;

exit:
    stop_hfp(adev);
    ALOGE("%s: Problem in HFP start: status(%d)", __func__, ret);
    return ret;

Related logs looks like this:

08-01 09:13:54.462   684  1590 E audio_hw_hfp: start_hfp: Problem in HFP start: status(-22)

Initially it failed at attempt to call pcm_open and bark something like /dev/snd/pcmC0D39p does't exists and it ideed does not exist on device (only 'c' ending one does):

pdx201:/ # ls -la /dev/snd/pcmC0D39*
crw-rw---- 1 system audio 116,  55 2021-03-22 16:40 /dev/snd/pcmC0D39c

I know that p stands for "playback" and c for capture, and I also know that devices it will try to use controlled by either hfp_pcm_dev_id parameter in <config_params> section of audio_platform.xml or by definitions of usecases in pcm_ids section of the same file. So I've tried to assign them to some values that will match existing sound devices, though this didn't work either. This time it failed at pcm_start call with error like this:

03-26 19:23:09.144     0     0 I msm-pcm-loopback soc: qcom,msm-pcm-loopback: msm_pcm_open: Instance = 2, Stream ID = MultiMedia6 (*)
03-26 19:23:09.145     0     0 E         : SM6150 ASM Loopback: ASoC: no backend DAIs enabled for SM6150 ASM Loopback
03-26 19:23:09.145   684   767 E audio_hw_hfp: start_hfp: pcm start for hfp sco rx failed

I can assume that I choose devices for this poorly (I was basically guessing) and if someone knows what values should be - I would very much appreciate your advice.

And one last note... I don't know if all of the above make any sense (i.e. at all related to lack of echo cancellation) because HFP stands for Hands free profile and apparently have something to do with bluetooth. Though I do see that it get mentioned in code path for enabling speaker. If someone knows if this is at all a correct goal to pursue for solving this issue - please tell me.

@AngryGami AngryGami added the bug label Mar 29, 2021
@MarijnS95
Copy link
Contributor

MarijnS95 commented Mar 29, 2021

Fyi HFP is Bluetooth's Hands Free Profile - in this case it is afaik used when the Android device is in the "HF role", ie. when Android runs on cars, so not relevant for phones.

Otherwise it makes little sense for the libhfp module to be changing hardware microphone and speaker volumes, since a phone would be in the AG (Audio Gateway) role when connected to a headset.

@AngryGami
Copy link
Author

Well... this means that this investigation was for nothing. Though problem with lack of echo cancellation is still there.

@MarijnS95
Copy link
Contributor

@AngryGami By the way thanks for using ```console! Note that there are many more file formats, such as ```c and ```xml which can be used to improve the looks and readability of the blocks above.

As for the issue itself echo cancellation has always been a tough problem with Sony using their own "audio devices" inside the HAL, ACDB being a black box and generally not enough time/resources/knowledge to properly dig into these :/

@AngryGami
Copy link
Author

Does this mean that this won't be fixed anytime soon?

@jerpelea
Copy link
Collaborator

@AngryGami the implementation used in the official SW and the OSS implementation are different. We can not estimate when it will be fixed or if it is even possible to fix it for this usecase on this specific device.

@jerpelea
Copy link
Collaborator

jerpelea commented Mar 29, 2021

@AngryGami if you want to help with the investigation you can compare which out device and which microphone are used on stock and with what is used on the OSS implementation if they are the same it can not be fixed otherwise it may be fixable

@AngryGami
Copy link
Author

@jerpelea This is not good news :(
Though can you elaborate why this might not be possible? Hardware is there, stock image works... what might be wrong with AOSP then?

I've tried to compare stock and oss implementations regarding this.
Logs for stock show following when phone is switched to speaker mode:

03-22 15:37:05.020   776 10440 D audio_hw_primary: select_devices: changing use case voicemmode1-call output device from(20: voice-handset, acdb 7) to (22: voice-speaker-stereo, acdb 14)
03-22 15:37:05.020   776 10440 D audio_hw_primary: select_devices: changing use case voicemmode1-call input device from(168: voice-dmic-ef, acdb 41) to (153: voice-speaker-mic, acdb 11)

and OSS do this:

03-22 15:15:00.155   687   767 D audio_hw_primary: select_devices: changing use case voicemmode1-call output device from(20: voice-handset, acdb 7) to (21: voice-speaker, acdb 101)
03-22 15:15:00.155   687   767 D audio_hw_primary: select_devices: changing use case voicemmode1-call input device from(151: voice-dmic-ef, acdb 41) to (154: voice-speaker-dmic-ef, acdb 43)

So here is what mixer_paths.xml for stock contains for mic:

    <path name="dmic1">
        <ctl name="TX_CDC_DMA_TX_3 Channels" value="One" />
        <ctl name="TX_AIF1_CAP Mixer DEC0" value="1" />
        <ctl name="TX DMIC MUX0" value="DMIC0" />
    </path>

    <path name="dmic1-adj-gain">
        <ctl name="TX_DEC1 Volume" value="88" />
    </path>

    <path name="speaker-mic">
        <path name="dmic1" />
        <path name="dmic3-adj-gain" />
    </path>

    <path name="voice-speaker-mic">
        <path name="speaker-mic" />
    </path>

and this is what oss have for mic:

    <path name="speaker-dmic-endfire">
        <ctl name="TX_CDC_DMA_TX_3 Channels" value="Two" />
        <ctl name="TX_AIF1_CAP Mixer DEC0" value="1" />
        <ctl name="TX DMIC MUX0" value="DMIC2" />
        <ctl name="TX_AIF1_CAP Mixer DEC1" value="1" />
        <ctl name="TX DMIC MUX1" value="DMIC3" />
    </path>

    <path name="voice-speaker-dmic-ef">
        <path name="speaker-dmic-endfire" />
    </path>

It is hard for me to make a lot of sense out of that but as far as I can understand - OSS version tries to use two microphones while stock only uses one. Though DMIC2 and DMIC3 are both "bottom" microphones while DMIC0 is "top" one (I've played with values a bit and figured this out). I also tried to match OSS configuration with what stock does and this didn't help either. Configuration that worked best for me (in terms of microphone gain and sound volume) is following:

    <path name="dmic1">
        <ctl name="TX_CDC_DMA_TX_3 Channels" value="One" />
        <ctl name="TX_AIF1_CAP Mixer DEC0" value="1" />
        <ctl name="TX DMIC MUX0" value="DMIC0" />
    </path>

    <path name="speaker-mic">
        <path name="dmic1" />
    </path>

    <path name="voice-speaker-dmic-ef">
        <path name="speaker-mic" />
        <ctl name="TX_DEC1 Volume" value="88" />
        <ctl name="TX_DEC0 Volume" value="120" />
    </path>
...
    <path name="handset-as-speaker">
        <ctl name="RX_MACRO RX0 MUX" value="AIF1_PB" />
        <ctl name="RX_CDC_DMA_RX_0 Channels" value="One" />
        <ctl name="RX INT0_1 MIX1 INP0" value="RX0" />
        <ctl name="RX INT0 DEM MUX" value="CLSH_DSM_OUT" />
        <ctl name="EAR_RDAC Switch" value="1" />
        <ctl name="RDAC3_MUX" value="RX1" />
        <ctl name="RX_RX0 Digital Volume" value="112" />
    </path>

    <path name="speaker">
        <ctl name="PCM Source" value="DSP" />
        <path name="handset-as-speaker" />
    </path>

    <path name="speaker-mono">
        <path name="speaker" />
    </path>

    <path name="voice-speaker">
        <path name="speaker-mono" />
        <ctl name="ASP TX1 Source" value="VMON" />
    </path>

So I've made phone use top microphone and bottom speaker in loudspeaker mode. This kinda mitigated lack of echo cancellation a bit (because speaker and microphone are further apart) but I'm not sure that was a step in proper direction.
I also noticed two strange things in the logs:

  1. Microphone acdb_id cannot be found or matched with speaker id in ACDB:
03-24 16:00:57.176   664   664 D audio_hw_primary: select_devices: changing use case voicemmode1-call input device from(151: voice-dmic-ef, acdb 41) to (154: voice-speaker-dmic-ef, acdb 41)
...
03-24 16:00:57.377   664   664 D audio_hw_primary: enable_snd_device: snd_device(154: voice-speaker-dmic-ef)
03-24 16:00:57.377   664   664 I soundtrigger: audio_extn_sound_trigger_update_device_status: device 0x9a of type 1 for Event 1, with Raise=1
03-24 16:00:57.377   664   664 D audio_route: Apply path: voice-speaker-dmic-ef
03-24 16:00:57.387   664   664 D ACDB-LOADER: ACDB -> send_voice_cal, acdb_rx = 101, acdb_tx = 41, feature_set = 0
03-24 16:00:57.388   664   664 E ACDB-LOADER: ACDB -> Error: invalid device pair!
  1. when device switched to speaker mode during call following message appears:
03-22 15:37:05.128   776 10440 D audio_hw_primary: enable_audio_route: apply mixer and update path: voicemmode1-call voice-speaker-stereo

and mixer path with this name look like this (identical to stock):

    <path name="voicemmode1-call">
        <ctl name="PRI_MI2S_RX_Voice Mixer VoiceMMode1" value="1" />
        <ctl name="VoiceMMode1_Tx Mixer TX_CDC_DMA_TX_3_MMode1" value="1" />
    </path>

    <path name="voicemmode1-call voice-speaker-stereo">
        <ctl name="VOC_EXT_EC MUX" value="PRI_MI2S_TX" />
        <path name="voicemmode1-call" />
    </path>

it doesn't have any references to neither voice-speaker-dmic-ef nor voice-speaker, though maybe it shouldn't.

@AngryGami
Copy link
Author

Speaker config for stock is:

    <path name="speaker">
        <ctl name="PCM Source" value="DSP" />
    </path>

    <path name="voice-speaker-stereo">
        <path name="speaker" />
        <ctl name="ASP TX1 Source" value="VMON" />
    </path>

and for AOSP it is:

    <path name="handset-as-speaker">
        <ctl name="RX_MACRO RX0 MUX" value="AIF1_PB" />
        <ctl name="RX_CDC_DMA_RX_0 Channels" value="One" />
        <ctl name="RX INT0_1 MIX1 INP0" value="RX0" />
        <ctl name="RX INT0 DEM MUX" value="CLSH_DSM_OUT" />
        <ctl name="EAR_RDAC Switch" value="1" />
        <ctl name="RDAC3_MUX" value="RX1" />
        <ctl name="RX_RX0 Digital Volume" value="92" />
    </path>

    <path name="speaker">
        <ctl name="PCM Source" value="DSP" />
        <path name="handset-as-speaker" />
    </path>

    <path name="speaker-mono">
        <path name="speaker" />
    </path>

    <path name="voice-speaker">
        <path name="speaker-mono" />
        <ctl name="ASP TX1 Source" value="VMON" />
    </path>

Which again, doesn't make a lot of sense to me. Though without this "handset-as-speaker" part (which apparently the only major difference between these two) - sound in general (i.e. ringing sound and music) disappears.

@AngryGami
Copy link
Author

changing use case voicemmode1-call output device from(20: voice-handset, acdb 7) to (22: voice-speaker-stereo, acdb 14)
changing use case voicemmode1-call input device from(168: voice-dmic-ef, acdb 41) to (153: voice-speaker-mic, acdb 11)
changing use case voicemmode1-call output device from(20: voice-handset, acdb 7) to (21: voice-speaker, acdb 101)
changing use case voicemmode1-call input device from(151: voice-dmic-ef, acdb 41) to (154: voice-speaker-dmic-ef, acdb 43)

Note that acdb ids for speaker and microphones are different between OSS and stock - is this fine? Especially considering ACDB-LOADER: ACDB -> Error: invalid device pair! error.

@jerpelea
Copy link
Collaborator

@AngryGami please set the same device ID on AOSP as on stock and check if the issue is solved

@AngryGami
Copy link
Author

How?

@jerpelea
Copy link
Collaborator

@MarijnS95 do you have time to explain ?

@AngryGami
Copy link
Author

I see that in platform.c

// device names table
    [SND_DEVICE_IN_VOICE_SPEAKER_DMIC] = "voice-speaker-dmic-ef",
    [SND_DEVICE_OUT_VOICE_SPEAKER] = "voice-speaker",
...
// acdb_ids table
    [SND_DEVICE_IN_VOICE_SPEAKER_DMIC] = 43,
    [SND_DEVICE_OUT_VOICE_SPEAKER] = 14,

i.e. SND_DEVICE_OUT_VOICE_SPEAKER do match stock, while SND_DEVICE_IN_VOICE_SPEAKER_DMIC doesn't though at same time log from OSS image says that voice-speaker, acdb 101 which doesn't match source code.
There is no override values for neither of these devices in /aosp2/aosp10-build/device/sony/seine/rootdir/vendor/etc/audio_platform_info.xml, so somehow something does replace SND_DEVICE_OUT_VOICE_SPEAKER with 101.

@AngryGami
Copy link
Author

It seems that SND_DEVICE_OUT_VOICE_SPEAKER acdb id is overriden because of this:

./device/sony/common/common-prop.mk:198:    vendor.audio.feature.spkr_prot.enable=true \

So I've disabled it :) and it made problem much much much worse :)
Sound from speaker become very distorted (as if it volume is way to high). Echo become even more clear.
But ACDB-LOADER: ACDB -> Error: invalid device pair! disappeared.

@ix5
Copy link
Contributor

ix5 commented Mar 29, 2021

Wow, that's a lot of unrelated to noise to wade through...

@AngryGami how about this: https://github.com/search?q=user%3Asonyxperiadev+acdb&type=commits

In case you want to override the acdb id from qcom's defaults (stemming from platform.c, as you correctly deduced), edit your device's/platform's audio_platform_info.xml.

But the issue seems to be with the selected usecase. I hope you can find out for yourself how to change the usecase instead of bugging poor Marijn who is overworked already.
Follow the chain - the resulting usecase you see in logs is almost never the one requested, e.g. voice-handset-dmic-ec or sth might actually stem from voice or sth.


Related: #683

@AngryGami
Copy link
Author

AngryGami commented Mar 29, 2021

Sorry for making you "wade through" my "unrelated noise".
I, as you might have noticed, have almost no idea what I'm doing and trying to learn from the code. Not a surprise that I make a lot of mistakes and therefore generate a lot of that noise. Thought this apparently does work since you guys are talking back at least, not usual silence.
As for your adviсe - again... it might make sense to you, but you must understand how alien it is for me.
E.g. what this means:

But the issue seems to be with the selected usecase.

or this:

Follow the chain - the resulting usecase you see in logs is almost never the one requested, e.g. voice-handset-dmic-ec or sth might actually stem from voice or sth.

I have very poor understanding of what this system is for and what tasks it performs and why. What chain you are talking about? Chain in the code somewhere? Or chain of nested <path elements in mixer_paths.xml file?

I know how to override acdb ids in audio_platform_info.xml - though, as I have wrote above this doesn't "just" work. I see from the code and from my experiments that if I, e.g. change SND_DEVICE_OUT_VOICE_SPEAKER in that file - log will still say it took 101 value because of functions like platform_get_spkr_prot_acdb_id which essentially override whatever choice I've made.

@jerpelea
Copy link
Collaborator

@ix5 please relax and lets try to help people learn
AngryGami does not have as much experience as we have and is trying to learn

@ix5
Copy link
Contributor

ix5 commented Mar 30, 2021

@AngryGami
Copy link
Author

AngryGami commented Mar 30, 2021

@ix5 Yep, that what I was doing entire day today (trying different variants). And yes, it looks like this helped with echo cancellation, though I had to tinker with microphone gain and speaker volume because sound is very distorted and there is a noticeable feedback (though this might be because of how I test stuff).

@ix5
Copy link
Contributor

ix5 commented Mar 30, 2021

That's good to know. If your changes indeed restore phone-speaker functionality during calls, you should make a pull request ASAP.
Is this only Android-10 related? Or could we benefit from this on Android 11 as well?

@AngryGami
Copy link
Author

@ix5 I don't know if this is a problem for Android 11. Maybe acdb files for Android 11 are different and values that are currently in audio_platform_info.xml are ok.

@ix5
Copy link
Contributor

ix5 commented Mar 30, 2021

In any case, I cannot reproduce any echo issues on calls on Android 11 on my pdx201. Please submit the updated acdb id(s) for Android 10.

It would be great of you to upload a build of Android 10 somewhere (with verity disabled) so that I can reproduce your issue.
Set this in your BoardConfig.mk:

BOARD_AVB_MAKE_VBMETA_IMAGE_ARGS += --flag 2

@AngryGami
Copy link
Author

@ix5

Please submit the updated acdb id(s) for Android 10.

I will, as soon I done with all the tests I want to do. So far it looks very promising, though feedback is bothering me a bit still.

It would be great of you to upload a build of Android 10 somewhere with verity disabled) so that I can reproduce your issue.

I'm not sure which issue you are talking about? Lack of echo cancellation? This seems to be fixed by changing acdb ids in audio_platform_info.xml. As for feedback and distorted sound I think this is artifact of my attempts to fix the problem with mixer_paths.xml more than anything.

@ix5
Copy link
Contributor

ix5 commented Mar 31, 2021

I'm not sure which issue you are talking about? Lack of echo cancellation? This seems to be fixed by changing acdb ids in audio_platform_info.xml. As for feedback and distorted sound I think this is artifact of my attempts to fix the problem with mixer_paths.xml more than anything.

I'd still like to take a look at an Android 10 build. Just make otapackage with -userdebug and upload the zip to somewhere.

@AngryGami
Copy link
Author

@ix5 you keep throwing things at me that I either don't understand or don't know how to do. Image that I have now doesn't have this issue anymore and the only thing that I've changed was these acdb ids. I don't have any shared hosting to put several GB of these files and my image contain some custom signatures and applications that I can't distribute. You are asking me to change my build config to exclude everything that I've added/changed, make a build with additional flags that I don't understand and don't know where to put only because you can't reproduce echo cancellation problem? This is kinda over top of my head currently.
I don't know why you can't reproduce this because I haven't made any changes to audio subsystem and used whatever "vanilla" pdx201 and seine configurations do. Basically I have slightly changed set of pdx201/aosp_hpxqau51.mk pdx201/aosp_hpxqau52.mk and pdx201/device.mk files that reuse literally everything else from sony repositories. So my build is 99.9% identical to what you should get from standard configuration from sony.

@ix5
Copy link
Contributor

ix5 commented Mar 31, 2021

I am asking you to provide a ready-made build so that I don't have to set up my own A10 build environment.
An ota package should clock in around 400-500MB, not multiple GB.

Simply overriding the acdb ids might "solve" your issue, but the root cause is still interesting to us. And I have a looming suspicion this won't be the last "bug" you open issues for. If you don't want to go to that (comparatively very minimal imo) effort, I don't know what to tell you.

@AngryGami
Copy link
Author

Ok, here is ota zip.

@AngryGami
Copy link
Author

So I can conclude that after these changes in audio_platform_info.xml for acdb ids echo cancellation for loud speaker finally works. Thanks everybody.

@jerpelea
Copy link
Collaborator

@AngryGami please push the needed changes

@AngryGami
Copy link
Author

@jerpelea What is wrong with pull request above?

@jerpelea
Copy link
Collaborator

cherry-picked

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

No branches or pull requests

4 participants