WIP: audio: optimize biquad filter by arch #3270

johnylin76 · 2020-08-04T12:26:19Z

Biquad filter is utilized by both eq_iir and crossover, and the original biquad implementation had the capability of hifi3 optimization according to compiler of architecture. We should move biquad filter code to a shareable place while remaining the optimization capability.

This commit extracts the biquad processing from iir_df2t(). It allows to reuse the biquad processing code independently of the equalizer implementation. Components such as crossover use biquads for processing input. But it could not do so because iir_df2t() was specific to single output audio processing. The motivation behind this change was to reuse the coefficients of the LR4 biquads for the crossover more freely. An LR4 filter is made of two biquads in series with same coefficients. Therefore to save memory, we can store one copy of each set of coefficient, and pass those to the biquad processing function. Signed-off-by: Sebastiano Carlucci <scarlucci@google.com>

This commit adds Crossover to the list of SOF components. A crossover filter can be used to split an input to different frequency bands. The number of outputs should be set statically in the topology. The user then uses the control bytes to route the frequency bands to different outputs. (similar to the demux component). This commit adds support for the following formats: - S16_LE - S24_LE - S32_LE Signed-off-by: Sebastiano Carlucci <scarlucci@google.com>

This commit adds the topology files for the crossover component. The control bytes are generated by the tools in tune/crossover. Signed-off-by: Sebastiano Carlucci <scarlucci@google.com>

This commit adds the tools to generate the control bytes for the crossover component. To generate the control bytes, run the example_crossover.m script. The parameters of the crossover components are: - number of outputs - sink assignments (routing crossover output to different pipelines) - frequency cutoffs To tweak the parameters modify the values in example_crossover.m and run it. Refer to sof/src/include/user/crossover.h for more information on how the crossover config is structured and how sink assignments are done. Signed-off-by: Sebastiano Carlucci <scarlucci@google.com>

Add Crossover component for tplg_parser. Signed-off-by: Pin-chih Lin <johnylin@google.com>

johnylin76 · 2020-08-05T09:54:35Z

@cujomalainey I still have some design questions while I looked into the code after the discussion. I could only fulfill as commit: 19cbe7a under my knowledge which is not the architecture what we discussed.

Are we able to pass the function as pointer from a library (libsof_eq_iir.so) to another one (libsof_crossover.so), by "extern" flag perhaps? However I think it would then imply libsof_crossover.so depends on libsof_eq_iir.so, are we able to make the link?

Thanks.

src/audio/CMakeLists.txt

cujomalainey · 2020-08-05T18:03:34Z

src/include/ipc/topology.h

I think this might be defunct now that UUID has landed

Aren't types still used for matching component driver as fallback according to the UUID commit 11a4a22 ? Do we really want to skip it for crossover?

Good point, this is generic, lets leave it

@johnylin76 you are right for ABI < 3.17, but this will land for ABI >= 3.17 so it no longer needs this.

tools/tplg_parser/include/tplg_parser/topology.h

cujomalainey · 2020-08-05T18:08:06Z

tools/tune/crossover/crossover_build_blob.m

this might need reworking to fit the new ABI if i remember correctly.

Could you explain more? Thanks

Given the crossover is built up of that struct that we want to remove that Seb created, the format of the crossover is likely to change so this function will likely need updated to match that new blob format. You will likely be able to call the iir blob build for the subsections. This will also make the crossover resiliant to any abi changes in the IIR.

There is a chance I am totally wrong about this, I am just trying to future proof the ABI but this might be the smarter solution

cujomalainey · 2020-08-05T18:09:20Z

No worries

I don't think we need to make source link in CMakeList.txt, we should only need to mark it as a dependency in the KConfig. We may need to convert iir into a library in the build system to make linking easier. We can consult @lgirdwood on the best way to do this in CMake

This is definitely a step in the right direction. We should squash these changes back to undo the original changes. I added some comments on some other stuff that might need to change to fix stuff up.

lgirdwood · 2020-08-10T10:05:39Z

@singalsu can you comment on the FIR/IIR code sharing and review. Thanks !

Moved iir_df2t() from src/audio/eq_iir to src/math as a common library for IIR and Crossover usage. Signed-off-by: Pin-chih Lin <johnylin@google.com>

lgirdwood

@singalsu any comments ?

singalsu

