[Asterisk-code-review] res_rtp_asterisk.c: Send RTCP as compound packets. (...asterisk[16])

[Kevin Harwell]

Kevin Harwell has posted comments on this change. ( https://gerrit.asterisk.org/c/asterisk/+/12863 )

Change subject: res_rtp_asterisk.c: Send RTCP as compound packets.
......................................................................


Patch Set 1: Code-Review-1

(3 comments)

https://gerrit.asterisk.org/#/c/12863/1/res/res_rtp_asterisk.c 
File res/res_rtp_asterisk.c:

https://gerrit.asterisk.org/#/c/12863/1/res/res_rtp_asterisk.c@4891 
PS1, Line 4891: 
You need to unlock the instance in the nominal path as well. Can do here if you want before the check, and won't have to do in the check as well.


https://gerrit.asterisk.org/#/c/12863/1/res/res_rtp_asterisk.c@4949 
PS1, Line 4949: 	ao2_lock(instance);
              : 	rtcpheader = bdata;
              : 
              : 	res = ast_rtcp_generate_report(instance, rtcpheader, rtcp_report, &sr);
              : 	if (res == 0 || res == 1) {
              : 		ast_debug(1, "Failed to add %s report to REMB packet!\n", sr ? "SR" : "RR");
              : 		ao2_unlock(instance);
              : 		return;
              : 	}
              : 
              : 	packet_len += res;
              : 
              : 	res = ast_rtcp_generate_sdes(instance, rtcpheader + packet_len, rtcp_report);
              : 	if (res == 0 || res == 1) {
              : 		ast_debug(1, "Failed to add SDES to REMB packet!\n");
              : 		ao2_unlock(instance);
              : 		return;
              : 	}
If I'm not missing anything this is pretty much the same code as above. Should be able to put into a function to call from both.


https://gerrit.asterisk.org/#/c/12863/1/res/res_rtp_asterisk.c@4967 
PS1, Line 4967: 
You need to unlock the instance in the nominal path as well. Can do just after the sdes call, and before the result check if you want, and won't have to do in the check as well.



-- 
To view, visit https://gerrit.asterisk.org/c/asterisk/+/12863
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: 16
Gerrit-Change-Id: Ieb140e9c22568a251a564cd953dd22cd33244605
Gerrit-Change-Number: 12863
Gerrit-PatchSet: 1
Gerrit-Owner: Benjamin Keith Ford <bford at digium.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-Comment-Date: Tue, 10 Sep 2019 22:48:46 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20190910/9a551328/attachment.html>