Wireshark · Ethereal-dev: [Ethereal-dev] [patch] Saving RTP

[Jaap Keuter <jaap.keuter@xxxxxxxxx>]

Hello list,

Working with the RTP stream analysis in the new released 0.10.13 I found
several flaws in the way streams are saved. This is the list of items I
worked on, all of which are fixed in the attached patch.

Add PCM u-law and A-law silence sample definition

The intermediate file is an augmented raw file. Augmentation was still
done in 16 bit linear i.s.o. based on the codec of the stream.
Also the 0x55 isn't the idle pattern for PCM u-law, it's of A-law.

Note: many functions use the statinfo packet type (pt) to determine
what to do. This can be PT_CN if the last packet of the stream was a
comfort noise packet. This makes this all fail. Instead use statinfo
regular packet type (reg_pt) for this.
Maybe this should be a seperate bugzilla entry.

Some better use of available symbols (SAVE_*_DIRECTION_MASK)

When saving the intermediate file into an au format file there is no
need to convert u-law into linear into u-law and then save it.
Save the u-law samples straight up.

Note: It would be even better if we could save the intermediate file
straight up and put the codec into the header of the au file. In fact
we can put PCM A-law in the header (codec=27), but so far I've only
found a single common Win32 application capable of playing it
(Apple QuickTime). This makes this approach unpractical in the field.

When mixing streams a possible lead-in has to have the proper silence
samples for the codec of the stream.

Three bugs are found in the code to determine if the RTP data can be
saved:
Twice checking the reverse direction mask i.s.o. the both directons mask.
Once comparing the forward codec to itself i.s.o. the reverse codec.

UI changes:
The default selection presented when saving the payload (raw + both)
is invalid. Default file type when saving payload is now au file format.

Cancel button on Save CSV dialog didn't work.

--- gtk/rtp_analysis.c.orig	2005-10-10 15:22:40.000000000 +0200
+++ gtk/rtp_analysis.c	2005-10-26 13:34:57.330157900 +0200
@@ -271,6 +271,9 @@
 
 #define TMPNAMSIZE 100
 
