Merged revisions 252089 via svnmerge from

https://origsvn.digium.com/svn/asterisk/trunk

........
  r252089 | twilson | 2010-03-12 16:04:51 -0600 (Fri, 12 Mar 2010) | 20 lines

  Only change the RTP ssrc when we see that it has changed

  This change basically reverts the change reviewed in
  https://reviewboard.asterisk.org/r/374/ and instead limits the
  updating of the RTP synchronization source to only those times when we
  detect that the other side of the conversation has changed the ssrc.

  The problem is that SRCUPDATE control frames are sent many times where
  we don't want a new ssrc, including whenever Asterisk has to send DTMF
  in a normal bridge. This is also not the first time that this mistake
  has been made. The initial implementation of the ast_rtp_new_source
  function also changed the ssrc--and then it was removed because of
  this same issue. Then, we put it back in again to fix a different
  issue. This patch attempts to only change the ssrc when we see that
  the other side of the conversation has changed the ssrc.

  It also renames some functions to make their purpose more clear.

  Review: https://reviewboard.asterisk.org/r/540/
........

git-svn-id: https://origsvn.digium.com/svn/asterisk/branches/1.4@252175 65c4cc65-6c06-0410-ace0-fbb531ad65f3

diff --git a/channels/chan_h323.c b/channels/chan_h323.c
index 700fdd174e35eeb0ca149a3703e8698a53a07267..f8a929d6e505aeed009a7d6f0869011be50d71ab 100644 (file)
--- a/channels/chan_h323.c
+++ b/channels/chan_h323.c
@@ -918,7 +918,11 @@ static int oh323_indicate(struct ast_channel *c, int condition, const void *data
                res = 0;
                break;
        case AST_CONTROL_SRCUPDATE:
-               ast_rtp_new_source(pvt->rtp);
+               ast_rtp_update_source(pvt->rtp);
+               res = 0;
+               break;
+       case AST_CONTROL_SRCCHANGE:
+               ast_rtp_change_source(pvt->rtp);
                res = 0;
                break;
        case AST_CONTROL_PROCEEDING:

diff --git a/channels/chan_mgcp.c b/channels/chan_mgcp.c
index 159657b855b65cffcf179c04cb011590631f43fa..3f8b49e0dda36cb072df1de1d136850bb621a99e 100644 (file)
--- a/channels/chan_mgcp.c
+++ b/channels/chan_mgcp.c
@@ -1442,7 +1442,10 @@ static int mgcp_indicate(struct ast_channel *ast, int ind, const void *data, siz
                ast_moh_stop(ast);
                break;
        case AST_CONTROL_SRCUPDATE:
-               ast_rtp_new_source(sub->rtp);
+               ast_rtp_update_source(sub->rtp);
+               break;
+       case AST_CONTROL_SRCCHANGE:
+               ast_rtp_change_source(sub->rtp);
                break;
        case -1:
                transmit_notify_request(sub, "");

diff --git a/channels/chan_sip.c b/channels/chan_sip.c
index 618fdd7ca513622fbae33aa0be017dd24326990d..6d80d466f38818540d8ea3393cbff4581ad11036 100644 (file)
--- a/channels/chan_sip.c
+++ b/channels/chan_sip.c
@@ -811,12 +811,11 @@ struct sip_auth {
 #define SIP_PAGE2_UDPTL_DESTINATION     (1 << 28)       /*!< 28: Use source IP of RTP as destination if NAT is enabled */
 #define SIP_PAGE2_DIALOG_ESTABLISHED    (1 << 29)       /*!< 29: Has a dialog been established? */
 #define SIP_PAGE2_RPORT_PRESENT         (1 << 30)       /*!< 30: Was rport received in the Via header? */
-#define SIP_PAGE2_CONSTANT_SSRC         (1 << 31)       /*!< 31: Don't change SSRC on reinvite */
 
 #define SIP_PAGE2_FLAGS_TO_COPY \
        (SIP_PAGE2_ALLOWSUBSCRIBE | SIP_PAGE2_ALLOWOVERLAP | SIP_PAGE2_VIDEOSUPPORT | \
        SIP_PAGE2_T38SUPPORT | SIP_PAGE2_RFC2833_COMPENSATE | SIP_PAGE2_BUGGY_MWI | \
-       SIP_PAGE2_UDPTL_DESTINATION | SIP_PAGE2_CONSTANT_SSRC)
+       SIP_PAGE2_UDPTL_DESTINATION)
 
 /* SIP packet flags */
 #define SIP_PKT_DEBUG          (1 << 0)        /*!< Debug this packet */
@@ -2952,9 +2951,6 @@ static int create_addr_from_peer(struct sip_pvt *dialog, struct sip_peer *peer)
                ast_rtp_set_rtptimeout(dialog->rtp, peer->rtptimeout);
                ast_rtp_set_rtpholdtimeout(dialog->rtp, peer->rtpholdtimeout);
                ast_rtp_set_rtpkeepalive(dialog->rtp, peer->rtpkeepalive);
-               if (ast_test_flag(&dialog->flags[1], SIP_PAGE2_CONSTANT_SSRC)) {
-                       ast_rtp_set_constantssrc(dialog->rtp);
-               }
                /* Set Frame packetization */
                ast_rtp_codec_setpref(dialog->rtp, &dialog->prefs);
                dialog->autoframing = peer->autoframing;
@@ -2965,9 +2961,6 @@ static int create_addr_from_peer(struct sip_pvt *dialog, struct sip_peer *peer)
                ast_rtp_set_rtptimeout(dialog->vrtp, peer->rtptimeout);
                ast_rtp_set_rtpholdtimeout(dialog->vrtp, peer->rtpholdtimeout);
                ast_rtp_set_rtpkeepalive(dialog->vrtp, peer->rtpkeepalive);
-               if (ast_test_flag(&dialog->flags[1], SIP_PAGE2_CONSTANT_SSRC)) {
-                       ast_rtp_set_constantssrc(dialog->vrtp);
-               }
        }
 
        ast_string_field_set(dialog, peername, peer->name);
@@ -3871,6 +3864,7 @@ static int sip_answer(struct ast_channel *ast)
                if (option_debug)
                        ast_log(LOG_DEBUG, "SIP answering channel: %s\n", ast->name);
 
+               ast_rtp_update_source(p->rtp);
                res = transmit_response_with_sdp(p, "200 OK", &p->initreq, XMIT_CRITICAL);
                ast_set_flag(&p->flags[1], SIP_PAGE2_DIALOG_ESTABLISHED);
        }
