[Home]

Summary:ASTERISK-02529: [patch]RTP debugging
Reporter:Filip Olsson (folsson)Labels:
Date Opened:2004-10-04 21:06:33Date Closed:2008-01-15 15:12:10.000-0600
Priority:MajorRegression?No
Status:Closed/CompleteComponents:Core/General
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) rtp_debugging4.txt
Description:Adds the CLI commands 'rtp debug ip <host[:port]>' for printing rtp packets, and 'rtp no debug' for not doing so.
Comments:By: Filip Olsson (folsson) 2004-10-04 21:09:07

Pretty much a rip from chan_sip. But does the job.

By: Mark Spencer (markster) 2004-10-05 17:16:03

Do you have a disclaimer on file?

By: Filip Olsson (folsson) 2004-10-05 20:13:29

Nope. You'll get it first thing tomorrow morning.
I'll also add 'rtp debug' to debug _all_ rtp-traffic

By: Olle Johansson (oej) 2004-10-06 06:53:40

Folsson, good patch! However
* Read the coding guidlines and fix whitespaces and curled brackets
* You register CLI commands, but do not unregister. However, I've not that familiar with RTP.c so I don't know if it's an unloadable module at all...

By: Filip Olsson (folsson) 2004-10-06 08:01:31

Uploaded a new patch with oej's points corrected(I think).
Don't even think about using the first one, it's diffed against the wrong version.
rtp.c can't be unloaded so there's no place to put the cli_unregisters.

By: Olle Johansson (oej) 2004-10-10 10:27:18

Better. According to *my* coding guidelines, I want comments when you add new variables - and if you understand some of the old uncommented variables, please add comments for them as well. It never hurts, but makes life much easier when debugging or when the next fellow coder wants to understand the code. Thank you!

* Minor indentation problem still in function rtp_debug_test_addr
* Some instances of "three spaces" instead of tabs to clear out
* You are using // comments instead of /* */

Doing fine. Please fix these issues!

By: Filip Olsson (folsson) 2004-10-10 10:53:32

Took another class in oej's how-to-write-good-looking-code-school

By: Olle Johansson (oej) 2004-10-10 11:01:29

Still have some work to do before you get an exam ;-)

+static int rtp_do_debug_ip(int fd, int argc, char *argv[])
+{
+   struct hostent *hp;
+   struct ast_hostent ahp;

^^^^^ What is this? Three spaces... ;-)

By: Filip Olsson (folsson) 2004-10-10 11:12:30

I think I learned how to use my new found skills in this one. Passed?

By: Olle Johansson (oej) 2004-10-10 11:15:53

The code looks much better now. Anyone that has tested the functionality?

By: twisted (twisted) 2004-10-10 11:28:56

/* The value of each payload format mapping: */
struct rtpPayloadType {
-  int isAstFormat; /* whether the following code is an AST_FORMAT */
-  int code;
+        int isAstFormat; /* whether the following code is an AST_FORMAT */
+        int code;
};

spaces!!

@@ -815,6 +832,7 @@
int x;
int first;
int startplace;
+        char iabuf[INET_ADDRSTRLEN];  
rtp = malloc(sizeof(struct ast_rtp));
if (!rtp)
return NULL;

and more spaces!!

Almost there :)

By: Filip Olsson (folsson) 2004-10-11 08:39:34

rtp_debugging4.txt is the latest. Please remove the others

By: Filip Olsson (folsson) 2004-10-11 08:45:22

BTW. Added the CLI command 'rtp debug', for debugging _all_ RTP traffic.

By: Olle Johansson (oej) 2004-10-11 12:41:43

Folsson, please check if you can add more verbose output for DTMF signals sent over RTP, that would be nice.

By: Mark Spencer (markster) 2004-10-12 22:45:48

I think this is ready for CVS, can you confirm that you did get your disclaimer filed?  Thanks!

By: Filip Olsson (folsson) 2004-10-13 03:30:10

I faxed it over to you a week ago. I could do it again if you didn't receive it?

By: Filip Olsson (folsson) 2004-10-22 08:29:52

What's the status on this one? CVS? Did you get the disclaimer?

By: Olle Johansson (oej) 2004-10-23 03:48:15

Be paticence, there is a lot of patches waiting in line for inclusion.
While waiting, why don't you mail the -dev list and ask people to test this patch and get feedback if they think it is useful or not? THey might also have ideas on improvement or just crazy stuff that might fit here as well...

By: duanecox (duanecox) 2004-10-26 09:02:05

FWIW I used this patch with great success to verify some RTP packets from rogue end devices.

There was some log errors generated when I issued a "reload" with this patch installed, I don't recall what they were now.

edited on: 10-28-04 15:38

By: duanecox (duanecox) 2004-10-29 18:18:42

Warning output when patch applied

devel*CLI> reload
Oct 29 18:18:25 WARNING[294926]: cli.c:837 ast_cli_register: Command 'rtpdebug' already registered (or something close enough)
Oct 29 18:18:25 WARNING[294926]: cli.c:837 ast_cli_register: Command 'rtpdebugip' already registered (or something close enough)
Oct 29 18:18:25 WARNING[294926]: cli.c:837 ast_cli_register: Command 'rtpnodebug' already registered (or something close enough)

By: Mark Spencer (markster) 2004-10-29 21:49:18

Added to CVS (with fix for mentioned issue)

By: Russell Bryant (russell) 2004-10-30 12:08:13

not included in 1.0

By: Digium Subversion (svnbot) 2008-01-15 15:12:10.000-0600

Repository: asterisk
Revision: 4129

U   trunk/rtp.c

------------------------------------------------------------------------
r4129 | markster | 2008-01-15 15:12:09 -0600 (Tue, 15 Jan 2008) | 2 lines

Add RTP debug support (bug ASTERISK-2529)

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=4129