Skip to content

API for using custom decimator filters#259

Merged
shuchitak merged 8 commits intoxmos:developfrom
shuchitak:custom_decimator
Dec 17, 2025
Merged

API for using custom decimator filters#259
shuchitak merged 8 commits intoxmos:developfrom
shuchitak:custom_decimator

Conversation

@shuchitak
Copy link
Copy Markdown
Contributor

@shuchitak shuchitak commented Dec 1, 2025

Main changes in this PR

  • Added an API (mic_array_init_custom_filter()) that allows initialising the mic array with custom decimation filters
  • Added mic array configuration structure (mic_array_conf_t) that the application needs to populate with memory for filter coeffs, state etc.
  • Simplified decimator and pdmrx class to not use templated parameters, instead work with the config structures (that are passed to the modified Init() API)
  • Added an example (app_custom_filter) to demonstrate custom filter usage
  • Extended BasicMicArray test to also test a custom filter configuration
  • Extended stage1.py, stage2.py and added a new script combined.py to support loading the filter .pkl file and generating a header file (that can be included in the application) with the filter params.
  • Removed prefab from code and documentation
  • Updated documentation to include the custom filter API
  • Added ThreeStageDecimator class to the library
  • Added support to mic_array_init_custom_filter() to work with a 3 stage filter
  • Changed the default 16KHz sample rate filter from small_2_stage_filter_int.pkl to good_2_stage_filter_int.pkl

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds support for custom decimation filters in the microphone array library by introducing a new API (mic_array_init_custom_filter) alongside the existing default API. The changes refactor the decimator and PDM RX components to accept runtime configuration instead of relying solely on compile-time template parameters.

Key Changes:

  • New custom filter API with configuration structs for flexible filter specification
  • Refactored TwoStageDecimator and StandardPdmRxService to use runtime configuration
  • Removed the deprecated prefab API (BasicMicArray)
  • Added new example (app_custom_filter) and updated existing tests
  • Enhanced Python scripts for generating filter header files

Reviewed changes

Copilot reviewed 73 out of 79 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
lib_mic_array/api/mic_array/mic_array_conf_struct.h New configuration structs for custom filter API
lib_mic_array/api/mic_array/mic_array_task.h Added mic_array_init_custom_filter function declaration
lib_mic_array/api/mic_array/cpp/Decimator.hpp Refactored to accept runtime config via Init()
lib_mic_array/api/mic_array/cpp/PdmRx.hpp Removed base class, changed to accept runtime buffers
lib_mic_array/api/mic_array/cpp/Prefab.hpp Removed deprecated prefab API
lib_mic_array/src/mic_array_task.cpp Simplified to use single mic array type with runtime config
lib_mic_array/src/mic_array_task_internal.hpp Refactored internal helpers for new API
python/*.py Enhanced filter generation scripts with header file output
examples/app_custom_filter/* New example demonstrating custom filter usage
examples/app_prefab/* Removed deprecated prefab example
tests/**/*.cpp Updated tests to use new API patterns
tests/**/*.py Updated test scripts for custom filter support
doc/**/*.rst Updated documentation for custom filter API

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

s2_filter = filters.Stage2Filter(s2_coef, s2_df)

if self.debug_print_filters:
self.print_two_stage_filter(s1_filter, s2_filter)
Copy link

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

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

Call to method MicArraySharedBase.print_two_stage_filter with too many arguments; should be no more than 1.

Copilot uses AI. Check for mistakes.
s2_filter = filters.Stage2Filter(s2_coef, s2_df)

if self.debug_print_filters:
def print_two_stage_filter(s1_filter, s2_filter):
Copy link

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

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

Normal methods should have 'self', rather than 's1_filter', as their first parameter.

Copilot uses AI. Check for mistakes.
Comment thread python/combined.py Outdated
import stage1
import stage2
from pathlib import Path
from header_utils import *
Copy link

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

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

Import pollutes the enclosing namespace, as the imported module header_utils does not define 'all'.

Copilot uses AI. Check for mistakes.
Comment thread python/stage1.py Outdated
from pathlib import Path

import mic_array.filters as filters
from header_utils import *
Copy link

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

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

Import pollutes the enclosing namespace, as the imported module header_utils does not define 'all'.

Copilot uses AI. Check for mistakes.
Comment thread python/stage2.py Outdated
from pathlib import Path

import mic_array.filters as filters
from header_utils import *
Copy link

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

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

Import pollutes the enclosing namespace, as the imported module header_utils does not define 'all'.

Copilot uses AI. Check for mistakes.
Comment thread python/combined.py Outdated
# Copyright 2025 XMOS LIMITED.
# This Software is subject to the terms of the XMOS Public Licence: Version 1.

import argparse
Copy link

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

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

Import of 'argparse' is not used.

Copilot uses AI. Check for mistakes.
Shuchita Khare added 4 commits December 5, 2025 12:08
…onf_t and pdm_rx_conf_t)

