[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@95
PS2, Line 95: 
            : #define DEFAULT_RTP_BUFFER_SIZE 100
> I think 100 may be too large for now. I think 30 is more reasonable.
I was wrong on this. The browser seems to like about a 250 buffer so we need to increase it.


https://gerrit.asterisk.org/#/c/8773/2/res/res_rtp_asterisk.c@5267
PS2, Line 5267: 	for (j = 3; j < length; j++) {
> Oh nevermind, I see what you did.
I have figured out why this confused me and why it doesn't work.

This NACK may be part of a combined RTCP packet. As a result [3] may not point to where you expect. The current position needs to be passed in and the position used be relative to that. You also need to ensure that you don't exceed the NACK and go into another packet.



-- 
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 12:28:59 +0000
Gerrit-HasComments: Yes
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20180416/5b74e0ee/attachment-0001.html>