Merge fix/speedchanger-rate-and-phase: SpeedChanger rounds its rate and carries its phase (#91, #92)

Both defects are upstream CircuitPython 10.3.0's own; fixed on the MicroPython
and CPython targets as documented deviations, with upstream reports in
docs/upstream-reports/. PR #93, 43 checks green.

Author: bdbarnett

CHANGELOG.md

@@ -1,5 +1,27 @@
 ## Unreleased
 
+- `audiospeed.SpeedChanger` **carries its phase across a source buffer**.
+  Upstream zeroes the accumulator every time it takes a new buffer, so what the
+  node renders depends on how the node above it chunks its output: the same
+  hold over 64- and 256-frame buffers differed in 1947 frames of 2048. A
+  sample-and-hold built from a pair therefore drifted (73 codes over 65536
+  frames at 48 kHz, 430 at 44.1) and spread its images instead of placing them
+  — a 1 kHz image under a 7 kHz tone held at 8 kHz read −39.9 dB where a
+  zero-order hold puts it at −0.22. All of it is 0 now, and the render matches
+  `source[(((n·up)>>16)·down)>>16]` exactly. A rate that divides the buffer
+  length was always right, which is why the parity gate never saw it.
+  `reset_buffer` still starts the stream over. Upstream still restarts, so this
+  is a named departure — `docs/upstream-diff.md`, report drafted (audioif#91).
+
+- `audiospeed`: the 16.16 rate **rounds** now instead of truncating. Upstream
+  casts, so a float landing a hair below its Q16 neighbour lost a whole LSB —
+  `1/1.0000000000000004` came back as 65535/65536, and a `SpeedChanger` pair
+  asked for "the hold rate = the running rate" was not an identity at 44.1 kHz
+  (27666 codes of error on a full-scale tone; 48 and 22.05 kHz happened to land
+  on 1.0 and looked fine). `Resampler`'s bound ratio rounds too: 48000/44100 is
+  71332, not 71331. Upstream CircuitPython still truncates, so this is a named
+  departure — `docs/upstream-diff.md`, report drafted (audioif#92).
+
 - `audioroute.Splitter` lost the head of any block bigger than its
   8192-frame ring. `audiocore.get_buffer` takes no length, so a source hands
   back what it has — a `RawSample` over a table returns the whole table — and

docs/upstream-diff.md

@@ -1,5 +1,101 @@
 # Deltas from upstream CircuitPython
 
+## `audiospeed`: the Q16 rate rounds here, upstream truncates (audioif#92)
+
+CircuitPython 10.3.0 converts a rate to 16.16 fixed point with a cast
+(`shared-module/audiospeed/__init__.c:18-21`):
+
+```c
+return (uint32_t)(rate * (1 << SPEED_SHIFT));
+```
+
+A cast truncates. So a float that lands a hair *below* its Q16 neighbour loses
+a whole LSB rather than arriving at it, and the worst case of the conversion is
+one full step instead of half a step. `Resampler` reaches the same arithmetic by
+another road — `calculate_rate` in `shared-module/audiospeed/Resampler.c:11`
+casts the same way — so binding a ratio from two sample rates truncates too.
+
+Measured on `bin/circuitpython`, whose `shared-module/audiospeed/` is stock
+10.3.0, and reproduced identically on this port before the fix:
+
+| asked for | upstream's Q16 | wanted |
+|---|---:|---:|
+| `0.5 - 1e-6` | 32767 | 32768 |
+| `1.0/4.0000000000000036` | 16383 | 16384 |
+| `1.0/1.0000000000000004` | 65535 | 65536 |
+| `48000/44100`, through a `Resampler` binding | 71331 | 71332 |
+
+The third row is the one that bites. A class asking for "the hold rate = the
+running rate" computes `fs / rate_hz` in Python float, gets 1.0000000000000004
+off a log-mapped knob, and the `SpeedChanger` pair built from it is **not** an
+identity — at 44.1 kHz only, because 48 and 22.05 kHz happen to land on 1.0. A
+full-scale 441 Hz tone through that pair read **max |wet − dry| = 27666 codes**
+where a wire reads 0.
+
+**This port rounds**: `src/audiospeed/SpeedChanger.c` (`rate_to_fp`),
+`src/audiospeed/Resampler.c` (`audiospeed_resampler_set_sample_rate`) and
+`src/cpython/audiospeed.py` (the `rate` setter and `_bind_sample_rate`). So a
+rate strictly between two Q16 steps renders different bytes here and on
+CircuitPython 10.3.0, and ours is the arithmetically closer one. A rate already
+*on* a step is untouched by the change, which is why no stored golden moved:
+`resampler_probe.py` only ever asks for 2.0, 1.0 and 0.5.
+
+`tests/parity/speedchanger_hold_probe.py` renders the divergence and
+`verify_dsp` carries a stated skip for it on circuitpython naming both issues,
+so the three-way is not red on a departure that is deliberate. It is also where
+a MicroPython build that predates these two fixes shows up: any such binary
+renders CircuitPython's bytes here to the byte, because the port was faithful.
+
+Found by the effects program's `Bitcrusher` (audiocomponents#71) and reported
+in [upstream-reports/speedchanger-rate-rounding.md](upstream-reports/speedchanger-rate-rounding.md).
+
+## `audiospeed`: the phase accumulator crosses a source buffer, upstream's restarts (audioif#91)
+
+`audiospeed_fetch_source_buffer` calls `audiospeed_reset_phase` when it takes a
+new buffer from the source (`shared-module/audiospeed/__init__.c:87`, and the
+same line in `__init__.h:39-41`), so whatever fraction — or whole number — of a
+source frame the accumulator was carrying is thrown away at every buffer
+boundary. Only the very first frame of a buffer is ever the right one to resume
+on, and it is right only when the rate divides the buffer length exactly.
+
+What that does to a `SpeedChanger` used as a sample-and-hold, which is how the
+palette builds a lo-fi rate reducer (decimate at N, restore at 1/N):
+
+| measured on the CPython twin, 256-frame source buffers unless said otherwise | upstream | this port |
+|---|---:|---:|
+| the same hold rendered over 64-, 100-, 256- and 1000-frame buffers, frames differing from the 64-frame render, N = 1.8433 | 8023, 8077, 8076 of 8192 | 0, 0, 0 |
+| frames differing from `source[(((n·up)>>16)·down)>>16]` over 16384, N = 1.8433 / 6.0 / 2.5 | 15657 / 16066 / 16090 | 0 / 0 / 0 |
+| run lengths outside the hold's own alphabet `{2, 3}`, N = 2.5 | 12 | 0 |
+| a 1 kHz image under a 7 kHz tone held at 8 kHz, where a zero-order hold puts it at −0.22 dB | −39.86 dB | −0.22 dB |
+| worst miss against `20·log₁₀|sinc(f·T)|` over 500 Hz … 5 kHz, N = 1.8433 | 1.41 dB | 0.02 dB |
+| a full-scale ramp of one code per frame, lag after 65536 frames, at 48 / 44.1 kHz | 73 / 430 codes | 0 / 1 codes |
+
+Two ratios see none of it and are worth naming, because they are why the gate
+was green: at N = 4 over 256-frame buffers the accumulator lands exactly on the
+boundary every time and upstream renders the right answer, and `resampler_probe`
+only ever asks for 2.0, 1.0 and 0.5, which do the same.
+
+**This port carries the remainder across**, `src/audiospeed/SpeedChanger.c` and
+`src/cpython/audiospeed.py`: `fetch_source_buffer` subtracts the frames the
+*previous* buffer held rather than zeroing, and `get_buffer` pulls buffers until
+the index the accumulator names lands inside one, instead of assuming it lands
+on frame 0. `reset_buffer` still zeroes, because a reset resets the source too
+and the stream genuinely restarts there.
+
+Two things the carry made necessary and that upstream does not have. A source
+handing back a buffer shorter than one frame is now treated as exhausted; with
+the carry, upstream's `src_index = 0` against a zero-frame buffer would be an
+unbounded loop rather than the out-of-bounds read it is today. And the
+subtraction is guarded (`phase >= consumed`), so the accumulator cannot wrap
+below zero. Neither changes anything for a well-formed source.
+
+Upstream's `phase` is `uint32_t` in Q16, so a source buffer longer than 65535
+frames cannot be indexed at all. That limit is upstream's and this change
+neither widens nor narrows it.
+
+Found by the effects program's `Bitcrusher` (audiocomponents#71) and reported
+in [upstream-reports/speedchanger-phase-carry.md](upstream-reports/speedchanger-phase-carry.md).
+
 ## `audiodelays.Flanger`: we do not reproduce upstream's int32 overflow (audioif#76)
 
 CircuitPython 10.3.0's `shared-module/audiodelays/Flanger.c:365` computes the

docs/upstream-reports/README.md

@@ -1,5 +1,14 @@
 # Drafts for adafruit/circuitpython
 
+Two later drafts are **not filed yet** and are held with the rest. Both are in
+`audiospeed`, which is new in 10.3.0, and both were measured on a build of that
+tag rather than on this port (see "Provenance of the numbers").
+
+| draft | what | shape of the fix |
+|---|---|---|
+| [speedchanger-rate-rounding.md](speedchanger-rate-rounding.md) | the 16.16 rate truncates, so `1/1.0000000000000004` is 65535/65536 and a `SpeedChanger` pair asked for unity is not one | one character, twice |
+| [speedchanger-phase-carry.md](speedchanger-phase-carry.md) | the phase accumulator is zeroed at every source buffer, so what the node renders depends on the block size above it: 1947 frames of 2048 differ between a 64- and a 256-frame source | a few lines |
+
 Six bugs this port found in CircuitPython, written up as issue bodies.
 **Filed 2026-08-28** (all re-verified by inspection of `10.3.0-rc.0` first):
 

docs/upstream-reports/speedchanger-phase-carry.md

@@ -0,0 +1,190 @@
+# Draft: `audiospeed` throws away its phase at every source buffer
+
+**Note to the poster — strip everything above the `---`.**
+
+The larger of the two `audiospeed` drafts, and the one with an audible symptom.
+Independent of [speedchanger-rate-rounding.md](speedchanger-rate-rounding.md) —
+different lines of the same file — but the repro reads better with that one
+applied, because otherwise the rates in it are a hair off as well. Suggested
+title:
+
+> `audiospeed_fetch_source_buffer` resets the phase, so a `SpeedChanger` drifts
+> and only renders correctly when the rate divides the buffer length
+
+Verified present on `10.3.0` 2026-09-17 on a build of that tag with
+`CIRCUITPY_AUDIOSPEED` on. The fix was verified by applying it to a port of the
+same code: every measurement below goes to its ideal value, and the node's
+output stops depending on the source's block size at all.
+
+---
+
+### `audiospeed_fetch_source_buffer` resets the phase, so a `SpeedChanger` drifts and only renders correctly when the rate divides the buffer length
+
+`shared-module/audiospeed/__init__.c`:
+
+```c
+bool audiospeed_fetch_source_buffer(audiospeed_base_t *self) {
+    ...
+    self->src_sample_count = len / bytes_per_frame;
+    self->source_done = (result == GET_BUFFER_DONE);
+    // Reset phase to index within this new buffer
+    audiospeed_reset_phase(&self->speed);
+    return true;
+}
+```
+
+and `audiospeed_get_buffer` assumes that reset a few lines later:
+
+```c
+            if (src_index >= self->src_sample_count) {
+                ...
+                if (!audiospeed_fetch_source_buffer(self)) {
+                    break;
+                }
+                src_index = 0; // phase was reset by fetch
+            }
+```
+
+The accumulator is in units of **source frames since the stream began**, not
+since this buffer began, so the remainder it is holding when a buffer runs out
+belongs to the next one. Zeroing it discards that remainder.
+
+A buffer boundary is only ever the right place to resume from frame 0 when the
+rate divides the buffer length exactly. At `rate` 1.8433 with a 256-frame
+upstream, 0.2 of a frame is dropped every 139 output frames; at `rate` 6 with
+the same upstream, four whole frames are dropped every 43.
+
+#### What it sounds like
+
+A `SpeedChanger` pair used as a sample-and-hold — decimate at N, restore at 1/N,
+which is how a lo-fi rate reducer is built out of this module — is the clearest
+case, because a hold has an exactly known answer.
+
+Measured with a 256-frame source, everything else at defaults:
+
+| | 10.3.0 | with the fix |
+|---|---:|---:|
+| the same hold over 64-, 100-, 256- and 1000-frame source buffers: frames differing from the 64-frame render, of 8192, N = 1.8433 | 8023, 8077, 8076 | 0, 0, 0 |
+| frames differing from `source[(((n·up)>>16)·down)>>16]`, of 16384, at N = 1.8433 / 6.0 / 2.5 | 15657 / 16066 / 16090 | 0 / 0 / 0 |
+| a 1 kHz image under a 7 kHz tone held at 8 kHz, where a zero-order hold puts it at −0.22 dB | −39.86 dB | −0.22 dB |
+| worst miss against `20·log₁₀\|sinc(f·T)\|` from 500 Hz to 5 kHz, N = 1.8433 | 1.41 dB | 0.02 dB |
+| a full-scale ramp of one code per frame: lag after 65536 frames at 48 / 44.1 kHz | 73 / 430 codes | 0 / 1 |
+
+The first row is the property worth keeping: **what a `SpeedChanger` renders
+should not depend on how the node above it happens to chunk its output**, and
+today it depends on it almost entirely. The rest follow from it — the staircase
+restarts 187 times a second at 48 kHz, so the held waveform is not the hold that
+was asked for, its images are spread instead of placed, and the stream drifts
+against everything playing beside it.
+
+A rate that divides the buffer length is exempt, which is why this is easy to
+miss: at N = 4 over 256-frame buffers, and at the 2.0 / 1.0 / 0.5 a `Resampler`
+is usually bound to, the accumulator lands on the boundary every time and the
+output is correct.
+
+#### Fix
+
+Subtract what the previous buffer held instead of zeroing, and let the caller
+ask the accumulator where it is rather than assuming frame 0:
+
+```diff
+ bool audiospeed_fetch_source_buffer(audiospeed_base_t *self) {
+     ...
+-    self->src_sample_count = len / bytes_per_frame;
+-    self->source_done = (result == GET_BUFFER_DONE);
+-    // Reset phase to index within this new buffer
+-    audiospeed_reset_phase(&self->speed);
++    // The frames just consumed are the ones the previous buffer held; the
++    // remainder belongs to this one.
++    audiospeed_consume_frames(&self->speed, self->src_sample_count);
++    self->src_sample_count = len / bytes_per_frame;
++    self->source_done = (result == GET_BUFFER_DONE);
+     return true;
+ }
+```
+
+with, in `__init__.h`:
+
+```c
+static inline void audiospeed_consume_frames(audiospeed_speed_t *self, uint32_t frames) {
+    uint32_t consumed = frames << SPEED_SHIFT;
+    self->phase = self->phase >= consumed ? self->phase - consumed : 0;
+}
+```
+
+and, in both arms of `audiospeed_get_buffer`, pulling until the index lands
+inside a buffer instead of assuming one pull is enough — at a rate above 1.0 the
+carry can be several frames, and the next buffer is free to be shorter than
+that:
+
+```diff
+-            uint32_t src_index = audiospeed_get_index(&self->speed);
+-            if (src_index >= self->src_sample_count) {
++            while (audiospeed_get_index(&self->speed) >= self->src_sample_count) {
+                 if (self->source_done) {
+                     self->source_exhausted = true;
+                     break;
+                 }
+                 if (!audiospeed_fetch_source_buffer(self)) {
+                     break;
+                 }
+-                src_index = 0; // phase was reset by fetch
+             }
++            if (audiospeed_get_index(&self->speed) >= self->src_sample_count) {
++                break;
++            }
++            uint32_t src_index = audiospeed_get_index(&self->speed);
+```
+
+One guard goes with it. `audiospeed_fetch_source_buffer` tests `len == 0`; with
+the carry it needs `len < bytes_per_frame`, because a buffer holding no whole
+frame can never advance the index. Today that case is an unconditional
+`src_index = 0` against a zero-frame buffer, which reads off the end of it, so
+the guard is worth having either way.
+
+`audiospeed_reset_buffer` should keep zeroing the phase: it resets the source as
+well, so the stream genuinely restarts there.
+
+#### Repro
+
+```python
+# What a SpeedChanger renders should not depend on the block size of the node
+# above it. Two identical holds, two different upstream buffer sizes.
+import array, math
+import audiocore, audiofilters, audiospeed
+
+RATE, FRAMES, DOWN = 48000, 4096, 1.8433
+
+values = array.array("h", bytes(FRAMES * 4))
+for frame in range(FRAMES):
+    value = int(round(16384 * math.sin(2 * math.pi * 100 * frame / RATE)))
+    values[frame * 2] = values[frame * 2 + 1] = value
+
+
+def hold(block):
+    wire = audiofilters.Filter(sample_rate=RATE, channel_count=2,
+                               buffer_size=block * 4)
+    wire.play(audiocore.RawSample(values, sample_rate=RATE, channel_count=2))
+    node = audiospeed.SpeedChanger(
+        audiospeed.SpeedChanger(wire, DOWN), 1.0 / DOWN)
+    out = array.array("h")
+    while len(out) < FRAMES * 2:
+        out.extend(array.array("h", bytes(audiocore.get_buffer(node)[1])))
+    return [out[index * 2] for index in range(FRAMES // 2)]
+
+
+small, large = hold(64), hold(256)
+wrong = sum(1 for a, b in zip(small, large) if a != b)
+print("%d of %d frames differ between a 64- and a 256-frame source"
+      % (wrong, len(small)))
+```
+
+```
+1947 of 2048 frames differ between a 64- and a 256-frame source
+```
+
+With the fix applied it prints `0 of 2048`.
+
+`audiocore.get_buffer` is not upstream; on a board the same two graphs played
+through an `AudioOut` differ audibly, since one of them is not the hold that was
+asked for.