|Summary:||ASTERISK-08375: [patch] Added RTCP Manager Events to rtp.c|
|Reporter:||Jon Creasy (johann8384)||Labels:|
|Date Opened:||2006-12-19 09:07:55.000-0600||Date Closed:||2007-06-21 18:07:07|
|Environment:||Attachments:||( 0) rtcp_manager_06112007.patch|
( 1) rtcp_manager_coding_guidelines.patch
( 2) rtcp_manager_revised.patch
( 3) rtcp_manager_trunk.patch
( 4) rtcp_manager.patch
|Description:||I have added manager events to rtp.c which are executed when an RTCP message is sent or received. |
****** ADDITIONAL INFORMATION ******
After discussing this change on the developer's list it was determined that a new privilege should be created for these events. The privilege is to be called "reporting". I have not completed that at this time.
|Comments:||By: Jared Smith (jsmith) 2006-12-19 11:43:48.000-0600|
Just one minor thing -- the patch should be against TRUNK, not Asterisk 1.4 beta 3. (Once we've got it working, feel free to backport the patch to 1.4 if you feel like it.) Other than that, it looks pretty straightforward to me. I only just glanced over it, so maybe someone with more C skills than I've got will spot something out of the ordinary.
By: Jon Creasy (johann8384) 2006-12-19 11:58:59.000-0600
I created a new patch against the trunk and have uploaded it. I was going to replace the original file but I did not have access.
By: Jon Creasy (johann8384) 2006-12-27 10:44:08.000-0600
I have created a newer patch which includes the addition of a manager privilege called "reporting" as discussed on the dev mailing list a week or so ago.
RTCP related events (as added by this patch) now use this new privilege.
By: Jon Creasy (johann8384) 2006-12-27 10:46:24.000-0600
I have tested this patch with the 1.4.0 release and it does work properly.
By: Anthony LaMantia (alamantia) 2006-12-27 11:56:12.000-0600
there are a few indentation problems in this patch that don't go along with the coding guidelines with the asterisk.
ie spaces before the parenthesis of an if statement. also i really think that the best manager privilege/scope for this information to be broadcast under would be just plain and simple "debug" rather then creating a new privilege just for these events.
By: Jon Creasy (johann8384) 2006-12-27 12:29:12.000-0600
I consulted some people in the list about the privilege. My reasoning was that we are using it for reporting purposes not debug purposes. A second reason is that it generates a whole heck of a lot of messages in some cases.
I'll address the indentation, sorry about that I didn't proofread it very carefully.
By: Jon Creasy (johann8384) 2006-12-27 12:47:18.000-0600
Here is a new patch with the indentation problem corrected.
By: Serge Vecher (serge-v) 2006-12-27 15:35:07.000-0600
1. Typo: "nReceptionRreports"
2. For consistency purposes, should the following be spelled out fully:
"RTPRecvStat" -> RTPReceiverStat
"RTPSndrStat" -> RTPSenderStat
By: Jon Creasy (johann8384) 2006-12-28 07:10:45.000-0600
Maybe so. I am not against updating those. I will look through them and decide what I think should be put on each one and post an update.
By: Serge Vecher (serge-v) 2007-01-18 11:20:42.000-0600
thank you, johann
By: Andrew Kohlsmith (akohlsmith) 2007-06-01 07:21:23
Patch applies with minimum fuzz to 1.4, testing...
By: Andrew Kohlsmith (akohlsmith) 2007-06-01 08:04:58
Ok, pardon my ignorance to SIP, but I see no way to tie in a call and its RTCP events with this patch. Example call's events:
DLSR: 27.9020 (sec)
(repeats for call duration)
I see no way of typing the SSRC to uniqueid or channel...
By: Jon Creasy (johann8384) 2007-06-11 21:34:46
Your absolutely right. I made no attempt whatsoever to attach a report to a specific call. I see where this could be very useful but it was irrelevant to my application.
I'll see if I can expand it in the coming weeks.
By: Jon Creasy (johann8384) 2007-06-11 21:44:42
New patch file based on Trunk Revision # 68873
There have been some changes since my last patch which are in conflict with the patch so I have made a new one.
By: Jason Parker (jparker) 2007-06-21 18:07:05
Added to svn trunk in revision 70961. Thanks! I'm sure a few people will be happy to see this added.