[Asterisk-code-review] core / pjsip: Add support for grouping streams together. (asterisk[15])

[Kevin Harwell]

Kevin Harwell has posted comments on this change. ( https://gerrit.asterisk.org/6936 )

Change subject: core / pjsip: Add support for grouping streams together.
......................................................................


Patch Set 3: Code-Review-1

(2 comments)

https://gerrit.asterisk.org/#/c/6936/3/main/stream.c
File main/stream.c:

https://gerrit.asterisk.org/#/c/6936/3/main/stream.c@70
PS3, Line 70: 	 * \brief The group that the stream is part of
            : 	 */
            : 	int group;
I think things could be simplified by moving the msid/mslabel stuff onto the stream itself. Although you probably would want two separate fields derived from the msid. One for the unique stream id and the other for the stream group. Then you wouldn't need a third field representing the group.

You could then also have a is_in_group method on the stream that tests if streams are part of the same group.


https://gerrit.asterisk.org/#/c/6936/3/res/res_pjsip_session.c
File res/res_pjsip_session.c:

https://gerrit.asterisk.org/#/c/6936/3/res/res_pjsip_session.c@611
PS3, Line 611: 		if (ast_strlen_zero(group_session_media->remote_mslabel) ||
             : 			strcmp(group_session_media->remote_mslabel, session_media->remote_mslabel)) {
If the remote_mslabel is derived from the msid this may not match sometimes since the msid would different for each stream. You'd need to break apart the msid into msid value and msid appdata (which is typically the group id).



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

Gerrit-Project: asterisk
Gerrit-Branch: 15
Gerrit-MessageType: comment
Gerrit-Change-Id: Id6299aa031efe46254edbdc7973c534d54d641ad
Gerrit-Change-Number: 6936
Gerrit-PatchSet: 3
Gerrit-Owner: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: Sean Bright <sean.bright at gmail.com>
Gerrit-Comment-Date: Tue, 31 Oct 2017 16:30:50 +0000
Gerrit-HasComments: Yes
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20171031/b79450aa/attachment.html>