Modify the echo reference capture pipeline #8101

ranj063 · 2023-08-25T21:42:42Z

The new topology looks like this (Note that there is no capture PCM for echo ref anymore and SSP0-Codec DAI feeds into the google-rtc-ace module directly)

This PR depends on the kernel PR thesofproject/linux#4550

The echo-ref pipeline ie the DAI capture pipeline involving the speaker codec is already part of the list of pipelines that gets set up and triggered when the DMIC capture starts. Therefore, there's no need for the echo-ref PCM to explicitly start the reference capture. So, remove it and connect the codec DAI to the google-rts-aec module directly. Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

lgirdwood

More removals than additions which is always good.
@RDharageswari @yongzhi1 have you been able to validate with AEC ?
@cujomalainey fyi.

RDharageswari · 2023-08-28T22:48:31Z

Hi Liam,
I am seeing some issues w.r.t third capture..Will continue to test and update

kv2019i · 2023-08-29T15:41:14Z

tools/topology/topology2/platform/intel/speaker-echo-ref.conf

-		source	$ECHO_REF_COPIER_MODULE
-		sink	"host-copier.$ECHO_REF_PCM_ID.capture"
-	}
-]


@ranj063 Not necessarily for this PR, but should we still have templates for both cases, echo ref to an AEC running on the DSP (like this case), and echo ref to user-space. I mean a given product will use only one option, not both, but in the past we've enabled both.

@kv2019i for aec in userspace, we would need the host copier and the PCM but we can't tie it to this topology. So I'd say we should leave it until.theres a need for it.

cujomalainey · 2023-08-29T17:46:49Z

Also not directly related to this PR, but if this is targeted for ChromeOS, can we remove the EQIIR 14.1, we won't use it for now.

tools/topology/topology2/platform/intel/speaker-echo-ref.conf

cujomalainey

~~cached comment~~

plbossart · 2023-08-29T18:19:51Z

Also not directly related to this PR, but if this is targeted for ChromeOS, can we remove the EQIIR 14.1, we won't use it for now.

why? it's there on purpose to remove the DC component. You absolutely want this in Chrome on DMIC pipelines, it's an endpoint effect here, not something that the user should tweak.
Adding @singalsu for more comments.

cujomalainey · 2023-08-29T20:22:32Z

Also not directly related to this PR, but if this is targeted for ChromeOS, can we remove the EQIIR 14.1, we won't use it for now.

why? it's there on purpose to remove the DC component. You absolutely want this in Chrome on DMIC pipelines, it's an endpoint effect here, not something that the user should tweak. Adding @singalsu for more comments.

If that is what it is for we would rather use the dcblock which is a exact duplicate of what we deploy in CRAS.

Also we have 0 users of it on intel platforms in the past 9 programs

plbossart · 2023-08-29T21:50:41Z

Also not directly related to this PR, but if this is targeted for ChromeOS, can we remove the EQIIR 14.1, we won't use it for now.

why? it's there on purpose to remove the DC component. You absolutely want this in Chrome on DMIC pipelines, it's an endpoint effect here, not something that the user should tweak. Adding @singalsu for more comments.

If that is what it is for we would rather use the dcblock which is a exact duplicate of what we deploy in CRAS.

Also we have 0 users of it on intel platforms in the past 9 programs

We really need @singalsu to comment, but I can tell you 100% of DMIC-based non-Chrome platforms use the EQIIR on the DMIC path. IIRC the EQIIR also boosts the gain. It's a big surprise that Chrome topologies skipped it.

lgirdwood · 2023-08-30T12:57:37Z

Hi Liam, I am seeing some issues w.r.t third capture..Will continue to test and update

@RDharageswari ok, pls approve when you are happy with test results.

singalsu · 2023-08-30T13:20:12Z

Also not directly related to this PR, but if this is targeted for ChromeOS, can we remove the EQIIR 14.1, we won't use it for now.

why? it's there on purpose to remove the DC component. You absolutely want this in Chrome on DMIC pipelines, it's an endpoint effect here, not something that the user should tweak. Adding @singalsu for more comments.

