Answering myself in a second attempt to get this applied, I've updated the
patches and tried to address the issues (see below) I raised before.
>
> - I'm using modified versions of rtp_add_address() and rtcp_add_address()
to
> create the RTP and RTCP conversations. These were previously unused but
> wanted to register RTP/RTCP as heuristic dissectors. Should they? At the
> moment I don't so there are new warnings that dissect_rt(c)p_heur() and
> heur_init() are defined but not used.
RTP/RTCP aren't currenty registered as heuristic dissectors, these patches
don't change this. Looking at the dissect_rt(c)p_heur functions, they only
decide to dissect a packet as their own if it matches a conversation and
that conversation has data set. But in reality, wherever the conversation
is created, the dissector_handle will already be set (by
conversation_set_dissector()), and no conversation data will be set... So
they don't seem useful to me at the moment.
The warnings serve as a useful reminder that heuristic functions have been
defined but are not currently being used.
> - These _add_address() functions are called in 3 different places (SDP,
> H.245 and RTSP). I've followed the way SDP and H.245 used the
> conversations, i.e. only interested in the server address+port (pass the
> same address and port twice with the flags NO_ADDR2 | NO_PORT2 for new
and
> NO_ADDR_B | NO_PORT_B for find). RTSP does it differently though, it
loooks
> like it's trying to make use of the client port (if known) as well. Will
> this ever catch more RTP/RTCP packets in practice, or is my use of the
> add_address() functions OK...? I'm not familiar with RTSP and don't have
> any captures to play with (I also let my editor cull some trailing
> whitespace in packet-rtsp.c)
OK, I've added another port parameter to the rtp_add_address() and
rtcp_add_address() functions, so now the conversation lookup behaviour
should be totally unchanged for RTSP, SDP and H245 (SDP and H.245 pass 0 for
the other port).
> - I removed the (unused) prototypes for proto_register_rtp() and
> proto_register_rtcp() from the rtp/rtcp header files.
>
I can see no harm in this.
> - If anyone found these fields annoying I could add preferences to turn
them
> off...
>
This was in reference to Andreas Sikkema saying that he found fields
referring to other packets annoying. I've added preferences (on by default)
allowing setup info to be tracked and shown individually for RTP and RTCP.
Regards,
Martin
Attachment:
packet-sdp.c.diff
Description: Binary data
Attachment:
packet-rtcp.c.diff
Description: Binary data
Attachment:
packet-rtcp.h.diff
Description: Binary data
Attachment:
packet-rtp.c.diff
Description: Binary data
Attachment:
packet-rtp.h.diff
Description: Binary data
Attachment:
packet-rtsp.c.diff
Description: Binary data
Attachment:
packet-h245.c.diff
Description: Binary data