- Removed S2_DEC_FACTOR and S2_TAP_COUNT template parameters from TwoStageDecimator
- Modified TwoStageDecimator::Init() API to accept mic_array_decimator_conf_t &decimator_conf
- Removed SUBBLOCKS template parameter from StandardPdmRxService
- Modified StandardPdmRxService::Init() API to accept pdm_rx_conf_t &pdm_rx_config
- Removed PdmRxService class and the associated CRTP stuff
- Modified default API implementation to use the simplified decimator and pdmrx classes
- Get tests and examples building
- Update MIPS and memory numbers
- Remove prefab tests and examples
- lib checks + fix doc build errors
- Added support in stage1.py, stage2.py and combined.py to output the filters in a .h file
- Added support in BasicMicArray test app CMakeLists.txt to autogenerate the custom filter header
file from the pkl file
- Remove num_mics from decimator and pdmrx conf (not needed for the custom filter feature
- Modified mic_array_decimator_conf_t to have a mic_array_filter_conf_t pointer and num_filter_stages, to
avoid having a MAX_FILTER_STAGES type limitation.
- Renamed some fields in the decimator and pdmrx conf to be more descriptive
- Renamed app_mic_array_custom_filter to app_custom_filter
- Renamed state_size to state_words_per_channel
- Renamed Init_new() back to Init()
@shuchitak shuchitak changed the title Custom decimator API for using custom decimator filters Dec 5, 2025
@shuchitak shuchitak requested a review from uvvpavel December 5, 2025 12:27
@shuchitak shuchitak force-pushed the custom_decimator branch 2 times, most recently from 8cca269 to 6120953 Compare December 5, 2025 15:33
- Removed READMEs from tests/
- Removed references to crtp from documentation
- Added documentation to stage1.py, stage2.py and combined.py
- Changelog update
- Address copilot review comments
- modified stage1.py and stage2.py to work for 3 stage filters
- Extended BasicMicArray test to test a <custom_filter>.pkl file that's specified in test_params.json
- Added support for specifying custom filter .pkl file in app_mips CMakeLists. User required to run the _customfs config manually to check the MIPS impact
- Refactor mic_array_task.cpp to remove duplicated code
- Renamed Decimator_3_stage.hpp to ThreeStageDecimator.hpp
- Added tests/signal/BasicMicArray/good_3_stage_filter_int.pkl
…r_int.pkl to good_2_stage_filter_int.pkl

- Update mips and memory profile numbers (~1.2 mips/channel increase in mips and extra ~1536 bytes of memory)
  This has increased MIPS and memory overhead but better THDN performance
Copy link
Copy Markdown

@uvvpavel uvvpavel left a comment

Choose a reason for hiding this comment

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

Great work! Some nitpicks, but all good really.

Side note:
Not related to your PR, but the library API has an inconsistent namespases. Some APIs start with mic_array_ while others with ma_. I think this would need to be resolved in the future.
(I'd prefer ma_, if anyone would ask me ;))

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This is so much better!

* Reads stage-1 and stage-2 filter parameters from @p decimator_conf and prepares
* internal state:
* The caller must ensure all pointers inside @p decimator_conf.filter_conf[0]
* and @p decimator_conf.filter_conf[0] are valid and remain alive for the
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Don't really like alive in docs, not sure how to change it tho...

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

fixed

* subsequent application stages.
* @endparblock
*
* @tparam MIC_COUNT Number of microphone output channels from the the mic array component
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Indent please :)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done

*
* `SubType::GetPdmBlock()` responsible for receiving a block of PDM data from
* `SubType::SendBlock()` as a pointer, deinterleaving the buffer contents,
* `GetPdmBlock()` responsible for receiving a block of PDM data from
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Nitpick: missing is

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done

Comment on lines +531 to +532
this->phase = CHANNELS_IN * this->pdm_out_words_per_channel;
this->num_phases = CHANNELS_IN * this->pdm_out_words_per_channel;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

could do something like:

this->num_phases = CHANNEL_IN * this->pdm_out_words_per_channel;
// starting at the last phase
this->phase = this->num_phase;

Comment thread python/README.rst Outdated
Comment on lines +157 to +163
The simplest way to use these coefficients in an application is to run something like:

.. code::

lib_mic_array\python>python combined.py filter_design\good_2_stage_filter_int.pkl --file-prefix custom_filter

and include the generated ``custom_filter.h`` file in the application.
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I'd say something like "run this from the repo root"

Then include cd lib_mic_array/python

And the finishing part should be its own sentence, like:
Then include the generated ..... in the application

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done

Comment thread python/header_utils.py
Comment on lines +30 to +31
print(f"#ifndef {args.file_prefix.upper()}_H", file=out)
print(f"#define {args.file_prefix.upper()}_H", file=out)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

#pragma once?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

not C/C++ standard :)

Comment thread python/header_utils.py
Comment on lines +32 to +33
print(f"\n/* Autogenerated by running 'python combined.py {args.coef_pkl_file} -fp {args.file_prefix}'. Do not edit */", file=out)
print(f"\n#include <stdint.h>", file=out)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

didnt think about it. thanks!

Comment on lines +92 to +102
/**
* @brief PDM RX output block (input to the decimator) for all microphones.
* @details
* Packed PDM samples as 32-bit words. The layout is a contiguous buffer
* sized `output_mic_count * pdm_out_words_per_channel` 32-bit words.
* Typically contains enough PDM words to produce one PCM sample per microphone
* after decimation.
*
* This buffer must be aligned to a 32-bit word boundary.
*/
uint32_t *pdm_out_block;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Do we want to include details on how to put the PDM data in? LSB or MSB first?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

good point. Its mentioned elsewhere in the documentation but I'll added it here as well.

Comment thread doc/rst/src/resource_usage.rst Outdated
.. note::

The default API requires approximately 6 KiB more memory than the custom configuration.
The default API requires approximately 4 KiB more memory than the custom configuration.
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

In the getting started guide this number is 3. Is this correct?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ah. I think I'll remove quoting an exact number. It keeps changing as the code changes

@shuchitak shuchitak merged commit f39e752 into xmos:develop Dec 17, 2025
2 checks passed
@shuchitak shuchitak deleted the custom_decimator branch December 17, 2025 15:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants