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.

Sign up for GitHub

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

Jump to bottom

Tools: Topology2: Add Multiband-DRC to sof-hda-efx-generic.tplg #8049

Merged
merged 1 commit into from
Sep 11, 2023
Merged

Tools: Topology2: Add Multiband-DRC to sof-hda-efx-generic.tplg #8049

merged 1 commit into from
Sep 11, 2023

Conversation

singalsu
Copy link
Collaborator

@singalsu singalsu commented Aug 16, 2023 •
edited
Loading 

This patch adds the topology2 files for multiband-DRC. The EFX
processing chain is gain - IIR - FIR - DRC or Multiband-DRC. The
DRC selection is done with macro EFX_DRC_COMPONENT and values
"singleband" and "multiband".

The multiband-drc has two ALSA controls, bytes and switch. The
switch control switches the processing on and off.

The generated topology files with multiband-drc are:

sof-hda-efx-mbdrc-generic.tplg
sof-hda-efx-mbdrc-generic-2ch.tplg
sof-hda-efx-mbdrc-generic-4cg.tplg

The topology files with DRC are as before:

sof-hda-efx-mbdrc-generic.tplg
sof-hda-efx-mbdrc-generic-2ch.tplg
sof-hda-efx-mbdrc-generic-4cg.tplg

The configuration blob default is updated to what is generated
by the current example_multiband_drc.m configuration script. The
pass-through blob is set as default for tplg2. The topology 1
blobs are updated similarly.

Picture of sof-hda-efx-mbdrc-generic.tplg with multi-band DRC:
sof-hda-efx-mbdrc-generic

PIcture of sof-hda-efx-mbdrc-generic.tplg with DRC:
sof-hda-efx-generic

