[Asterisk-code-review] res rtp asterisk: Add support for sending NACK requests. (asterisk[15])

[Sean Bright]

Sean Bright has posted comments on this change. ( https://gerrit.asterisk.org/9224 )

Change subject: res_rtp_asterisk: Add support for sending NACK requests.
......................................................................


Patch Set 1:

(9 comments)

https://gerrit.asterisk.org/#/c/9224/1/include/asterisk/data_buffer.h
File include/asterisk/data_buffer.h:

https://gerrit.asterisk.org/#/c/9224/1/include/asterisk/data_buffer.h@114
PS1, Line 114:  * \brief Retrieve a data payload from the data buffer
Retrieve -> Remove?


https://gerrit.asterisk.org/#/c/9224/1/include/asterisk/data_buffer.h@129
PS1, Line 129:  * \brief Retrieve the first payload from the data buffer
Retrieve -> Remove?


https://gerrit.asterisk.org/#/c/9224/1/main/data_buffer.c
File main/data_buffer.c:

https://gerrit.asterisk.org/#/c/9224/1/main/data_buffer.c@307
PS1, Line 307: 	struct data_buffer_payload_entry *buffer_payload;
Minor: This can be local to the if block below.


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

https://gerrit.asterisk.org/#/c/9224/1/res/res_rtp_asterisk.c@2759
PS1, Line 2759: 	if (elem < value) {
              : 		return -1;
              : 	}
              : 
              : 	if (value < elem) {
              : 		return 1;
              : 	}
              : 
              : 	return 0;
How about:

 return elem - value;


https://gerrit.asterisk.org/#/c/9224/1/res/res_rtp_asterisk.c@2773
PS1, Line 2773: 	if (elem == value) {
              : 		return 1;
              : 	}
              : 
              : 	return 0;
How about:

 return elem == value;


https://gerrit.asterisk.org/#/c/9224/1/res/res_rtp_asterisk.c@4122
PS1, Line 4122: 	rate = rtp_get_rate(rtp->f.subclass.format);
This is only used in the block at line 4138. You can probably just inline the function call.


https://gerrit.asterisk.org/#/c/9224/1/res/res_rtp_asterisk.c@4205
PS1, Line 4205: 	rate = rtp_get_rate(rtp->f.subclass.format);
This is only used in the block at 4227. You can probably just inline the function call.


https://gerrit.asterisk.org/#/c/9224/1/tests/test_data_buffer.c
File tests/test_data_buffer.c:

https://gerrit.asterisk.org/#/c/9224/1/tests/test_data_buffer.c@251
PS1, Line 251: 		if (ret != 0) {
if (ret)


https://gerrit.asterisk.org/#/c/9224/1/tests/test_data_buffer.c@277
PS1, Line 277: 		if (ret != 0) {
if (ret)



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

Gerrit-Project: asterisk
Gerrit-Branch: 15
Gerrit-MessageType: comment
Gerrit-Change-Id: Idab644b08a1593659c92cda64132ccc203fe991d
Gerrit-Change-Number: 9224
Gerrit-PatchSet: 1
Gerrit-Owner: Benjamin Keith Ford <bford at digium.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Sean Bright <sean.bright at gmail.com>
Gerrit-Comment-Date: Wed, 20 Jun 2018 13:33:53 +0000
Gerrit-HasComments: Yes
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20180620/da05707f/attachment.html>