thesofproject / sof

Sound Open Firmware
Other
541 stars 312 forks source link

[FEATURE] Define filter blobs for processing components at topology top level #2535

Closed singalsu closed 4 years ago

singalsu commented 4 years ago

Is your feature request related to a problem? Please describe. The amount of processing variants in tools/topology/sof/pipe-x-y.m4 are growing and the macro names can be confusing. We have pipelines with 40 Hz high-pass some with +20 dB gain and some with 0 dB gain and it can't be seen from the macro names. Also some pipelines use coefficients for 16 kHz rate and some 48 kHz rate.

The pipeline macro names could be then just the processing components names in the order the are in pipeline in stream direction.

Describe the solution you'd like It would be better to define the needed filter blobs at topology top level to keep the pipeline macros more generic. It would also help to avoid side effects/conflicts with not unique controls identifiers those can conflict with other pipelines those might want to use a different filter blob.

Describe alternatives you've considered I tested this in an experimental PR with modified pipeline macro. The last e.g. three arguments for PIPELINE_PCM_ADD could pass blobs to processing components:

# Low Latency capture pipeline 2 on PCM 0 using max 2 channels of s24le.
# 1000us deadline on core 0 with priority 0
PIPELINE_PCM_ADD(sof/pipe-eq-iir-volume-capture.m4,
    2, 0, 2, s24le,
    1000, 0, 0,
    48000, 48000, 48000,,, eq_iir_coef_highpass_40hz_0db_48khz.m4)

This would not change functionality in the topologies but tidy the topology directories and avoid human mistakes when using the macros.

Additional context Add any other context or screenshots about the feature request here.

singalsu commented 4 years ago

@lgirdwood What do you think of this proposal? I'm not good with m4 so I don't know if this is the best approach but at least it seemed to work when I tried.

lgirdwood commented 4 years ago

@singalsu looks sound.

fredoh9 commented 4 years ago

This can be closed. Remaining enhancements are described in https://github.com/thesofproject/sof/issues/2707

singalsu commented 4 years ago

@fredoh9 Yup, I'm also now using this work in testbench to test EQs with not-flat response that improves a lot tests coverage.

Currently PIPELINE_FILTER1 is hard coded to IIR and PIPELINE_FILTER2 is hard coded to FIR. It works for now but eventually the middle layer should pass the configuration to any type first processing component in pipeline with PIPELINE_FILTER1 and use "...2" or "...3" when there's more processing components in the pipeline those use the configuration blob. I've in set in the beamformer PR the pipeline macros to get the blob reference in PIPELINE_FILTER1 that currently would create a conflict if there would be EQs in same topology.