fix(voip/mlow): multi-frame admission, coded-inactive decode, malformed-frame concealment, and regenerable vectors - #1111
fix(voip/mlow): multi-frame admission, coded-inactive decode, malformed-frame concealment, and regenerable vectors#1111jlucaso1 wants to merge 18 commits into
Conversation
A 1:1 call whose peer is WhatsApp Desktop was silent. That client emits 120 ms packets (TOC 0x58) routinely, and the decoder discarded every one of them: decode_active_frame ran a hardcoded three-iteration loop, so the operating-point guard rejected any duration other than 60 ms and returned a silence slot. Nothing surfaced as an error, because a dropped frame is indistinguishable from a peer that is not speaking. A packet is not one unit of decode. The reference derives the loop count from the declared duration while the geometry inside each iteration is unchanged: num_frames = (packet_len_ms + 10) / 20, with the internal frame length and subframe count fixed for anything above 10 ms. So 20, 60 and 120 ms differ only in how many times the same decode repeats, and the frame index only ever distinguishes the first internal frame from the rest. Derive the count from the TOC and drive the loop, the output length, the lag buffer and the postfilter average from it. This admits 20 and 120 ms as well as the 60 ms that already worked. 10 ms stays rejected: it halves both the internal frame length and the subframe count, a geometry the synthesis does not implement, and decoding it would consume the payload at the wrong symbol count and desync the range coder. Two existing tests used 20 ms as their example of a dropped frame and now use 10 ms and a 32 kHz TOC instead. That is a deliberate contract change, not a convenience edit: 20 ms decodes now.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughMLow decoding now supports active 20 ms, 60 ms, and 120 ms packets by chaining internal 20 ms frames. Unsupported active durations are silenced before range decoding, playout scales to packet size, and tests and tooling validate multi-frame reference alignment. ChangesVariable MLow decoding
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant RTPPeer
participant MlowDecoder
participant RangeCoder
participant PlayoutBuffer
RTPPeer->>MlowDecoder: provide packet and TOC duration
MlowDecoder->>MlowDecoder: compute internal frame count
MlowDecoder->>RangeCoder: decode supported chained frames
MlowDecoder->>PlayoutBuffer: store decoded PCM and packet sample count
PlayoutBuffer-->>RTPPeer: emit packet-sized playout audio
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
| Filename | Overview |
|---|---|
| wacore/src/voip/mlow/decoder.rs | Adds variable-duration decoding, coded-inactive handling, endpoint validation, and rollback concealment; the previously reported Desktop 120 ms silence remains. |
| wacore/src/voip/engine.rs | Derives jitter-buffer targets and ceilings from the decoder’s declared packet duration. |
| wacore/src/voip/mlow/rangecoder.rs | Exposes range-decoder storage and consumption information used by endpoint validation. |
| wacore/src/voip/mlow/smpl_decode.rs | Gates symbols that are absent from coded-inactive frames. |
| scripts/regenerate-mlow-vectors.sh | Adds pinned, reproducible generation of MLow cross-check fixtures. |
| scripts/mlow-vectors/mlow_frames.c | Adds the reference encoder/decoder harness used by fixture regeneration. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart LR
RTP[Inbound MLow RTP payload] --> TOC[Parse declared duration and mode]
TOC --> Decode[Decode internal frames]
Decode --> Endpoint{Endpoint valid?}
Endpoint -->|Yes| PCM[Decoded PCM]
Endpoint -->|No| Rollback[Restore decoder state]
Rollback --> Silence[Full-duration concealed silence]
PCM --> Jitter[Playout jitter buffer]
Silence --> Jitter
Jitter --> Speaker[20 ms playout slices]
Reviews (11): Last reviewed commit: "fix(voip): take the endpoint window from..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
📦 Binary size report
.text per crate
Baseline: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@wacore/src/voip/mlow/testdata/PROVENANCE.md`:
- Around line 81-95: Add a concise note to the “Multi-frame (120 ms) packets”
provenance section stating that the committed `mlow_120ms_frames.json` and
`ref_120ms_expected.raw` fixtures are an intentional truncated prefix/subset of
the full regeneration output, which may contain approximately 55 frames.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 80934515-007b-4aab-b0e7-46389ed938bd
📒 Files selected for processing (4)
wacore/src/voip/mlow/decoder.rswacore/src/voip/mlow/testdata/PROVENANCE.mdwacore/src/voip/mlow/testdata/mlow_120ms_frames.jsonwacore/src/voip/mlow/testdata/ref_120ms_expected.raw
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f1e719112d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
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.
…ing 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.
The top field of the smpl TOC is a two-bit SID/VoA selector, not a pair of flags. Only 10 is a SID; 00 means "normal frame sent while there is no voice activity", which is what a peer with DTX off sends through every pause. The decoder treated anything that was not active-voice as silence and returned a zero slot, so that background noise was dropped on the floor. Measured against the reference on a stream encoded with DTX off: the coded-inactive frames are 12% of the samples, where the reference produces 0.047 rms and we produced exactly 0. Overall correlation with the reference rises from 0.969 to 0.9999 once they decode. Two symbols are gated on the same flag in the reference and are absent from the bitstream when it is clear: the voicing symbol (such a frame is unvoiced by definition) and the LSF interpolation index. Reading either would consume symbols that were never written and desync every field after them, so the LSF decode takes the flag and skips them, and the pulse decode selects its table with the real value rather than a hardcoded 1. Two existing tests changed, both because they pinned the old behavior: - the SID test used TOC 0x00, which is coded-inactive rather than SID, and now uses 0x80. What it asserts is unchanged. - decoder_silence_frames_produce_zero asserted over every near-silent frame in e2e_vectors.json. That fixture had its inactive-TOC frames ZEROED when it was generated, to match the behavior this commit removes, so those entries are not an oracle for silence. The assertion now covers real SID frames, and the coded-inactive path is covered against an unmodified vector instead.
Dismissed because a newer commit was pushed; Greptile will re-review the current head.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
jlucaso1
changed the title
Review caught an integration effect the decoder-level tests could not see. The prebuffer target and the latency ceiling were both written around a 60ms peer frame, which held while the decoder only ever produced those. A 120ms packet breaks both assumptions at once. One such packet is 1920 samples, which IS the prime target, so priming ended on the very first arrival with no cushion at all - the case the target's own comment describes as a zero cushion that underruns on any late packet. Worse, two in flight are 3840 samples against a 2400-sample ceiling, so the buffer was trimmed on arrival and dropped audio continuously rather than only under a burst. Derive both from what the peer actually sends: prime to two packets so the steady state never drains below one, and let the ceiling hold that cushion plus a drain slice. The 60ms case keeps its existing numbers exactly. Also note in the provenance that the 120ms fixture is an intentional 8-packet prefix; regenerating over the whole input yields ~55 and looks like a break.
Dismissed because a newer commit was pushed; Greptile will re-review the current head.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@wacore/src/voip/engine.rs`:
- Around line 64-67: Centralize the packet-sizing rationale in playout_bounds.
In wacore/src/voip/engine.rs lines 64-67, retain the baseline constant
definition and link to playout_bounds while removing duplicated scaling details;
at lines 473-475, document packet_samps and its initial value only; at lines
1202-1203, describe only the decoded-length assignment.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: b61f0423-5f9b-44ca-a829-d0ff24c74558
📒 Files selected for processing (2)
wacore/src/voip/engine.rswacore/src/voip/mlow/testdata/PROVENANCE.md
The scaling explanation belonged at playout_bounds; the constant, the field and the assignment now state only their local contract.
Dismissed because a newer commit was pushed; Greptile will re-review the current head.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6c31c4ef09
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Review caught a regression the previous commit introduced. The cushion learned the peer's packet size from the DECODED LENGTH, which is synthesized on several paths: a SID, a dropped frame and the standard-Opus escape all emit a fixed 60ms slot regardless of what the packet declared. So a 120ms peer entering DTX reported 960 samples, the ceiling fell from 4160 to 2400 while up to 4160 were queued, and the feed-side trim discarded buffered speech that had not been played yet - clipping the tail of every utterance at the DTX transition. Take the duration from the TOC instead. That is what governs arrival cadence, it is unaffected by what a given path chooses to emit, and a peer that really does move to 60ms is still learned.
Dismissed because a newer commit was pushed; Greptile will re-review the current head.
Dismissed because a newer commit was pushed; Greptile will re-review the current head.
|
Correction to my previous comment: when I wrote that the repro "is committed", it was not — it was still sitting uncommitted in my worktree. It is now, at Three commits just pushed:
The repro is |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 79ecd92e81
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
wacore/src/voip/mlow/decoder.rs (1)
122-179: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
last_packet_sampsgets set from a TOC that hasn't been validated yet — the guard can reject the very TOC you just trusted.Look, I care a lot about call quality staying rock-solid, and this is a real gap in that story.
self.last_packet_sampsis assigned fromtoc.sample_rate/toc.frame_msat Lines 127-129, unconditionally, before the operating-point guard at Lines 150-176 even runs. If the guard subsequently rejects the frame (bad rate,low_rate=1, or an unsupportedframe_ms), the decoder still emits a fixed 960-sample silence slot — butlast_packet_sampshas already been overwritten with the rejected frame's declared duration.Downstream in
engine.rs,pcm.packet_samps = pcm.decoder.last_packet_samps()feeds directly intoeffective_playout_cap. Since that function raises the cap immediately whenever the declared size grows (no backlog check for growth, only for shrink), a single corrupted/off-spec TOC that happens to declare a larger duration (e.g. a garbled 32kHz/120ms byte) can inflate the latency ceiling on the spot. It self-corrects on the next legitimate packet, so I'm calling this minor rather than a call-breaker — but a jitter buffer's cushion sizing shouldn't be swayed by a frame the decoder itself just declared undecodable.Scope the assignment to only run for frames that are actually going to be trusted (SID, std-opus escape, or a supported active geometry) — not for the guard-rejected case.
🛠️ Proposed fix
let toc = parse_mlow_toc(frame[0]); - if toc.frame_ms > 0 && toc.sample_rate > 0 { - self.last_packet_samps = (toc.sample_rate / 1000 * toc.frame_ms) as usize; - } + let declared_samps = (toc.frame_ms > 0 && toc.sample_rate > 0) + .then(|| (toc.sample_rate / 1000 * toc.frame_ms) as usize); if toc.std_opus { + if let Some(s) = declared_samps { + self.last_packet_samps = s; + } let out_len = (16000 / 1000 * toc.frame_ms) as usize; ... return vec![0.0; out_len]; } if toc.sid { + if let Some(s) = declared_samps { + self.last_packet_samps = s; + } ... return vec![0.0; OPUS_FRAME_SAMPS]; } let frames = internal_frames(toc.frame_ms); let off_point = ...; if let Some((dim, val)) = off_point { + // Do not trust this TOC's declared duration for cushion sizing: the guard just + // determined it isn't a geometry this decoder can decode. self.dropped_unsupported += 1; ... return vec![0.0; OPUS_FRAME_SAMPS]; } + if let Some(s) = declared_samps { + self.last_packet_samps = s; + } let frames = frames.expect("the guard above rejected every unsupported duration"); self.decode_active_frame(frame, frames * SMPL_INTF_LEN, frames, toc.active)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wacore/src/voip/mlow/decoder.rs` around lines 122 - 179, Move the last_packet_samps assignment in decode_frame so rejected active frames cannot update it. Preserve geometry updates for SID, standard-Opus escapes, and supported active frames, while ensuring the operating-point guard returns silence without changing the previously trusted packet sample count.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/regenerate-mlow-vectors.sh`:
- Around line 52-69: Update the script’s header comment block to document the
MLOW_ALLOW_DIRTY_REFERENCE environment variable, including that setting it to 1
permits fixture generation from a dirty oracle worktree. Keep the existing
behavior in the dirty-worktree guard unchanged.
In `@wacore/src/voip/engine.rs`:
- Around line 1220-1223: Replace the multi-line rationale above pcm.packet_samps
with a single-line pointer to MlowDecoder::last_packet_samps and its documented
declared-duration behavior; keep the assignment unchanged and do not duplicate
the SID/DTX explanation at this call site.
---
Outside diff comments:
In `@wacore/src/voip/mlow/decoder.rs`:
- Around line 122-179: Move the last_packet_samps assignment in decode_frame so
rejected active frames cannot update it. Preserve geometry updates for SID,
standard-Opus escapes, and supported active frames, while ensuring the
operating-point guard returns silence without changing the previously trusted
packet sample count.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: e2c41672-6d66-42ef-ba05-f1adeb0aa82e
📒 Files selected for processing (5)
scripts/regenerate-mlow-vectors.shwacore/src/voip/engine.rswacore/src/voip/mlow/decoder.rswacore/src/voip/mlow/testdata/PROVENANCE.mdwacore/src/voip/mlow/testdata/live_120ms_frames.json
The range decoder returns zero past either end of its storage without setting its error flag, by design. A malformed body therefore decodes into plausible looking symbols rather than failing, and the synthesis runs on invented parameters. On real 120 ms packets from a WhatsApp Desktop peer that diverged to full scale: 19 of 40 packets clipping, 4.21% of samples at rail. Audibly that is loud distortion where the peer is speaking. The reference guards this with an endpoint check the port never carried: a valid stream finishes within two bytes of its storage, and anything else is treated as a lost frame. Comparing where the decode ended against what the body held is the only way to see the overrun, precisely because the reads that cause it are silent. Add that check, before the postfilter as in the reference, and conceal a failing frame as silence. State already advanced during the decode is left alone, which is also what the reference does. This does not make those packets decode: under the entropy grammar implemented here every one of them is malformed, and the reference agrees. It replaces loud clipping with the concealment a malformed frame is supposed to get, and it makes a peer we cannot read visible in a log instead of silently wrong.
Dismissed because a newer commit was pushed; Greptile will re-review the current head.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3a736610de
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Two defects in the endpoint check, both from review. The sticky range-error flag became unreachable: the malformed branch returns before the assignment, and `dec.err != 0` is part of its own condition, so `had_error` reported a clean decode for exactly the range-decoder failures it documents and the regression suites read it for. Set it before returning. Concealment also left the damage behind. The overrun is only detectable after the last internal frame, by which point the loop has advanced the LSF predictor, the CELP history and prev_nlsf from parameters invented past the end of the bad body. The next packet was then synthesized partly from those: measured as full-scale samples and RMS 0.290 against 0.114 from a fresh decoder. Snapshot the state before the loop and restore it when concealing. This diverges from the reference, which does not roll back. The reference only ever decodes streams it produced itself; this decoder meets streams it cannot read, so the leak is reachable here and audible in the following frame. The copy costs a few KB once per packet against 20-120 ms of audio.
… rationale A clean worktree at the pinned revision still says nothing about the archive the harness links: switching revisions without rebuilding leaves a stale .a that links fine and attributes its fixtures to a commit it was never built from. Compare it against the checkout and refuse when it is older. Also document MLOW_ALLOW_DIRTY_REFERENCE in the script header, since a hard fail should not require reading the body to find its escape hatch, and trim the declared-duration rationale in the engine to a pointer at the field that owns it.
Dismissed because a newer commit was pushed; Greptile will re-review the current head.
Companion to the file removal, which landed a commit early by mistake. AGENTS.md forbids real PII in test vectors, including ones derived from production captures, and this document itself states that no captured call audio is present. The 120 ms fixture was compressed speech from a live peer: calling the words neutral removes the conversation, not the person's voice, which is what a working decoder would reconstruct. Committing it contradicted both. The code path keeps its coverage from synthetic bytes in endpoint_check_rejects_an_overrunning_body. The field evidence that real packets take that path belongs in the issue thread as text, not shipped with the repository as someone's voice.
jlucaso1
changed the title
|
All seven open review points actioned, PR body rewritten to match what this branch actually does now. Letting CI run the full suite rather than the local one. The PII finding was the important one and it was right.
Concealment was leaking state, and the measurement made that concrete. Snapshot before the loop, restore when concealing. This diverges from the reference, which does not roll back — the reference only decodes streams it produced itself, while this decoder meets streams it cannot read, so the leak is reachable here and audible in the following frame. Stale-archive check added: a clean worktree at the pinned revision says nothing about the On "fix live 120 ms decoding before enabling it" — that was correct against the revision it was written for, where these packets became loud noise. With the endpoint check they are concealed, so the outcome for those calls is the same silence as before this PR, reached by an integrity check rather than a blind drop. The decode itself is still unresolved and the PR body now says so under Still broken, including the offer to isolate the admission change if you would rather not ship it while the decode is open. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9fe07677e8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Review caught that the per-packet snapshot added for concealment was copying `dbg_exc_pre`, the test-only excitation capture. That buffer accumulates every synthesized subframe for the whole stream and is never cleared, so cloning it once per packet made decoding quadratic in stream length. The cost was not theoretical: the wacore suite went from 39s to 3332s, and I had already written the 56-minute figure off as the codec being heavy in debug. It was this. Hand-write Clone so the trace starts empty. It is a diagnostic capture, not codec state that concealment needs to restore.
…etadata `git reset --hard` on the same branch moves the branch ref and leaves .git/HEAD untouched, so the previous mtime check would accept an archive built at another revision and attribute its fixtures to the pinned commit. Compare the archive against the reference sources instead: one of them being newer is the direct statement that the archive does not correspond to the tree whose revision the script is about to record.
Dismissed because a newer commit was pushed; Greptile will re-review the current head.
|
Both actioned — and the trace one was a genuinely good catch, with a bigger effect than the comment claims. The snapshot was making decoding quadratic. Cloning
On the archive check, you are right that Full suite green at 1567 tests, and fast again. |
|
60 ms frames from a Desktop peer, as requested. They decode cleanly, so this is your second branch: the fault is specific to the 120 ms path, not latent in the entropy layer at every duration. One thing that makes the comparison stronger than I first realised, and worth stating precisely: both captures come from the same WhatsApp Desktop installation on the same machine. Only the logged-in account differs. So this is one encoder binary emitting 60 ms for one account and 120 ms for another — not two client builds. The capture40 consecutive active frames, all
The They decode cleanlySame rig, same recorder, same measurements as the 120 ms report:
264 frames decoded across the call, zero out-of-operating-point drops. The boundary ratio below 1.0 means blocks stitch seamlessly — no seam-stepping at all, which is the artifact that dominates the 120 ms output. So: same encoder, 3 internal frames decode correctly and 6 do not, with identical per-frame geometry. That fits your disassembly — 40 frames, hex (TOC byte included)[
"123711932c56c5e88360a956a559b684528f70933bc0",
"5026c9d7b7b54cfd7a7d3e2473c2ebeb98237b",
"502a94e211c08255ed119379bf45537fb020",
"50027db3e07b572fa0f518fe62f593617b638e42e21f1258c5f94a456a0a4f2dfed11c20000a4d52777df91d846be5cb5866a7426428c7d348ce38097ee676ab840eb90e4ba3a40cb88d357b0e8c330cd417351bf13f2909dfc8e328094c087a5557626ed0539cec694194a0e5d9b391cfccbbfbeb2256a1bb09b90010de9c9fa54deb8aaae9a12f82d227",
"50bade9bbef0bea6c2474bd5ec0830675bf3f38ff07ab9b3c681e704c116eed36d98f38c6644cee1f361127113f1120c1823e673beedb6660f40e54b6844a73931a2a90292683c053653d5ed9d0b22037fa09c72c5f3692728793c29c6a8fdef1dcc7639d874674eab19e804bb21aa53f28d308ab9c98e1091b3a8dee009bc7801c50559f54e",
"50815aceb42ac222ec590a0de959760fe34c3adba7a6a6f8e58fc838442b316bf72c2fe5900cff33500be066977616a567c48698dd65b49de630673bf6f3ee95c5f997797567a779007aba230fcf3229f301e0da1b89353548ea8d9f55852ad8f38772f94ef4b82f37632efeddd09a0d6f9dd6dc841e4a",
"508afcd16075dedaafac40caa17a1ec95b3e15c07e9556fe2b19acce621908d78f67a6388718bc5b00441b758b4d6a4625ae552a49d6d5647390a8fbefb8de8d17e8824736f8193b4373a2af1355e3dec8f76296aa85fe055a49530398f4336faf25dc21e0bcdb989955ce537494bb3c1d3b54ed79621e8480c19586d19ea0d617cb98b92633312fdfdb78",
"50a39ffd92edb40612c3e39ff1575f6e03ffbe98cc9bc6197efc5c1bbff3f80e7e45121ce814ac60865fc988210aa6bbed9e2071a8d26b17600b900936cd107546c924d146f6d0d5b1a21045554e8a2ae0bd65cdc6e306378f502829dbfa217c22810574dc5976171896495e6a6e1b63a84c65f4d230c09eebdd241cf2c4b2f178b8403ada40",
"121071db1fe1d09e57040a45850432e71290d53cfb2ff89eaf2398",
"5032c9a3648e8339f16dad57998380e58b7ba5bd9a117b0998f80874f9846bb6382424427fadddbd66e36cc59011a2001cd94f8a9915dd88b16c98a5b81fc0bf06aba25797bdd9c84780be2170525ce834d39ea633896b6e80a5ff246018278fb8d8e41646cecd90",
"5002a23a2e23d928218a0207b3a85f2b659b023169d33897b9484aab89a5242c8e42f1a0875d44b32d9f56014dcda09f6b4ed769ddb566e289e4eba456e6d525c10712698bb0b8453db4c319043ef32b4ad2ec63b72b0fb3768522c24aba7c249181c7977ea184a06cb7fc170f33a5e6c8ae7e60b20484b63e795a438deac437ec1b9cf19a5172c0",
"5029bc416ef10b52550161035a9ad62e4f6869b62b4948856a4020fd3b8eb2ff37ffbea3d7efce79abf034483716e90311eecd373ef903a927eb5b2bf340",
"1233946ee7e045ec644511542bf97a5740",
"50240647f741c3cafe441b84a92922d01c2215c0",
"5007e1721ba02be1804e04dae3661ebe11fcc0",
"502573a6724878da742ee2a0b363d0deac905c",
"503b4f6f7cb05050b7436a40fe2001b9e790",
"5015b62de146daa3827bf8de94a9cccd651248a0",
"12248231d2cbf96133ab8b0e705b15bcb66320",
"502477aa01935e84a23365b833de10dde860",
"1217aeba0b689858fa411af1d89e5e77ee50",
"5001c10831717dfadd80ed15855c0a7ddcfe21e1",
"12226a3413643f50bf122835767d0e60d322a0",
"502c0ca1a6b315ed45afc070ae1b0db087ee7dca578c51077a02feb57d38fb77f771fe2b842556f88514bc78091b09d46aec2b3a43a4747db35414fec6392a2de1fa6277bd26301620e370a0490104ad8ba10e89ad4b1b66c518acbef940256b7d9780",
"50e82ea124edc58a52dbe5542b5b43448d95e6a368a9a78788c46c9ea123459d944cd21ed2baec26aa0e7c98912ecdf2a6b327cadfc4c25ad5d3084ad33d02dedaef4c81a5cbe80ed78d8bd0032ce486ba53174198c542f3544997da2cae47786c9aec5e00bfa77befb999d11126a753fb54b2c53abe23bc08df0a2e31ff4efb15f481c04aaf253d6a2dacb836a3ebdf8c424166eeabc08f2afb24fcc54f4fb7306323d55e0cbdc0",
"50b376c89769e7d42fa4f52669dbc3f3dddcf050a0ecf62ac1edd626ad0023cc59bc31ac1d4b27894c42c00ed8a38ede9a50ce8b618e3a75cfbbc302ac171809b5e746d8e71b9191dfcf199ab09d915736e4b70c57e69940f07115fe52e92bd4239642c78e39c452d33dc52557572f8f416f7071e46cac12be828e853bc4b1715aef1a2a8f6f8d0c94c740b706de23197c24bb5797bbce12bd6cef9b372dbccf9b1906a66360149e9dd42737497cc0",
"508cb7083bca5460d0e4133f92ce9a92755f5b8a8b5d42a47d747787ae3f6e0bbd3f0684b6100477e986727ca9f34db78e1c19616a5bc67af5400ca31ef2055a8fe3af3052135fc9482418c2542c3de2891b5806e8713f6754f93726fcc3e00c01f66a8cdaa393f5c7a982018b6c6b495f47f5f3566a5e36c577c6066b9ef81446cb4dbecd29af81325b1b3dfb34c19609b4c02b803c1853098fee2b3a5edd6115271080",
"50e3b0e6f3cc1f64b68ab771d078bcff8ecd42ef16a36a7571cc603b529f4a960a99da41ef2f15b170944865a80dd458976c475a3c50001a8124fa57953e02ba38fb817a1760f3704b0a8c96180bb7d1f29a4a3560c356a2f12e80a2660158f6eb1a31d7b197b1b8f378ce2c0e7bf22157fa7b70bee0a4e1605f60bc18e537845773da2cd614bdc51dd08ee6bb6cd9b95a7a3dc032868648",
"50b349f026be11d8b417e7963e066a2ca192aad7ef84b1d40ab73d303c4408aa3031ac21473f678fa52c64691dfa034d90235961f86d1b6414010be64c40a08d14b6c72d91d895d9757002503eb1c3a3b928041713ed4b4287df7758598a7463e991a6a3d57b4b9435d9b79868f924250d11f2afd2db9215777ac725e4d92d06f64ee81cf4d0743fc578ea1eeb",
"50b53bc5f0f2534d0cc40289b1aa7c4c57b4a9476f92c3fdfd9dc9a789e88aca0d911756a384822e4ffd613df02e37f55d6c7025ce32254370be806e91cff099afcc4bfa98d9998b93ffce74f1ec694cfb3c79986be7b31b4e64c3387ae53eef240d9df66b3f81169acc8f2a969e0ec89d4fc7a7a9483adb1bef5ced8a31938c80",
"50fa612448137e615ba51b1d9b89708b27cd711db4e6efbf0fbc59525160b9261eae3983ed382fa0130dc437e0b6e85a8e126dbeceb99aa08f93cd2be84617bfc7642f76a4805b5cd460ec8d710bf1935fbb177f422beb97db1026b1d84f771343b95d636da329ff871fce63ebca28c829",
"1200fee63fc045752a1e96a25a60561bf516644bbcd2a438b2cd3419396a46ea095323be4a",
"5024c85a21389d38d5126edaaebfdda74e32",
"5041555a592f82c95f3f31845c5260bb91a4c55699ed170b8e1699206f9b14849201c4d33dd71d03141098388a8bf756389d15c04f1faf3fa67c6bf276ba182ecaa1c9f77974390ac9729374c3700d54380366573e0653fcbd0399e99629f0e1f24b2a81962c6dc44482ea9be46a6a6c067b1d5e1a743e7e488d5c3d5040d9e32537afcc5524f7f1e0f865de",
"502fa1818c393f0915fe753cd470fb300bd3d69c46e34fe17433cb9e0fddd0866545956dcbc10707756c2762e266be4a2d948611f1dc1c7ef46652e3987038869631bad2bba120a4d7d7c45a8c67460fa80cb6582a15feb31af2dbb1b2cbdd3ca338ce83816db330ef1cdaeb5bf2b1ad35d109ae5be17686586bba33cae2fdf229e9263963d703be48df5383fbf249262a3c3d25ae3e0899064a4098b0f1491fa17ec589dc350cd8d0",
"5042923d5f957ed7db61ea8a099b945e0260ef3222016d19159929dce0a41e02e49221532ab93b7cdd2ce86987bd497fc8f02fe423d93beb9d342cd8ac7615351067659fc306553917edd1cd3e39e3416abbb82328b7dab31f1873ed53691153bd41ddf1507cac6db50c950643f97c98f01ecc327eb1b75b75de3d39c9263422030f0ff85b7c00aa660931a8f5f99e8e32dd713e2ac184e7673855017d296312a93c618c68e5f83b1f28102b80",
"50c31003d0445172d1269322ef4674269003f35de3c5449f1aee0c84980d511f7cd941505e83b67101b2ad37ea80e8a864a2c7098bf1a2620a994d0dc608ac01fbb371f8ad260c621ad2802d1ebccb014fd70a78a9d80a0b454a687073c7236e018818c2dc8d88e87ec6c3405ff88726aa810ed0e7ddda1bd33a71eb825fd89feb71c390e6be750a600bc4b2a66f76060384e9543dd9a6f0ca1a7e6edfd6035b7eab61411853f81e875f55655866c33d4556c2c366d21b50",
"50c22a73844cf01fe2a0f8bbf51a81a1112ab6ba6177b369e133b2db263b9083371f9f63d3df45b4a8897a987c06a75329942785e2eafdbd78cdf2eadd950f7f2930782cdc406d3169bb9d16d00d04cbb7b037773963f9fd4ac383f70a9d998dd156bda8e2386483c50e2298a20f0c47063de4557f9f9e4470136cfab4af2ada6f11846f3de98656258fc89d5039baf810d16b00376b61a90eb62e591e3780fedc4c0ac8091bf964567dbb2f2cf2e0",
"50993ef9021862fa2a4e9d01194970dcd1e21f082a797f06c12540b37fc060e6b723ddb3d85a9f1f5406fde81e4e7cde77b90572c2bd57387a3d3b42566d4362db6868878eea2f3d18d1a53649e5be294a7152957b156a4866ad88d8f7e1c5d91734363b85a503c863bd5aa05f8e29e96a221239eac40b1bb1aa6f9cc12db5683d71291528e6236781c25b34c74eea5d87bb1631cb46683ba0c59316a9eccec8942b0486ab3e2c5d7d874a",
"50743af77bf7738680b35e2734cee3cc17e237a506eafa6ef2a0469d4bb6fe0290796b878bcb049a5d438c0ce8190a5d8f427f2f214a0d0ec3de43d59784958a552ca61a448e0e0e8cdf325ea0c0a7e10dba59333c2a24d67187baca567963b55603292c4369c9f581b1634e87d1cf8548daa3fe9c79a6f69929af1d6f8b04a1abb8349c70deb2a655fd37444e0c3bbeb1da0416894a40451bcd5815de918367c9606f5c6290d7628c54a28df80e76a74e"
]One residual variable, so you can weigh it yourselfSame binary, but the account differs, and a client can be configured per account by server-side props or rollout. The offer carries On
|
|
Thank you for the 60 ms dump — that was the measurement that moved this, and the detail that both captures come from the same installation with only the account differing is what made it decisive. Two of the three open questions are now closed. Your reading is right, and it goes further than "not latent at every duration". The 60 ms packets decode cleanly here too, with It also is not a defect in this port. I ran your 120 ms bodies through the C reference itself, and it fails on them the same way — zeroing its output through Quantifying the failure, with the decoder reconstructed per packet so no attempt inherits another's state:
The overrun is a factor of 3 to 5 — the decode asks for roughly four times the bits the body holds. Not a boundary case. Four hypotheses died on your data:
Geometry matches the reference exactly ( Two notes on where this leaves the PR. Admission is fixed and confirmed by your live test, so real 20 ms and 60 ms frames from this peer now decode — that part is a genuine improvement you can use today. For 120 ms the audible result is still silence, now reached by an integrity check rather than a blind drop, and without the clipping an earlier revision of this branch produced. And I have not committed your frames as a test fixture: they are real speech, and this repo does not carry captured audio in tests. The bytes stay here in the thread as evidence, and the regression tests are built from synthetic vectors instead. |
… fork The endpoint check rejected a decode that ended more than two bytes past its storage, following the C fork's smpl_check_end_result. The shipped decoder is looser: WhatsAppNative.dll @ 0x180337e30 does `add r8d, 0x4` before its second compare, accepting up to four. Two is the stricter of the pair, so the port concealed frames the official client plays -- a false report of corruption that silences audio which is fine. The upper slack exists to absorb the range coder's trailing carry bytes, which the encoder does not emit, so the exact bound is not arbitrary and copying it from the fork was the wrong source. The predicate moves into `endpoint_is_valid` so the window is stated once and can be pinned directly. It is tested as arithmetic rather than through a payload because no synthetic body lands on +3 on demand. This does not change the 120 ms Desktop packets tracked on #1105: those over-run by a factor of 3 to 5, nowhere near four bytes, and they are still concealed.
|
Update, and one question that would redirect this if the answer is what I suspect. I reverse-engineered the shipped decoder's 120 ms path and it contains no mechanism that explains these packets. Three candidates, all eliminated against
So the shipped decoder would run these bytes with the same geometry we do. Combined with the C reference also failing on them, there is no known decoder that reads these packets — which makes the bytes themselves the thing to question. One real difference did fall out, and it is now fixed ( The question. Where exactly did you capture the 120 ms payloads, and what RTP payload type did they carry? The reason it matters: RED redundancy is enabled per packet from the payload type ( I audited the depacketizer's arithmetic (
If it is the second case, the raw pre-strip payload for a handful of packets plus their payload type would settle it immediately. If it is the first, then the packets are genuinely unreadable by every decoder we can inspect, and the next step is instrumenting the Windows client's own decode entry to see what it receives for the same call. No rush on this — the rest of the PR is independent of the answer and does not depend on resolving 120 ms. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8845bf3209
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Conflito em wacore/src/voip/engine.rs (on_timeout): main extraiu playout_due para mixar o frame de grupo antes do caminho PCM 1:1; este ramo parametrizou drain_playout com o pacote do peer. Mantida a estrutura de main, com o teto dinamico aplicado no ramo PCM, onde packet_samps/playout_cap vivem. Corrige tambem um #[test] duplicado em mlow/decoder.rs, que separava o doc-comment de endpoint_check_rejects_an_overrunning_body da sua fn.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 89249a7ffa
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
| self.decode_active_frame(frame, OPUS_FRAME_SAMPS) | ||
| let frames = frames.expect("the guard above rejected every unsupported duration"); | ||
| self.decode_active_frame(frame, frames * SMPL_INTF_LEN, frames, toc.active) |
There was a problem hiding this comment.
When a group-call participant sends one of the newly admitted 120 ms MLOW packets, the group RTP path passes all 1,920 decoded samples into ParticipantAudioMixer, whose fixed GROUP_MIX_PREFILL_SAMPLES is also 1,920. The participant therefore becomes primed after a single packet, and the mixer drains that packet in exactly its 120 ms nominal arrival interval, leaving no jitter cushion and producing silence whenever the next packet is late. The direct-call buffer now scales its target by packet duration, but the group mixer still assumes 60 ms packets; scale its per-participant prefill/cap similarly or restrict variable-duration admission on this path.
Useful? React with 👍 / 👎.
2 participants
Fixes #1105 partially — see Still broken at the end. Several MLow decoder findings from one investigation, kept together so the whole picture is reviewable at once.
1. Multi-frame packets were dropped (the reported bug)
A 1:1 call whose peer is WhatsApp Desktop was silent. That client emits 120 ms packets (TOC
0x58) routinely and the decoder discarded every one:decode_active_frameran a hardcoded three-iteration loop, so the operating-point guard rejected any duration other than 60 ms. Nothing surfaced as an error, because a dropped frame looks exactly like a peer who is not speaking.A packet is not one unit of decode. The reference derives the loop count from the declared duration while the geometry inside each iteration is unchanged —
num_frames = (packet_len_ms + 10) / 20, with frame length and subframe count fixed above 10 ms. So 20, 60 and 120 ms differ only in how many times the same decode repeats. 10 ms stays rejected: it halves both, a geometry the synthesis does not implement.Confirmed against the shipped client, not just the C fork:
smpl_core_decodeinWhatsAppNative.dll@0x1803479d0decompiles to exactly that geometry, andsmpl_decode_toc@0x180337110gives0x58= 16 kHz / 120 ms / mono / high-rate. A 120 ms packet is one continuous range-coded block at offset +1; multiframe packing exists but requires(toc & 0x82) == 0x82, which0x58does not satisfy.The reporter confirmed on a live call that admission is fixed — 230 packets, zero drops, zero range-decoder flags — and that real 20 ms frames from the same peer were also being dropped before this and now decode.
2. Frames coded inactive were silenced
The top field of the TOC is a two-bit SID/VoA selector, not a pair of flags.
00is "normal frame sent while there is no voice activity" — background noise, sent through every pause when DTX is off. Only10is a SID. We treated anything not active-voice as silence.Found by measuring why the 60 ms reference vector correlates only 0.969: the coded-inactive frames are 12% of the samples, where the reference produces 0.047 rms and we produced exactly 0. With the fix, correlation goes to 0.999914.
Two symbols are gated on the same flag in the reference and are absent from the bitstream when it is clear — the voicing symbol and the LSF interpolation index. Reading either consumes symbols that were never written and desyncs everything after, so the LSF decode takes the flag and skips them.
3. A malformed body now gets concealed instead of synthesized
The range decoder returns zero past either end of its storage without setting its error flag, by design. A malformed body therefore decodes into plausible-looking symbols and the synthesis runs on invented parameters.
The reference guards this with an endpoint check the port never carried: a valid stream finishes within two bytes of its storage. Anything else is a lost frame. Added, before the postfilter as in the reference, plus a
warnso a peer we cannot read is visible in a log rather than silently quiet.Concealment also rolls the decoder state back. The overrun is only detectable after the last internal frame, by which point the loop has advanced the LSF predictor, CELP history and
prev_nlsffrom parameters invented past the end of the bad body — measured leaking into the next packet as full-scale samples, RMS 0.290 against 0.114 from a fresh decoder. This deliberately diverges from the reference, which does not roll back: it only ever decodes streams it produced itself, while this decoder meets streams it cannot read.4. Playout sizing follows the peer's packet
The prebuffer target and latency ceiling were written around a 60 ms frame. A 120 ms packet is a full target on its own, so priming ended with no cushion; worse, two in flight exceeded the ceiling and were trimmed on arrival, dropping audio continuously. Both now derive from the peer's packet, taken from the declared duration rather than the decoded length — a SID emits a fixed slot, and reading it off the output shrank the cushion at every DTX transition. The ceiling also only falls once the backlog fits underneath, so a shrinking packet cannot discard queued speech.
5. The vectors are regenerable
PROVENANCE.mddescribed oracles in prose while the harnesses lived on nobody's machine. Producing the 120 ms vector took an afternoon of reverse engineering for two invisible reasons:smpl_CreateCodec()must run before the first encode, and the output buffer is a hard bound rather than a capacity under CBR.scripts/mlow-vectors/mlow_frames.cplusscripts/regenerate-mlow-vectors.shclose that, following the existingregenerate-*.shconvention. The script reproduces the committed fixtures byte for byte against a pinned oracle revision, refuses a dirty reference worktree, and refuses an archive older than the checkout — neither can be attributed to a recorded commit.Contract changes, called out
Four existing tests changed. Three pinned behavior this PR deliberately changes (20 ms and
0x00are no longer examples of dropped frames). The fourth is worth pausing on:decoder_silence_frames_produce_zeroasserted over a fixture whose inactive-TOC frames had been ZEROED when it was generated, perPROVENANCE.md, to match the behavior finding 2 removes. The oracle had been edited to agree with the decoder, and that is what kept the bug invisible.Validation
Tests:
multi_frame_decode_matches_the_reference(correlation 0.999894 against the reference decoder's own output),dtx_off_frames_decode_to_audio,endpoint_check_accepts_a_well_formed_packetandendpoint_check_rejects_an_overrunning_body,a_concealed_frame_does_not_contaminate_the_next,playout_scales_its_cushion_to_the_peer_packet,a_smaller_packet_does_not_trim_the_existing_backlog,declared_duration_survives_a_dtx_transition, plus the geometry and fixture tripwires.e2e_decode_matches_usesmplpasses untouched.Still broken: 120 ms from a Desktop peer produces no audio
Admission is fixed, the decode is not. Real 120 ms packets from that client fail the endpoint check — every one of the 40 the reporter captured — so they are concealed as silence rather than played. The user-visible outcome for those calls is the same silence as before this PR, now reached by an integrity check instead of a blind drop, and without the loud clipping an earlier revision of this branch produced.
The reporter has since supplied a dump of 60 ms packets from the same WhatsApp Desktop installation, on the same machine, differing only in which account was logged in. Those decode cleanly. That is one encoder binary emitting a duration we can read and a duration we cannot, which settles what the previous revision of this section left open — it is neither a corrupted capture nor a different client build.
It also is not a defect in this port. The C reference fails on the same bytes, zeroing its output through
smpl_check_end_resultatsmpl_core_decoder.c:541. Whatever these packets are, the fork cannot read them either.Measured, with the decoder reconstructed per packet so no attempt inherits another's state:
The overrun is a factor of 3 to 5, not a boundary case: the decode asks for roughly four times the bits the body holds. The endpoint check is
storage <= ceil(ec_tell/8) <= storage+2(smpl_param_coding.c:663).Four hypotheses were tested and eliminated:
0x54(60 ms,low_rateset) passes 39 of 40 — but none produces speech: pitch periodicity lands at 0.07–0.26 against 0.758 for the genuine 60 ms control. Passing that check is weak evidence on its own, because it tolerates two bytes and the range decoder returns zeros past either end without setting its error flag. Normalized autocorrelation over lags 53–200 is what actually separates speech from a plausible-looking desync.0x58packets of 145–351 bytes across 10–20 kbit/s, and this decoder reads them at 0.999870 correlation with zero malformed frames. Below roughly 8 kbit/s the fork switches tolow_rate(0x5c); the Desktop packets stay0x58at 2.4–10.7 kbit/s.Geometry matches the reference exactly (
num_frames = (ms+10)/20,num_subframes = 1 << (1 - low_rate + (ms>10)),low_rate= bit 2), and all 107 constant tables are byte-identical between the fork andWhatsAppNative.dll. So the remaining question is narrow and well-posed: what does the shipped decoder do for six internal frames that the fork does not do for three? That is where the search continues.The rest of this PR stands on its own — findings 2 through 5 are validated against the reference and fix real defects — but if you would rather not ship multi-frame admission while its decode is unresolved, the guard change is easy to isolate.