@@ -3905,7 +3899,7 @@ static int sip_write(struct ast_channel *ast, struct ast_frame *frame)
                                if ((ast->_state != AST_STATE_UP) &&
                                    !ast_test_flag(&p->flags[0], SIP_PROGRESS_SENT) &&
                                    !ast_test_flag(&p->flags[0], SIP_OUTGOING)) {
-                                       ast_rtp_new_source(p->rtp);
+                                       ast_rtp_update_source(p->rtp);
                                        if (!global_prematuremediafilter) {
                                                p->invitestate = INV_EARLY_MEDIA;
                                                transmit_provisional_response(p, "183 Session Progress", &p->initreq, 1);
@@ -4153,11 +4147,11 @@ static int sip_indicate(struct ast_channel *ast, int condition, const void *data
                res = -1;
                break;
        case AST_CONTROL_HOLD:
-               ast_rtp_new_source(p->rtp);
+               ast_rtp_update_source(p->rtp);
                ast_moh_start(ast, data, p->mohinterpret);
                break;
        case AST_CONTROL_UNHOLD:
-               ast_rtp_new_source(p->rtp);
+               ast_rtp_update_source(p->rtp);
                ast_moh_stop(ast);
                break;
        case AST_CONTROL_VIDUPDATE:     /* Request a video frame update */
@@ -4168,7 +4162,10 @@ static int sip_indicate(struct ast_channel *ast, int condition, const void *data
                        res = -1;
                break;
        case AST_CONTROL_SRCUPDATE:
-               ast_rtp_new_source(p->rtp);
+               ast_rtp_update_source(p->rtp);
+               break;
+       case AST_CONTROL_SRCCHANGE:
+               ast_rtp_change_source(p->rtp);
                break;
        case -1:
                res = -1;
@@ -15085,14 +15082,6 @@ static int handle_request_invite(struct sip_pvt *p, struct sip_request *req, int
                                res = -1;
                                goto request_invite_cleanup;
                        }
-                       if (ast_test_flag(&p->flags[1], SIP_PAGE2_CONSTANT_SSRC)) {
-                               if (p->rtp) {
-                                       ast_rtp_set_constantssrc(p->rtp);
-                               }
-                               if (p->vrtp) {
-                                       ast_rtp_set_constantssrc(p->vrtp);
-                               }
-                       }
                } else {        /* No SDP in invite, call control session */
                        p->jointcapability = p->capability;
                        if (option_debug > 1)
@@ -17574,9 +17563,6 @@ static int handle_common_options(struct ast_flags *flags, struct ast_flags *mask
        } else if (!strcasecmp(v->name, "t38pt_usertpsource")) {
                ast_set_flag(&mask[1], SIP_PAGE2_UDPTL_DESTINATION);
                ast_set2_flag(&flags[1], ast_true(v->value), SIP_PAGE2_UDPTL_DESTINATION);
-       } else if (!strcasecmp(v->name, "constantssrc")) {
-               ast_set_flag(&mask[1], SIP_PAGE2_CONSTANT_SSRC);
-               ast_set2_flag(&flags[1], ast_true(v->value), SIP_PAGE2_CONSTANT_SSRC);
        } else
                res = 0;
 
@@ -18650,8 +18636,6 @@ static int reload_config(enum channelreloadreason reason)
                                default_maxcallbitrate = DEFAULT_MAX_CALL_BITRATE;
                } else if (!strcasecmp(v->name, "matchexterniplocally")) {
                        global_matchexterniplocally = ast_true(v->value);
-               } else if (!strcasecmp(v->name, "constantssrc")) {
-                       ast_set2_flag(&global_flags[1], ast_true(v->value), SIP_PAGE2_CONSTANT_SSRC);
                } else if (!strcasecmp(v->name, "shrinkcallerid")) {
                        if (ast_true(v->value)) {
                                global_shrinkcallerid = 1;

diff --git a/channels/chan_skinny.c b/channels/chan_skinny.c
index 812aa16fb5524ee0417739facf64eac532f52d08..48be77b9c3d7cfd8ced8e26d976c8163c36dffac 100644 (file)
--- a/channels/chan_skinny.c
+++ b/channels/chan_skinny.c
@@ -2868,7 +2868,10 @@ static int skinny_indicate(struct ast_channel *ast, int ind, const void *data, s
        case AST_CONTROL_PROCEEDING:
                break;
        case AST_CONTROL_SRCUPDATE:
-               ast_rtp_new_source(sub->rtp);
+               ast_rtp_update_source(sub->rtp);
+               break;
+       case AST_CONTROL_SRCCHANGE:
+               ast_rtp_change_source(sub->rtp);
                break;
        default:
                ast_log(LOG_WARNING, "Don't know how to indicate condition %d\n", ind);

diff --git a/configs/sip.conf.sample b/configs/sip.conf.sample
index 914ccb775edb64cdba4e0c05f63f81aab26df33a..838196d8041e9659d6e05bb13c557e5f7903732a 100644 (file)
--- a/configs/sip.conf.sample
+++ b/configs/sip.conf.sample
@@ -385,8 +385,6 @@ srvlookup=yes                   ; Enable DNS SRV lookups on outbound calls
                                 ; instead of INVITE. This can be combined with 'nonat', as
                                 ; 'canreinvite=update,nonat'. It implies 'yes'.
 
-;constantssrc=yes               ; Don't change the RTP SSRC when our media stream changes
-
 ;----------------------------------------- REALTIME SUPPORT ------------------------
 ; For additional information on ARA, the Asterisk Realtime Architecture,
 ; please read realtime.txt and extconfig.txt in the /doc directory of the
@@ -546,7 +544,6 @@ srvlookup=yes                   ; Enable DNS SRV lookups on outbound calls
 ; maxcallbitrate              maxcallbitrate
 ; rfc2833compensate           mailbox
 ; t38pt_usertpsource          username
-; constantssrc                template
 ;                             fromdomain
 ;                             regexten
 ;                             fromuser
@@ -559,7 +556,6 @@ srvlookup=yes                   ; Enable DNS SRV lookups on outbound calls
 ;                             sendrpid
 ;                             outboundproxy
 ;                             rfc2833compensate
-;                             constantssrc
 ;                             t38pt_usertpsource
 ;                             contactpermit         ; Limit what a host may register as (a neat trick
 ;                             contactdeny           ; is to register at the same IP as a SIP provider,

diff --git a/include/asterisk/frame.h b/include/asterisk/frame.h
index 5cfe0eb0af86d8546891848c4d60360e0bdeae0e..7f45d91f0ffd4719cf27343eb6695dbe4090d494 100644 (file)
--- a/include/asterisk/frame.h
+++ b/include/asterisk/frame.h
@@ -85,7 +85,8 @@ struct ast_codec_pref {
        \arg \b HOLD    Call is placed on hold
        \arg \b UNHOLD  Call is back from hold
        \arg \b VIDUPDATE       Video update requested
-       \arg \b SRCUPDATE       The source of media has changed
+       \arg \b SRCUPDATE The source of media has changed (RTP marker bit must change)
+       \arg \b SRCCHANGE Media source has changed (RTP marker bit and SSRC must change)
 
 */
 
@@ -290,6 +291,7 @@ enum ast_control_frame_type {
        AST_CONTROL_UNHOLD = 17,        /*!< Indicate call is left from hold */
        AST_CONTROL_VIDUPDATE = 18,     /*!< Indicate video frame update */
        AST_CONTROL_SRCUPDATE = 20,     /*!< Indicate source of media has changed */
+       AST_CONTROL_SRCCHANGE = 21,     /*!< Media has changed and requires a new RTP SSRC */
 };
 
 #define AST_SMOOTHER_FLAG_G729         (1 << 0)

diff --git a/include/asterisk/rtp.h b/include/asterisk/rtp.h
index fada0bcb473a2048fc6767afc7b04504a85b3a90..955e8bf99ca0ca45e16f34fb2e549c49339460a2 100644 (file)
--- a/include/asterisk/rtp.h
+++ b/include/asterisk/rtp.h
@@ -179,10 +179,11 @@ int ast_rtp_sendcng(struct ast_rtp *rtp, int level);
 
 int ast_rtp_settos(struct ast_rtp *rtp, int tos);
 
-/*! \brief When changing sources, don't generate a new SSRC */
-void ast_rtp_set_constantssrc(struct ast_rtp *rtp);
+/*! \brief Indicate that we need to set the marker bit */
+void ast_rtp_update_source(struct ast_rtp *rtp);
 
-void ast_rtp_new_source(struct ast_rtp *rtp);
+/*! \brief Indicate that we need to set the marker bit and change the ssrc */
+void ast_rtp_change_source(struct ast_rtp *rtp);
 
 /*! \brief  Setting RTP payload types from lines in a SDP description: */
 void ast_rtp_pt_clear(struct ast_rtp* rtp);

diff --git a/main/channel.c b/main/channel.c
index a4ffd629d3ee51aae154b9edb3db0026f7bbc61a..4141d51bf5c3d974a5951148feca6e79e8a0be65 100644 (file)
--- a/main/channel.c
+++ b/main/channel.c
@@ -1972,6 +1972,7 @@ int ast_waitfordigit_full(struct ast_channel *c, int ms, int audiofd, int cmdfd)
                                case AST_CONTROL_RINGING:
                                case AST_CONTROL_ANSWER:
                                case AST_CONTROL_SRCUPDATE:
+                               case AST_CONTROL_SRCCHANGE:
                                        /* Unimportant */
                                        break;
                                default:
@@ -2571,6 +2572,7 @@ static int attribute_const is_visible_indication(enum ast_control_frame_type con
        case AST_CONTROL_PROCEEDING:
        case AST_CONTROL_VIDUPDATE:
        case AST_CONTROL_SRCUPDATE:
+       case AST_CONTROL_SRCCHANGE:
        case AST_CONTROL_RADIO_KEY:
        case AST_CONTROL_RADIO_UNKEY:
        case AST_CONTROL_OPTION:
@@ -2663,6 +2665,7 @@ int ast_indicate_data(struct ast_channel *chan, int _condition,
        case AST_CONTROL_PROCEEDING:
        case AST_CONTROL_VIDUPDATE:
        case AST_CONTROL_SRCUPDATE:
+       case AST_CONTROL_SRCCHANGE:
        case AST_CONTROL_RADIO_KEY:
        case AST_CONTROL_RADIO_UNKEY:
        case AST_CONTROL_OPTION:
@@ -3367,6 +3370,7 @@ struct ast_channel *__ast_request_and_dial(const char *type, int format, void *d
                                case AST_CONTROL_UNHOLD:
                                case AST_CONTROL_VIDUPDATE:
                                case AST_CONTROL_SRCUPDATE:
+                               case AST_CONTROL_SRCCHANGE:
                                case -1:                        /* Ignore -- just stopping indications */
                                        break;
 
@@ -4316,6 +4320,7 @@ static enum ast_bridge_result ast_generic_bridge(struct ast_channel *c0, struct
                        case AST_CONTROL_UNHOLD:
                        case AST_CONTROL_VIDUPDATE:
                        case AST_CONTROL_SRCUPDATE:
+                       case AST_CONTROL_SRCCHANGE:
                                ast_indicate_data(other, f->subclass, f->data, f->datalen);
                                if (jb_in_use) {
                                        ast_jb_empty_and_reset(c0, c1);

diff --git a/main/rtp.c b/main/rtp.c
index 4f54b71a56e24d8b6e1fa970e2e4b4e9e2bdeeb6..76deb3c693b95bc1a37750b4dd1090c0945b9a97 100644 (file)
--- a/main/rtp.c
+++ b/main/rtp.c
@@ -174,7 +174,6 @@ struct ast_rtp {
        struct ast_codec_pref pref;
        struct ast_rtp *bridged;        /*!< Who we are Packet bridged to */
        int set_marker_bit:1;           /*!< Whether to set the marker bit or not */
-       unsigned int constantssrc:1;
 };
 
 /* Forward declarations */
@@ -1175,6 +1174,7 @@ struct ast_frame *ast_rtp_read(struct ast_rtp *rtp)
        unsigned int *rtpheader;
        struct rtpPayloadType rtpPT;
        struct ast_rtp *bridged = NULL;
+       AST_LIST_HEAD_NOLOCK(, ast_frame) frames;
        
        /* If time is up, kill it */
        if (rtp->sending_digit)
@@ -1253,11 +1253,23 @@ struct ast_frame *ast_rtp_read(struct ast_rtp *rtp)
        seqno &= 0xffff;
        timestamp = ntohl(rtpheader[1]);
        ssrc = ntohl(rtpheader[2]);
-       
-       if (!mark && rtp->rxssrc && rtp->rxssrc != ssrc) {
-               if (option_debug || rtpdebug)
-                       ast_log(LOG_DEBUG, "Forcing Marker bit, because SSRC has changed\n");
-               mark = 1;
+
+       AST_LIST_HEAD_INIT_NOLOCK(&frames);
+       /* Force a marker bit and change SSRC if the SSRC changes */
+       if (rtp->rxssrc && rtp->rxssrc != ssrc) {
+               struct ast_frame *f, srcupdate = {
+                       AST_FRAME_CONTROL,
+                       .subclass = AST_CONTROL_SRCCHANGE,
+               };
+ 
+               if (!mark) {
+                       if (option_debug || rtpdebug) {
+                               ast_log(LOG_DEBUG, "Forcing Marker bit, because SSRC has changed\n");
+                       }
+                       mark = 1;
+               }
+               f = ast_frisolate(&srcupdate);
+               AST_LIST_INSERT_TAIL(&frames, f, frame_list);
        }
 
        rtp->rxssrc = ssrc;
@@ -1280,7 +1292,7 @@ struct ast_frame *ast_rtp_read(struct ast_rtp *rtp)
 
        if (res < hdrlen) {
                ast_log(LOG_WARNING, "RTP Read too short (%d, expecting %d)\n", res, hdrlen);
-               return &ast_null_frame;
+               return AST_LIST_FIRST(&frames) ? AST_LIST_FIRST(&frames) : &ast_null_frame;
        }
 
        rtp->rxcount++; /* Only count reasonably valid packets, this'll make the rtcp stats more accurate */
@@ -1342,7 +1354,11 @@ struct ast_frame *ast_rtp_read(struct ast_rtp *rtp)
                } else {
                        ast_log(LOG_NOTICE, "Unknown RTP codec %d received from '%s'\n", payloadtype, ast_inet_ntoa(rtp->them.sin_addr));
                }
-               return f ? f : &ast_null_frame;
+               if (f) {
+                       AST_LIST_INSERT_TAIL(&frames, f, frame_list);
+                       return AST_LIST_FIRST(&frames);
+               }
+               return &ast_null_frame;
        }
        rtp->lastrxformat = rtp->f.subclass = rtpPT.code;
        rtp->f.frametype = (rtp->f.subclass < AST_FORMAT_MAX_AUDIO) ? AST_FRAME_VOICE : AST_FRAME_VIDEO;
@@ -1358,7 +1374,8 @@ struct ast_frame *ast_rtp_read(struct ast_rtp *rtp)
                        f->len = ast_tvdiff_ms(ast_samp2tv(rtp->dtmf_duration, rtp_get_rate(f->subclass)), ast_tv(0, 0));
                        rtp->resp = 0;
                        rtp->dtmf_timeout = rtp->dtmf_duration = 0;
-                       return f;
+                       AST_LIST_INSERT_TAIL(&frames, f, frame_list);
+                       return AST_LIST_FIRST(&frames);
                }
        }
 
@@ -1391,7 +1408,9 @@ struct ast_frame *ast_rtp_read(struct ast_rtp *rtp)
                        rtp->f.subclass |= 0x1;
        }
        rtp->f.src = "RTP";
-       return &rtp->f;
+
+       AST_LIST_INSERT_TAIL(&frames, &rtp->f, frame_list);
+       return AST_LIST_FIRST(&frames);
 }
 
 /* The following array defines the MIME Media type (and subtype) for each
@@ -2063,18 +2082,26 @@ int ast_rtp_settos(struct ast_rtp *rtp, int tos)
        return res;
 }
 
-void ast_rtp_set_constantssrc(struct ast_rtp *rtp)
+void ast_rtp_update_source(struct ast_rtp *rtp)
 {
-       rtp->constantssrc = 1;
+       if (rtp) {
+               rtp->set_marker_bit = 1;
+               if (option_debug > 2) {
+                       ast_log(LOG_DEBUG, "Setting the marker bit due to a source update\n");
+               }
+       }
 }
 
-void ast_rtp_new_source(struct ast_rtp *rtp)
+void ast_rtp_change_source(struct ast_rtp *rtp)
 {
        if (rtp) {
+               unsigned int ssrc = ast_random();
+
                rtp->set_marker_bit = 1;
-               if (!rtp->constantssrc) {
-                       rtp->ssrc = ast_random();
+               if (option_debug > 2) {
+                       ast_log(LOG_DEBUG, "Changing ssrc from %u to %u due to a source change\n", rtp->ssrc, ssrc);
                }
+               rtp->ssrc = ssrc;
        }
 }