topology2: add google rtc aec support

[RanderWang]

No description provided.

[kv2019i]

In general logic looks good, but we need to work on the naming and definitions. Is this specific to "google-rtc-audio-processing" or are parts of the PR something that apply to allt types of AEC modules? We need to identify which parts are specific to Google RTC, and which are generic definitions that can be used by all AEC implementes (current there are no others in SOF upstream but this is a very common type of DSP audio module).

[plbossart]

AEC is not a generic solution, there's just too many variants.

The other concern is that this is starting to be too much for a nocodec topology, which has become a dumping ground. We are not going to add all possible variants of aec, smart-amp, wov in nocodec.conf. We need to revisit the goals and how tests will be handled.

NAK NAK NAK.

[plbossart]

not ok with the assumption that the AEC is directly connected to a copier. This may or may not be true if noise suppression comes into play. That's why we introduced the 3 input paths (raw, AEC, AEC+NS).

In other words, we need to defer the connections to the very top level and not create artificial restrictions we'll have to undo later.

Adding @cujomalainey and @singalsu for additional comments.

[plbossart]

we should never assume how an element is connected.

Specifically in this case, it's possible that there is a noise-suppression inserted before the host copier, or that there are two input paths, one with a direct host copier and one with the noise suppression.

I think we need to leave the choice of how an algorithm is inserted to the top-level topology. Don't introduce restrictions when we already know they don't always hold.

[RanderWang]

@plbossart In current plan, we have three pipelines for output: (1) raw (2) AEC (3) AEC+NS. This PR focus on pipeline (2) and if we have NS module, we will change one of AEC pipeline to AEC+NS.

Do you prefer to merge (2) and (3) with some flag ?

[plbossart]

@RanderWang not following what you meant by "we will change one of AEC pipeline to AEC+NS".

It's the same AEC in both cases, no? This is a hierarchical set of paths.

[plbossart]

This is my understanding of the ask:

DMIC ---> copier ---> host
             |
             +------> AEC -----> copier -----> host
                                    |
                                    +----------> NS -------> copier -----------> host

[RanderWang]

thanks. My understanding is. The advantage is that it will skip 2 copiers to improve performance.

Copier DMIC -->   EQ -----> copier module ---------> host
                                    |----------------> AEC ------> host
                                    |------> AEC ------> NS---> host

[cujomalainey]

@plbossart we are planning to roll AEC out on spk+dmic, it would be good to leave a HS+HS-mic option for experimentation in the config for us to test with

[plbossart]

Sorry, I don't get your point @cujomalainey. It might be easier to provide Rander with a picture of the desired topology...

[cujomalainey]

Ideally we would have a copy of each of the above images for SPK+DMIC, and a copy of the second for HS + HS Mic on top of the standard volume only passthrough options

[plbossart]

I meant an image of how the noise reduction will be added @cujomalainey. I suggested a hierarchy in #7237 (comment) and @RanderWang suggested two separate instances of AEC in #7237 (comment)

[cujomalainey]

@plbossart ah thanks I missed that, hierarchy is better, less instances.

[singalsu]

@RanderWang Can you share a tplgtool.py created picture of this topology?

[cujomalainey]

Adding @cujomalainey and @singalsu for additional comments.

@plbossart thanks. @thesofproject/google should be CCed on all things related to our AEC and related components. Thanks

[cujomalainey]

I'll reinforce this again as in my other PR. I will not approve commits that relate to google code if I don't have the documentation to approve what I am reviewing. please prioritize these documents

[RanderWang]

I'll reinforce this again as in my other PR. I will not approve commits that relate to google code if I don't have the documentation to approve what I am reviewing. please prioritize these documents

According to the question, a documents of topology2 and ipc4 is needed. @lgirdwood this is a big task

[RanderWang]

@RanderWang Can you share a tplgtool.py created picture of this topology?

@singalsu Do we need gain for each pipeline ? if not we can remove them and one copier module so that we have better performance

[singalsu]

@RanderWang Can you share a tplgtool.py created picture of this topology?

@singalsu Do we need gain for each pipeline ? if not we can remove them and one copier module so that we have better performance

Thanks for the picture. I would place AEC up one layer and have and leave gains there after AEC as placeholder for other processing. Gain may be needed for mic mute feature anyway with some device button & led. But it depends on what the google rtc aec topology requirement is. Can you please comment @cujomalainey @johnylin76 .

[cujomalainey]

I don't think we need gain on capture, I believe it includes an AGC @perahgren to correct me if I am wrong

[RanderWang]

@cujomalainey could you please help to answer #7237 (comment). What is your picture of pipeline ?

[RanderWang]

@plbossart @cujomalainey this diagram is for the pipeline. There is no NS support now, so I temporarily gain for NS. And Copier.SSP.8.1 provides loop back data of speaker playback stream.

DMIC ------->     copier ---> host
                    |
                    +------> AEC -----> copier -----> host
                                           |
                                           +----------> NS -------> copier host

[lgirdwood]

I'll reinforce this again as in my other PR. I will not approve commits that relate to google code if I don't have the documentation to approve what I am reviewing. please prioritize these documents

According to the question, a documents of topology2 and ipc4 is needed. @lgirdwood this is a big task

Ack - @mmaka1 and @mwasko have some sof-docs text and diagrams now in progress to help explain the new features being upstreamed into sof-docs.
@jsarha can you have a look at topology2 and make sure we have the correct docs for the topology features being added here.
@cujomalainey are you able to add a FEATURE issue on sof-docs listing the areas that need more docs with some priority. This will help us prioritize new documentation.

[kv2019i]

Thanks @RanderWang , all my concerns now addressed.

[sys-pt1s]

Can one of the admins verify this patch?

[kv2019i]

@RanderWang Status of this?

[RanderWang]

@RanderWang Status of this?

@kv2019i rebase to latest code. @cujomalainey do you have any comments ? If not, let's merge it.

[marc-hb]

https://sof-ci.01.org/sofpr/PR7237/build7147/devicetest/index.html failed with some aplay errors.

PS: maybe a direct link to the new sof-docs would be useful?

[RanderWang]

https://sof-ci.01.org/sofpr/PR7237/build7147/devicetest/index.html failed with some aplay errors.

PS: maybe a direct link to the new sof-docs would be useful?

This PR only affects sof-mtl-max98357a-rt5682.conf, not the nocodec topology. sof-docs for google-rtc-aec ? Actually this PR follows the topology1 for google-rtc-aec and it is better for google to provide how their aec work.

[marc-hb]

SOFCI TEST

[cujomalainey]

getting close, thanks

[RanderWang]

@cujomalainey updated, thanks