From: Matthew Jordan Date: Thu, 7 Mar 2013 15:48:06 +0000 (+0000) Subject: Add a 'secret' probation strictrtp mode to handle delayed changes in RTP source X-Git-Tag: 13.0.0-beta1~2036 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=80b8c2349c427a94a428670f1183bdc693936813;p=thirdparty%2Fasterisk.git Add a 'secret' probation strictrtp mode to handle delayed changes in RTP source Often, Asterisk may realize that a change in the source of an RTP stream is about to occur and ask that the RTP engine reset it's lock on the current RTP source. In certain scenarios, it may take awhile for the new remote system to send RTP packets, while the old remote system may continue providing RTP during that time period. This causes Asterisk to re-lock onto the old source, thereby rejecting the new source when the old source stops sending RTP and the new source begins. This patch prevents that by having a constant secondary, 'secret' probation mode enabled when an RTP source has been chosen. RTP packets from other sources are always considered, but never chosen unless the current RTP source stops sending RTP. Review: https://reviewboard.asterisk.org/r/2364 (closes issue AST-1124) Reported by: John Bigelow Tested by: John Bigelow (closes issue AST-1125) Reported by: John Bigelow Tested by: John Bigelow ........ Merged revisions 382573 from http://svn.asterisk.org/svn/asterisk/branches/11 git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@382589 65c4cc65-6c06-0410-ace0-fbb531ad65f3 --- diff --git a/channels/chan_sip.c b/channels/chan_sip.c index 9538441926..627924bed9 100644 --- a/channels/chan_sip.c +++ b/channels/chan_sip.c @@ -25310,13 +25310,13 @@ static int handle_request_invite(struct sip_pvt *p, struct sip_request *req, str if (get_ip_and_port_from_sdp(req, SDP_AUDIO, &addr)) { ast_log(LOG_WARNING, "Failed to set an alternate media source on glared reinvite. Audio may not work properly on this call.\n"); } else { - ast_rtp_instance_set_alt_remote_address(p->rtp, &addr); + ast_rtp_instance_set_remote_address(p->rtp, &addr); } if (p->vrtp) { if (get_ip_and_port_from_sdp(req, SDP_VIDEO, &addr)) { ast_log(LOG_WARNING, "Failed to set an alternate media source on glared reinvite. Video may not work properly on this call.\n"); } else { - ast_rtp_instance_set_alt_remote_address(p->vrtp, &addr); + ast_rtp_instance_set_remote_address(p->vrtp, &addr); } } } diff --git a/include/asterisk/rtp_engine.h b/include/asterisk/rtp_engine.h index 72c162d20c..c8a144b73f 100644 --- a/include/asterisk/rtp_engine.h +++ b/include/asterisk/rtp_engine.h @@ -443,8 +443,6 @@ struct ast_rtp_engine { void (*packetization_set)(struct ast_rtp_instance *instance, struct ast_codec_pref *pref); /*! Callback for setting the remote address that RTP is to be sent to */ void (*remote_address_set)(struct ast_rtp_instance *instance, struct ast_sockaddr *sa); - /*! Callback for setting an alternate remote address */ - void (*alt_remote_address_set)(struct ast_rtp_instance *instance, struct ast_sockaddr *sa); /*! Callback for changing DTMF mode */ int (*dtmf_mode_set)(struct ast_rtp_instance *instance, enum ast_rtp_dtmf_mode dtmf_mode); /*! Callback for getting DTMF mode */ @@ -791,29 +789,6 @@ struct ast_frame *ast_rtp_instance_read(struct ast_rtp_instance *instance, int r */ int ast_rtp_instance_set_remote_address(struct ast_rtp_instance *instance, const struct ast_sockaddr *address); - -/*! - * \brief Set the address of an an alternate RTP address to receive from - * - * \param instance The RTP instance to change the address on - * \param address Address to set it to - * - * \retval 0 success - * \retval -1 failure - * - * Example usage: - * - * \code - * ast_rtp_instance_set_alt_remote_address(instance, &address); - * \endcode - * - * This changes the alternate remote address that RTP will be sent to on instance to the address given in the sin - * structure. - * - * \since 1.8 - */ -int ast_rtp_instance_set_alt_remote_address(struct ast_rtp_instance *instance, const struct ast_sockaddr *address); - /*! * \brief Set the address that we are expecting to receive RTP on * diff --git a/main/rtp_engine.c b/main/rtp_engine.c index 7010c1c0c6..b25cb95b36 100644 --- a/main/rtp_engine.c +++ b/main/rtp_engine.c @@ -61,8 +61,6 @@ struct ast_rtp_instance { struct ast_sockaddr local_address; /*! Address that we are sending RTP to */ struct ast_sockaddr remote_address; - /*! Alternate address that we are receiving RTP from */ - struct ast_sockaddr alt_remote_address; /*! Instance that we are bridged to if doing remote or local bridging */ struct ast_rtp_instance *bridged; /*! Payload and packetization information */ @@ -335,20 +333,6 @@ int ast_rtp_instance_set_remote_address(struct ast_rtp_instance *instance, return 0; } -int ast_rtp_instance_set_alt_remote_address(struct ast_rtp_instance *instance, - const struct ast_sockaddr *address) -{ - ast_sockaddr_copy(&instance->alt_remote_address, address); - - /* oink */ - - if (instance->engine->alt_remote_address_set) { - instance->engine->alt_remote_address_set(instance, &instance->alt_remote_address); - } - - return 0; -} - int ast_rtp_instance_get_and_cmp_local_address(struct ast_rtp_instance *instance, struct ast_sockaddr *address) { diff --git a/res/res_rtp_asterisk.c b/res/res_rtp_asterisk.c index fadace763a..9e9a92c826 100644 --- a/res/res_rtp_asterisk.c +++ b/res/res_rtp_asterisk.c @@ -169,6 +169,12 @@ static int worker_terminate; #define COMPONENT_RTP 1 #define COMPONENT_RTCP 2 +/*! \brief RTP learning mode tracking information */ +struct rtp_learning_info { + int max_seq; /*!< The highest sequence number received */ + int packets; /*!< The number of remaining packets before the source is accepted */ +}; + /*! \brief RTP session description */ struct ast_rtp { int s; @@ -233,14 +239,13 @@ struct ast_rtp { enum strict_rtp_state strict_rtp_state; /*!< Current state that strict RTP protection is in */ struct ast_sockaddr strict_rtp_address; /*!< Remote address information for strict RTP purposes */ - struct ast_sockaddr alt_rtp_address; /*!strict_rtp_state = STRICT_RTP_LEARN; - rtp_learning_seq_init(rtp, (uint16_t)rtp->seqno); + rtp_learning_seq_init(&rtp->rtp_source_learn, (uint16_t)rtp->seqno); } static void ast_rtp_on_ice_rx_data(pj_ice_sess *ice, unsigned comp_id, unsigned transport_id, void *pkt, pj_size_t size, const pj_sockaddr_t *src_addr, unsigned src_addr_len) @@ -1592,13 +1595,13 @@ static int create_new_socket(const char *type, int af) * \brief Initializes sequence values and probation for learning mode. * \note This is an adaptation of pjmedia's pjmedia_rtp_seq_init function. * - * \param rtp pointer to rtp struct used with the received rtp packet. - * \param seq sequence number read from the rtp header + * \param info The learning information to track + * \param seq sequence number read from the rtp header to initialize the information with */ -static void rtp_learning_seq_init(struct ast_rtp *rtp, uint16_t seq) +static void rtp_learning_seq_init(struct rtp_learning_info *info, uint16_t seq) { - rtp->learning_max_seq = seq - 1; - rtp->learning_probation = learning_min_sequential; + info->max_seq = seq - 1; + info->packets = learning_min_sequential; } /*! @@ -1606,29 +1609,23 @@ static void rtp_learning_seq_init(struct ast_rtp *rtp, uint16_t seq) * \brief Updates sequence information for learning mode and determines if probation/learning mode should remain in effect. * \note This function was adapted from pjmedia's pjmedia_rtp_seq_update function. * - * \param rtp pointer to rtp struct used with the received rtp packet. + * \param info Structure tracking the learning progress of some address * \param seq sequence number read from the rtp header - * \return boolean value indicating if probation mode is active at the end of the function + * \retval 0 if probation mode should exit for this address + * \retval non-zero if probation mode should continue */ -static int rtp_learning_rtp_seq_update(struct ast_rtp *rtp, uint16_t seq) +static int rtp_learning_rtp_seq_update(struct rtp_learning_info *info, uint16_t seq) { - int probation = 1; - - ast_debug(1, "%p -- probation = %d, seq = %d\n", rtp, rtp->learning_probation, seq); - - if (seq == rtp->learning_max_seq + 1) { + if (seq == info->max_seq + 1) { /* packet is in sequence */ - rtp->learning_probation--; - rtp->learning_max_seq = seq; - if (rtp->learning_probation == 0) { - probation = 0; - } + info->packets--; } else { - rtp->learning_probation = learning_min_sequential - 1; - rtp->learning_max_seq = seq; + /* Sequence discontinuity; reset */ + info->packets = learning_min_sequential - 1; } + info->max_seq = seq; - return probation; + return (info->packets == 0); } static void rtp_add_candidates_to_ice(struct ast_rtp_instance *instance, struct ast_rtp *rtp, struct ast_sockaddr *addr, int port, int component, @@ -1719,7 +1716,8 @@ static int ast_rtp_new(struct ast_rtp_instance *instance, rtp->seqno = ast_random() & 0xffff; rtp->strict_rtp_state = (strictrtp ? STRICT_RTP_LEARN : STRICT_RTP_OPEN); if (strictrtp) { - rtp_learning_seq_init(rtp, (uint16_t)rtp->seqno); + rtp_learning_seq_init(&rtp->rtp_source_learn, (uint16_t)rtp->seqno); + rtp_learning_seq_init(&rtp->alt_source_learn, (uint16_t)rtp->seqno); } /* Create a new socket for us to listen on and use */ @@ -3552,34 +3550,36 @@ static struct ast_frame *ast_rtp_read(struct ast_rtp_instance *instance, int rtc /* If strict RTP protection is enabled see if we need to learn the remote address or if we need to drop the packet */ if (rtp->strict_rtp_state == STRICT_RTP_LEARN) { - ast_debug(1, "%p -- start learning mode pass with addr = %s\n", rtp, ast_sockaddr_stringify(&addr)); + ast_debug(1, "%p -- Probation learning mode pass with source address %s\n", rtp, ast_sockaddr_stringify(&addr)); /* For now, we always copy the address. */ ast_sockaddr_copy(&rtp->strict_rtp_address, &addr); /* Send the rtp and the seqno from header to rtp_learning_rtp_seq_update to see whether we can exit or not*/ - if (rtp_learning_rtp_seq_update(rtp, ntohl(rtpheader[0]))) { - ast_debug(1, "%p -- Condition for learning hasn't exited, so reject the frame.\n", rtp); + if (rtp_learning_rtp_seq_update(&rtp->rtp_source_learn, seqno)) { + ast_debug(1, "%p -- Probation at seq %d with %d to go; discarding frame\n", + rtp, rtp->rtp_source_learn.max_seq, rtp->rtp_source_learn.packets); return &ast_null_frame; } - ast_debug(1, "%p -- Probation Ended. Set strict_rtp_state to STRICT_RTP_CLOSED with address %s\n", rtp, ast_sockaddr_stringify(&addr)); + ast_verb(4, "%p -- Probation passed - setting RTP source address to %s\n", rtp, ast_sockaddr_stringify(&addr)); rtp->strict_rtp_state = STRICT_RTP_CLOSED; - } else if (rtp->strict_rtp_state == STRICT_RTP_CLOSED) { - if (ast_sockaddr_cmp(&rtp->strict_rtp_address, &addr)) { - /* Hmm, not the strict addres. Perhaps we're getting audio from the alternate? */ - if (!ast_sockaddr_cmp(&rtp->alt_rtp_address, &addr)) { - /* ooh, we did! You're now the new expected address, son! */ - ast_sockaddr_copy(&rtp->strict_rtp_address, - &addr); - } else { - const char *real_addr = ast_strdupa(ast_sockaddr_stringify(&addr)); - const char *expected_addr = ast_strdupa(ast_sockaddr_stringify(&rtp->strict_rtp_address)); - - ast_debug(1, "Received RTP packet from %s, dropping due to strict RTP protection. Expected it to be from %s\n", - real_addr, expected_addr); - + } + if (rtp->strict_rtp_state == STRICT_RTP_CLOSED) { + if (!ast_sockaddr_cmp(&rtp->strict_rtp_address, &addr)) { + /* Always reset the alternate learning source */ + rtp_learning_seq_init(&rtp->alt_source_learn, seqno); + } else { + /* Start trying to learn from the new address. If we pass a probationary period with + * it, that means we've stopped getting RTP from the original source and we should + * switch to it. + */ + if (rtp_learning_rtp_seq_update(&rtp->alt_source_learn, seqno)) { + ast_debug(1, "%p -- Received RTP packet from %s, dropping due to strict RTP protection. Will switch to it in %d packets\n", + rtp, ast_sockaddr_stringify(&addr), rtp->alt_source_learn.packets); return &ast_null_frame; } + ast_verb(4, "%p -- Switching RTP source address to %s\n", rtp, ast_sockaddr_stringify(&addr)); + ast_sockaddr_copy(&rtp->strict_rtp_address, &addr); } } @@ -3958,24 +3958,12 @@ static void ast_rtp_remote_address_set(struct ast_rtp_instance *instance, struct if (strictrtp && rtp->strict_rtp_state != STRICT_RTP_OPEN) { rtp->strict_rtp_state = STRICT_RTP_LEARN; - rtp_learning_seq_init(rtp, rtp->seqno); + rtp_learning_seq_init(&rtp->rtp_source_learn, rtp->seqno); } return; } -static void ast_rtp_alt_remote_address_set(struct ast_rtp_instance *instance, struct ast_sockaddr *addr) -{ - struct ast_rtp *rtp = ast_rtp_instance_get_data(instance); - - /* No need to futz with rtp->rtcp here because ast_rtcp_read is already able to adjust if receiving - * RTCP from an "unexpected" source - */ - ast_sockaddr_copy(&rtp->alt_rtp_address, addr); - - return; -} - /*! \brief Write t140 redundacy frame * \param data primary data to be buffered */