sip codec negotiation of dynamic rtp payloads error fix

This patch fixes how chan_sip handles dynamic rtp payload types
it does not understand.  At the moment if a dynamic payload's mime
type does not match one we understand, the payload does not get
removed from our payload table.  As a result of this, the payload
is set to whatever dynamic codec we use internally for that payload
number on outgoing INVITES.  This is incorrect.

This patch fixes this by properly checking the rtpmap set function's
return code to make sure it was found.  The function can return both
-1 and -2 depending on the source of the mismatch.  We were just
checking -1 explicitly.

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

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

diff --git a/channels/chan_sip.c b/channels/chan_sip.c
index 9a9fcc61d9c1c250f445edbc626cdc7ea51c42c8..513d874da5dafc811cad1fcfcb425f21ef931a80 100644 (file)
--- a/channels/chan_sip.c
+++ b/channels/chan_sip.c
@@ -8891,8 +8891,8 @@ static int process_sdp_a_audio(const char *a, struct sip_pvt *p, struct ast_rtp_
        } else if (sscanf(a, "rtpmap: %30u %127[^/]/%30u", &codec, mimeSubtype, &sample_rate) == 3) {
                /* We have a rtpmap to handle */
                if (*last_rtpmap_codec < SDP_MAX_RTPMAP_CODECS) {
-                       if (ast_rtp_codecs_payloads_set_rtpmap_type_rate(newaudiortp, NULL, codec, "audio", mimeSubtype,
-                           ast_test_flag(&p->flags[0], SIP_G726_NONSTANDARD) ? AST_RTP_OPT_G726_NONSTANDARD : 0, sample_rate) != -1) {
+                       if (!(ast_rtp_codecs_payloads_set_rtpmap_type_rate(newaudiortp, NULL, codec, "audio", mimeSubtype,
+                           ast_test_flag(&p->flags[0], SIP_G726_NONSTANDARD) ? AST_RTP_OPT_G726_NONSTANDARD : 0, sample_rate))) {
                                if (debug)
                                        ast_verbose("Found audio description format %s for ID %d\n", mimeSubtype, codec);
                                //found_rtpmap_codecs[last_rtpmap_codec] = codec;
@@ -8964,7 +8964,7 @@ static int process_sdp_a_video(const char *a, struct sip_pvt *p, struct ast_rtp_
                if (*last_rtpmap_codec < SDP_MAX_RTPMAP_CODECS) {
                        /* Note: should really look at the '#chans' params too */
                        if (!strncasecmp(mimeSubtype, "H26", 3) || !strncasecmp(mimeSubtype, "MP4", 3)) {
-                               if (ast_rtp_codecs_payloads_set_rtpmap_type_rate(newvideortp, NULL, codec, "video", mimeSubtype, 0, sample_rate) != -1) {
+                               if (!(ast_rtp_codecs_payloads_set_rtpmap_type_rate(newvideortp, NULL, codec, "video", mimeSubtype, 0, sample_rate))) {
                                        if (debug)
                                                ast_verbose("Found video description format %s for ID %d\n", mimeSubtype, codec);
                                        //found_rtpmap_codecs[last_rtpmap_codec] = codec;

diff --git a/include/asterisk/rtp_engine.h b/include/asterisk/rtp_engine.h
index 39633c84c8ef4d8e72d123fb25e403613083dc06..5313fc11b17470ef8d49684d3d781d678b7b9ad9 100644 (file)
--- a/include/asterisk/rtp_engine.h
+++ b/include/asterisk/rtp_engine.h
@@ -964,7 +964,8 @@ void ast_rtp_codecs_payloads_set_m_type(struct ast_rtp_codecs *codecs, struct as
  * \param options Optional options that may change the behavior of this specific payload
  *
  * \retval 0 success
- * \retval -1 failure
+ * \retval -1 failure, invalid payload numbe
+ * \retval -2 failure, unknown mimetype
  *
  * Example usage:
  *

diff --git a/main/rtp_engine.c b/main/rtp_engine.c
index e673fc4f30cf6ecb395a62689a5269169dd76924..4159170c902070e6396f32fd0c2cbf4369ffe362 100644 (file)
--- a/main/rtp_engine.c
+++ b/main/rtp_engine.c
@@ -562,8 +562,8 @@ int ast_rtp_codecs_payloads_set_rtpmap_type_rate(struct ast_rtp_codecs *codecs,
                }
 
                /* if both sample rates have been supplied, and they don't match,
-                                     then this not a match; if one has not been supplied, then the
-                                     rates are not compared */
+                * then this not a match; if one has not been supplied, then the
+                * rates are not compared */
                if (sample_rate && t->sample_rate &&
                    (sample_rate != t->sample_rate)) {
                        continue;