[Asterisk-code-review] res pjsip session: Check for removed stream state. (asterisk[15])

Joshua Colp asteriskteam at digium.com
Fri Sep 15 10:54:57 CDT 2017


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

Change subject: res_pjsip_session: Check for removed stream state.
......................................................................


Patch Set 2: Code-Review-1

(1 comment)

https://gerrit.asterisk.org/#/c/6464/2/res/res_pjsip_session.c
File res/res_pjsip_session.c:

https://gerrit.asterisk.org/#/c/6464/2/res/res_pjsip_session.c@1444
PS2, Line 1444: 						|| ast_stream_get_state(stream) == AST_STREAM_STATE_REMOVED) {
I don't think this is ideal. We want to preserve ordering of streams as much as possible. This allows things to be reused and is easier on the remote client. It's only when we absolutely can't (in the case of the limit being reached) that we should remove a stream. The code in res_pjsip_sdp_rtp already handles when a stream is removed and does the right thing, and it should even work when no formats are provided. I think the correct thing here is to simply skip the compatible check further down if the state is removed.



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

Gerrit-Project: asterisk
Gerrit-Branch: 15
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic86f8b65a4a26a60885b28b8b1a0b22e1b471d42
Gerrit-Change-Number: 6464
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: Fri, 15 Sep 2017 15:54:57 +0000
Gerrit-HasComments: Yes
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20170915/6f9d4871/attachment.html>


More information about the asterisk-code-review mailing list