[Asterisk-code-review] res_pjsip: Add 100rel option "peer_supported". (asterisk[16])

George Joseph asteriskteam at digium.com
Wed Sep 7 07:37:43 CDT 2022


Attention is currently required from: N A, Maximilian Fridrich.

George Joseph has posted comments on this change. ( https://gerrit.asterisk.org/c/asterisk/+/18838 )

Change subject: res_pjsip: Add 100rel option "peer_supported".
......................................................................


Patch Set 5: Code-Review-1

(4 comments)

File include/asterisk/res_pjsip.h:

https://gerrit.asterisk.org/c/asterisk/+/18838/comment/b7f5c60b_fb6082e9 
PS5, Line 422: AST_SIP_100REL_SUPPORTED
Might want to be a bit clearer that AST_SIP_100REL_SUPPORTED is for uac.
You should also note what the config value is for each enum. I.E.  "yes", "no", "required", "peer_supported".


File res/res_pjsip/pjsip_config.xml:

https://gerrit.asterisk.org/c/asterisk/+/18838/comment/6ce99959_4f23507f 
PS5, Line 35: 							<enum name="no">
            : 							<para>If set to <literal>no</literal>, do not support transmission of
            : 							reliable provisional responses.</para>
            : 							</enum>
            : 							<enum name="required">
            : 							<para>If set to <literal>required</literal>, require provisional responses to
            : 							be sent reliably.</para>
            : 							</enum>
            : 							<enum name="peer_supported">
            : 							<para>If set to <literal>peer_supported</literal>, send provisional responses
            : 							reliably if the request by the peer contained 100rel in the Supported or
            : 							Require header. If the request by the peer did not contain 100rel in the Supported
            : 							and Require header, send responses them normally. Requests originating from Asterisk
            : 							will contain 100rel in the Supported header.</para>
            : 							</enum>
            : 							<enum name="yes">
            : 							<para>If set to <literal>yes</literal>, indicate the support of reliable provisional
            : 							responses and PRACK them if required by the peer. If the peer mereley indicates
            : 							support of 100rel in the Supported header, do not send 1xx responses reliable, but
            : 							indicate the support in the Supported header.</para>
            : 							</enum>
            : 						</enumlist>
            : 					</description>
            : 
For each option, you should explicitly state the behavior when Asterisk is the UAC and when it's the UAS.


File res/res_pjsip/pjsip_configuration.c:

https://gerrit.asterisk.org/c/asterisk/+/18838/comment/fd6a0d5b_2e577a3f 
PS5, Line 201: static int prack_to_str(const void *obj, const intptr_t *args, char **buf)
You need to update this function as well to provide the reverse mapping to string.
You should convert this to use the new 100rel parameter as the source at the same time.


File res/res_pjsip_session.c:

https://gerrit.asterisk.org/c/asterisk/+/18838/comment/657b6acf_b6701084 
PS5, Line 3804: 	if (endpoint->rel100 == AST_SIP_100REL_PEER_SUPPORTED && rdata->msg_info.supported != NULL) {
I think a comment explaining what's going on would be appropriate.



-- 
To view, visit https://gerrit.asterisk.org/c/asterisk/+/18838
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: 16
Gerrit-Change-Id: Id6d95ffa8f00dab118e0b386146e99f254f287ad
Gerrit-Change-Number: 18838
Gerrit-PatchSet: 5
Gerrit-Owner: Maximilian Fridrich <m.fridrich at commend.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at sangoma.com>
Gerrit-Reviewer: N A <mail at interlinked.x10host.com>
Gerrit-Reviewer: Sean Bright <sean at seanbright.com>
Gerrit-Attention: N A <mail at interlinked.x10host.com>
Gerrit-Attention: Maximilian Fridrich <m.fridrich at commend.com>
Gerrit-Comment-Date: Wed, 07 Sep 2022 12:37:43 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20220907/ca6705bb/attachment.html>


More information about the asterisk-code-review mailing list