[Asterisk-code-review] res pjsip: Re-use IP version of signaling (SIP) for media (R... (asterisk[13])

[Alexander Traud]

Alexander Traud has posted comments on this change.

Change subject: res_pjsip: Re-use IP version of signaling (SIP) for media (RTP).
......................................................................


Patch Set 4:

> some test failures

Instead of "some", please, be as specific as possible. 4 cases from the automated Asterisk Test Suite failed, which prevented the integration of this change here. All cases belong to the SIP channel driver PJSIP and are part of the Basic-Call tests:
tests/channels/pjsip/basic_calls/outgoing/nominal/echo
tests/channels/pjsip/basic_calls/outgoing/nominal/playback
tests/channels/pjsip/basic_calls/two_parties/nominal/alice_initiated/alice_hang
tests/channels/pjsip/basic_calls/two_parties/nominal/alice_initiated/bob_hangs_up

In the next step, explain the contributor what is expected from him. Please, go through this Wiki page to reproduce these failures on your computer: <https://wiki.asterisk.org/wiki/display/AST/Installing+the+Asterisk+Test+Suite>. Start with one case. When you have problems to create the dependencies or are not able to reproduce the case, simply report at [I have no idea].

> audio isn't detected on the channel

In those cases, ast_sip_session->contact->via_addr is not set and therefore, the test for the IP version is wrong. I changed that, so that the code runs into the error case, then. With that, the change-set here passes all test cases. However, that would be cheating, because the user would receive an LOG_ERROR. Therefore, I added ast_assert(0) to reliably fail all test cases, which run into the error case.

ast_sip_session_create_invite
-create_local_sdp
--add_sdp_streams
---create_outgoing_sdp_stream
via_addr is only set, when a SIP-REGISTER is involved. In those test cases, a SIP contact is called directly by its SIP-URI, without registering. Furthermore, via_addr, ast_sip_session->pjsip_inv_session->pjsip_dialog->via_addr, nor the linked list ast_sip_session->pjsip_inv_session->pjsip_dialog->inv_hdr(PJSIP_H_VIA) contain an IP address already. The ast_sip_session->ast_sip_contact->uri is set, but might contain a domain which is not resolved by DNS, yet.

Consequently, my change does not catch all cases. Because I am not able to find/access an IP address and I do not have a bird's eye view, this is an architectural issue in res_pjsip. Anyway, please, regate this change so we see whether even more test cases fail (the Asterisk Test Suite does not work on my computer/setup). It is going to fail, but at least that tells us the scenarios which are affected. Digium can then:
A) remove ast_assert(0) and accept this as partial fix
B) abandon this

For me, this issue here is a blocker. From my understanding, this is a blocker for all VoIP/SIP providers because they abstract the IP version and do not know the IP version of their users in advance. I cannot envision a scenario which can live with this issue except an IPv4-only server. I do not have the resources to continue with this change-set because the remaining cases are not mine.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I01a85a8c6723fcc12e86139f80e090e2078d04bb
Gerrit-PatchSet: 4
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: Alexander Traud <pabstraud at compuserve.com>
Gerrit-Reviewer: Alexander Traud <pabstraud at compuserve.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Mark Michelson <mmichelson at digium.com>
Gerrit-HasComments: No