microbit-foundation / micropython-microbit-v2

Temporary home for MicroPython for micro:bit v2 as we stablise it before pushing upstream
MIT License
44 stars 25 forks source link

AudioFrame with zero or negative values as arguments #186

Closed microbit-carlos closed 5 months ago

microbit-carlos commented 5 months ago

Using the latest version (at the time of writing) of the recording & playback branch: https://github.com/microbit-foundation/micropython-microbit-v2/commit/0b06914c71c18533da90df85230ac198578669bf Hex: https://github.com/microbit-foundation/micropython-microbit-v2/actions/runs/8416237764?pr=163

An AudioFrame with 0 duration or rate raises a ValueError:

>>> audio.AudioFrame(0)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ValueError: size out of bounds
>>> audio.AudioFrame(1000, 0)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ValueError: rate out of bounds
>>> 

A negative rate also raises ValueError:

>>> audio.AudioFrame(1000, -1)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ValueError: rate out of bounds
>>> 

But negative value results in the default length

>>> len(audio.AudioFrame())
32
>>> len(audio.AudioFrame(-2))
32
>>> 

On the other hand, microphone.record(0) return a zero-length AudioFrame (which might be related to https://github.com/microbit-foundation/micropython-microbit-v2/issues/185):

>>> len(microphone.record(0))
0
>>> len(microphone.record(1000, rate=0))
0
microbit-carlos commented 5 months ago

I'm guessing this is because the default value for duration in AudioFrame is -1: https://github.com/microbit-foundation/micropython-microbit-v2/blob/c91ae4e83c5a1a82f647740277f7d4a462192160/src/codal_port/modaudio.c#L316-L319

Should that be set to None and raise a ValueError with any negative values?

dpgeorge commented 5 months ago

'm guessing this is because the default value for duration in AudioFrame is -1

Yes, correct.

Should that be set to None and raise a ValueError with any negative values?

I have now made this change.

dpgeorge commented 5 months ago

microphone.record(0) return a zero-length AudioFrame

That's also now fixed, as is the case of passing in a non-zero rate.