adafruit / circuitpython

CircuitPython - a Python implementation for teaching coding with microcontrollers
https://circuitpython.org
Other
4.12k stars 1.22k forks source link

Stopping playback of source or sink connected to `audiofilters.Filter` causes device to become unresponsive #9778

Closed relic-se closed 2 weeks ago

relic-se commented 3 weeks ago

CircuitPython version

Adafruit CircuitPython 9.2.0 on 2024-10-28; Pimoroni Pico Plus 2 with rp2350b

Code/REPL

import board
import audiobusio
import audiocore
import audiodelays
import audiofilters
import time

audio = audiobusio.I2SOut(bit_clock=board.GP0, word_select=board.GP1, data=board.GP2)

wave_file = open("StreetChicken.wav", "rb")
wave = audiocore.WaveFile(wave_file)

effect1 = audiofilters.Filter( # No filter specified, passes audio through
    sample_rate=wave.sample_rate,
    bits_per_sample=wave.bits_per_sample,
    channel_count=wave.channel_count,
)
effect1.play(wave, loop=True)

effect2 = audiodelays.Echo(
    sample_rate=wave.sample_rate,
    bits_per_sample=wave.bits_per_sample,
    channel_count=wave.channel_count,
)
effect2.play(effect1)

audio.play(effect2)

time.sleep(2)
effect1.stop()

Behavior

The echo effect is heard for a moment at around half speed, then output audio suffers terrible distortion and the device requires hard reset. USB device (/dev/ttyACM0) is not recognized.

Description

No response

Additional information

I've tested this in a slightly different scenario where audiodelays.Echo precedes audiofilters.Filter, and this error does not happen if audiodelays.Echo is stopped first. If audiofilters.Filter is stopped, the error described above occurs.

...
effect1 = audiodelays.Echo(
    sample_rate=wave.sample_rate,
    bits_per_sample=wave.bits_per_sample,
    channel_count=wave.channel_count,
)
effect1.play(wave, loop=True)

effect2 = audiofilters.Filter( # No filter specified, passes audio through
    sample_rate=wave.sample_rate,
    bits_per_sample=wave.bits_per_sample,
    channel_count=wave.channel_count,
)
effect2.play(effect1)

audio.play(effect2)

time.sleep(2)
effect1.stop() # No error
#effect2.stop() # Error
relic-se commented 3 weeks ago

I've just tested this scenario but with using two audiofilters.Filter objects and got the error. Then again with two audiodelays.Echo objects and wasn't able to recreate the crash. So, it looks like I'm to blame here (https://github.com/adafruit/circuitpython/pull/9744). I'll do more investigation into what could be causing the fault.

gamblor21 commented 3 weeks ago

If you haven't found it yet, this sounds like get_buffer is still being called and not finding a sample so calling random memory.

relic-se commented 3 weeks ago

I've closely compared audiodelays.Echo and audiofilters.Filter and haven't found a direct solution yet. On the other hand, I've found a new problem. Playing a Filter through a Delay which hasn't yet been loaded with a sample causes a crash. When doing this the other way around (Delay -> Filter) or Delay -> Delay, this error doesn't occur.

import audiodelays
import audiofilters

properties = {
    "sample_rate": 48000,
    "bits_per_sample": 16,
    "channel_count": 2,
}

effect1 = audiofilters.Filter(**properties)
effect2 = audiodelays.Echo(**properties)

effect2.play(effect1)
relic-se commented 3 weeks ago

I'm not at my computer at the moment so I won't be able to push a PR out until this evening, but I believe I found the issue.

The length variable is never being set to 0 when self->sample == NULL (like it is here, https://github.com/dcooperdalrymple/circuitpython/blob/24a8927d0ce1d77addd584bf809ab73cb0386e75/shared-module/audiodelays/Echo.c#L397), causing an infinite while loop (https://github.com/dcooperdalrymple/circuitpython/blob/24a8927d0ce1d77addd584bf809ab73cb0386e75/shared-module/audiofilters/Filter.c#L224).

I'm not sure why exactly this causes the odd audio playback. I'll make sure to test the results when I am able.

relic-se commented 3 weeks ago

Fix has been implemented in https://github.com/adafruit/circuitpython/pull/9787. The solution was rather simple and all examples now function correctly.