RTP: If multiple codecs are used in RTP stream flow, all are shown in codecs column

Change-Id: Ica8b3bc2b6b59790805764ec88c6f4e3f8689a85
Reviewed-on: https://code.wireshark.org/review/28435
Petri-Dish: Pascal Quantin <pascal.quantin@gmail.com>
Tested-by: Petri Dish Buildbot
Reviewed-by: Pascal Quantin <pascal.quantin@gmail.com>

6 files changed, 73 insertions, 37 deletions

diff --git a/ui/cli/tap-rtp.c b/ui/cli/tap-rtp.c
index 0d4e55dff7..e19fb83d61 100644
--- a/ui/cli/tap-rtp.c
+++ b/ui/cli/tap-rtp.c
@@ -78,7 +78,7 @@ rtpstreams_stat_draw_cb(rtpstream_tapinfo_t *tapinfo _U_)
             calc.dst_addr_str,
             calc.dst_port,
             calc.ssrc,
-            calc.payload_str,
+            calc.all_payload_type_names,
             calc.packet_count,
             calc.lost_num,
             calc.lost_perc,
diff --git a/ui/qt/rtp_stream_dialog.cpp b/ui/qt/rtp_stream_dialog.cpp
index 8082231937..a2e9c400fe 100644
--- a/ui/qt/rtp_stream_dialog.cpp
+++ b/ui/qt/rtp_stream_dialog.cpp
@@ -100,7 +100,7 @@ public:
         setText(dst_addr_col_, calc.dst_addr_str);
         setText(dst_port_col_, QString::number(calc.dst_port));
         setText(ssrc_col_, QString("0x%1").arg(calc.ssrc, 0, 16));
-        setText(payload_col_, calc.payload_str);
+        setText(payload_col_, calc.all_payload_type_names);
         setText(packets_col_, QString::number(calc.packet_count));
         setText(lost_col_, QObject::tr("%1 (%L2%)").arg(calc.lost_num).arg(QString::number(calc.lost_perc, 'f', 1)));
         setText(max_delta_col_, QString::number(calc.max_delta, 'f', 3)); // This is RTP. Do we need nanoseconds?
