amy_global's transfer_flag/transfer_storage/transfer_length_bytes/transfer_stored_bytes/transfer_file_handle fields are overloaded for three unrelated purposes, with no guard against them clobbering each other:
- AUDIO/FILE wire transfer (
transfer.c) -- incoming byte-stream ingestion via parse_transfer_message, gated by amy_message_is_transfer_chunk() in api.c.
- SAMPLE capture (
amy.c amy_fill_buffer(), guarded by transfer_flag == AMY_TRANSFER_TYPE_SAMPLE) -- render-loop-driven capture of AMY's own output or audio-in into a PCM preset buffer (amy.start_sample()/stop_sample()). Not wire-message-driven at all.
- Disk-sample async load scratch space (
pcm.c:367-368, parse.c:505-507) -- zF (disk_sample) stashes midinote/preset into transfer_stored_bytes/transfer_file_handle while the file load is scheduled onto the MicroPython task, before pcm_load_file() reads them back out. amy.h's own field comments admit this ("using this for midi note before file load" / "using this for preset number before file load").
Since transfer_flag is a single scalar and all three reuse the same storage/length/handle fields, nothing stops them from stepping on each other. Concretely: calling amy.start_sample(...) (arms SAMPLE) and then amy.load_sample(...) (arms AUDIO) before the first finishes silently clobbers the sample capture's state -- no error, just corrupted output. Same risk between any pair of the three.
Proposed fix: give each concern its own dedicated state instead of sharing transfer_*:
amy_global.sample_capture (storage/length/stored/source) for SAMPLE, independent of transfer state.- Two dedicated scratch fields for the disk-sample-load handoff, instead of borrowing
transfer_stored_bytes/transfer_file_handle.
amy_global.transfer (flag/storage/length/stored/handle) left for AUDIO/FILE only.
Touches amy.h (the struct), transfer.c, amy.c, pcm.c, parse.c. Not urgent -- none of these three are normally invoked concurrently in practice -- but worth fixing before it bites someone
amy_global'stransfer_flag/transfer_storage/transfer_length_bytes/transfer_stored_bytes/transfer_file_handlefields are overloaded for three unrelated purposes, with no guard against them clobbering each other:transfer.c) -- incoming byte-stream ingestion viaparse_transfer_message, gated byamy_message_is_transfer_chunk()inapi.c.amy.camy_fill_buffer(), guarded bytransfer_flag == AMY_TRANSFER_TYPE_SAMPLE) -- render-loop-driven capture of AMY's own output or audio-in into a PCM preset buffer (amy.start_sample()/stop_sample()). Not wire-message-driven at all.pcm.c:367-368,parse.c:505-507) --zF(disk_sample) stashesmidinote/presetintotransfer_stored_bytes/transfer_file_handlewhile the file load is scheduled onto the MicroPython task, beforepcm_load_file()reads them back out.amy.h's own field comments admit this ("using this for midi note before file load" / "using this for preset number before file load").Since
transfer_flagis a single scalar and all three reuse the same storage/length/handle fields, nothing stops them from stepping on each other. Concretely: callingamy.start_sample(...)(arms SAMPLE) and thenamy.load_sample(...)(arms AUDIO) before the first finishes silently clobbers the sample capture's state -- no error, just corrupted output. Same risk between any pair of the three.Proposed fix: give each concern its own dedicated state instead of sharing
transfer_*:amy_global.sample_capture(storage/length/stored/source) for SAMPLE, independent of transfer state.transfer_stored_bytes/transfer_file_handle.amy_global.transfer(flag/storage/length/stored/handle) left for AUDIO/FILE only.Touches
amy.h(the struct),transfer.c,amy.c,pcm.c,parse.c. Not urgent -- none of these three are normally invoked concurrently in practice -- but worth fixing before it bites someone