audio: validate arguments before narrowing them, and end an MP3 with …

[peterbay]

Code written by Claude Code, guided and corrected by @peterbay.

The problem

Five defects across the audio modules. Four are arguments narrowed into a uint8_t or uint32_t field before anything checked them, so a value past the field's range wrapped into a valid-looking one. The fifth stops a looped MP3 from restarting.

The changes

• audiomp3 returned an error where a stream had simply ended. An input underflow at the end of the data is how an MP3 that was cut off mid-frame finishes, not a failure, but get_buffer returned GET_BUFFER_ERROR for it and the caller stopped playback instead of looping. It returns GET_BUFFER_DONE when the underflow is at end of file, and GET_BUFFER_ERROR for a real decode error as before.

• audiomp3 passed stream_lseek its offset and whence the wrong way round, in two places. The signature is (stream, offset, whence). The seek past an ID3 header asked to move SEEK_CUR bytes from whence size, failed, and fell through to the read-and-discard loop below, which is why it still worked. The rewind on loop passed (SEEK_SET, 0), which worked only because SEEK_SET is 0. The success test on the first one is also wrong: a stream that cannot seek returns its unchanged position, so comparing against zero is not enough. It now compares the position before and after.

• audiomixer narrowed the voice index before checking it. args[ARG_voice].u_int went into a uint8_t and was then compared against voice_count - 1, so voice=256 became 0 and played on the first voice instead of raising. Changed to mp_arg_validate_int_range on the value as parsed.

• audiomixer wrote one word past the mix buffer. The mono path stepped for (i = 0; i < n; i += 2) and wrote word_buffer[i] and word_buffer[i + 1], so an odd n wrote one past the end. The loop now bounds i + 1.

• audiobusio.PDMIn narrowed bit_depth and oversample before the divisibility check, so 256 passed it as zero, and sample_rate was not checked at all. All three are validated on the parsed value now.

• audiocore's shared sample_rate setter validated nothing. It is mixed into every audio sample through AUDIOSAMPLE_FIELDS, so it could undo the constructors' own check; zero reached code that divides by it.

• I2SOut.left_justified was declared MP_ARG_OBJ and read as .u_bool. Passing it explicitly stored an object in the union and the flag came back as whatever that pointer aliased to, so left_justified=False read as true. Declared MP_ARG_BOOL.

Testing

Seeed XIAO nRF52840 Sense with an Adafruit Audio BFF, on two builds differing only by these changes. Playback ran through a mixer voice held at level 0, so the decoder was driven without anything audible.

	before	after
mixer.play(sample, voice=256) on a 2-voice mixer	accepted, voice 0 plays	ValueError: voice must be 0-1
sample.sample_rate = 0	accepted, reads back 0	ValueError: sample_rate must be >= 1
PDMIn(..., oversample=256)	accepted	ValueError: oversample must be multiple of 8.
an MP3 cut off mid-frame, played with loop=True	stops after 0.8 s	still looping after 6 s

PDMIn's bit_depth=256 and sample_rate=0 are rejected on this port either way, because its HAL supports only bit_depth=16 and sample_rate=16000 and refuses them for its own reason; oversample is the one it leaves to the binding. A complete MP3 never reaches the changed branch — it ends on a whole frame — so the loop test used a truncated copy.

Two of these are not covered above. The mixer's one-word overrun needs an odd n, which the buffer sizes here do not produce. left_justified sets NRF_I2S->CONFIG.FORMAT on this port, but memorymap here only maps RAM, so the register could not be read back from Python.

[dhalbert]

Thanks - these are nice fixes, and they all make sense to me.

[peterbay]

Testing and diagnostic script.
audio_validate_before_narrowing.py