diff --git a/components/sip_client/README.md b/components/sip_client/README.md index 212aef08..80475184 100644 --- a/components/sip_client/README.md +++ b/components/sip_client/README.md @@ -202,11 +202,15 @@ binary_sensor: is reported once when the event starts, independent of the RTP marker bit — Yealink DECT handsets (W70B/W73H) and the 3CX Android app send the event without it. Repeat packets of the same event, including the retransmitted end - packets, do not re-fire the trigger. -- **SIP INFO** with `application/dtmf-relay` (`Signal=1`) or `application/dtmf`, - answered with `200 OK`. `*` and `#` are accepted both literally and as the - event codes `10` / `11`. INFO requests outside an established call are - answered but do not fire the trigger. + packets, do not re-fire the trigger. A 4-byte payload on any other dynamic + payload type (96–127) is also taken as a telephone-event, for ATAs that never + offer one in their SDP or send it on a different PT than negotiated. +- **SIP INFO**, answered with `200 OK`. Content types `application/dtmf-relay`, + `application/dtmf` and `audio/telephone-event` are accepted, as is a body with + no Content-Type at all. The digit is read from a `Signal=` / `d=` / `dtmf=` + line or from a body that is nothing but the digit; `*` and `#` are accepted + both literally and as the event codes `10` / `11`. INFO requests outside an + established call are answered but do not fire the trigger. In-band audio tones (no telephone-event, no INFO) are **not** decoded. diff --git a/components/sip_client/dtmf.cpp b/components/sip_client/dtmf.cpp index d87ff1f7..cf6111da 100644 --- a/components/sip_client/dtmf.cpp +++ b/components/sip_client/dtmf.cpp @@ -40,32 +40,43 @@ char signal_token_to_char(const std::string &token) { return is_dtmf_char(c) ? c : 0; } -// Value of "Signal=" in a dtmf-relay body. The name is matched -// case-insensitively; "Duration=" and friends are left alone because the '=' -// must follow the name. -std::string signal_token(const std::string &body) { - std::string lower = to_lower(body); - for (size_t p = lower.find("signal"); p != std::string::npos; p = lower.find("signal", p + 6)) { - size_t i = p + 6; - while (i < body.size() && (body[i] == ' ' || body[i] == '\t')) i++; - if (i >= body.size() || body[i] != '=') continue; - i++; - while (i < body.size() && (body[i] == ' ' || body[i] == '\t')) i++; - std::string token; - while (i < body.size() && !std::isspace((unsigned char) body[i])) token += body[i++]; - return token; - } - return ""; +bool is_dtmf_content_type(const std::string &content_type) { + // Empty: gateways that send a body with no Content-Type at all. + if (trim(content_type).empty()) return true; + std::string type = to_lower(content_type); + return type.find("application/dtmf") != std::string::npos || + type.find("audio/telephone-event") != std::string::npos; +} + +// Key of a "=" body line, lowercased; empty when the line has no +// '=' at all. +std::string line_key(const std::string &line) { + size_t eq = line.find('='); + if (eq == std::string::npos) return ""; + return to_lower(trim(line.substr(0, eq))); } } // namespace char parse_dtmf_info(const std::string &content_type, const std::string &body) { - // Content-Type may carry parameters, e.g. "application/dtmf-relay;charset=utf-8". - std::string type = to_lower(trim(content_type.substr(0, content_type.find(';')))); - if (type == "application/dtmf-relay") return signal_token_to_char(signal_token(body)); - if (type == "application/dtmf") return signal_token_to_char(trim(body)); - return 0; + if (!is_dtmf_content_type(content_type)) return 0; + if (body.empty()) return 0; + + // "Signal=1", "d=1" or "dtmf=1" on a line of its own. Other keys, notably + // "Duration=250", must not be read as a digit. + size_t pos = 0; + while (pos < body.size()) { + size_t eol = body.find('\n', pos); + std::string line = body.substr(pos, eol == std::string::npos ? std::string::npos : eol - pos); + std::string key = line_key(line); + if (key == "signal" || key == "d" || key == "dtmf") + return signal_token_to_char(trim(line.substr(line.find('=') + 1))); + if (eol == std::string::npos) break; + pos = eol + 1; + } + + // No key/value pair: some devices send just the digit as the whole body. + return signal_token_to_char(trim(body)); } } // namespace sip_client diff --git a/components/sip_client/dtmf.h b/components/sip_client/dtmf.h index 9395c711..5dfaa837 100644 --- a/components/sip_client/dtmf.h +++ b/components/sip_client/dtmf.h @@ -1,4 +1,5 @@ #pragma once +#include #include #include @@ -15,6 +16,15 @@ inline char dtmf_event_to_char(uint8_t event) { return 0; } +// Whether a packet on `pt` should be decoded as a telephone-event even though +// it is not the negotiated DTMF payload type. Some ATAs send RFC 2833 without +// ever offering telephone-event in their SDP, or on a different dynamic PT than +// the one negotiated; a 4-byte payload on a dynamic PT is the telephone-event +// shape (event, E|R|volume, duration), never an audio frame. +inline bool is_unnegotiated_telephone_event(uint8_t pt, uint8_t audio_pt, size_t payload_len) { + return pt != audio_pt && pt >= 96 && pt <= 127 && payload_len == 4; +} + inline bool is_dtmf_char(char c) { return (c >= '0' && c <= '9') || c == '*' || c == '#' || (c >= 'A' && c <= 'D'); } @@ -45,15 +55,18 @@ class DtmfRxDedup { uint32_t last_timestamp_{0}; }; -// DTMF carried in a SIP INFO body, as sent by the 3CX Android app and by desk -// phones configured for "SIP INFO" DTMF. Returns the digit, or 0 when the INFO -// is not a DTMF INFO. +// DTMF carried in a SIP INFO body, as sent by the 3CX Android app, by ATAs +// and by desk phones configured for "SIP INFO" DTMF. Returns the digit, or 0 +// when the INFO is not a DTMF INFO. // // application/dtmf-relay: "Signal=1\r\nDuration=160" // application/dtmf: "1" +// audio/telephone-event: either shape // -// Senders disagree on how * and # are written -- literally, or as the RFC 4733 -// event codes 10 and 11 -- so both spellings are accepted. +// The signal is read from a `Signal=` / `d=` / `dtmf=` line, or from a body +// that is nothing but the digit. Senders disagree on how * and # are written +// -- literally, or as the RFC 4733 event codes 10 and 11 -- so both spellings +// are accepted. A body without a Content-Type is parsed too; gateways omit it. char parse_dtmf_info(const std::string &content_type, const std::string &body); } // namespace sip_client diff --git a/components/sip_client/rtp_session.cpp b/components/sip_client/rtp_session.cpp index dd85b41b..a4fd2153 100644 --- a/components/sip_client/rtp_session.cpp +++ b/components/sip_client/rtp_session.cpp @@ -97,6 +97,7 @@ bool RtpSession::start(uint16_t local_port) { this->dtmf_queue_.clear(); this->dtmf_active_ = false; this->dtmf_rx_.reset(); + this->rx_dtmf_pt_warned_ = false; this->last_tx_ms_ = millis(); // Treat the session start as "just sent audio" so the grace period applies // from here; a session with no microphone starts emitting silence right after. @@ -105,6 +106,10 @@ bool RtpSession::start(uint16_t local_port) { this->recv_buf_.resize(1500); ESP_LOGI(TAG, "RTP started on port %u (pt=%u %s, dtmf_pt=%d)", local_port, this->codec_->desc().pt, this->codec_->desc().rtpmap, this->dtmf_pt_); + if (this->dtmf_pt_ < 0) { + ESP_LOGW(TAG, "Remote did not negotiate telephone-event (RFC 2833); inbound DTMF " + "relies on SIP INFO or an unnegotiated payload type"); + } return true; } @@ -285,9 +290,15 @@ void RtpSession::receive_() { size_t header_len = 12 + 4 * (this->recv_buf_[0] & 0x0F); // CSRC count if ((size_t) len <= header_len) continue; - if (this->dtmf_pt_ >= 0 && pt == (uint8_t) this->dtmf_pt_) { + size_t payload_len = (size_t) len - header_len; + bool negotiated_dtmf = this->dtmf_pt_ >= 0 && pt == (uint8_t) this->dtmf_pt_; + if (negotiated_dtmf || is_unnegotiated_telephone_event(pt, expect_pt, payload_len)) { + if (!negotiated_dtmf && !this->rx_dtmf_pt_warned_) { + this->rx_dtmf_pt_warned_ = true; + ESP_LOGI(TAG, "Accepting inbound DTMF on unnegotiated payload type %u", pt); + } // RFC 4733 payload: event, E|R|volume, duration (16 bit). - if ((size_t) len >= header_len + 4 && this->on_dtmf_) { + if (payload_len >= 4 && this->on_dtmf_) { uint32_t event_ts = ((uint32_t) this->recv_buf_[4] << 24) | ((uint32_t) this->recv_buf_[5] << 16) | ((uint32_t) this->recv_buf_[6] << 8) | this->recv_buf_[7]; diff --git a/components/sip_client/rtp_session.h b/components/sip_client/rtp_session.h index cb0c6998..8653ebb5 100644 --- a/components/sip_client/rtp_session.h +++ b/components/sip_client/rtp_session.h @@ -80,6 +80,7 @@ class RtpSession { uint32_t dtmf_timestamp_{0}; int dtmf_end_packets_{0}; DtmfRxDedup dtmf_rx_{}; + bool rx_dtmf_pt_warned_{false}; std::function on_audio_{}; std::function on_dtmf_{}; diff --git a/tests/native/sip_sdp/README.md b/tests/native/sip_sdp/README.md index d700fe17..3b9e520e 100644 --- a/tests/native/sip_sdp/README.md +++ b/tests/native/sip_sdp/README.md @@ -47,7 +47,9 @@ tests/native/sip_sdp/run.sh | `info_star_hash_spellings` | `Signal=*` and `Signal=10`/`11` both accepted | | `info_plain_dtmf` | `application/dtmf` bare-digit body | | `info_duration_is_not_a_digit` | `Duration=250` must not be read as DTMF `D` | -| `info_non_dtmf_is_ignored` | other INFO bodies / no Content-Type → no digit | +| `info_non_dtmf_is_ignored` | other INFO content types / out-of-range codes → no digit | +| `info_alternate_keys_and_types` | `d=` / `dtmf=` keys, `audio/telephone-event`, missing Content-Type | +| `unnegotiated_telephone_event_pt` | 4-byte payload on a dynamic PT is DTMF; audio PT and static PTs are not | ### `G711Codec` (`test_g711_codec.cpp`) diff --git a/tests/native/sip_sdp/test_dtmf.cpp b/tests/native/sip_sdp/test_dtmf.cpp index be137503..805b77f5 100644 --- a/tests/native/sip_sdp/test_dtmf.cpp +++ b/tests/native/sip_sdp/test_dtmf.cpp @@ -5,12 +5,20 @@ #include using esphome::sip_client::DtmfRxDedup; +using esphome::sip_client::is_unnegotiated_telephone_event; using esphome::sip_client::parse_dtmf_info; namespace { int g_failures = 0; +void require(bool condition, const char *message) { + if (!condition) { + std::cerr << "FAIL: " << message << '\n'; + g_failures++; + } +} + void require_eq_char(char actual, char expected, const char *message) { if (actual != expected) { std::cerr << "FAIL: " << message << " (got '" << (actual == 0 ? '0' : actual) @@ -101,12 +109,34 @@ void test_info_duration_is_not_a_digit() { void test_info_non_dtmf_is_ignored() { require_eq_char(parse_dtmf_info("application/media_control+xml", ""), 0, "video fast-update INFO must not fire DTMF"); - require_eq_char(parse_dtmf_info("", "Signal=1"), 0, "no Content-Type -> no DTMF"); require_eq_char(parse_dtmf_info("application/dtmf-relay", "Signal=99\r\n"), 0, "out-of-range event code"); require_eq_char(parse_dtmf_info("application/dtmf-relay", "Signal=\r\n"), 0, "empty signal"); } +void test_info_alternate_keys_and_types() { + // ATAs and gateways vary the key, the type, and whether they send a type at + // all; hass-sip accepts all of these and the door relay depends on it. + require_eq_char(parse_dtmf_info("application/dtmf-relay", "d=7\r\n"), '7', "d= key"); + require_eq_char(parse_dtmf_info("application/dtmf-relay", "dtmf=3"), '3', "dtmf= key"); + require_eq_char(parse_dtmf_info("audio/telephone-event", "Signal=8\r\nDuration=160"), '8', + "audio/telephone-event type"); + require_eq_char(parse_dtmf_info("", "Signal=1\r\n"), '1', "body with no Content-Type"); + require_eq_char(parse_dtmf_info("", "6"), '6', "bare digit with no Content-Type"); +} + +void test_unnegotiated_telephone_event_pt() { + // Some ATAs never offer telephone-event in SDP, or send it on a PT other than + // the negotiated one. A 4-byte payload on a dynamic PT is the RFC 4733 shape. + require(is_unnegotiated_telephone_event(/*pt=*/96, /*audio_pt=*/0, /*payload_len=*/4), + "dynamic PT with a 4-byte payload is a telephone-event"); + require(is_unnegotiated_telephone_event(127, 8, 4), "top of the dynamic range"); + require(!is_unnegotiated_telephone_event(96, 96, 4), "the negotiated audio PT is never DTMF"); + require(!is_unnegotiated_telephone_event(0, 8, 4), "static PTs are not probed"); + require(!is_unnegotiated_telephone_event(96, 0, 160), "an audio frame is not a telephone-event"); + require(!is_unnegotiated_telephone_event(96, 0, 3), "a runt payload is not a telephone-event"); +} + } // namespace int main() { @@ -120,6 +150,8 @@ int main() { test_info_plain_dtmf(); test_info_duration_is_not_a_digit(); test_info_non_dtmf_is_ignored(); + test_info_alternate_keys_and_types(); + test_unnegotiated_telephone_event_pt(); if (g_failures != 0) { std::cerr << g_failures << " failure(s)\n";