diff --git a/amy/test.py b/amy/test.py index bdef1983..cb67f08d 100644 --- a/amy/test.py +++ b/amy/test.py @@ -1299,6 +1299,33 @@ def run(self): amy_send_at(time=500, osc=0, preset=1024, wave=amy.PCM_MIX, vel=2, note=50) +class TestDiskSampleStopsOnNoteOff(AmyTest): + """A streamed preset must honor the immediate-stop note-off like an + in-memory one does. pcm_note_off stops by seeking phase to the end, but + render_pcm resets phase to 0 every block for file presets, so the stop + used to be discarded and the clip played on to end-of-file. Note-off at + 100ms, sample is ~256ms, so a regression shows up as extra audio.""" + + def run(self): + amy.disk_sample('sounds/partial_sources/CL SHCI A3.wav', preset=1024, midinote=57) + amy_send_at(time=50, osc=0, preset=1024, wave=amy.PCM_MIX, vel=2, note=57) + amy_send_at(time=100, osc=0, vel=0) + + +class TestDiskSampleLoopModeRefused(AmyTest): + """A streamed preset can't loop (no seekable table), so asking for PCM_LOOP + is refused outright rather than accepted and quietly not looped. The mode + is the half that gets dropped, so the osc keeps the preset and its default + PCM_PLAY_STOP -- which means note-off stops it. Output should therefore be + identical to TestDiskSampleStopsOnNoteOff, which asks for no mode at all.""" + + def run(self): + amy.disk_sample('sounds/partial_sources/CL SHCI A3.wav', preset=1024, midinote=57) + amy_send_at(time=50, osc=0, preset=1024, wave=amy.PCM_MIX, vel=2, note=57, + mode=amy.PCM_LOOP) + amy_send_at(time=100, osc=0, vel=0) + + class TestDiskSampleStereo(AmyTest): def run(self): diff --git a/docs/synth.md b/docs/synth.md index 5fed2922..f68a0faa 100644 --- a/docs/synth.md +++ b/docs/synth.md @@ -359,6 +359,26 @@ amy.send(wave=amy.PCM, preset=99, vel=1, mode=amy.PCM_LOOP_FOREVER, eg0='0,1,100 amy.send(vel=0) # note off ``` +**Looping needs an in-memory sample.** Presets loaded with `disk_sample` stream +from the file through a small buffer instead of sitting in memory, so there is +nothing to loop back into and the loop marks can't be honored. + +AMY therefore refuses to enter that configuration rather than accepting it and +quietly not looping. Both halves are checked as they are set — when you change +`mode`, and when you change `preset` — and the command that would create the +impossible pair is dropped with a warning naming the file: + +``` +amy: osc 0 preset 1024 streams from drums/kick.wav, which cannot loop; + ignoring mode=2. Use load_sample() to loop. +``` + +The `mode` is the half dropped when you set both at once, so the sample still +plays (once, honoring note-off) rather than leaving you a loop mode pointing at +nothing. Setting a streamed preset on an oscillator that is *already* in a loop +mode drops the `preset` instead, and says so. Load the sample with +`load_sample` if you need it to loop. + ### Sampler (aka Memory PCM) You can also load your own samples into AMY memory at runtime by sending PCM data over the wire protocol. Use `load_sample` in `amy.py` as an example: diff --git a/src/amy.c b/src/amy.c index 8e27cc81..c1a915b2 100644 --- a/src/amy.c +++ b/src/amy.c @@ -723,8 +723,14 @@ void amy_event_to_deltas_queue(amy_event *e, uint16_t base_osc, struct delta **q amy_global.highest_bus = e->bus; } EVENT_TO_DELTA_I(wave, WAVE) - EVENT_TO_DELTA_I(mode, MODE) + // PRESET before MODE, and it matters: pcm_loop_config_allowed() refuses + // whichever of the pair arrives second and makes the configuration + // impossible. A single message asking for a file-backed preset AND a + // PCM_LOOP* mode is the common way to hit that, and refusing the *mode* + // leaves something useful (the sample, playing once) where refusing the + // preset would leave a loop mode pointing at nothing. EVENT_TO_DELTA_I(preset, PRESET) + EVENT_TO_DELTA_I(mode, MODE) EVENT_TO_DELTA_F(midi_note, MIDI_NOTE) EVENT_TO_DELTA_COEFS(amp_coefs, AMP) EVENT_TO_DELTA_FREQ_COEFS(freq_coefs, FREQ) @@ -1366,7 +1372,21 @@ void play_delta(struct delta *d) { sine_note_on(d->osc, freq_of_logfreq(synth[d->osc]->logfreq_coefs[COEF_CONST])); } } - DELTA_TO_SYNTH_I(MODE, mode) + // MODE and PRESET are assigned together, because neither is valid on its + // own: a PCM_LOOP* mode on a file-backed preset is a configuration AMY + // cannot honor. Whichever of the pair this delta carries is checked + // against the value already on the osc, and refused with a warning rather + // than accepted and quietly doing something else at note-on. (Only one + // param matches per delta, so this reads as two cases but runs as one.) + if (d->param == MODE || d->param == PRESET) { + bool setting_mode = (d->param == MODE); + uint16_t mode = setting_mode ? (uint16_t)d->data.i : synth[d->osc]->mode; + uint16_t preset = setting_mode ? synth[d->osc]->preset : (uint16_t)d->data.i; + if (pcm_loop_config_allowed(d->osc, mode, preset, setting_mode)) { + if (setting_mode) synth[d->osc]->mode = mode; + else synth[d->osc]->preset = preset; + } + } DELTA_TO_SYNTH_I(BUS, bus) DELTA_TO_SYNTH_F(FEEDBACK, feedback) DELTA_TO_SYNTH_F(RATIO, logratio) @@ -1375,9 +1395,6 @@ void play_delta(struct delta *d) { DELTA_TO_SYNTH_I(NOTE_SOURCE_CHANNEL, s_note_source_channel) DELTA_TO_SYNTH_I(EG0_TYPE, eg_type[0]) DELTA_TO_SYNTH_I(EG1_TYPE, eg_type[1]) - if (d->param == PRESET) { - synth[d->osc]->preset = (uint16_t)d->data.i; - } if (d->param == PORTAMENTO) synth[d->osc]->portamento_alpha = portamento_ms_to_alpha(d->data.i); if (d->param == PHASE) { // Phase sets the *initial* phase of the osc. diff --git a/src/amy.h b/src/amy.h index aee1754c..a760713d 100644 --- a/src/amy.h +++ b/src/amy.h @@ -1235,6 +1235,12 @@ extern void custom_mod_trigger(uint16_t osc); extern int16_t * pcm_load(uint16_t preset_number, uint32_t length, uint32_t samplerate, uint8_t channels, uint8_t midinote, uint32_t loopstart, uint32_t loopend); extern const int16_t *pcm_get_sample_ram_for_preset(uint16_t preset_number, uint32_t *length); extern int pcm_load_file(); +// Guard against configuring a PCM loop on a file-backed (streamed) preset, +// which can never loop. Called with the PROPOSED mode and preset as each is +// set; returns false if that command should be dropped (having warned). +// mode_is_the_new_part picks which of the two the message blames. +extern bool pcm_loop_config_allowed(uint16_t osc, uint16_t mode, uint16_t preset_number, + bool mode_is_the_new_part); extern void pcm_unload_preset(uint16_t preset_number); extern void pcm_unload_all_presets(); diff --git a/src/pcm.c b/src/pcm.c index a466ec4b..53dbe274 100644 --- a/src/pcm.c +++ b/src/pcm.c @@ -160,6 +160,57 @@ static void fclose_if_file(memorypcm_preset_t *preset) { } } +static bool mode_is_looping(uint16_t mode) { + return mode == PCM_LOOP || mode == PCM_LOOP_STOP || mode == PCM_LOOP_FOREVER; +} + +// True if `preset_number` streams from a file rather than sitting in memory. +// `filename_out`, when non-NULL, receives the file name for the message. +static bool preset_is_file(uint16_t preset_number, const char **filename_out) { + if (AMY_IS_UNSET(preset_number)) return false; + memorypcm_preset_t rom_local; + memorypcm_preset_t *preset = get_preset_for_preset_number(preset_number, &rom_local); + if (preset == NULL || preset->type != AMY_PCM_TYPE_FILE) return false; + if (filename_out != NULL) *filename_out = preset->filename; + return true; +} + +// A file-backed preset streams through a small sliding buffer rather than +// sitting in a table we can index freely, so there is nothing to loop back +// into: render_pcm refills from the file each block and rewinds phase to the +// top of the fresh buffer. A PCM_LOOP* mode on such a preset can never do +// what it says. +// +// Rather than accept the command and quietly do something else at note-on, +// refuse it where it is issued -- when the mode changes, and when the preset +// number changes -- so the configuration never reaches a state it can't +// honor, and the user hears about it while the offending command is still in +// front of them. Called with the *proposed* mode and preset; returns false if +// the command should be dropped. +// +// (Whole-file looping *would* be implementable on top of the fseek+re-parse +// rewind pcm_note_on already does; what a stream can never honor is the +// loopstart/loopend marks. That's a bigger change than this one.) +bool pcm_loop_config_allowed(uint16_t osc, uint16_t mode, uint16_t preset_number, + bool mode_is_the_new_part) { + // mode means nothing outside PCM, so don't second-guess other waves. + if (synth[osc]->wave != PCM) return true; + if (!mode_is_looping(mode)) return true; + const char *filename = NULL; + if (!preset_is_file(preset_number, &filename)) return true; + if (mode_is_the_new_part) { + fprintf(stderr, "amy: osc %d preset %d streams from %s, which cannot loop; " + "ignoring mode=%d. Use load_sample() to loop.\n", + osc, preset_number, filename ? filename : "a file", mode); + } else { + fprintf(stderr, "amy: preset %d streams from %s, which cannot loop, but osc %d " + "is in mode=%d; ignoring preset=%d. Set a non-loop mode first, " + "or use load_sample().\n", + preset_number, filename ? filename : "a file", osc, mode, preset_number); + } + return false; +} + void pcm_note_on(uint16_t osc) { if(AMY_IS_SET(synth[osc]->preset)) { memorypcm_preset_t rom_local; @@ -205,17 +256,20 @@ void pcm_mod_trigger(uint16_t osc) { void pcm_note_off(uint16_t osc) { if(AMY_IS_SET(synth[osc]->preset)) { - uint32_t length = 0; - memorypcm_preset_t rom_local; - memorypcm_preset_t *preset = - get_preset_for_preset_number(synth[osc]->preset, &rom_local); - if(preset != NULL) { - length = preset->length; - } if (msynth[osc]->state == PCM_PLAY_STOP || msynth[osc]->state == PCM_LOOP_STOP) { - // PCM mode where note off causes immediate stop: Set phase to the end - synth[osc]->phase = F2P(length / (float)(1 << PCM_INDEX_BITS)); + // PCM mode where note off causes immediate stop. + // + // This used to seek phase past the end of the sample and let + // render_pcm notice on the next block. That worked only for + // in-memory presets: a streamed one refills from the file and + // resets phase to 0 every block, so the seek was thrown away and + // the clip played on to end-of-file, ignoring note-off entirely. + // PCM_PLAY_STOP is the DEFAULT mode, so that hit every + // disk_sample() note-off. Stopping the osc says what we mean and + // works for both kinds -- and it no longer needs the preset + // lookup that the seek needed just to find the sample length. + synth[osc]->status = SYNTH_OFF; } else if (msynth[osc]->state == PCM_LOOP_FOREVER) { // Sending one note-off to a LOOP_FOREVER loop downgrades it to a stoppable loop. msynth[osc]->state = PCM_LOOP; diff --git a/tests/ref/TestDiskSampleLoopModeRefused.wav b/tests/ref/TestDiskSampleLoopModeRefused.wav new file mode 100644 index 00000000..641129e0 Binary files /dev/null and b/tests/ref/TestDiskSampleLoopModeRefused.wav differ diff --git a/tests/ref/TestDiskSampleStopsOnNoteOff.wav b/tests/ref/TestDiskSampleStopsOnNoteOff.wav new file mode 100644 index 00000000..641129e0 Binary files /dev/null and b/tests/ref/TestDiskSampleStopsOnNoteOff.wav differ