+#define SILENCE_PCMU	(guint8)0xFF
+#define SILENCE_PCMA	(guint8)0x55
+
 /* structure that holds general information about the connection 
 * and structures for both directions */
 typedef struct _user_data_t {
@@ -827,7 +830,7 @@
 {
 	guint i;
 	const guint8 *data;
-	gint16 tmp;
+	guint8 tmp;
 
 	/*  is this the first packet we got in this direction? */
 	if (statinfo->flags & STAT_FLAG_FIRST) {
@@ -871,8 +874,18 @@
 		* XXX not done yet */
 		for(i=0; i < (statinfo->delta_timestamp - rtpinfo->info_payload_len -
 			rtpinfo->info_padding_count); i++) {
-			tmp = (gint16 )ulaw2linear((unsigned char)(0x55));
-			fwrite(&tmp, 2, 1, saveinfo->fp);
+			switch (statinfo->reg_pt) {
+			case PT_PCMU:
+				tmp = SILENCE_PCMU;
+				break;
+			case PT_PCMA:
+				tmp = SILENCE_PCMA;
+				break;
+			default:
+				tmp = 0;
+				break;
+			}
+			fwrite(&tmp, 1, 1, saveinfo->fp);
 			saveinfo->count++;
 		}
 		fflush(saveinfo->fp);
@@ -2395,7 +2408,7 @@
 		user_data->dlg.save_csv_as_w);
 	
 	window_set_cancel_button(user_data->dlg.save_csv_as_w, 
-		GTK_FILE_SELECTION(user_data->dlg.save_csv_as_w)->cancel_button, NULL);
+		GTK_FILE_SELECTION(user_data->dlg.save_csv_as_w)->cancel_button, window_cancel_button_cb);
 	
 	SIGNAL_CONNECT(user_data->dlg.save_csv_as_w, "delete_event", window_delete_event_cb, NULL);
 	SIGNAL_CONNECT(user_data->dlg.save_csv_as_w, "destroy",
@@ -2485,7 +2498,7 @@
 	
 		switch (channels) {
 			/* only forward direction */
-			case 1: {
+			case SAVE_FORWARD_DIRECTION_MASK: {
 				progbar_count = user_data->forward.saveinfo.count;
 				progbar_quantum = user_data->forward.saveinfo.count/100;
 				while ((fread = read(forw_fd, f_pd, 1)) > 0) {
@@ -2499,8 +2512,7 @@
 					count++;
 
 					if (user_data->forward.statinfo.pt == PT_PCMU){
-						tmp = (gint16 )ulaw2linear(*f_pd);
-						*pd = (unsigned char)linear2ulaw(tmp);
+						*pd = *f_pd;
 					}
 					else if(user_data->forward.statinfo.pt == PT_PCMA){
 						tmp = (gint16 )alaw2linear(*f_pd);
@@ -2526,7 +2538,7 @@
 				break;
 			}
 			/* only reversed direction */
-			case 2: {
+			case SAVE_REVERSE_DIRECTION_MASK: {
 				progbar_count = user_data->reversed.saveinfo.count;
 				progbar_quantum = user_data->reversed.saveinfo.count/100;
 				while ((rread = read(rev_fd, r_pd, 1)) > 0) {
@@ -2540,8 +2552,7 @@
 					count++;
 
 					if (user_data->forward.statinfo.pt == PT_PCMU){
-						tmp = (gint16 )ulaw2linear(*r_pd);
-						*pd = (unsigned char)linear2ulaw(tmp);
+						*pd = r_pd;
 					}
 					else if(user_data->forward.statinfo.pt == PT_PCMA){
 						tmp = (gint16 )alaw2linear(*r_pd);
@@ -2567,7 +2578,7 @@
 				break;
 			}
 			/* both directions */
-			default: {
+			case SAVE_BOTH_DIRECTION_MASK: {
 				(user_data->forward.saveinfo.count > user_data->reversed.saveinfo.count) ? 
 						(progbar_count = user_data->forward.saveinfo.count) : 
 							(progbar_count = user_data->reversed.saveinfo.count);
@@ -2593,13 +2604,27 @@
 					count++;
 					if(f_write_silence > 0) {
 						rread = read(rev_fd, r_pd, 1);
-						*f_pd = 0;
+						switch (user_data->forward.statinfo.reg_pt) {
+						case PT_PCMU:
+							*f_pd = SILENCE_PCMU;
+							break;
+						case PT_PCMA:
+							*f_pd = SILENCE_PCMA;
+							break;
+						}							
 						fread = 1;
 						f_write_silence--;
 					}
 					else if(r_write_silence > 0) {
 						fread = read(forw_fd, f_pd, 1);
-						*r_pd = 0;
+						switch (user_data->forward.statinfo.reg_pt) {
+						case PT_PCMU:
+							*r_pd = SILENCE_PCMU;
+							break;
+						case PT_PCMA:
+							*r_pd = SILENCE_PCMA;
+							break;
+						}							
 						rread = 1;
 						r_write_silence--;
 					}
@@ -2646,14 +2671,14 @@
 		int fd;
 		switch (channels) {
 			/* only forward direction */
-			case 1: {
+			case SAVE_FORWARD_DIRECTION_MASK: {
 				progbar_count = user_data->forward.saveinfo.count;
 				progbar_quantum = user_data->forward.saveinfo.count/100;
 				fd = forw_fd;
 				break;
 			}
 			/* only reversed direction */
-			case 2: {
+			case SAVE_REVERSE_DIRECTION_MASK: {
 				progbar_count = user_data->reversed.saveinfo.count;
 				progbar_quantum = user_data->reversed.saveinfo.count/100;
 				fd = rev_fd;
@@ -2707,7 +2732,7 @@
 static void save_voice_as_ok_cb(GtkWidget *ok_bt _U_, gpointer fs _U_)
 {
 	gchar *g_dest;
-	/*GtkWidget *wav, *au, *sw;*/
+	/*GtkWidget *wav, *sw;*/
 	GtkWidget *au, *raw;
 	GtkWidget *rev, *forw, *both;
 	user_data_t *user_data;
@@ -2842,7 +2867,7 @@
 			return;
 		}
 		/* make sure pt's don't differ */
-		if ((channels == SAVE_REVERSE_DIRECTION_MASK) && (user_data->forward.statinfo.pt != user_data->forward.statinfo.pt)){
+		if ((channels == SAVE_BOTH_DIRECTION_MASK) && (user_data->forward.statinfo.pt != user_data->reversed.statinfo.pt)){
 			simple_dialog(ESD_TYPE_ERROR, ESD_BTN_OK,
 				"Can't save in a file: Forward and reverse direction differ in type");
 			return;
@@ -2851,7 +2876,7 @@
 	else if (format == SAVE_RAW_FORMAT)
 	{
 		/* can't save raw in both directions */
-		if (channels == SAVE_REVERSE_DIRECTION_MASK){
+		if (channels == SAVE_BOTH_DIRECTION_MASK){
 			simple_dialog(ESD_TYPE_ERROR, ESD_BTN_OK,
 				"Can't save in a file: Unable to save raw data in both directions");
 			return;
@@ -2969,6 +2994,8 @@
 	  (GtkAttachOptions) (GTK_FILL),
 	  (GtkAttachOptions) (0), 0, 0);
 	*/ 
+
+	gtk_toggle_button_set_active(GTK_TOGGLE_BUTTON(au_rb), TRUE);
 	
 	channels_label = gtk_label_new ("Channels:");
 	gtk_widget_show (channels_label);