[Asterisk-code-review] res rtp asterisk: Add support for receiving and handling NAC... (asterisk[15])

[Joshua Colp]

Joshua Colp has posted comments on this change. ( https://gerrit.asterisk.org/8773 )

Change subject: res_rtp_asterisk: Add support for receiving and handling NACK requests.
......................................................................


Patch Set 2:

(2 comments)

https://gerrit.asterisk.org/#/c/8773/2/res/res_rtp_asterisk.c
File res/res_rtp_asterisk.c:

https://gerrit.asterisk.org/#/c/8773/2/res/res_rtp_asterisk.c@5267
PS2, Line 5267: 	for (j = 3; j < length; j++) {
> Document why magic number 3 is used.
On further investigation I think the FCI information starts at 1.

The RTCP code automatically skips past the RTCP and packet source SSRC, so 0 would be the media source SSRC when we get here and therefore 1 would be the first FCI.


https://gerrit.asterisk.org/#/c/8773/2/res/res_rtp_asterisk.c@5269
PS2, Line 5269: 		pid = current_word >> 16;
I did some testing with this and I don't think the logic for getting the pid is correct:

[Apr 16 11:23:54] NOTICE[19415][C-00000001]: res_rtp_asterisk.c:5270 ast_rtp_rtcp_handle_nack: Looking for seqno 7936, latest sent 20865
[Apr 16 11:23:54] NOTICE[19415][C-00000001]: res_rtp_asterisk.c:5276 ast_rtp_rtcp_handle_nack: Could not find 7936 to retransmit
[Apr 16 11:23:54] NOTICE[19415][C-00000001]: res_rtp_asterisk.c:5270 ast_rtp_rtcp_handle_nack: Looking for seqno 5888, latest sent 18042
[Apr 16 11:23:54] NOTICE[19415][C-00000001]: res_rtp_asterisk.c:5276 ast_rtp_rtcp_handle_nack: Could not find 5888 to retransmit
[Apr 16 11:24:03] NOTICE[19415][C-00000001]: res_rtp_asterisk.c:5270 ast_rtp_rtcp_handle_nack: Looking for seqno 7936, latest sent 18647
[Apr 16 11:24:03] NOTICE[19415][C-00000001]: res_rtp_asterisk.c:5276 ast_rtp_rtcp_handle_nack: Could not find 7936 to retransmit
[Apr 16 11:24:03] NOTICE[19425][C-00000003]: res_rtp_asterisk.c:5270 ast_rtp_rtcp_handle_nack: Looking for seqno 5376, latest sent 24711
[Apr 16 11:24:03] NOTICE[19425][C-00000003]: res_rtp_asterisk.c:5276 ast_rtp_rtcp_handle_nack: Could not find 5376 to retransmit
[Apr 16 11:24:12] NOTICE[19425][C-00000003]: res_rtp_asterisk.c:5270 ast_rtp_rtcp_handle_nack: Looking for seqno 4864, latest sent 26261
[Apr 16 11:24:12] NOTICE[19423][C-00000002]: res_rtp_asterisk.c:5270 ast_rtp_rtcp_handle_nack: Looking for seqno 4864, latest sent 15963
[Apr 16 11:24:12] NOTICE[19423][C-00000002]: res_rtp_asterisk.c:5276 ast_rtp_rtcp_handle_nack: Could not find 4864 to retransmit
[Apr 16 11:24:12] NOTICE[19425][C-00000003]: res_rtp_asterisk.c:5276 ast_rtp_rtcp_handle_nack: Could not find 4864 to retransmit

The value it got for 'pid' was also 0 sometimes which is not what I'd expect.



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

Gerrit-Project: asterisk
Gerrit-Branch: 15
Gerrit-MessageType: comment
Gerrit-Change-Id: I7f7f124af3b9d5d2fd9cffc6ba8cb48a6fff06ec
Gerrit-Change-Number: 8773
Gerrit-PatchSet: 2
Gerrit-Owner: Benjamin Keith Ford <bford at digium.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Comment-Date: Mon, 16 Apr 2018 11:42:29 +0000
Gerrit-HasComments: Yes
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20180416/265373e0/attachment-0001.html>