[Heikki Vatiainen <hessu@xxxxxxxxx>]

andreas.sikkema@xxxxxxxxxxx wrote:

> > Should rtp_add_address be called when the media address and ports are
> > found in the SDP?
> 
> In one word, yes. If traffic to this ip adddress / port combination is 
> found, the packet is dissected as RTP or RTCP. Both protocols need a 
> call to rtp_add_address or rtcp_add_address!

Russell, you might want to try the patch I have attached below. I
tested it against a unicast SIP originated RTP stream and multiple
SAP based multicast RTP/RTCP streams.

The main problem with the patch is that you have to "see" the
packet containing the SDP content that describes the upcoming RTP
stream. When I did the patch, I once again forgot that the packets
are not always dissected immediately and for that reason the SDP
line containing the RTP address/port may not be processed by the
functions contained in the patch.

You can work around this problem by opening the SDP content and
then pressing the "Reset" button in the bottom of the Ethereal
window. Once you open the correct SDP packet, the RTP and RTCP
streams are registered and "Reset" makes Ethereal go through all
the packets again. The difference is that this time it knows to
look for the newly discovered RTP/ RTCP streams.


Index: packet-sdp.c
===================================================================
RCS file: /cvsroot/ethereal/packet-sdp.c,v
retrieving revision 1.17
diff -u -r1.17 packet-sdp.c
--- packet-sdp.c	2000/11/19 21:01:06	1.17
+++ packet-sdp.c	2000/11/30 19:48:49
@@ -34,18 +34,128 @@
 #include <sys/types.h>
 #endif
 
+#ifdef HAVE_ARPA_INET_H
+#include <arpa/inet.h>    /* For inet_pton() */
+#endif
+
+#include <errno.h>
 #include <string.h>
 #include <ctype.h>
 
 #include <glib.h>
 #include "packet.h"
 #include "strutil.h"
+#include "packet-rtp.h"
+#include "packet-rtcp.h"
 
 static int proto_sdp = -1;
 
 static int ett_sdp = -1;
 
+/* The format of `value' for unicast is `IN IP4 130.230.83.29' and for
+ * multicast `IN IP4 224.2.1.1/ttl/range' where `range' is the number
+ * of addresses in the contiguous range above the base address
+ *
+ * Just like in sdp_check_rtp(), split using delimiter ` '.
+ */
+static guint32
+sdp_get_ipv4addr(const u_char *value, int valuelen)
+{
+	gchar **fields, buf[60], **addr_fields;
+	int i, is_ip4, ret, maxlen;
+	guint32 addr;
+
+	memset(buf, '\0', sizeof buf);
+	maxlen = (valuelen < (int)sizeof buf) ? valuelen : sizeof buf;
+	strncpy(buf, value, maxlen);
+
+	is_ip4 = 0;
+	fields = g_strsplit(buf, " ", -1);
+	for (i = 0; fields[i] != NULL; i++) {
+		if (i == 2 && g_strncasecmp(fields[1], "IP4", strlen("IP4")) == 0)
+			is_ip4 = 1;
+	}
+	if (!is_ip4) {
+		g_strfreev(fields);
+		return 0;
+	}
+
+	/* Split `address/ttl/range' and discard ttl and range */
+	addr_fields = g_strsplit(fields[2], "/", -1);
+	errno = 0;
+	ret = inet_pton(AF_INET, addr_fields[0], &addr);
+	g_strfreev(fields);
+	g_strfreev(addr_fields);
+
+	return (ret > 0 && errno == 0) ? addr : 0;
+}
+
+/* The format of value is something like `video 49170/2 RTP/AVP 31' or
+ * `audio 18800 RTP/AVP 0', which means that can we split the value
+ * into fields using delimiter ` ', and then check if the third string
+ * starts with `RTP'.
+ *
+ * For RTP, the integer in port/integer notation is always 2 (as far
+ * as I know it) and simply means that RTP uses port and RTCP uses
+ * port+1.
+ */
 static void
+sdp_check_rtp(guint32 ipv4addr, const u_char *value, int valuelen)
+{
+	gchar **fields, buf[60], *endptr, **ports;
+	int i, is_rtp, maxlen;
+	unsigned long baseport;
+
+	if (ipv4addr == 0) return;
+
+	memset(buf, '\0', sizeof buf);
+	maxlen = (valuelen < (int)sizeof buf) ? valuelen : sizeof buf;
+	memcpy(buf, value, maxlen);
+
+	is_rtp = 0;
+	fields = g_strsplit(buf, " ", -1);
+	for (i = 0; fields[i] != NULL; i++) {
+		if (i == 2 && g_strncasecmp(fields[i], "RTP", strlen("RTP")) == 0) {
+			is_rtp = 1;
+			break;
+		}
+	}
+
+	if (!is_rtp) {
+		g_strfreev(fields);
+		return;
+	}
+
+	/* Split `port/integer'. If `/integer' part was not present,
+	 * ports[1] will be NULL */
+	ports = g_strsplit(fields[1], "/", -1);
+	errno = 0;
+	baseport = strtoul(ports[0], &endptr, 10);
+	if (endptr == ports[0] || errno != 0) {
+		g_strfreev(fields);
+		g_strfreev(ports);
+		return;
+	}
+
+	rtp_add_address((const unsigned char *)&ipv4addr, (int)baseport);
+
+	/* Grr, unconditionally add RTCP since at least in the
+	 * multicast SAP announcements I captured, none of the
+	 * advertisers actually used the port range.
+	 *
+	 */
+#if 0
+	 if (ports[1] != NULL)
+#endif
+		 rtcp_add_address((const unsigned char *)&ipv4addr, (int)(baseport + 1));
+
+	g_strfreev(fields);
+	g_strfreev(ports);
+
+	return;
+}
+
+static void
 dissect_sdp(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree)
 {
 	proto_tree	*sdp_tree;
@@ -60,6 +170,9 @@
 	int		valuelen;
 	const char	*typename;
 	int		datalen;
+	guint32         ipv4addr = 0;
+	const u_char    *rtp_value = NULL;
+	int             rtp_valuelen = 0;
 
 	CHECK_DISPLAY_AS_DATA(proto_sdp, tvb, pinfo, tree);
 
@@ -156,6 +269,7 @@
 			typename = "Phone Number";
 			break;
 		case 'c':
+			ipv4addr = sdp_get_ipv4addr(value, valuelen);
 			typename = "Connection Information";
 			break;
 		case 'b':
@@ -171,6 +285,8 @@
 		case 'm':
 			section = 'm';
 			typename = "Media Description, name and address";
+			rtp_value = value;
+			rtp_valuelen = valuelen;
 			break;
 		case 'k':
 			typename = "Encryption Key";
@@ -197,6 +313,13 @@
 		    format_text(value, valuelen));
 		offset = next_offset;
 	}
+
+	/* Call sdp_check_rtp() here, instead under case 'm'
+	 * label. This should keep us from registering wrong values if
+	 * the sender did not follow the order mandated in RFC 2327
+	 */
+	if (rtp_value)
+		sdp_check_rtp(ipv4addr, rtp_value, rtp_valuelen);
 
 	datalen = tvb_length_remaining(tvb, offset);
 	if (datalen > 0) {