Fix DTMF deduplication to use (timestamp, event code) composite key - #796
Open
cuihang wants to merge 1 commit into
Open
Fix DTMF deduplication to use (timestamp, event code) composite key#796cuihang wants to merge 1 commit into
cuihang wants to merge 1 commit into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #796 +/- ##
==========================================
+ Coverage 65.25% 66.69% +1.44%
==========================================
Files 51 41 -10
Lines 6588 8096 +1508
==========================================
+ Hits 4299 5400 +1101
- Misses 1915 2209 +294
- Partials 374 487 +113 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
The DTMF handler previously deduplicated packets using only the RTP timestamp. Per RFC 4733, all packets of a given digit share the same timestamp, so this correctly filters redundant packets within one digit. However, some SIP devices or carriers reuse the previous digit's timestamp for the next digit, causing legitimate digits to be silently dropped. Replace the timestamp-only dedup with a (timestamp, event code) composite key so that: - Same timestamp + same event code: redundant packets still deduped - Same timestamp + different event code (e.g. *, 0, 1): each digit reported individually - No dependency on the RTP marker bit, avoiding missed digits on loss Add TestMediaPortDTMFSameTimestamp covering the scenario where two different digits share the same RTP timestamp.
cuihang
force-pushed
the
fix/dtmf-dedup-event-code
branch
from
August 16, 2026 19:04
94cae6a to
135a5fc
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.
Problem
The DTMF handler in
dtmfHandler(media_port.go) deduplicated incoming DTMF packets using only the RTP timestamp:RFC 4733 requires all packets of a given digit to share identical timestamps, so this correctly filters redundant packets within one digit. However, some SIP devices or carriers reuse the timestamp of the previous digit when sending the next digit. When this happens, the next digit is incorrectly filtered out and never reported.
Fix
Replace the timestamp-only dedup with a (timestamp, event code) composite key:
Behavior
*,0,1)Known limitation
If an upstream sends two identical digits (e.g. two
0s) with the exact same timestamp and event code, the(timestamp, event code)key alone cannot distinguish them. Resolving that case requires full DTMF lifecycle state tracking (marker bit, End bit, duration reset, sequence number gaps). That is a more complex enhancement beyond this fix; in the worst case (all fields identical) the receiver cannot distinguish the digits at all.Changes
pkg/sip/media_port.go: ReplacelastDTMFTimestamp atomic.Uint32withlastDTMFEvent atomic.Uint64; updatedtmfHandlerto decode first, then dedup on composite key.pkg/sip/media_port_test.go: Update field reference; addTestMediaPortDTMFSameTimestampverifying two different digits (0and1) sharing the same RTP timestamp are both reported.Test plan
TestMediaPortDTMF— all 12 existing subtests pass (digits 1/12/123 × loss none/first/last/middle)TestMediaPortDTMFSameTimestamp— new test passes: digits0and1with same timestamp → both reported as01