Fix G.722/G.711 mixed recording half-rate + flaky rtpchannel suite (3.2.5)#156
Merged
Conversation
….2.5) Recording fix: a recording on a G.722 (wideband) leg was stamped at 8 kHz in the WAV header while the feed path wrote true 16 kHz frames, so the leg played back at half speed. The recorder's rate was derived from codecx.is_wideband(), which only inspected the live wire PT — still 255 when `record` is issued before the first inbound packet (the normal SIP ordering). CodecBundle now also tracks the negotiated (SDP) payload type, so is_wideband()/native_samplerate() report wideband from the start. Adds a regression test that records on the G.722 leg and asserts the WAV is stamped 16 kHz. Test-harness fixes (the consistently-failing rtpchannel suite): - projectrtpchannel.js: the "check event emitter" test was an arrow function calling this.timeout(2000). In an arrow, `this` is the describe Suite, and Suite.timeout() (mocha 11) re-arms every sibling test's timeout timer — so all 12 already-passed tests were re-armed and their stale timers fired 2 s later, retroactively flipping them to "done() called multiple times / Timeout 2000ms". Only visible in the full suite (the process stays alive long enough for the timers to fire). Switched to a regular function. - index.js: the channel event callback swallowed listener throws via console.trace, so a failed assertion inside a close/record/play handler surfaced only as an unrelated timeout. Re-raise asynchronously instead so it becomes a visible uncaughtException. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
64af1d8 to
75a59e3
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two independent bug fixes plus a patch version bump to 3.2.5 (ready for npm).
1. G.722/G.711 mixed recording played back at half rate (the reported bug)
When one leg is G.722 (16 kHz) and the other G.711 (8 kHz), a recording on the G.722 leg played back at half speed.
Cause: the recorder's WAV sample rate was derived from
codecx.is_wideband(), which only inspected the live wire payload type. That's still255whenrecordis issued before the first inbound RTP packet decodes (the normal SIP ordering), so a G.722 channel was misclassified as narrowband and the file stamped 8 kHz — while the per-tick feed path wrote true 16 kHz wideband frames. 16 kHz audio in an 8 kHz container = half-speed playback.Fix:
CodecBundlenow also tracks the negotiated (SDP) payload type, set at remote-config (known before any packet).is_wideband()/native_samplerate()consider it, so the stamp and the fed audio agree from the first frame. Added a regression test that records on the G.722 leg and asserts the WAV is stamped 16 kHz.2. Consistently-failing
rtpchanneltest suite (12 tests)The full
npm testrun consistently failed all 12rtpchanneltests withdone() called multiple times / Timeout 2000ms, even though the tests actually pass in isolation.Cause (traced via mocha timer instrumentation): the
"check event emitter"test is an arrow function callingthis.timeout(2000). In an arrow,thisis the enclosingdescribeSuite, and mocha 11'sSuite.timeout()re-arms the timeout timer of every test in the suite. That re-armed all 12 already-passed sibling tests; their stale 2000 ms timers fired 2 s later and retroactively flipped them to failed. Only visible in the full suite, where the process stays alive long enough for the timers to fire (in isolation it exits first).Fix: switched the test to a regular
function()sothis.timeout()binds to the test, not the suite. (Swept the wholetest/tree — this was the only offending case.)3. Harness honesty
index.jsswallowed throws from the channel event callback viaconsole.trace, so a failed assertion inside aclose/record/playhandler surfaced only as an unrelated timeout. It now re-raises asynchronously (setImmediate) so the real error becomes a visibleuncaughtException.Verification
npm test: 143 passing, 6 pending, 0 failing.cargo test --lib(single-threaded): 77 passing.🤖 Generated with Claude Code