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

[Benjamin Keith Ford]

Benjamin Keith Ford 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:

(2 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. […]
Oh yeah, duh. Must've completely overlooked that when writing it, yet somehow I remember the failure cases :)


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. […]
Yeah, it may be beneficial to refactor it anyways since there could be more RTCP packets added in the future that will need this same format prefixing it.



-- 
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: Benjamin Keith Ford <bford at digium.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-Comment-Date: Wed, 11 Sep 2019 16:06:23 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Kevin Harwell <kharwell at digium.com>
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20190911/d03e4b24/attachment.html>