audioverb.Tank: two fixes (#168, #169)

docs/upstream-diff.md

@@ -3172,7 +3172,7 @@ a CircuitPython module would not exist on a stock board, so a `Plate` written
 against it would silently be a different effect there. `audiofreeverb` is not
 touched by any of this.
 
-Four details worth recording:
+Six details worth recording:
 
 - **Every line write is a magnitude truncation, not a rounding, and that is
   what makes the tail reach exact zero.** A recirculating `int16` network that
@@ -3205,6 +3205,41 @@ Four details worth recording:
   a starved chain gets silence and the tail stops with the source rather than
   ringing on. A class that wants the tail rung out feeds the tank silence for
   as long as its `tail_samples` says.
+- **The tilt's pole runs while `tone_db` is 0** (audiodsp#168, 2026-09-28).
+  At 0 the output skips the tilt, because its gains are exactly 1 and
+  `s + (v - s)` is not always `v` in `float`, so a node held at 0 renders what
+  it always did. Until then the pole stopped there too, and held the state it
+  had when Tone reached 0: moved off 0 after the tail had died, the node
+  played `state * (low_gain - high_gain)` out of exact silence, 1 382 LSB at
+  48 kHz stereo for a move back to +12 dB. Now Tone comes back exactly as it
+  would from 2^-24 dB, where both gains round to 1 and the pole never
+  stopped. *Rejected:* following the signal while out (`s = v`, a store
+  rather than a multiply-add). Out of silence it is silent too, but the tilt
+  then comes back as a pole at the last sample rather than at the signal's
+  low band, and the first millisecond or so of wet is wrong: back to +12 dB
+  while noise plays, 2 402 LSB off at 48 kHz stereo, against 2 995 frozen.
+  Holding the state at 0 is wrong the same way (1 914).
+- **`set(delays=..., taps=...)` re-cuts a playing node in place**
+  (audiodsp#169, 2026-09-28). The line table and the tap table size and index
+  the allocation, so until then `set()` refused them and the only way to
+  change a reverb's size or character mid-stream was a new node. That lost
+  audio: the node pulls its source a buffer at a time and plays 256 frames a
+  block, so between blocks it can hold the unplayed rest of a source buffer,
+  and those frames went with the old node (512 frames of a 1024- or
+  2048-frame source for a move 1536 frames in; a RawSample handed whole
+  replayed from its start). Now the node keeps its source and the frames it
+  holds; the new tables are checked on a copy, the network is allocated (or
+  cleared, at the same size) and starts exactly as a newly built node's does,
+  lines, filters, predelay and modulation phase, and options in the same call
+  apply after it. `sample_rate`, `channel_count` and `max_predelay_ms` stay
+  fixed. No new name: the two keywords are the constructor's. *Rejected:* a
+  way to read the held frames out of the old node and hand them to a new one
+  (a new public method, and a pointer into the source's memory handed across
+  objects); and the old node leaving its frames somewhere a new node playing
+  the same source would find them (hidden state shared between objects, and
+  wrong the moment anything pulls the source in between). `reset_buffer`
+  still drops the held frames, as every node in the family does;
+  `clear()` has always kept them.
 
 **What it costs.** Counted from the source, with everything switched on and
 the default 14-tap table: about 62 multiplies per stereo frame -- input

src/audioverb/Tank.c

@@ -196,19 +196,102 @@ static mp_obj_t audioverb_tank_play(mp_obj_t self_in, mp_obj_t sample) {
 }
 static MP_DEFINE_CONST_FUN_OBJ_2(audioverb_tank_play_obj, audioverb_tank_play);
 
-static mp_obj_t audioverb_tank_set(size_t n_args, const mp_obj_t *args,
-    mp_map_t *kw_args) {
-    audioverb_tank_obj_t *self = MP_OBJ_TO_PTR(args[0]);
-    (void)n_args;
-    for (size_t i = 0; i < kw_args->alloc; ++i) {
-        if (!mp_map_slot_is_filled(kw_args, i)) {
+// `delays` and `taps` in `set()` re-cut the network in place (audiodsp#169).
+// A class that changes a reverb's size or character used to have to build a
+// new node for it, and the old node took with it any source frames it had
+// pulled and not yet played -- up to a source buffer less a block, lost from
+// the dry as well as the wet. The node now keeps its source and those frames;
+// the network starts empty, exactly as a newly built node's does. Everything
+// that can be refused is checked before anything running changes.
+static bool tank_is_topology_keyword(qstr name) {
+    return name == MP_QSTR_delays || name == MP_QSTR_taps;
+}
+
+static void tank_check_options(const mp_map_t *kw) {
+    for (size_t i = 0; i < kw->alloc; ++i) {
+        if (!mp_map_slot_is_filled(kw, i)) {
+            continue;
+        }
+        qstr name = mp_obj_str_get_qstr(kw->table[i].key);
+        if (tank_is_topology_keyword(name)) {
             continue;
         }
-        qstr name = mp_obj_str_get_qstr(kw_args->table[i].key);
         if (tank_is_shape_keyword(name)) {
             mp_raise_msg_varg(&mp_type_TypeError,
                 MP_ERROR_TEXT("'%q' is fixed at construction"), name);
         }
+        bool known = false;
+        for (size_t option = 0; option < MP_ARRAY_SIZE(tank_option_names);
+             ++option) {
+            if (tank_option_names[option].name == name) {
+                known = true;
+                break;
+            }
+        }
+        if (!known) {
+            mp_raise_msg_varg(&mp_type_TypeError,
+                MP_ERROR_TEXT("unknown Tank option '%q'"), name);
+        }
+    }
+}
+
+static void tank_recut(audioverb_tank_obj_t *self, mp_obj_t delays,
+    mp_obj_t taps) {
+    uint32_t frames[AUDIODSP_TANK_LINES];
+    uint32_t frame_count = 0;
+    float tap_values[AUDIODSP_TANK_MAX_TAPS * 4u];
+    uint32_t tap_count = 0;
+    if (delays != MP_OBJ_NULL) {
+        float values[AUDIODSP_TANK_LINES];
+        frame_count = tank_read_floats(delays, values, AUDIODSP_TANK_LINES);
+        for (uint32_t line = 0; line < frame_count; ++line) {
+            frames[line] = values[line] < 0.0f ? 0u : (uint32_t)values[line];
+        }
+    }
+    if (taps != MP_OBJ_NULL) {
+        tap_count = tank_read_floats(taps, tap_values,
+            AUDIODSP_TANK_MAX_TAPS * 4u);
+    }
+    audiodsp_tank_config_t recut;
+    tank_raise_status(audiodsp_tank_recut(&recut, &self->config,
+        delays != MP_OBJ_NULL ? frames : NULL, frame_count,
+        taps != MP_OBJ_NULL ? tap_values : NULL, tap_count));
+    const uint32_t old_samples = audiodsp_tank_buffer_samples(&self->config);
+    const uint32_t samples = audiodsp_tank_buffer_samples(&recut);
+    int16_t *old_lines = self->state.lines[0];
+    int16_t *lines = old_lines;
+    if (samples != old_samples) {
+        // Allocated and cleared before the pump is held off, so a refused
+        // allocation leaves the node as it was, and the pump waits only for
+        // the swap.
+        lines = m_malloc((size_t)samples * sizeof(int16_t));
+        memset(lines, 0, (size_t)samples * sizeof(int16_t));
+    }
+    audiodsp_pump_lock_acquire();
+    self->config = recut;
+    if (lines == old_lines) {
+        // The same size: cleared where it is, under the lock, as clear() is.
+        memset(lines, 0, (size_t)samples * sizeof(int16_t));
+    }
+    audiodsp_tank_state_init(&self->state, &self->config, lines);
+    audiodsp_pump_lock_release();
+    if (lines != old_lines) {
+        m_del(int16_t, old_lines, old_samples);
+    }
+}
+
+static mp_obj_t audioverb_tank_set(size_t n_args, const mp_obj_t *args,
+    mp_map_t *kw_args) {
+    audioverb_tank_obj_t *self = MP_OBJ_TO_PTR(args[0]);
+    (void)n_args;
+    tank_check_options(kw_args);
+    mp_map_elem_t *delays =
+        mp_map_lookup(kw_args, MP_OBJ_NEW_QSTR(MP_QSTR_delays), MP_MAP_LOOKUP);
+    mp_map_elem_t *taps =
+        mp_map_lookup(kw_args, MP_OBJ_NEW_QSTR(MP_QSTR_taps), MP_MAP_LOOKUP);
+    if (delays != NULL || taps != NULL) {
+        tank_recut(self, delays != NULL ? delays->value : MP_OBJ_NULL,
+            taps != NULL ? taps->value : MP_OBJ_NULL);
     }
     tank_apply_kwargs(self, kw_args);
     audiodsp_pump_lock_acquire();

src/circuitpython_spike/shared-bindings/audioverb/Tank.c

@@ -45,8 +45,9 @@
 //|         mix: float = 0.3,
 //|     ) -> None:
 //|         """Create a reverberation tank. ``max_predelay_ms``, ``delays`` and
-//|         ``taps`` size the allocation and cut the topology, and none of the
-//|         three can change afterwards.
+//|         ``taps`` size the allocation and cut the topology.
+//|         ``max_predelay_ms`` cannot change afterwards; ``delays`` and
+//|         ``taps`` can be re-cut with `set`, which empties the network.
 //|
 //|         ``delays`` is twelve line lengths in frames - the four input
 //|         diffusers, then each tank half's modulated all-pass, delay,
@@ -254,21 +255,98 @@ MP_DEFINE_CONST_FUN_OBJ_2(audioverb_tank_play_obj, audioverb_tank_play);
 
 //|     def set(self, **options: float) -> None:
 //|         """Change settings mid-stream. The lines keep their contents; only
-//|         what the network does to them changes."""
+//|         what the network does to them changes.
+//|
+//|         ``delays`` and ``taps`` are the exception: either one re-cuts the
+//|         network in place, and every line and filter starts empty, exactly
+//|         as a newly built tank's would. The source, and any source frames
+//|         the tank has pulled and not yet played, stay where they are, so
+//|         the dry does not skip. The other shape keywords (``sample_rate``,
+//|         ``channel_count``, ``max_predelay_ms``) are fixed at construction."""
 //|         ...
-static mp_obj_t audioverb_tank_set(size_t n_args, const mp_obj_t *args,
-    mp_map_t *kw_args) {
-    audioverb_tank_obj_t *self = MP_OBJ_TO_PTR(args[0]);
-    (void)n_args;
-    for (size_t i = 0; i < kw_args->alloc; ++i) {
-        if (!mp_map_slot_is_filled(kw_args, i)) {
+// `delays` and `taps` re-cut the network in place (audiodsp#169): the
+// MicroPython binding's tank_recut, line for line, without the pump lock this
+// runtime does not have.
+static bool tank_is_topology_keyword(qstr name) {
+    return name == MP_QSTR_delays || name == MP_QSTR_taps;
+}
+
+static void tank_check_options(const mp_map_t *kw) {
+    for (size_t i = 0; i < kw->alloc; ++i) {
+        if (!mp_map_slot_is_filled(kw, i)) {
+            continue;
+        }
+        qstr name = mp_obj_str_get_qstr(kw->table[i].key);
+        if (tank_is_topology_keyword(name)) {
             continue;
         }
-        qstr name = mp_obj_str_get_qstr(kw_args->table[i].key);
         if (tank_is_shape_keyword(name)) {
             mp_raise_msg_varg(&mp_type_TypeError,
                 MP_ERROR_TEXT("'%q' is fixed at construction"), name);
         }
+        bool known = false;
+        for (size_t option = 0; option < MP_ARRAY_SIZE(tank_option_names);
+             ++option) {
+            if (tank_option_names[option].name == name) {
+                known = true;
+                break;
+            }
+        }
+        if (!known) {
+            mp_raise_msg_varg(&mp_type_TypeError,
+                MP_ERROR_TEXT("unknown Tank option '%q'"), name);
+        }
+    }
+}
+
+static void tank_recut(audioverb_tank_obj_t *self, mp_obj_t delays,
+    mp_obj_t taps) {
+    uint32_t frames[AUDIODSP_TANK_LINES];
+    uint32_t frame_count = 0;
+    float tap_values[AUDIODSP_TANK_MAX_TAPS * 4u];
+    uint32_t tap_count = 0;
+    if (delays != MP_OBJ_NULL) {
+        float values[AUDIODSP_TANK_LINES];
+        frame_count = tank_read_floats(delays, values, AUDIODSP_TANK_LINES);
+        for (uint32_t line = 0; line < frame_count; ++line) {
+            frames[line] = values[line] < 0.0f ? 0u : (uint32_t)values[line];
+        }
+    }
+    if (taps != MP_OBJ_NULL) {
+        tap_count = tank_read_floats(taps, tap_values,
+            AUDIODSP_TANK_MAX_TAPS * 4u);
+    }
+    audiodsp_tank_config_t recut;
+    tank_raise_status(audiodsp_tank_recut(&recut, &self->config,
+        delays != MP_OBJ_NULL ? frames : NULL, frame_count,
+        taps != MP_OBJ_NULL ? tap_values : NULL, tap_count));
+    const uint32_t old_samples = audiodsp_tank_buffer_samples(&self->config);
+    const uint32_t samples = audiodsp_tank_buffer_samples(&recut);
+    int16_t *old_lines = self->state.lines[0];
+    int16_t *lines = old_lines;
+    if (samples != old_samples) {
+        lines = m_malloc((size_t)samples * sizeof(int16_t));
+    }
+    memset(lines, 0, (size_t)samples * sizeof(int16_t));
+    self->config = recut;
+    audiodsp_tank_state_init(&self->state, &self->config, lines);
+    if (lines != old_lines) {
+        m_del(int16_t, old_lines, old_samples);
+    }
+}
+
+static mp_obj_t audioverb_tank_set(size_t n_args, const mp_obj_t *args,
+    mp_map_t *kw_args) {
+    audioverb_tank_obj_t *self = MP_OBJ_TO_PTR(args[0]);
+    (void)n_args;
+    tank_check_options(kw_args);
+    mp_map_elem_t *delays =
+        mp_map_lookup(kw_args, MP_OBJ_NEW_QSTR(MP_QSTR_delays), MP_MAP_LOOKUP);
+    mp_map_elem_t *taps =
+        mp_map_lookup(kw_args, MP_OBJ_NEW_QSTR(MP_QSTR_taps), MP_MAP_LOOKUP);
+    if (delays != NULL || taps != NULL) {
+        tank_recut(self, delays != NULL ? delays->value : MP_OBJ_NULL,
+            taps != NULL ? taps->value : MP_OBJ_NULL);
     }
     tank_apply_kwargs(self, kw_args);
     audiodsp_tank_config_finish(&self->config);

src/cpython/_audiodsp.c

@@ -1931,6 +1931,49 @@ static PyObject *tank_state_reset(audiodsp_tank_object_t *self,
     Py_RETURN_NONE;
 }
 
+// A re-cut of a running node (audiodsp#169): the new tables checked on a
+// copy, then the network rebuilt empty on them, exactly as tank_state_init
+// leaves a new one. `delays` or `taps` may be None to keep that table.
+static PyObject *tank_state_recut(audiodsp_tank_object_t *self,
+    PyObject *args) {
+    PyObject *delays = Py_None;
+    PyObject *taps = Py_None;
+    if (!PyArg_ParseTuple(args, "OO:recut", &delays, &taps)) return NULL;
+    uint32_t frames[AUDIODSP_TANK_LINES];
+    uint32_t frame_count = 0;
+    float tap_values[AUDIODSP_TANK_MAX_TAPS * 4u];
+    uint32_t tap_count = 0;
+    if (delays != Py_None) {
+        float values[AUDIODSP_TANK_LINES];
+        if (tank_state_floats(delays, values, AUDIODSP_TANK_LINES,
+            &frame_count) < 0) return NULL;
+        for (uint32_t line = 0; line < frame_count; ++line) {
+            frames[line] = values[line] < 0.0f ? 0u : (uint32_t)values[line];
+        }
+    }
+    if (taps != Py_None) {
+        if (tank_state_floats(taps, tap_values, AUDIODSP_TANK_MAX_TAPS * 4u,
+            &tap_count) < 0) return NULL;
+    }
+    audiodsp_tank_config_t recut;
+    if (tank_state_status(audiodsp_tank_recut(&recut, &self->config,
+        delays != Py_None ? frames : NULL, frame_count,
+        taps != Py_None ? tap_values : NULL, tap_count)) < 0) return NULL;
+    const uint32_t samples = audiodsp_tank_buffer_samples(&recut);
+    int16_t *lines = self->lines;
+    if (samples != audiodsp_tank_buffer_samples(&self->config)) {
+        lines = PyMem_Calloc((size_t)samples, sizeof(int16_t));
+        if (lines == NULL) return PyErr_NoMemory();
+        PyMem_Free(self->lines);
+        self->lines = lines;
+    } else {
+        memset(lines, 0, (size_t)samples * sizeof(int16_t));
+    }
+    self->config = recut;
+    audiodsp_tank_state_init(&self->state, &self->config, lines);
+    Py_RETURN_NONE;
+}
+
 static PyObject *tank_state_process(audiodsp_tank_object_t *self,
     PyObject *argument) {
     Py_buffer input = {0};
@@ -1956,6 +1999,7 @@ static PyMethodDef tank_state_methods[] = {
     {"configure", (PyCFunction)tank_state_configure, METH_VARARGS, NULL},
     {"finish", (PyCFunction)tank_state_finish, METH_NOARGS, NULL},
     {"reset", (PyCFunction)tank_state_reset, METH_NOARGS, NULL},
+    {"recut", (PyCFunction)tank_state_recut, METH_VARARGS, NULL},
     {"process", (PyCFunction)tank_state_process, METH_O, NULL},
     {NULL, NULL, 0, NULL},
 };

src/cpython/audioverb.py

@@ -29,7 +29,8 @@
     8..11   tank half B: the same four
 
 `taps` is four values per tap -- channel, line index, offset in frames, gain --
-and at most 32 of them. Both default to Dattorro's own published table, scaled
+and at most 32 of them. Either can be re-cut later with `set()`, which empties
+the network and keeps the source where it is. Both default to Dattorro's own published table, scaled
 from 29761 Hz to whatever `sample_rate` says. His figures, for a class that
 wants to scale or re-cut them itself:
 
@@ -87,10 +88,11 @@
     "mix": 11,
 }
 
-#: Keywords that shape the allocation rather than setting an option. They are
-#: constructor-only, and `set()` refuses them rather than silently ignoring
-#: them.
+#: Keywords that shape the allocation rather than setting an option. The first
+#: three are constructor-only, and `set()` refuses them rather than silently
+#: ignoring them; `delays` and `taps` in `set()` re-cut the network in place.
 _SHAPE = ("sample_rate", "channel_count", "max_predelay_ms", "delays", "taps")
+_FIXED = ("sample_rate", "channel_count", "max_predelay_ms")
 
 
 class Tank(_AudioSample):
@@ -130,11 +132,29 @@ def _apply(self, options):
 
     def set(self, **options):
         """Change settings mid-stream. The lines keep their contents; only what
-        the network does to them changes."""
+        the network does to them changes.
+
+        `delays` and `taps` are the exception: either one re-cuts the network
+        in place, and every line and filter starts empty, exactly as a newly
+        built tank's would. The source, and any source frames the tank has
+        pulled and not yet played, stay where they are, so the dry does not
+        skip (audiodsp#169). A class that changes a reverb's size or character
+        used to need a new node for it, and the frames went with the old one.
+        `sample_rate`, `channel_count` and `max_predelay_ms` are fixed at
+        construction."""
         self._check()
-        for name in _SHAPE:
+        for name in _FIXED:
             if name in options:
                 raise TypeError("%r is fixed at construction" % (name,))
+        delays = options.pop("delays", None)
+        taps = options.pop("taps", None)
+        for name in options:
+            if name not in _OPTIONS:
+                raise TypeError("unknown Tank option %r" % (name,))
+        if delays is not None or taps is not None:
+            self._state.recut(
+                None if delays is None else [float(v) for v in delays],
+                None if taps is None else [float(v) for v in taps])
         self._apply(options)
         self._state.finish()