[Asterisk-code-review] SDP: Rework SDP offer/answer model and update capabilities m... (asterisk[master])

[Richard Mudgett]

Richard Mudgett has posted comments on this change. ( https://gerrit.asterisk.org/5819 )

Change subject: SDP: Rework SDP offer/answer model and update capabilities merges.
......................................................................


Patch Set 4:

(3 comments)

https://gerrit.asterisk.org/#/c/5819/4/main/sdp_state.c
File main/sdp_state.c:

https://gerrit.asterisk.org/#/c/5819/4/main/sdp_state.c@320
PS4, Line 320: 	max_streams = ast_sdp_options_get_max_streams(options);
> Mark has done things with a per-type max streams limit. We should ensure we
The modify callbacks are able to enforce any other restriction on number and type of streams.  This is a sledgehammer because pjproject cannot support more than a maximum total number of streams without corrupting memory.


https://gerrit.asterisk.org/#/c/5819/4/main/sdp_state.c@536
PS4, Line 536: 	if (cb) {
> Did you do this just so you can declare, neg_topology, etc if needed?
I don't understand what you mean here.  Are you saying I didn't need to declare neg_topology?  I don't actually need to declare it.  I could just make one extremely long and convoluted line instead to make it harder to understand if you want. :)


https://gerrit.asterisk.org/#/c/5819/4/main/sdp_state.c@817
PS4, Line 817: 	if (ast_sockaddr_isnull(address)) {
             : 		/* No address is set on the stream state. */
             : 		return -1;
             : 	}
> Why this check before using the global connection address?
The global connection address is used to set the port on address and only if the global connection address is valid.  If there is no address it makes no sense to give it a port.



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

Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If07fe6d79fbdce33968a9401d41d908385043a06
Gerrit-Change-Number: 5819
Gerrit-PatchSet: 4
Gerrit-Owner: Richard Mudgett <rmudgett at digium.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
Gerrit-Comment-Date: Fri, 16 Jun 2017 15:38:59 +0000
Gerrit-HasComments: Yes
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20170616/e93e01df/attachment.html>