@@ -127,7 +127,7 @@ public:
         switch(col) {
         case src_addr_col_:
         case dst_addr_col_:
-        case payload_col_: // XXX Return numeric value?
+        case payload_col_:
             return text(col);
         case src_port_col_:
             return stream_info_->id.src_port;
@@ -170,7 +170,7 @@ public:
         case ssrc_col_:
             return stream_info_->id.ssrc < other_rstwi.stream_info_->id.ssrc;
         case payload_col_:
-            return stream_info_->payload_type < other_rstwi.stream_info_->payload_type; // XXX Compare payload_type_name instead?
+            return g_strcmp0(stream_info_->all_payload_type_names, other_rstwi.stream_info_->all_payload_type_names);
         case packets_col_:
             return stream_info_->packet_count < other_rstwi.stream_info_->packet_count;
         case lost_col_:
diff --git a/ui/rtp_stream.h b/ui/rtp_stream.h
index 7298c94d31..11c45d9fdf 100644
--- a/ui/rtp_stream.h
+++ b/ui/rtp_stream.h
@@ -38,10 +38,12 @@ extern "C" {
 typedef struct _rtpstream_info {
     rtpstream_id_t  id;
 
-    guint8          payload_type; /**< Numeric payload type */
-    gchar          *payload_type_name; /**< Payload type name */
-    gboolean        is_srtp;
+    guint8          first_payload_type; /**< Numeric payload type */
+    const gchar    *first_payload_type_name; /**< Payload type name */
+    const gchar    *payload_type_names[256]; /**< Seen payload type names. Array index is payload type (byte), filled only during TAP_ANALYSE */
+    gchar          *all_payload_type_names; /**< All seen payload names for a stream in one string */
 
+    gboolean        is_srtp;
     guint32         packet_count;
     gboolean        end_stream; /**< Used to track streams across payload types */
     int             rtp_event;
@@ -59,7 +61,7 @@ typedef struct _rtpstream_info {
 
     tap_rtp_stat_t  rtp_stats;  /**< here goes the RTP statistics info */
     gboolean        problem;    /**< if the streams had wrong sequence numbers or wrong timestamps */
-    gchar          *ed137_info;
+    const gchar    *ed137_info; /** pointer to static text, no freeing is required */
 } rtpstream_info_t;
 
 /** tapping modes */
diff --git a/ui/tap-rtp-common.c b/ui/tap-rtp-common.c
index 267172b53a..488ce866f6 100644
--- a/ui/tap-rtp-common.c
+++ b/ui/tap-rtp-common.c
@@ -71,8 +71,7 @@ void rtpstream_info_copy_deep(rtpstream_info_t *dest, const rtpstream_info_t *sr
     *dest = *src;  /* memberwise copy of struct */
     copy_address(&(dest->id.src_addr), &(src->id.src_addr));
     copy_address(&(dest->id.dst_addr), &(src->id.dst_addr));
-    dest->payload_type_name = g_strdup(src->payload_type_name);
-    dest->ed137_info = g_strdup(src->ed137_info);
+    dest->all_payload_type_names = g_strdup(src->all_payload_type_names);
 }
 
 /****************************************************************************/
@@ -91,12 +90,10 @@ rtpstream_info_t *rtpstream_info_malloc_and_copy_deep(const rtpstream_info_t *sr
 /* free rtpstream_info_t referenced values */
 void rtpstream_info_free_data(rtpstream_info_t *info)
 {
-    if (info->ed137_info != NULL) {
-        g_free(info->ed137_info);
-    }
-    if (info->payload_type_name!= NULL) {
-        g_free(info->payload_type_name);
+    if (info->all_payload_type_names != NULL) {
+        g_free(info->all_payload_type_names);
     }
+
     rtpstream_id_free(&info->id);
 }
 
@@ -251,6 +248,43 @@ register_tap_listener_rtpstream(rtpstream_tapinfo_t *tapinfo, const char *fstrin
 * the RTP/RTCP header and (optionally) the actual payload.
 */
 
+static const gchar *PAYLOAD_UNKNOWN_STR = "Unknown";
+
+static void update_payload_names(rtpstream_info_t *stream_info, const struct _rtp_info *rtpinfo)
+{
+    GString *payload_type_names;
+    const gchar *new_payload_type_str;
+
+    /* Ensure that we have non empty payload_type_str */
+    if (rtpinfo->info_payload_type_str != NULL) {
+        new_payload_type_str = rtpinfo->info_payload_type_str;
+    }
+    else {
+        /* String is created from const strings only */
+        new_payload_type_str = val_to_str_ext_const(rtpinfo->info_payload_type,
+            &rtp_payload_type_short_vals_ext,
+            PAYLOAD_UNKNOWN_STR
+        );
+    }
+    stream_info->payload_type_names[rtpinfo->info_payload_type] = new_payload_type_str;
+
+    /* Join all existing payload names to one string */
+    payload_type_names = g_string_sized_new(40); /* Preallocate memory */
+    for(int i=0; i<256; i++) {
+        if (stream_info->payload_type_names[i] != NULL) {
+            if (payload_type_names->len > 0) {
+                g_string_append(payload_type_names, ", ");
+            }
+            g_string_append(payload_type_names, stream_info->payload_type_names[i]);
+        }
+    }
+    if (stream_info->all_payload_type_names != NULL) {
+        g_free(stream_info->all_payload_type_names);
+    }
+    stream_info->all_payload_type_names = payload_type_names->str;
+    g_string_free(payload_type_names, FALSE);
+}
+
 #define RTPFILE_VERSION "1.0"
 
 /*
@@ -337,8 +371,8 @@ int rtpstream_packet_cb(void *arg, packet_info *pinfo, epan_dissect_t *edt _U_,
     rtpstream_info_init(&new_stream_info);
     rtpstream_id_copy_pinfo(pinfo,&(new_stream_info.id),FALSE);
     new_stream_info.id.ssrc = rtpinfo->info_sync_src;
-    new_stream_info.payload_type = rtpinfo->info_payload_type;
-    new_stream_info.payload_type_name = (char *)rtpinfo->info_payload_type_str;
+    new_stream_info.first_payload_type = rtpinfo->info_payload_type;
+    new_stream_info.first_payload_type_name = rtpinfo->info_payload_type_str;
 
     if (tapinfo->mode == TAP_ANALYSE) {
         /* check whether we already have a stream with these parameters in the list */
@@ -357,6 +391,8 @@ int rtpstream_packet_cb(void *arg, packet_info *pinfo, epan_dissect_t *edt _U_,
         if (!stream_info) {
             new_stream_info.start_fd = pinfo->fd;
             new_stream_info.start_rel_time = pinfo->rel_ts;
+            new_stream_info.first_payload_type = rtpinfo->info_payload_type;
+            new_stream_info.first_payload_type_name = rtpinfo->info_payload_type_str;
 
             /* reset RTP stats */
             new_stream_info.rtp_stats.first_packet = TRUE;
@@ -376,6 +412,10 @@ int rtpstream_packet_cb(void *arg, packet_info *pinfo, epan_dissect_t *edt _U_,
 
         /* get RTP stats for the packet */
         rtppacket_analyse(&(stream_info->rtp_stats), pinfo, rtpinfo);
+        if (stream_info->payload_type_names[rtpinfo->info_payload_type] == NULL ) {
+            update_payload_names(stream_info, rtpinfo);
+        }
+
         if (stream_info->rtp_stats.flags & STAT_FLAG_WRONG_TIMESTAMP
                 || stream_info->rtp_stats.flags & STAT_FLAG_WRONG_SEQ)
             stream_info->problem = TRUE;
@@ -432,15 +472,7 @@ void rtpstream_info_calculate(const rtpstream_info_t *strinfo, rtpstream_info_ca
         calc->dst_port = strinfo->id.dst_port;
         calc->ssrc = strinfo->id.ssrc;
 
-        if (strinfo->payload_type > 95) {
-                if (strinfo->payload_type_name != NULL) {
-                        calc->payload_str = wmem_strdup(NULL, strinfo->payload_type_name);
-                } else {
-                        calc->payload_str = wmem_strdup_printf(NULL, "Unknown(%u)", strinfo->payload_type);
-                }
-        } else {
-                calc->payload_str = val_to_str_ext_wmem(NULL, strinfo->payload_type, &rtp_payload_type_vals_ext, "Unknown (%u)");
-        }
+        calc->all_payload_type_names = wmem_strdup(NULL, strinfo->all_payload_type_names);
 
         calc->packet_count = strinfo->packet_count;
         /* packet count, lost packets */
@@ -489,7 +521,7 @@ void rtpstream_info_calc_free(rtpstream_info_calc_t *calc)
 {
         wmem_free(NULL, calc->src_addr_str);
         wmem_free(NULL, calc->dst_addr_str);
-        wmem_free(NULL, calc->payload_str);
+        wmem_free(NULL, calc->all_payload_type_names);
 }
 
 /*
diff --git a/ui/tap-rtp-common.h b/ui/tap-rtp-common.h
index 0b997b60e1..d13229f45a 100644
--- a/ui/tap-rtp-common.h
+++ b/ui/tap-rtp-common.h
@@ -49,7 +49,7 @@ typedef struct _rtpstream_info_calc {
     gchar *dst_addr_str;
     guint16 dst_port;
     guint32 ssrc;
-    gchar *payload_str; /* Name of coded derived from fixed or dynamic codec names */
+    gchar *all_payload_type_names; /* Name of codec derived from fixed or dynamic codec names */
     guint32 packet_count;
     guint32 total_nr;
     guint32 packet_expected; /* Count of expected packets, derived from lenght of RTP stream */
diff --git a/ui/voip_calls.c b/ui/voip_calls.c
index ff373506fa..39d02b0159 100644
--- a/ui/voip_calls.c
+++ b/ui/voip_calls.c
@@ -608,7 +608,7 @@ rtp_packet(void *tap_offset_ptr, packet_info *pinfo, epan_dissect_t *edt, void c
                 && (tmp_listinfo->id.ssrc == rtp_info->info_sync_src) && (tmp_listinfo->end_stream == FALSE)) {
             /* if the payload type has changed, we mark the stream as finished to create a new one
                this is to show multiple payload changes in the Graph for example for DTMF RFC2833 */
-            if ( tmp_listinfo->payload_type != rtp_info->info_payload_type ) {
+            if ( tmp_listinfo->first_payload_type != rtp_info->info_payload_type ) {
                 tmp_listinfo->end_stream = TRUE;
             } else if ( ( ( tmp_listinfo->ed137_info == NULL ) && (rtp_info->info_ed137_info != NULL) ) ||
                         ( ( tmp_listinfo->ed137_info != NULL ) && (rtp_info->info_ed137_info == NULL) ) ||
@@ -636,27 +636,29 @@ rtp_packet(void *tap_offset_ptr, packet_info *pinfo, epan_dissect_t *edt, void c
         strinfo = rtpstream_info_malloc_and_init();
         rtpstream_id_copy_pinfo(pinfo,&(strinfo->id),FALSE);
         strinfo->id.ssrc = rtp_info->info_sync_src;
-        strinfo->payload_type = rtp_info->info_payload_type;
+        strinfo->first_payload_type = rtp_info->info_payload_type;
         strinfo->is_srtp = rtp_info->info_is_srtp;
         /* if it is dynamic payload, let use the conv data to see if it is defined */
-        if ( (strinfo->payload_type >= PT_UNDF_96) && (strinfo->payload_type <= PT_UNDF_127) ) {
+        if ( (strinfo->first_payload_type >= PT_UNDF_96) && (strinfo->first_payload_type <= PT_UNDF_127) ) {
             /* Use existing packet info if available */
             p_conv_data = (struct _rtp_conversation_info *)p_get_proto_data(wmem_file_scope(), pinfo, proto_get_id_by_filter_name("rtp"), 0);
             if (p_conv_data && p_conv_data->rtp_dyn_payload) {
-                const gchar *encoding_name = rtp_dyn_payload_get_name(p_conv_data->rtp_dyn_payload, strinfo->payload_type);
+                const gchar *encoding_name = rtp_dyn_payload_get_name(p_conv_data->rtp_dyn_payload, strinfo->first_payload_type);
                 if (encoding_name) {
-                    strinfo->payload_type_name = wmem_strdup(NULL, encoding_name);
+                    strinfo->first_payload_type_name = encoding_name;
                 }
             }
         }
-        if (!strinfo->payload_type_name) strinfo->payload_type_name = (gchar*)val_to_str_ext_wmem(NULL, strinfo->payload_type, &rtp_payload_type_short_vals_ext, "%u");
+        if (!strinfo->first_payload_type_name) {
+            strinfo->first_payload_type_name = (gchar*)val_to_str_ext(strinfo->first_payload_type, &rtp_payload_type_short_vals_ext, "%u");
+        }
         strinfo->start_fd = pinfo->fd;
         strinfo->start_rel_time = pinfo->rel_ts;
         strinfo->setup_frame_number = rtp_info->info_setup_frame_num;
         strinfo->call_num = -1;
         strinfo->rtp_event = -1;
         if (rtp_info->info_ed137_info != NULL) {
-            strinfo->ed137_info = wmem_strdup(NULL, rtp_info->info_ed137_info);
+            strinfo->ed137_info = rtp_info->info_ed137_info;
         } else {
             strinfo->ed137_info = NULL;
         }
@@ -729,7 +731,7 @@ rtp_draw(void *tap_offset_ptr)
                 duration = (guint32)(nstime_to_msec(&rtp_listinfo->stop_rel_time) - nstime_to_msec(&rtp_listinfo->start_rel_time));
                 new_gai->frame_label = g_strdup_printf("%s (%s) %s%s%s",
                         (rtp_listinfo->is_srtp)?"SRTP":"RTP",
-                        rtp_listinfo->payload_type_name,
+                        rtp_listinfo->first_payload_type_name,
                         (rtp_listinfo->rtp_event == -1)?
                         "":val_to_str_ext_const(rtp_listinfo->rtp_event, &rtp_event_type_values_ext, "Unknown RTP Event"),
                         (rtp_listinfo->ed137_info!=NULL?" ":""),
@@ -815,7 +817,7 @@ rtp_packet_draw(void *tap_offset_ptr)
                         duration = (guint32)(nstime_to_msec(&rtp_listinfo->stop_fd->rel_ts) - nstime_to_msec(&rtp_listinfo->start_fd->rel_ts));
                         new_gai->frame_label = g_strdup_printf("%s (%s) %s",
                                                                (rtp_listinfo->is_srtp)?"SRTP":"RTP",
-                                                               rtp_listinfo->payload_type_str,
+                                                               rtp_listinfo->first_payload_type_str,
                                                                (rtp_listinfo->rtp_event == -1)?
                                                                "":val_to_str_ext_const(rtp_listinfo->rtp_event, &rtp_event_type_values_ext, "Unknown RTP Event"));
                         new_gai->comment = g_strdup_printf("%s Num packets:%u  Duration:%u.%03us SSRC:0x%X",