Commit 7dc7542a95 for asterisk.org
commit 7dc7542a950cc64f2838bc0afd372d9263584e99
Author: juanjo <juanjo@aroztegi.com>
Date: Tue Nov 3 19:22:53 2026 +0100
res_rtp_asterisk: Don't let symmetric RTP resume media on a held stream
When the remote party puts a call on hold, res_pjsip_sdp_rtp answers
recvonly or inactive and stops the RTP instance, which clears the remote
address so that nothing more is sent. With rtp_symmetric enabled, the
next RTP packet from the held party set the remote address again in
ast_rtp_read(), and Asterisk resumed sending media for the rest of the
hold. A sendonly holder keeps sending, so this happened on every such
hold; on an inactive or c=0.0.0.0 hold, one late packet was enough.
Symmetric RTP now only changes where RTP is sent if the remote side has
explicitly requested that media be sent to it. A stopped instance has no
such request until new SDP makes one, as an unhold does, so the held
party's RTP no longer restarts it. A unit test covers the stopped case.
Aided by Claude Opus 5.5 (investigation, code).
diff --git a/res/res_rtp_asterisk.c b/res/res_rtp_asterisk.c
index d5c5968038..e3b05f4786 100644
--- a/res/res_rtp_asterisk.c
+++ b/res/res_rtp_asterisk.c
@@ -8690,7 +8690,16 @@ static struct ast_frame *ast_rtp_read(struct ast_rtp_instance *instance, int rtc
/* If symmetric RTP is enabled see if the remote side is not what we expected and change where we are sending audio */
if (ast_rtp_instance_get_prop(instance, AST_RTP_PROPERTY_NAT)) {
- if (ast_sockaddr_cmp(&remote_address, &addr)) {
+ struct ast_sockaddr target_address;
+
+ /*
+ * Only change where we are sending RTP when symmetric RTP is enabled if the remote
+ * side has explicitly requested we send media to them. After the instance was stopped
+ * for a remote hold there is no such request, so the remote's own RTP must not start
+ * us sending to it.
+ */
+ ast_rtp_instance_get_requested_target_address(instance, &target_address);
+ if (!ast_sockaddr_isnull(&target_address) && ast_sockaddr_cmp(&remote_address, &addr)) {
/* do not update the originally given address, but only the remote */
ast_rtp_instance_set_incoming_source_address(instance, &addr);
ast_sockaddr_copy(&remote_address, &addr);
diff --git a/tests/test_res_rtp.c b/tests/test_res_rtp.c
index 211bc610d9..1fade341c5 100644
--- a/tests/test_res_rtp.c
+++ b/tests/test_res_rtp.c
@@ -747,6 +747,50 @@ AST_TEST_DEFINE(mes)
return AST_TEST_PASS;
}
+AST_TEST_DEFINE(symmetric_rtp_stopped)
+{
+ RAII_VAR(struct ast_rtp_instance *, instance1, NULL, ast_rtp_instance_destroy);
+ RAII_VAR(struct ast_rtp_instance *, instance2, NULL, ast_rtp_instance_destroy);
+ RAII_VAR(struct ast_sched_context *, test_sched, NULL, ast_sched_context_destroy_wrapper);
+ struct ast_sockaddr remote;
+
+ switch (cmd) {
+ case TEST_INIT:
+ info->name = "symmetric_rtp_stopped";
+ info->category = "/res/res_rtp/";
+ info->summary = "symmetric RTP does not restart a stopped instance";
+ info->description =
+ "Stops an instance that has symmetric RTP enabled, as a "
+ "remote hold does, then sends it RTP from its old remote "
+ "address. Validates that the instance does not take that "
+ "address back as where to send its own RTP.";
+ return AST_TEST_NOT_RUN;
+ case TEST_EXECUTE:
+ break;
+ }
+
+ test_sched = ast_sched_context_create();
+
+ if ((test_init_rtp_instances(&instance1, &instance2, test_sched, TEST_TYPE_NONE)) < 0) {
+ ast_log(LOG_ERROR, "Failed to initialize test!\n");
+ return AST_TEST_FAIL;
+ }
+
+ ast_rtp_instance_set_prop(instance1, AST_RTP_PROPERTY_NAT, 1);
+ test_write_frames(instance2, 1000, 1);
+ test_read_frames(instance1, 1);
+
+ ast_rtp_instance_stop(instance1);
+ test_write_frames(instance2, 1001, 1);
+ test_read_frames(instance1, 1);
+
+ ast_rtp_instance_get_remote_address(instance1, &remote);
+ ast_test_validate(test, ast_sockaddr_isnull(&remote),
+ "Stopped instance took the sender's address as its remote address");
+
+ return AST_TEST_PASS;
+}
+
AST_TEST_DEFINE(payload_merge_abandoned_offer)
{
struct ast_rtp_codecs offered = AST_RTP_CODECS_NULL_INIT;
@@ -901,6 +945,7 @@ static int unload_module(void)
{
AST_TEST_UNREGISTER(rtp_port_exhaustion);
AST_TEST_UNREGISTER(payload_merge_preserves_preferences);
+ AST_TEST_UNREGISTER(symmetric_rtp_stopped);
AST_TEST_UNREGISTER(payload_merge_abandoned_offer);
AST_TEST_UNREGISTER(mes);
AST_TEST_UNREGISTER(nack_no_packet_loss);
@@ -925,6 +970,7 @@ static int load_module(void)
AST_TEST_REGISTER(sr_rr_nominal);
AST_TEST_REGISTER(fir_nominal);
AST_TEST_REGISTER(mes);
+ AST_TEST_REGISTER(symmetric_rtp_stopped);
AST_TEST_REGISTER(payload_merge_abandoned_offer);
AST_TEST_REGISTER(payload_merge_preserves_preferences);
return AST_MODULE_LOAD_SUCCESS;