[asterisk-commits] mmichelson: branch mmichelson/correct_sdp_answer r206699 - /team/mmichelson/c...

SVN commits to the Asterisk project asterisk-commits at lists.digium.com
Wed Jul 15 11:55:12 CDT 2009


Author: mmichelson
Date: Wed Jul 15 11:55:08 2009
New Revision: 206699

URL: http://svn.asterisk.org/svn-view/asterisk?view=rev&rev=206699
Log:
Remove all XXX MCM comments.


Modified:
    team/mmichelson/correct_sdp_answer/channels/chan_sip.c

Modified: team/mmichelson/correct_sdp_answer/channels/chan_sip.c
URL: http://svn.asterisk.org/svn-view/asterisk/team/mmichelson/correct_sdp_answer/channels/chan_sip.c?view=diff&rev=206699&r1=206698&r2=206699
==============================================================================
--- team/mmichelson/correct_sdp_answer/channels/chan_sip.c (original)
+++ team/mmichelson/correct_sdp_answer/channels/chan_sip.c Wed Jul 15 11:55:08 2009
@@ -5335,11 +5335,6 @@
 
 		numberofports = 1;
 		len = -1;
-		/* XXX MCM It appears that if we come across multiple audio streams, then the final one
-		 * we parse is the one that will actually be answered in the answering SDP.
-		 *
-		 * Same appears to apply to video and image streams, too.
-		 */
 		if ((sscanf(m, "audio %d/%d RTP/AVP %n", &x, &numberofports, &len) == 2 && len > 0) ||
 		    (sscanf(m, "audio %d RTP/AVP %n", &x, &len) == 1 && len > 0)) {
 			audio = TRUE;
@@ -5359,11 +5354,6 @@
 					ast_verbose("Found RTP audio format %d\n", codec);
 				ast_rtp_set_m_type(newaudiortp, codec);
 			}
-		/* XXX MCM It appears that video streams are parsed on incoming offers no matter whether video
-		 * support is enabled or not. This is a good thing with regards to this branch. The information
-		 * is stored in the "newvideortp" structure. I'll need to check down below to see what is actually
-		 * done with this info...
-		 */
 		} else if ((sscanf(m, "video %d/%d RTP/AVP %n", &x, &numberofports, &len) == 2 && len > 0) ||
 		    (sscanf(m, "video %d RTP/AVP %n", &x, &len) == 1 && len >= 0)) {
 			/* If it is not audio - is it video ? */
@@ -5783,16 +5773,6 @@
 	p->jointnoncodeccapability = newnoncodeccapability;	/* DTMF capabilities */
 
 	ast_rtp_pt_copy(p->rtp, newaudiortp);
-	/* XXX MCM. Ah cripes, so even though we're storing the video information in the newvideortp
-	 * structure, we don't actually store anything about it on p unless there is a vrtp. vrtp's existence
-	 * is predicated on "videosupport" being enabled in sip.conf.
-	 *
-	 * So something needs to be added to this function so we can store a simple boolean on p telling if
-	 * video was offered. That way, in add_sdp, we can use this info to tell if we need to add an m= line
-	 * for video even if video support is not enabled.
-	 *
-	 * Similar measures need to be taken for image and audio streams as well.
-	 */
 	if (p->vrtp)
 		ast_rtp_pt_copy(p->vrtp, newvideortp);
 
@@ -6801,14 +6781,6 @@
 
 		/* Ok, we need video. Let's add what we need for video and set codecs.
 		   Video is handled differently than audio since we can not transcode. */
-		/* XXX MCM To simplify matters, a good idea would probably be to use whatever boolean was stored
-		 * on p to set the 'needvideo' variable. Then, within this block, we can just set vdest.sin_port to
-		 * 0.
-		 *
-		 * Actually, taking a closer look, 'needvideo' would have to be altered all over the place if I were to
-		 * try to overload it like this. A better method would just be to wait until the end of this function to
-		 * decide about whether to add video to the SDP or not.
-		 */
 		if (needvideo) {
 			/* Determine video destination */
 			if (p->vredirip.sin_addr.s_addr) {
@@ -6872,9 +6844,6 @@
 		}
 
 		/* Now send any other common audio and video codecs, and non-codec formats: */
-		/* XXX MCM Will need to make sure that we don't actually start adding video codecs if 
-		 * video is not supported.
-		 */
 		for (x = 1; x <= (needvideo ? AST_FORMAT_MAX_VIDEO : AST_FORMAT_MAX_AUDIO); x <<= 1) {
 			if (!(capability & x))	/* Codec not requested */
 				continue;
@@ -6925,11 +6894,6 @@
 			ast_build_string(&m_video_next, &m_video_left, "\r\n");
 	}
 
-	/* XXX MCM the 'add_t38' variable is determined by the caller of the function and will be set true
-	 * if we wish to offer/answer t38 for this stream. We need to be prepared to place t38 streams with
-	 * port set to 0 even if the caller of this function is not going to request t38 be added if the offer
-	 * we are answering had t38 in it.
-	 */
 	if (add_t38 && p->udptl) {
 		struct sockaddr_in udptlsin;
 		struct sockaddr_in udptldest = { 0, };




More information about the asterisk-commits mailing list