[Asterisk-code-review] pjsip / res rtp asterisk: Add support for sending REMB (asterisk[master])

[Matthew Fredrickson]

Matthew Fredrickson has posted comments on this change. ( https://gerrit.asterisk.org/8683 )

Change subject: pjsip / res_rtp_asterisk: Add support for sending REMB
......................................................................


Patch Set 1: Code-Review-1

(4 comments)

https://gerrit.asterisk.org/#/c/8683/1/channels/chan_pjsip.c
File channels/chan_pjsip.c:

https://gerrit.asterisk.org/#/c/8683/1/channels/chan_pjsip.c@973
PS1, Line 973: 			ast_debug(3, "Channel %s stream %d is of type '%s', not video!\n",
Could we make this debug say something more descriptive, like "Trying to write RTCP REMB on non-video stream of channel %s stream %d type %s".  Also, maybe should be an error instead of debug?


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

https://gerrit.asterisk.org/#/c/8683/1/res/res_rtp_asterisk.c@4527
PS1, Line 4527: 	if (!rtp || !rtp->rtcp || (feedback->fmt != AST_RTP_RTCP_FMT_REMB)) {
Maybe warn here if we get a request to send feedback->fmt that's != AST_RTP_RTCP_FMT_REMB


https://gerrit.asterisk.org/#/c/8683/1/res/res_rtp_asterisk.c@4532
PS1, Line 4532: 	if (!ast_rtp_instance_get_prop(instance, AST_RTP_PROPERTY_REMB)) {
Should we debug/error message here?


https://gerrit.asterisk.org/#/c/8683/1/res/res_rtp_asterisk.c@4574
PS1, Line 4574: 		rtp_write_rtcp_fir(instance, rtp, &remote_address);
Good change!



-- 
To view, visit https://gerrit.asterisk.org/8683
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic53f821c1560d8924907ad82c4d9c0bc322b38cd
Gerrit-Change-Number: 8683
Gerrit-PatchSet: 1
Gerrit-Owner: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: Matthew Fredrickson <creslin at digium.com>
Gerrit-Comment-Date: Fri, 06 Apr 2018 14:28:01 +0000
Gerrit-HasComments: Yes
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20180406/9a3f8a51/attachment.html>