Add audiodelays & audioeffects to unix coverage port #9793
Conversation
.. and add a very basic audioeffects test, showing that it plausibly is working I had to address several build errors that occurred in the Unix build, mostly related to conversion from FP types to integral types (replaced by explicit casts) and by accidental mixing of regular & f-suffixed floating constants (replaced with the MICROPY_FLOAT_CONST macro) Particularly this change could use consideration: ```diff - self->max_echo_buffer_len = self->sample_rate / 1000.0f * max_delay_ms * (self->channel_count * sizeof(uint16_t)); // bytes + self->max_echo_buffer_len = (uint32_t)(self->sample_rate / 1000.0f * max_delay_ms) * (self->channel_count * sizeof(uint16_t)); // bytes ``` The buffer length is being calculated in floating point based on the millisecond delay & the sample rate. The result could then be a fractional number such as 529.2 for a 12ms delay at 44.1kHz. Multiplying a floating number by the size required for each echo buffer item (`(self->channel_count * sizeof(uint16_t))`) could yield a number of bytes that doesn't correspond to an integral number of buffer items. I grouped the float->int conversion so that it converts the number of echo buffer items to an integer and then multiplies by the size of the item.
|
I think your solution to fix the implicit conversion will do the trick. There will be some imperceptible deviation from the actual ms value, but I think that's inevitable. Thanks for scanning through both modules and making those updates throughout. I'll try the filter tests out when I get the chance. I see you haven't created a test for the |
|
I only created one test as an example. I'd hope that after this PR is merged you'd consider creating more tests; you're likely to do better at identifying what's important to test than me, given your familiarity with the code. |
|
@jepler I tried testing this but I'm not sure that I've figured out the correct way to run it.
I briefly tried running the new test on it's own with But that raises an ImportError not being able to find audiofilterhelper, which I see is commited along with this PR in testlib folder. I'm not sure how to get the micropython binary to find it there when run like this. As a workaround I tried copying that file into |
There was a problem hiding this comment.
I figured out a way to run it individually by copying audiofilterhelper.py into the folder with the py file being executed. I'm sure that is not the intended way.
I was able to successfully run the test inside of the unix port and confirm its output against the .exp file. Didn't test anything on microcontroller.
I'm assuming that my issues running make test to do the full test suite are either me misunderstanding how to use it, or not having proper environment fully set up.
Looks good to me.
|
You can run a single test with |
.. and add a very basic audioeffects test, showing that it plausibly is working
I had to address several build errors that occurred in the Unix build, mostly related to conversion from FP types to integral types (replaced by explicit casts) and by
accidental mixing of regular & f-suffixed floating constants (replaced with the MICROPY_FLOAT_CONST macro)
Particularly this change could use consideration:
The buffer length is being calculated in floating point based on the millisecond delay & the sample rate. The result could then be a fractional number such as 529.2 for a 12ms delay at 44.1kHz. Multiplying a floating number by the size required for each echo buffer item (
(self->channel_count * sizeof(uint16_t))) could yield a number of bytes that doesn't correspond to an integral number of buffer items. I grouped the float->int conversion so that it converts the number of echo buffer items to an integer and then multiplies by the size of the item.ping @relic-se -- I've found it helpful to have host-based tests for synthio. this enables the same thing for filters and echo. Also, see above, maybe I found a bug?