If that is what it is for we would rather use the dcblock which is a exact duplicate of what we deploy in CRAS.
Also we have 0 users of it on intel platforms in the past 9 programs

We really need @singalsu to comment, but I can tell you 100% of DMIC-based non-Chrome platforms use the EQIIR on the DMIC path. IIRC the EQIIR also boosts the gain. It's a big surprise that Chrome topologies skipped it.

The generic Linux user space does not have enhancement for the microphone capture as CRAS in ChromeOS has. Without IIR and gain boost the capture would be very silent. We have got complains about it when it was lacking. The high-pass IIR is better than just gain for boosting the signal since suppressing the lowest frequencies gives more headroom for gain. Also without the IIR we see with various quality microphones components in PCs, that the DC levels that the HW DCCOMP in DMIC is not capable to suppress so it's useful to have IIR or dcblock there to ensure capture is without DC offset. We've used for convenience the IIR EQ instead of dcblock + EQ.

I'm adding to tplg2 dcblock and a topology for hda-generic so using it will be possible in IPC4 and tplg2. We could consider it as default for HDA analog capture if the MCPS is lower than IIR (we should be able to optimize). The HDA headset codec also produces a long DC pulse in the capture start.

This nocodec topology is just for testing, it's not a reference, so the audio quality aspect with it is not critical. But we should have a good set of components included in my opinion since this undergoes a lot of testing. The hda-generic topologies are a reference for PCs so there we should have a good generic example.

singalsu · 2023-08-30T14:18:21Z

@ranj063 I recommend to keep the IIR there and proceed. We can soon after switch it to dcblock when the conversion work is tested and merged.

I missed in my previous comment that this wasn't nocodec where we have similar looking capture pipelines. Also this PR is not related to IIR so it would be anyway other PR.

The DMIC capture path should support both 4ch and 2ch audio formats. Also, since the Google AEC module only support 16-bit input format, modify the output of the refeence capture DAI accordingly. Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

yongzhi1 · 2023-09-06T21:27:20Z

Think module-copier.18.1 is redundant, but the patch works fine.

ranj063 · 2023-09-06T22:32:12Z

Think module-copier.18.1 is redundant, but the patch works fine.

@yongzhi1 yes lets fix it in a follow up

RDharageswari · 2023-09-08T00:27:02Z

I am still seeing errors reported by FW. with kernel+topology and firmware for AEC. Working with Ranjani

The blob contains the input/output audio formats but these are already passed during module init based on hw_params. So no need to have the byte control for it. Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

ranj063 · 2023-09-08T15:01:31Z

I am still seeing errors reported by FW. with kernel+topology and firmware for AEC. Working with Ranjani

@RDharageswari can you please check if the PR works now. I have removed the AEC blob for setting audio formats

RDharageswari

works good for me.

kv2019i

@ranj063 Feel free to merge if this is ready and kernel status is ok as well.

lgirdwood · 2023-09-11T14:58:43Z

CI has had network issues. Rerun.

lgirdwood · 2023-09-11T14:58:54Z

SOFCI TEST

lgirdwood · 2023-09-13T13:55:38Z

@wszypelt @lrudyX CI seems to be in pending state, any chance someone could manually restart. Thanks !

wszypelt · 2023-09-14T09:21:05Z

@lgirdwood in Internal Intel CI system, all green :)

singalsu · 2023-09-26T14:10:24Z

tools/topology/topology2/include/pipelines/cavs/google-rtc-aec-capture.conf

                }

-		google-rtc-aec."1" {
-			Object.Control.bytes."1" {


Does the real AEC version without mockup use the bytes control. Was this added just for the mockup? If the real AEC has bytes control shouldn't we have it in topology to be able to exercise it with about right ballpark size random bytes blob?

ranj063 requested a review from jsarha as a code owner August 25, 2023 21:42

ranj063 mentioned this pull request Aug 25, 2023

[BUG] [MTL] AEC functionality depends on Echo reference endpoint to be started #8075

Closed

ranj063 force-pushed the main-082323 branch from b71183e to 4754cbe Compare August 25, 2023 21:45

ranj063 requested review from RDharageswari, plbossart and yongzhi1 August 25, 2023 21:45

ranj063 force-pushed the main-082323 branch from 4754cbe to b148f23 Compare August 25, 2023 21:59

lgirdwood reviewed Aug 28, 2023

View reviewed changes

kv2019i reviewed Aug 29, 2023

View reviewed changes

cujomalainey reviewed Aug 29, 2023

View reviewed changes

tools/topology/topology2/platform/intel/speaker-echo-ref.conf Show resolved Hide resolved

cujomalainey approved these changes Aug 29, 2023

View reviewed changes

ranj063 mentioned this pull request Aug 31, 2023

[BUG] [MTL] Playback clips captured from mock RTC AEC device is double speeded #8022

Closed

yongzhi1 approved these changes Sep 6, 2023

View reviewed changes

topology2: google-rtc-aec: Remove byte control

c7c0e42

The blob contains the input/output audio formats but these are already passed during module init based on hw_params. So no need to have the byte control for it. Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

RDharageswari approved these changes Sep 8, 2023

View reviewed changes

kv2019i approved these changes Sep 11, 2023

View reviewed changes

lgirdwood changed the title ~~Modify the echo reference capture pipeline~~ [DNM pending kernel PR] Modify the echo reference capture pipeline Sep 14, 2023

This was referenced Sep 18, 2023

ASoC: SOF: Intel: sort pipeline based on priority thesofproject/linux#4593

Merged

topology2: set pipeline priority for google aec capture stream #8204

Merged

singalsu reviewed Sep 26, 2023

View reviewed changes

yongzhi1 mentioned this pull request Sep 29, 2023

Audio: Convert Google RTC audio processing to module API #8258

Merged

1 task

ranj063 changed the title ~~[DNM pending kernel PR] Modify the echo reference capture pipeline~~ Modify the echo reference capture pipeline Oct 3, 2023

kv2019i merged commit 9e9120a into thesofproject:main Oct 3, 2023

ranj063 deleted the main-082323 branch October 3, 2023 15:16

Modify the echo reference capture pipeline #8101

Modify the echo reference capture pipeline #8101

Uh oh!

Conversation

ranj063 commented Aug 25, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lgirdwood left a comment

Choose a reason for hiding this comment

Uh oh!

RDharageswari commented Aug 28, 2023

Uh oh!

kv2019i Aug 29, 2023

Choose a reason for hiding this comment

Uh oh!

ranj063 Aug 30, 2023

Choose a reason for hiding this comment

Uh oh!

cujomalainey commented Aug 29, 2023

Uh oh!

Uh oh!

cujomalainey left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

plbossart commented Aug 29, 2023

Uh oh!

cujomalainey commented Aug 29, 2023

Uh oh!

plbossart commented Aug 29, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lgirdwood commented Aug 30, 2023

Uh oh!

singalsu commented Aug 30, 2023

Uh oh!

singalsu commented Aug 30, 2023

Uh oh!

yongzhi1 commented Sep 6, 2023

Uh oh!

ranj063 commented Sep 6, 2023

Uh oh!

RDharageswari commented Sep 8, 2023

Uh oh!

ranj063 commented Sep 8, 2023

Uh oh!

RDharageswari left a comment

Choose a reason for hiding this comment

Uh oh!

kv2019i left a comment

Choose a reason for hiding this comment

Uh oh!

lgirdwood commented Sep 11, 2023

Uh oh!

lgirdwood commented Sep 11, 2023

Uh oh!

lgirdwood commented Sep 13, 2023

Uh oh!

wszypelt commented Sep 14, 2023

Uh oh!

singalsu Sep 26, 2023

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

ranj063 commented Aug 25, 2023 •

edited

Loading

cujomalainey left a comment •

edited

Loading

plbossart commented Aug 29, 2023 •

edited

Loading