singalsu
singalsu commented Aug 16, 2023
View reviewed changes
tools/topology/topology2/cavs-mixin-mixout-efx-hda.conf Outdated
@@ -55,7 +55,12 @@ Object.Pipeline {
}
Object.Widget.drc.1 {
Object.Control.bytes."1" {
name '2 Main playback DRC'
name 'Post Mixer $ANALOG_PLAYBACK_PCM DRC'
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This small fix could be separate commit.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep.

tools/topology/topology2/development/tplg-targets.cmake Outdated Show resolved Hide resolved
@singalsu singalsu force-pushed the tplg2_add_multiband_drc branch from f7b160c to 8982a62 Compare August 22, 2023 13:58
lgirdwood
lgirdwood reviewed Aug 22, 2023
View reviewed changes
Copy link
Member

@lgirdwood lgirdwood left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR may need split

tools/topology/topology1/m4/multiband_drc_coef_passthrough.m4 Outdated Show resolved Hide resolved
@singalsu singalsu force-pushed the tplg2_add_multiband_drc branch from 8982a62 to 70fba1f Compare August 23, 2023 08:28
@singalsu singalsu mentioned this pull request Aug 23, 2023
Merged
lgirdwood
lgirdwood reviewed Aug 23, 2023
View reviewed changes
Copy link
Member

@lgirdwood lgirdwood left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you have slef reviewed and identified the changes needed.

tools/topology/topology2/cavs-mixin-mixout-efx-hda.conf Outdated
@@ -55,7 +55,12 @@ Object.Pipeline {
}
Object.Widget.drc.1 {
Object.Control.bytes."1" {
name '2 Main playback DRC'
name 'Post Mixer $ANALOG_PLAYBACK_PCM DRC'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep.

@singalsu singalsu force-pushed the tplg2_add_multiband_drc branch from 70fba1f to d1a7471 Compare August 25, 2023 11:17
@singalsu singalsu marked this pull request as ready for review August 25, 2023 11:20
@singalsu singalsu requested a review from a team August 25, 2023 11:21
lgirdwood
lgirdwood reviewed Aug 25, 2023
View reviewed changes
tools/topology/topology2/include/components/multiband_drc/passthrough.conf Outdated
@@ -0,0 +1,71 @@
# Exported Control Bytes 16-Aug-2023
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We really need to have a comment explaining how these are generated.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, I'll do a PR to change the text in the tool.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now fixed in this PR, re-exported the blobs. Tool PR is #8142.

@singalsu
Tools: Topology2: Add Multiband-DRC
8e7a015 
This patch adds the topology2 files for multiband-DRC. The EFX
processing chain is gain - IIR - FIR - DRC or Multiband-DRC. The
DRC selection is done with macro EFX_DRC_COMPONENT and values
"singleband" and "multiband".

The multiband-drc has two ALSA controls, bytes and switch. The
switch control switches the processing on and off.

The generated topology files with multiband-drc are:

sof-hda-efx-mbdrc-generic.tplg
sof-hda-efx-mbdrc-generic-2ch.tplg
sof-hda-efx-mbdrc-generic-4cg.tplg

The topology files with DRC are as before:

sof-hda-efx-mbdrc-generic.tplg
sof-hda-efx-mbdrc-generic-2ch.tplg
sof-hda-efx-mbdrc-generic-4cg.tplg

The configuration blob default is updated to what is generated
by the current example_multiband_drc.m configuration script. The
pass-through blob is set as default for tplg2. The topology 1
blobs are updated similarly.

Signed-off-by: Seppo Ingalsuo <[email protected]>
@singalsu singalsu force-pushed the tplg2_add_multiband_drc branch from d1a7471 to 8e7a015 Compare September 1, 2023 13:40
kv2019i
kv2019i approved these changes Sep 5, 2023
View reviewed changes
Copy link
Collaborator

@kv2019i kv2019i left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ranj063 @jsarha for tplg conventions, ok?

johnylin76
johnylin76 approved these changes Sep 6, 2023
View reviewed changes
Copy link
Contributor

@johnylin76 johnylin76 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks.

@johnylin76
Copy link
Contributor

Hi @singalsu I have a question about multi-stream mix-out playback. (not related to this PR, just come into my mind while reviewing).

For GOOGLE_AEC topology, the AEC module has one input for echo reference, which is sourced from a DEMUX on playback pipeline. The expectation for compound topology graph of both multi-stream and AEC may be formed as follows:

PCM0P (pipeline 2) ---> Volume ------|
                                     V
                                   Mixer (pipeline 1) -----> DEMUX ---> SSP0
                                     ^                         |
PCM1P (pipeline 3) ---> EQIIR  ------|                         |
                                             |-----------------|
                                             V
PCM2C (pipeline 5) <--------- NR <--------- AEC <--|
                                                   |
                                                 Mixer (pipeline 4) <--- DMIC1
                                                   |
PCM3C (pipeline 6) <-------------------------------|

Do you think this is technically feasible?
FYI, AEC can be run without active echo reference, the playback and capture path are independent of each other.

@singalsu
Copy link
Collaborator Author

singalsu commented Sep 7, 2023

Do you think this is technically feasible?
FYI, AEC can be run without active echo reference, the playback and capture path are independent of each other.

@johnylin76 The capture direction looks good, except typo with mixer, you must have meant demux there.

In playback direction the IIR location would be typically for all playback just before demux but if you need the other stream un-equalized this would be the way. I don't see why such topology would not work. I once tried 48 -> 16 kHz SRC with a mock-up AEC a reference path that wasn't working due to a scheduler limitation but here the reference path is without processing.

@kv2019i @mengdonglin @plbossart please chime in if this something not committed. Also SOF releases have differences in scheduler & pipelines capability, v2.2 may be more limited.

@lgirdwood lgirdwood merged commit 84809f9 into thesofproject:main Sep 11, 2023
37 of 41 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kv2019i kv2019i kv2019i approved these changes

@johnylin76 johnylin76 johnylin76 approved these changes

@lgirdwood lgirdwood lgirdwood approved these changes

@cujomalainey cujomalainey cujomalainey approved these changes

@ranj063 ranj063 Awaiting requested review from ranj063 ranj063 is a code owner

@jsarha jsarha Awaiting requested review from jsarha jsarha is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@singalsu @johnylin76 @cujomalainey @lgirdwood @kv2019i