Thanks for re-structuring this, looks OK to me! A clean patch against git master is needed without previous changes to IIR for crossover.

cujomalainey · 2020-08-11T20:54:02Z

src/math/CMakeLists.txt

 # SPDX-License-Identifier: BSD-3-Clause

-add_local_sources(sof numbers.c trig.c decibels.c)
+add_local_sources(sof numbers.c trig.c decibels.c iir_df2t_generic.c iir_df2t_hifi3.c)


ah is this how we are making the iir math a lib and reusing it?

cujomalainey · 2020-08-11T21:00:06Z

src/include/user/crossover.h

+	uint32_t reserved[4];
+
+	uint32_t assign_sink[SOF_CROSSOVER_MAX_STREAMS];
+	struct sof_eq_iir_biquad_df2t coef[];


this change here, can we not use the blob building functions in the respective octave code for the crossover?

johnylin76 · 2020-08-12T07:52:24Z

@singalsu Thanks for the review. I've updated to #3263 for moving iir_df2t into src/math.

lgirdwood · 2020-08-13T09:46:33Z

src/include/sof/audio/eq_iir/iir.h

+ * \param coef coefficients of the biquad Q2.30
+ * \param delay delay of the biquads Q3.61
+ */
+static inline int32_t iir_process_biquad(int32_t in, int32_t *coef,


oh wait a minute - I've just noticed this in a header instead of in a math library ? I think we want to put all the common IIR, FIR etc functions in math library C files.
@johnylin76 can you and @singalsu align on with maths function should belong in the library. We can then do the function move before merging the new component PRs.

@lgirdwood This code is outdated, in #3263 the inline header function is removed and iir_df2t is moved to math library, which could be utilized by both eq_iir and crossover.

oh ok great - can we close this then or will there be new updates after #3263 is merged ?

johnylin76 · 2020-08-14T06:54:09Z

changes are combined to #3263

sebcarlucci and others added 5 commits August 4, 2020 20:15

tools: topology: Add crossover topology files

4cabd42

This commit adds the topology files for the crossover component. The control bytes are generated by the tools in tune/crossover. Signed-off-by: Sebastiano Carlucci <scarlucci@google.com>

tools: tplg_parser: Add Crossover component

ab38ccb

Add Crossover component for tplg_parser. Signed-off-by: Pin-chih Lin <johnylin@google.com>

lgirdwood requested a review from singalsu August 4, 2020 20:56

johnylin76 force-pushed the johnylin_b branch from 4d5d5a0 to 19cbe7a Compare August 5, 2020 09:40

cujomalainey reviewed Aug 5, 2020

View reviewed changes

src/audio/CMakeLists.txt Outdated Show resolved Hide resolved

cujomalainey reviewed Aug 5, 2020

View reviewed changes

tools/tplg_parser/include/tplg_parser/topology.h Outdated Show resolved Hide resolved

cujomalainey reviewed Aug 5, 2020

View reviewed changes

src: math: move iir_df2t function to src/math

471ffa7

Moved iir_df2t() from src/audio/eq_iir to src/math as a common library for IIR and Crossover usage. Signed-off-by: Pin-chih Lin <johnylin@google.com>

johnylin76 force-pushed the johnylin_b branch from 19cbe7a to 471ffa7 Compare August 11, 2020 11:10

lgirdwood approved these changes Aug 11, 2020

View reviewed changes

singalsu approved these changes Aug 11, 2020

View reviewed changes

cujomalainey mentioned this pull request Aug 11, 2020

audio: Add Crossover Filter component #3263

Merged

cujomalainey reviewed Aug 11, 2020

View reviewed changes

lgirdwood requested changes Aug 13, 2020

View reviewed changes

johnylin76 closed this Aug 14, 2020

lgirdwood deleted the johnylin_b branch January 27, 2021 09:25

WIP: audio: optimize biquad filter by arch #3270

WIP: audio: optimize biquad filter by arch #3270

Uh oh!

Conversation

johnylin76 commented Aug 4, 2020

Uh oh!

johnylin76 commented Aug 5, 2020

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cujomalainey commented Aug 5, 2020

Uh oh!

lgirdwood commented Aug 10, 2020

Uh oh!

lgirdwood left a comment

Choose a reason for hiding this comment

Uh oh!

singalsu left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

johnylin76 commented Aug 12, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

johnylin76 commented Aug 14, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants