microbit-foundation / micropython-microbit-v2

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

AudioFrame playback chunk and the simulator #195

Closed microbit-carlos closed 1 month ago

microbit-carlos commented 2 months ago

From @microbit-matt-hillsdon in https://github.com/microbit-foundation/micropython-microbit-v2/pull/163#issuecomment-2032427825, broken out here to be able to keep discussions on their own thread:

The existing sim implementation of playing AudioBuffers has always been problematic because of the 4ms chunk size. For audio frames of longer duration I had more hope, but at the moment the chunk size used is the same even though we likely have a much larger buffer in the frame itself.

Our work-in-progress record/playback sim implementation works OK if you increase LOG_AUDIO_CHUNK_SIZE from 5 to 6. (it's possible that on slower computers a bigger buffer still might help, I've not had a chance to test yet). We can't just do that as I think it means regular audio frames are pitch/rate shifted but it might show a way forward for the sim.

The sim changes without the LOG_AUDIO_CHUNK_SIZE change can be seen here: https://review-python-simulator.usermbit.org/beta-updates/demo.html (microbit-foundation/micropython-microbit-v2-simulator#113) - see sample "Record". For now you'll have to rebuild locally to see the benefit of the buffer size change. We've not yet looked at edge cases etc. but I think this problem will remain.

microbit-carlos commented 2 months ago

@dpgeorge are you able to provide some guidance here?

jaustin commented 1 month ago

@dpgeorge this is currently the only thing blocking putting audio/record in the simulator for beta so we'd appreciate a view on this - especially if it needs a design change as we iterate the audio stuff anyway to make sure it works. (I'm thinking about our conversation about how a contiguous memoryview is more natural and efficient compared to a generator of smaller buffers)

dpgeorge commented 1 month ago

I think I have a way forward here to allow for a minimum AudioFrame size of 32 bytes, but also have a larger output buffer, and the output buffer is filled as much as possible before calling down in to the lower layer (CODAL, simulator) to play the data.

dpgeorge commented 1 month ago

I have pushed some commits to the audio-recording branch which should resolve this issue. You can now define (at the C level, in mpconfigport.h) the following macro to increase the audio output buffer size:

#define AUDIO_OUTPUT_BUFFER_SIZE (256) // this can be any value, defaults to 32 bytes

That will be the number of bytes that are sent at a time down to the lower layers of the audio pipeline (eg to the simulator).

@microbit-matt-hillsdon see if that works!

microbit-matt-hillsdon commented 1 month ago

Thank you, this is way better for playback of recordings and improves the situation for traditional audio frame use.

Updated here: https://review-python-simulator.usermbit.org/beta-updates/demo.html https://review-python-editor-v3.microbit.org/beta-micropython/ (At some point in these changes we've broken regular AudioFrame use on Firefox - see notes on https://github.com/microbit-foundation/micropython-microbit-v2-simulator/pull/113#issuecomment-2129572527).

Needs some testing on slower hardware but I think this is more an asyncify/event loop tick resolution issue than a performance one so I'm optimistic.

I used the smallest viable buffer (64) because of an issue with wait=True.

As I understand it, we stop waiting when we've finished sending the audio to play not when it's finished playing. The Web Audio side currently request an extra buffer when we start playing (to avoid stalling) and more buffers as we finish playing each buffer. So the last writeData returns with two 64 sample buffers still to play.

Initially we had a sim-only issue where we'd overlap audio if there was a subsequent play after a wait=True play but we've fixed that. So the remaining issue is that control is arguably returned to the user program early. These issues are somewhat noticeable in the "Audio" sample on the sim demo page.

It might still be possible to overlap different types of audio by the two buffers time - e.g. I think we could start playing speech while still processing the last two raw audio buffers.

I'm not sure these issues have a straightforward fix and they might be acceptable.

There's one more thing I wanted to check: If you ever play an AudioFrame then we now seem to play silence after it indefinitely. Is that an intentional change? We don't think this happened before the MicroPython update and maybe happened in this latest change. I'm dropping the silent frames in the C-side of the HAL at the moment.

dpgeorge commented 1 month ago

Thank you, this is way better for playback of recordings and improves the situation for traditional audio frame use

Great! I'm glad it works.

There's one more thing I wanted to check: If you ever play an AudioFrame then we now seem to play silence after it indefinitely. Is that an intentional change?

Oops, that's a mistake on my side. Now fixed, see the most recent commit on the audio-recording branch. Please can you test this to see if it fixes it for you?

As I understand it, we stop waiting when we've finished sending the audio to play not when it's finished playing.

This is a bug on the MicroPython side that we will try to resolve. See https://github.com/microbit-foundation/micropython-microbit-v2/issues/182#issuecomment-2084104624, and related https://github.com/microbit-foundation/micropython-microbit-v2/issues/206

microbit-matt-hillsdon commented 1 month ago

There's one more thing I wanted to check: If you ever play an AudioFrame then we now seem to play silence after it indefinitely. Is that an intentional change?

Oops, that's a mistake on my side. Now fixed, see the most recent commit on the audio-recording branch. Please can you test this to see if it fixes it for you?

Looks good now that I've updated the sim to 17aaa115ad4ad285377b2eb210f37b0e94f61aa9 (same demo links as above). Thanks.

dpgeorge commented 1 month ago

Looks good now that I've updated the sim to ...

Very good! Thanks for confirming.

I think we can probably close this issue now.