From d111081366810859b691f1b0354c76b1044ef6c6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Lucas?= <55464917+jlucaso1@users.noreply.github.com> Date: Sat, 25 Jul 2026 17:20:25 -0300 Subject: [PATCH 1/2] chore(voip/mlow): make the decoder cross-check vectors regenerable The MLow fixtures were documented recipes pointing at binaries that live on nobody's machine. PROVENANCE.md described each oracle in prose, but the harnesses themselves were never in the tree, so refreshing a vector meant rediscovering how to drive the C reference. That cost real time: the encoder fails every call with SMPL_ENC_NO_GLOBAL_DATA unless smpl_CreateCodec() runs first, and opus_encode flattens that into a bare OPUS_INTERNAL_ERROR with no hint of the cause. Vendor the harness and a script that drives it. Running the script against a built reference reproduces the committed 120 ms fixtures byte for byte, so a regeneration that changes them is a real change rather than tool drift. Add a tripwire for the two halves of that vector. They come out of one run and mean nothing apart: refreshing only one leaves the comparison reading mismatched data, which shows up as a correlation number that moved rather than as an obvious failure. The test pins the PCM length against the frame count and pins that every frame is still the TOC the fixture exists to cover. Record honestly what the harness does NOT reproduce. Run at 60 ms with DTX off it emits the same TOC mix as the older capture fixture, but not the same bytes: packet sizes bracket the committed ones without landing on them, so the original encoder configuration was never recorded. Regenerating those would replace the vectors rather than reproduce them. --- scripts/mlow-vectors/mlow_frames.c | 130 ++++++++++++++++++++ scripts/regenerate-mlow-vectors.sh | 69 +++++++++++ wacore/src/voip/mlow/decoder.rs | 27 ++++ wacore/src/voip/mlow/testdata/PROVENANCE.md | 37 ++++-- 4 files changed, 252 insertions(+), 11 deletions(-) create mode 100644 scripts/mlow-vectors/mlow_frames.c create mode 100755 scripts/regenerate-mlow-vectors.sh diff --git a/scripts/mlow-vectors/mlow_frames.c b/scripts/mlow-vectors/mlow_frames.c new file mode 100644 index 000000000..c8d96fc8d --- /dev/null +++ b/scripts/mlow-vectors/mlow_frames.c @@ -0,0 +1,130 @@ +// Encode a raw PCM file as MLow/smpl packets and decode each one back through the same +// reference, so a single run produces both halves of a decoder cross-check vector. +// +// Output, one line per packet, on stdout: +// +// +// +// Diagnostics (packet index, byte count, TOC, sample count) go to stderr, so stdout can be +// redirected straight into a fixture builder. +// +// Two things are easy to get wrong and cost an afternoon each: +// +// * smpl_CreateCodec() must run before the first encode. Without it every encode fails with +// SMPL_ENC_NO_GLOBAL_DATA (-112), which opus_encode flattens into a bare OPUS_INTERNAL_ERROR +// (-3) with no hint that global tables were the problem. +// * The output buffer handed to opus_encode is a hard bound, not a capacity. Under CBR the +// encoder pads to exactly that size, so an oversized buffer fails the pad. Keep it near the +// real packet size. + +#include +#include +#include + +#include "opus.h" + +// Declared here rather than pulled from a private header: the reference exposes it as a plain +// symbol and the harness only needs the one entry point. +extern int smpl_CreateCodec(void); + +#define FS 16000 +#define MAX_PACKET 400 + +static void usage(const char *argv0) { + fprintf(stderr, + "usage: %s [packets] [bitrate]\n" + " input.raw s16le mono @ 16 kHz\n" + " frame_ms 10, 20, 60 or 120\n" + " packets how many to emit (default: until EOF)\n" + " bitrate encoder bitrate in bps (default 20000)\n", + argv0); +} + +int main(int argc, char **argv) { + if (argc < 3) { + usage(argv[0]); + return 2; + } + const char *path = argv[1]; + const int frame_ms = atoi(argv[2]); + const int want = argc > 3 ? atoi(argv[3]) : -1; + const int bitrate = argc > 4 ? atoi(argv[4]) : 20000; + + if (frame_ms != 10 && frame_ms != 20 && frame_ms != 60 && frame_ms != 120) { + fprintf(stderr, "error: frame_ms must be 10, 20, 60 or 120 (got %d)\n", frame_ms); + return 2; + } + const int samps = FS / 1000 * frame_ms; + + if (smpl_CreateCodec() != 0) { + fprintf(stderr, "error: smpl_CreateCodec failed\n"); + return 1; + } + + FILE *f = fopen(path, "rb"); + if (!f) { + perror("open input"); + return 2; + } + + int err = 0; + OpusEncoder *enc = opus_encoder_create(FS, 1, OPUS_APPLICATION_VOIP, &err); + if (err != OPUS_OK || !enc) { + fprintf(stderr, "error: encoder create: %s\n", opus_strerror(err)); + return 1; + } + opus_encoder_ctl(enc, OPUS_SET_USING_SMPL(1)); + opus_encoder_ctl(enc, OPUS_SET_BITRATE(bitrate)); + opus_encoder_ctl(enc, OPUS_SET_FORCE_CHANNELS(1)); + + OpusDecoder *dec = opus_decoder_create(FS, 1, &err); + if (err != OPUS_OK || !dec) { + fprintf(stderr, "error: decoder create: %s\n", opus_strerror(err)); + return 1; + } + opus_decoder_ctl(dec, OPUS_SET_USING_SMPL(1)); + + opus_int16 *pcm = malloc((size_t)samps * sizeof(opus_int16)); + opus_int16 *out = malloc((size_t)samps * sizeof(opus_int16)); + unsigned char packet[MAX_PACKET]; + if (!pcm || !out) { + fprintf(stderr, "error: out of memory\n"); + return 1; + } + + int emitted = 0; + for (int n = 0; want < 0 || n < want; n++) { + if (fread(pcm, sizeof(opus_int16), (size_t)samps, f) != (size_t)samps) { + break; + } + opus_int32 len = opus_encode(enc, pcm, samps, packet, sizeof(packet)); + if (len <= 0) { + fprintf(stderr, "error: encode packet %d: %s\n", n, opus_strerror(len)); + return 1; + } + int ns = opus_decode(dec, packet, len, out, samps, 0); + if (ns <= 0) { + fprintf(stderr, "error: decode packet %d: %s\n", n, opus_strerror(ns)); + return 1; + } + for (opus_int32 i = 0; i < len; i++) { + printf("%02x", packet[i]); + } + printf(" "); + for (int i = 0; i < ns; i++) { + unsigned short v = (unsigned short)out[i]; + printf("%02x%02x", v & 0xff, (v >> 8) & 0xff); + } + printf("\n"); + fprintf(stderr, "packet %d: %d bytes, TOC 0x%02x, %d samples\n", n, len, packet[0], ns); + emitted++; + } + + fprintf(stderr, "emitted %d packet(s) of %d ms\n", emitted, frame_ms); + free(pcm); + free(out); + opus_encoder_destroy(enc); + opus_decoder_destroy(dec); + fclose(f); + return emitted > 0 ? 0 : 1; +} diff --git a/scripts/regenerate-mlow-vectors.sh b/scripts/regenerate-mlow-vectors.sh new file mode 100755 index 000000000..c63888f6d --- /dev/null +++ b/scripts/regenerate-mlow-vectors.sh @@ -0,0 +1,69 @@ +#!/usr/bin/env bash +# Regenerate the MLow decoder cross-check fixtures from the `smpl` C reference. +# +# Consumers never run this — the fixtures are committed and the tests read them directly. It +# exists so the vectors stop being a documented recipe with a missing binary: everything needed +# to reproduce them, including the two non-obvious setup steps, lives in +# `scripts/mlow-vectors/mlow_frames.c`. +# +# Requires a built `smpl` reference (the WhatsApp MLow fork of libopus): +# +# cd "$MLOW_REFERENCE" && ./autogen.sh && ./configure --disable-shared --disable-doc \ +# --disable-extra-programs && make -j"$(nproc)" +# +# Then: +# +# MLOW_REFERENCE=/path/to/opus_mlow scripts/regenerate-mlow-vectors.sh +# +# Regenerating changes committed fixtures. Re-run the decoder suite afterwards and treat any +# correlation change as a finding, not as a number to paper over: +# +# cargo test -p wacore --features voip-mlow --lib + +set -euo pipefail + +repo_root="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +testdata="$repo_root/wacore/src/voip/mlow/testdata" +harness_src="$repo_root/scripts/mlow-vectors/mlow_frames.c" + +ref="${MLOW_REFERENCE:-}" +if [[ -z "$ref" ]]; then + echo "error: set MLOW_REFERENCE to the built smpl/opus reference checkout" >&2 + exit 1 +fi +if [[ ! -f "$ref/.libs/libopus.a" ]]; then + echo "error: $ref/.libs/libopus.a not found; build the reference first (see the header of this script)" >&2 + exit 1 +fi + +work="$(mktemp -d)" +trap 'rm -rf "$work"' EXIT + +echo "==> building the harness against $ref" +cc -O2 \ + -I "$ref/include" -I "$ref/src" -I "$ref/celt" -I "$ref" \ + -o "$work/mlow_frames" "$harness_src" "$ref/.libs/libopus.a" -lm + +# One packet per line: " ". Split into the frames fixture and the +# reference PCM the decoder test compares against. +emit() { + local frame_ms="$1" packets="$2" frames_json="$3" pcm_raw="$4" + echo "==> ${frame_ms}ms x ${packets} -> $(basename "$frames_json"), $(basename "$pcm_raw")" + "$work/mlow_frames" "$testdata/synth_mic.raw" "$frame_ms" "$packets" > "$work/vectors.txt" + python3 - "$work/vectors.txt" "$frames_json" "$pcm_raw" <<'PY' +import json, sys +src, frames_path, pcm_path = sys.argv[1:4] +frames, pcm = [], bytearray() +for line in open(src): + payload, samples = line.split() + frames.append(payload) + pcm += bytes.fromhex(samples) +json.dump(frames, open(frames_path, "w"), indent=1) +open(pcm_path, "wb").write(pcm) +print(f" {len(frames)} frames, {len(pcm)//2} samples, TOCs: {sorted({f[:2] for f in frames})}") +PY +} + +emit 120 8 "$testdata/mlow_120ms_frames.json" "$testdata/ref_120ms_expected.raw" + +echo "==> done; re-run: cargo test -p wacore --features voip-mlow --lib" diff --git a/wacore/src/voip/mlow/decoder.rs b/wacore/src/voip/mlow/decoder.rs index c699ec47e..293e2df61 100644 --- a/wacore/src/voip/mlow/decoder.rs +++ b/wacore/src/voip/mlow/decoder.rs @@ -436,6 +436,33 @@ mod tests { assert!(!dec.had_error(), "the drop must not open the range decoder"); } + /// The two halves of a cross-check vector come out of one harness run and mean nothing apart: + /// refreshing only one leaves the comparison reading mismatched data, which surfaces as a + /// correlation number that moved rather than as an obvious error. Pin what ties them, and pin + /// that the fixture still exercises the multi-frame path it was added for. + #[test] + fn multi_frame_fixture_halves_stay_in_step() { + let frames: Vec = + serde_json::from_str(include_str!("testdata/mlow_120ms_frames.json")) + .expect("mlow_120ms_frames.json"); + let pcm_bytes = include_bytes!("testdata/ref_120ms_expected.raw").len(); + + assert!(!frames.is_empty(), "fixture is empty"); + for (i, f) in frames.iter().enumerate() { + let toc = hex::decode(f).expect("hex frame")[0]; + assert_eq!( + toc, 0x58, + "frame {i} is TOC {toc:#04x}, not the 120 ms packet this fixture exists to cover" + ); + } + assert_eq!( + pcm_bytes, + frames.len() * 6 * SMPL_INTF_LEN * 2, + "reference PCM does not match the frame count; regenerate both halves together with \ + scripts/regenerate-mlow-vectors.sh" + ); + } + /// The content check: decode a stream of real 120 ms packets and compare against the reference /// decoder's own output for the same bytes. Geometry alone is not enough, since running the loop /// the wrong number of times would still produce plausibly-shaped audio while consuming the diff --git a/wacore/src/voip/mlow/testdata/PROVENANCE.md b/wacore/src/voip/mlow/testdata/PROVENANCE.md index 6d516a261..19b2db523 100644 --- a/wacore/src/voip/mlow/testdata/PROVENANCE.md +++ b/wacore/src/voip/mlow/testdata/PROVENANCE.md @@ -79,17 +79,32 @@ exact wire bytes (config-1 `0x10` and config-2 `0x12` frames included). The trip committed stream still carries `0x10`, `0x12`, and `0x50` TOCs so the per-config decode branches stay covered; regenerating it requires the external encoder above on `synth_mic.raw`. -## Multi-frame (120 ms) packets +## Multi-frame (120 ms) packets — regenerable in one command | fixture | consumer / test | oracle recipe | | --- | --- | --- | -| `mlow_120ms_frames.json` | `decoder.rs::multi_frame_decode_matches_the_reference` | `smpl` C reference encoding `synth_mic.raw` at 120 ms, hex frames | -| `ref_120ms_expected.raw` | same test | the same C reference decoding those frames; s16le @ 16 kHz | - -Also not Rust-reproducible: this crate's encoder only emits 60 ms packets. Both files come from one -run of a harness linked against the `smpl` C reference, which encodes `synth_mic.raw` in 1920-sample -(120 ms) frames and decodes each packet back, emitting ` ` per line. The -encoder needs `smpl_CreateCodec()` before the first `opus_encode` (it fails -`SMPL_ENC_NO_GLOBAL_DATA` otherwise), `OPUS_SET_USING_SMPL(1)`, and a `max_data_bytes` the CBR pad -can satisfy. Every frame is TOC `0x58`, which the test asserts so the fixture cannot silently drift -off the multi-frame path. +| `mlow_120ms_frames.json` | `decoder.rs::multi_frame_decode_matches_the_reference` | `scripts/regenerate-mlow-vectors.sh` | +| `ref_120ms_expected.raw` | same test | same run of the same script | + +```sh +MLOW_REFERENCE=/path/to/opus_mlow scripts/regenerate-mlow-vectors.sh +``` + +The harness it builds lives in `scripts/mlow-vectors/mlow_frames.c`: it encodes `synth_mic.raw` at +the requested duration through the `smpl` C reference and decodes each packet back, emitting both +halves of the vector in one pass. Verified to reproduce the committed files byte for byte, so a +regeneration that changes them is a real change and not tool drift. + +Both halves must be regenerated together — `decoder.rs::multi_frame_fixture_halves_stay_in_step` +fails if the PCM length stops matching the frame count, and asserts every frame is still TOC `0x58` +so the fixture cannot quietly drift off the multi-frame path. + +## What is still not reproducible, and why + +The fixtures above the multi-frame section predate the harness and were produced by tools that no +longer exist here. The harness reproduces their SHAPE — run at 60 ms with DTX off it emits the same +TOC mix as `inbound_capture_frames.json` (13x `0x10`, 2x `0x12`, 95x `0x50`) — but not their exact +bytes: packet sizes bracket the committed ones without landing on them, so the original run used an +encoder configuration (or reference build) that was not recorded. Regenerating them would therefore +REPLACE those vectors rather than reproduce them, which is a deliberate decision to make with the +correlation thresholds in hand, not a mechanical refresh. From fb71117c3898f745384de152222f94e7411808f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Lucas?= <55464917+jlucaso1@users.noreply.github.com> Date: Sat, 25 Jul 2026 17:42:14 -0300 Subject: [PATCH 2/2] chore(voip/mlow): fail a short vector regeneration instead of committing it Review found the hole this tooling was supposed to close. The harness exited successfully whenever it emitted at least one packet, so a truncated input or a read error produced a shorter vector and the script overwrote both committed fixtures with it. The tripwire did not catch that either: it checked the PCM length against the frame count, a relative property that a shorter vector still satisfies. Treat a short count as the failure it is, distinguishing a read error from a clean EOF, and assert the absolute packet count in the tripwire so a shrunken fixture fails loudly. Also pin the oracle. The claim that a changed fixture means a real change only holds against the reference revision the bytes came from, which was never recorded; a different checkout could move the output and the text told the reader that could not happen. Name the revision and have the script report the one it built against, warning when they differ. --- scripts/mlow-vectors/mlow_frames.c | 25 ++++++++++++++++++++- scripts/regenerate-mlow-vectors.sh | 19 ++++++++++++++++ wacore/src/voip/mlow/decoder.rs | 9 +++++++- wacore/src/voip/mlow/testdata/PROVENANCE.md | 9 ++++++-- 4 files changed, 58 insertions(+), 4 deletions(-) diff --git a/scripts/mlow-vectors/mlow_frames.c b/scripts/mlow-vectors/mlow_frames.c index c8d96fc8d..0eb9cc08c 100644 --- a/scripts/mlow-vectors/mlow_frames.c +++ b/scripts/mlow-vectors/mlow_frames.c @@ -93,8 +93,16 @@ int main(int argc, char **argv) { } int emitted = 0; + int short_read = 0; for (int n = 0; want < 0 || n < want; n++) { if (fread(pcm, sizeof(opus_int16), (size_t)samps, f) != (size_t)samps) { + // A partial frame at EOF, or a read error. Either way this run cannot produce the + // vector that was asked for, and callers overwrite committed fixtures with whatever + // comes out of it -- so remember it and fail below rather than emit a short vector. + short_read = 1; + if (ferror(f)) { + perror("read input"); + } break; } opus_int32 len = opus_encode(enc, pcm, samps, packet, sizeof(packet)); @@ -121,10 +129,25 @@ int main(int argc, char **argv) { } fprintf(stderr, "emitted %d packet(s) of %d ms\n", emitted, frame_ms); + + // Exiting 0 with fewer packets than requested would let a regeneration quietly replace a + // fixture with a smaller one: the relative length checks downstream stay satisfied, so the + // coverage loss is invisible. A short count is a failure, not a partial success. + int ok = 1; + if (emitted == 0) { + fprintf(stderr, "error: no packets emitted\n"); + ok = 0; + } else if (want >= 0 && emitted != want) { + fprintf(stderr, + "error: asked for %d packet(s) but the input only yielded %d%s\n", + want, emitted, short_read ? " (short read)" : ""); + ok = 0; + } + free(pcm); free(out); opus_encoder_destroy(enc); opus_decoder_destroy(dec); fclose(f); - return emitted > 0 ? 0 : 1; + return ok ? 0 : 1; } diff --git a/scripts/regenerate-mlow-vectors.sh b/scripts/regenerate-mlow-vectors.sh index c63888f6d..d663f6efc 100755 --- a/scripts/regenerate-mlow-vectors.sh +++ b/scripts/regenerate-mlow-vectors.sh @@ -6,6 +6,14 @@ # to reproduce them, including the two non-obvious setup steps, lives in # `scripts/mlow-vectors/mlow_frames.c`. # +# The committed fixtures were produced with this exact oracle: +# +# https://github.com/edgardmessias/opus_mlow at 84b076e0809412df22e8a0d26f944610c4a3e40f +# +# Byte-for-byte reproduction is only claimed against that revision. A different checkout can +# change the output because the ORACLE changed, which is not the same thing as a decoder change, +# so the script prints the revision it actually built against and warns when it differs. +# # Requires a built `smpl` reference (the WhatsApp MLow fork of libopus): # # cd "$MLOW_REFERENCE" && ./autogen.sh && ./configure --disable-shared --disable-doc \ @@ -36,6 +44,17 @@ if [[ ! -f "$ref/.libs/libopus.a" ]]; then exit 1 fi +# Recorded so a regeneration says which oracle produced it; see the header. +expected_rev="84b076e0809412df22e8a0d26f944610c4a3e40f" +actual_rev="$(git -C "$ref" rev-parse HEAD 2>/dev/null || echo unknown)" +if [[ "$actual_rev" == "unknown" ]]; then + echo "warning: $ref is not a git checkout; cannot confirm the oracle revision" >&2 +elif [[ "$actual_rev" != "$expected_rev" ]]; then + echo "warning: oracle is $actual_rev, fixtures were generated with $expected_rev" >&2 + echo " differences below may come from the reference, not from this repository" >&2 +fi +echo "==> oracle: $ref @ $actual_rev" + work="$(mktemp -d)" trap 'rm -rf "$work"' EXIT diff --git a/wacore/src/voip/mlow/decoder.rs b/wacore/src/voip/mlow/decoder.rs index 293e2df61..1e9785501 100644 --- a/wacore/src/voip/mlow/decoder.rs +++ b/wacore/src/voip/mlow/decoder.rs @@ -447,7 +447,14 @@ mod tests { .expect("mlow_120ms_frames.json"); let pcm_bytes = include_bytes!("testdata/ref_120ms_expected.raw").len(); - assert!(!frames.is_empty(), "fixture is empty"); + // An absolute count, not just "non-empty": a regeneration that ends early produces a + // shorter vector whose PCM length still matches its own frame count, so a relative check + // would accept it and the coverage loss would be invisible. + assert_eq!( + frames.len(), + 8, + "fixture no longer holds the 8 packets it was generated with; a short regeneration silently reduces coverage" + ); for (i, f) in frames.iter().enumerate() { let toc = hex::decode(f).expect("hex frame")[0]; assert_eq!( diff --git a/wacore/src/voip/mlow/testdata/PROVENANCE.md b/wacore/src/voip/mlow/testdata/PROVENANCE.md index 19b2db523..423e25d7d 100644 --- a/wacore/src/voip/mlow/testdata/PROVENANCE.md +++ b/wacore/src/voip/mlow/testdata/PROVENANCE.md @@ -92,8 +92,13 @@ MLOW_REFERENCE=/path/to/opus_mlow scripts/regenerate-mlow-vectors.sh The harness it builds lives in `scripts/mlow-vectors/mlow_frames.c`: it encodes `synth_mic.raw` at the requested duration through the `smpl` C reference and decodes each packet back, emitting both -halves of the vector in one pass. Verified to reproduce the committed files byte for byte, so a -regeneration that changes them is a real change and not tool drift. +halves of the vector in one pass. + +The committed bytes were produced against one specific oracle — +`github.com/edgardmessias/opus_mlow` at `84b076e0809412df22e8a0d26f944610c4a3e40f`. Reproduction is +byte for byte **against that revision**, which is what makes a changed fixture a real change rather +than tool drift; against a different checkout the reference itself may have moved, so the script +prints the revision it built with and warns when it does not match. Both halves must be regenerated together — `decoder.rs::multi_frame_fixture_halves_stay_in_step` fails if the PCM length stops matching the frame count, and asserts every frame is still TOC `0x58`