Introduces support for real-time text conversation over RTP (RFC 4103).

[xkaraman]

• Co-authored-by: Henning Westerholt hw@gilawa.com

Based on a PR from Bharat Ramaswamy Nandakumar bharat@bramsoft.com

• Co-authored-by: Kevin Basov kevin.basov@cgi.com
• Co-authored-by: Sean Bright sean@seanbright.com

Previous changes from PR #998

• chan_pjsip - added write_text function to the pjsip_tech driver
• Added case for AST_FRAME_TEXT in chan_pjsip_write_stream
• Added case to handle text in chan_pjsip_write
• pjsip/dialplan_functions.c: added logic to handle media for TEXT
• res_pjsip.h: added variables to deal with endpoint media
• channel.c: added logic to add a stream_num for a frame
• frame.c: added code to handle and fix issues with red.
• res_pjsip_sdp_rtp.c: added logic to setup SDP.
• Added logic to handle tos and cos text
• Added logic to RED for red enabled
• res_pjsip_session.c: aded logic for max_text_streams
• res_rtp_asterisk.c: Added logic to handle Red and dealing with a repeating RTP
  issue
• pjsip_configuration.c: Added logic to handle text for PJSIP library

New additions:

• Maintain ABI compatibility
• Add alembic
• Add docs
• Adjust length based on stream
• fix whitespace formatting on some cases
• refactoring existing SDP attribute logic to use the existing framework
• fix redundant generation SDP payload mismatch
• remove not needed t140 debug fix, the t140 lengths are reset in the red case
• extend error logging and add some debug messages
• add more comment documentation regarding edge cases and workarounds
• add missing reference counting decrement for scheduler deletion
• add native bridging support for real-time RTP support
• add streaming support for real-time text
• update pjsip_config to new versions

UserNote: This introduces support for real-time text conversation over RTP (RFC 4103).
To enable it you need to add t140 and preferable also red as supported codecs.
You can configure this functionality by three new parameters in the endpoint
section of pjsip.conf: tos_text, cos_text and max_text_streams. If you are
using a database, please have a look to the upgrade notes.

UpgradeNote: This change introduce a schema modification for the ps_endpoint table by
alembic. If you don't use the scripts to update your database, please
adapt the table manually to the new structure. New fields were tos_text,
cos_text and max_text_streams.

Resolves: #997
Resolves: #1128

[sangoma-oss-cla]

All committers have signed the CLA.

[github-actions]

REMINDER: If this PR applies to other branches, please add a comment with the appropriate "cherry-pick-to" headers as per the Create a Pull Request process.

If you don't want it cherry-picked, please add a comment with cherry-pick-to: none so we don't keep asking.

If, after adding "cherry-pick-to" comments, you change your mind, please edit the comment to DELETE the header lines and add cherry-pick-to: none.

The currently active branches are now 20, 21, 22 and master.

[github-actions]

Workflow PRCheck failed
master-ari1: FAILED TEST: rest_api/bridges/error

[nikolaciprich]

Hi, tried new patches, as @henningw requested, now * crashes immediately after initiating call:

#0  0x00007f3724ebd905 in red_write (data=0x7f372c041e70) at res_rtp_asterisk.c:9149
#1  0x00000000005cdf12 in ast_sched_runq (con=0x223fad0) at sched.c:822
#2  0x00000000005cc0cf in sched_run (data=0x223fad0) at sched.c:169
#3  0x0000000000623c8d in dummy_start (data=0x2186610) at utils.c:1607
#4  0x00007f376b3adea5 in start_thread () from /lib64/libpthread.so.0
#5  0x00007f3769374b0d in clone () from /lib64/libc.so.6
(gdb) bt full
#0  0x00007f3724ebd905 in red_write (data=0x7f372c041e70) at res_rtp_asterisk.c:9149
        instance = 0x7f372c041e70
        rtp = 0xdeaddeaddeaddead
        __PRETTY_FUNCTION__ = "red_write"
#1  0x00000000005cdf12 in ast_sched_runq (con=0x223fad0) at sched.c:822
        current = 0x7f372c069890
        when = {tv_sec = 1740413884, tv_usec = 906862}
        numevents = 0
        res = 0
        __PRETTY_FUNCTION__ = "ast_sched_runq"
#2  0x00000000005cc0cf in sched_run (data=0x223fad0) at sched.c:169
        ms = 1
        ts = {tv_sec = 1740413884, tv_nsec = 905757000}
        con = 0x223fad0
        __PRETTY_FUNCTION__ = "sched_run"
#3  0x0000000000623c8d in dummy_start (data=0x2186610) at utils.c:1607
        __clframe = {__cancel_routine = 0x4604c4 <ast_unregister_thread>, __cancel_arg = 0x7f371639f700, __do_it = 1, __cancel_type = 0}
        ret = 0x0
        a = {start_routine = 0x5cbebb <sched_run>, data = 0x223fad0, name = 0x21dab80 "sched_run", ' ' <repeats 12 times>, "started at [  216] sched.c ast_sched_start_thread()"}
        __PRETTY_FUNCTION__ = "dummy_start"
#4  0x00007f376b3adea5 in start_thread () from /lib64/libpthread.so.0
No symbol table info available.
#5  0x00007f3769374b0d in clone () from /lib64/libc.so.6

note however I'm trying patch with 20.11.1, as it's the last release I'm able to build on my test env (centos7). I'll prepare newer test env to check whether this changes things.

[henningw]

Thanks for the feedback @nikolaciprich, we will have a look. Which client do you use for testing, also the tipcon1?

[nikolaciprich]

Thanks for the feedback @nikolaciprich, we will have a look. Which client do you use for testing, also the tipcon1?

NP. yes, tipcon1, wasn't able to find anything else freely available.. if you have a tip for another client, please let me know.

[henningw]

Ok, I see. Yes, the client situation for tests is not so good. Can you please share your settings to easier reproduce it? E.g.:

• max_text_streams
• cos_text
• tos_text
• allow codecs (t140 only or t140 and t140red)
• set redundancy level in the client(s)

[seanbright]

cherry-pick-to: 23
cherry-pick-to: 22
cherry-pick-to: 20

[seanbright]

I was looking through this patch a bit and how red works in asterisk in general when I came across:

asterisk/res/res_rtp_asterisk.c

Line 9184 in 2d7948f

rtp->red->schedid = ast_sched_add(rtp->sched, generations, red_write, instance);

According to the ast_sched_add(...) docs, the second argument is:

how many milliseconds to wait for event to occur

So I think it should be buffer_time and not generations. Might want to look at that as part of your work on this PR.

[henningw]

Yeah, we also wondered why it was done like this in our internal review. It was already present in the original patch, but its probably a bug. We will test tomorrow again for that.

[seanbright]

I've confirmed that it is a bug introduced when the RTP engine was modularized (split out into res_rtp_asterisk). This was the code in rtp.c that set up the scheduler before that change:

asterisk/main/rtp.c

Line 4845 in 08971ce

r->schedid = ast_sched_add(rtp->sched, ti, red_write, rtp);

I am going to open a separate PR to address that since it is a bug fix.

[henningw]

@seanbright just to give an update - we were able to reproduce the crash with another (commercial) client. My colleague added additional locking to the PR and its now not crashing anymore. We will do another review tomorrow morning and push an updated version also for you to test.

[gtjoseph]

Please follow the requirements for code contribution and formatting commit messages...

• Squash the commits into one.
• Don't put any non-standard headers after the standard ones.
• You need to add a UserNote announcing the new functionality.
• You need to add an UpgradeNote announcing a database schema change.
• The PR description must have the Fixes header or the related issue won't be closed.

See:
https://docs.asterisk.org/Development/Policies-and-Procedures/Code-Contribution/
https://docs.asterisk.org/Development/Policies-and-Procedures/Commit-Messages/

[github-actions]

Workflow PRCheck completed successfully

[henningw]

@seanbright With the addition of 4538db6 it does not crash anymore in our tests, maybe you can give it a try as well. Regarding the other comments from your side and others, we will review and address them.

[github-actions]

Workflow Check failed
master-build: FAILED: Alembic Tests

[henningw]

New changes as requested from the different reviews were added as individual commits. To give a summary about the testing status, things are looking now much better in our opinion. Below the details.

1. There are now several automatic asterisk tests based on pjsua for the RTT feature, will be added as dedicated PR to the asterisk-testsuite repository
2. RTT calls between pjsua and pjsua work with audio using T140
3. RTT calls between tipcon1 and pjsua using T140 work with and without audio
4. RTT calls between tipcon1 and pjsua using RED, with and without audio, work with an asymmetric codec on the B side; this is likely a pjsua bug, still investigated and probably to be addressed at pjsip project
5. RTT calls between pjsua and pjsua using T140 do not work without audio, but this is likely a bug in Asterisk related to stopping of the RTP engine due to a re-invite on the B side originated from asterisk with deactivated audio stream, needs more investigation
6. RTT calls between pjsua and pjsua using RED do not yet work due to a pjsua issue, as pjsua apparently does not yet support it properly. There were bug fixes for this in yesterday’s pjsip 2.17 release, but it needs more investigation
7. RTT calls between ectouch and pjsua using T140 work with audio; it cannot be disabled due to the client
8. RTT calls between tipcon1 and ectouch using RED work with audio; it cannot be disabled due to the client

If you like please give it another test from your side. Test clients were tipcon1 v1.5.1, pjsua 2.17, ectouch is not free available.

[gtjoseph]

There are now several automatic asterisk tests based on pjsua for the RTT feature, will be added as dedicated PR to the asterisk-testsuite repository

The python bindings for pjsua have been problematic for the testsuite and we no longer build it in the workflows. It may take a bit of effort to get it working again but we can cross that bridge when we come to it.

[henningw]

There are now several automatic asterisk tests based on pjsua for the RTT feature, will be added as dedicated PR to the asterisk-testsuite repository

The python bindings for pjsua have been problematic for the testsuite and we no longer build it in the workflows. It may take a bit of effort to get it working again but we can cross that bridge when we come to it.

Thanks. We used the linux application, to be precise, so it should be fine.

[henningw]

@gtjoseph your requested changes should be addressed now.
@seanbright You are also having some requested changes due to your testing result some time ago link Please give it another try. It would be also great if you could modify the cherry-pick comment link to not include branch 20 anymore due to the previous discussion with @gtjoseph .

[github-actions]

Workflow Check failed
master-build: FAILED: Alembic Tests

[gtjoseph]

. It would be also great if you could modify the cherry-pick comment #1128 (comment) to not include branch 20 anymore due to the previous discussion with @gtjoseph .

Thinking further, I'd prefer just not removing the code, Asterisk 20 receives full support until October of this year and security fixes until October 2027. Since this PR touches many files, not putting it into 20 could make cherry-picking other PRs into 20 fail and require manual intervention.

[henningw]

. It would be also great if you could modify the cherry-pick comment #1128 (comment) to not include branch 20 anymore due to the previous discussion with @gtjoseph .

Thinking further, I'd prefer just not removing the code, Asterisk 20 receives full support until October of this year and security fixes until October 2027. Since this PR touches many files, not putting it into 20 could make cherry-picking other PRs into 20 fail and require manual intervention.

The red_write(..) scheduler code does nothing after the initial change set. So you suggested to remove it, which we have done. This removal also fixed a error log, that always came at shutdown related to this, as I noticed.

I don't think it makes sense to add a compatibility layer for chan_sip to the branch 20 to this PR. I would be also not in a position to spend significant time in this area, as its an obselete SIP stack (and deprecated since asterisk 17). Naturally we never tested this PR with chan_sip, so it will probably not work.

So the options I see after finalising this PR:

1. Add it to branches 20, 22, 23, RTT for chan_sip will be broken, RTT for chan_pjsip will be available for all stable branches. Eventually smaller issues during cherry-picks for 20
2. Add it to branches 22 and 23, no RTT for chan_pjsip on 20. Eventually smaller issues during cherry-pick for 20. RTT for chan_pjsip will be available for 22 and 23 branches
3. Don't add it to stable branches at all, no RTT for chan_pjsip until asterisk 24 is released (october 2026)

My preference would be 1 or alternatively 2, if affecting RTT for chan_sip is not accetable for the project team.

[gtjoseph]

I know you're frustrated and I'm sorry for the churn but this PR touches so many things that not including it in all active branches will make life difficult for years down the road. Basically, anyone who adds pjsip options or touches anything near any of your changes will need to create multiple pull requests until October 2029 when Asterisk 22 goes EOL. It's also not acceptable to break something that currently works.

You're not being asked to spend "significant" time on this. In rtp_red_init for instance, you could simply get the ast_rtp_glue from the instance and only start the timer if the glue was registered by chan_sip.

[henningw]

I know you're frustrated and I'm sorry for the churn but this PR touches so many things that not including it in all active branches will make life difficult for years down the road. Basically, anyone who adds pjsip options or touches anything near any of your changes will need to create multiple pull requests until October 2029 when Asterisk 22 goes EOL. It's also not acceptable to break something that currently works.

You're not being asked to spend "significant" time on this. In rtp_red_init for instance, you could simply get the ast_rtp_glue from the instance and only start the timer if the glue was registered by chan_sip.

Thanks for the hint. I am not frustrated, just it seems from the outside that the requirements from your side keep changing. I will look into the suggested approach.

[henningw]

@gtjoseph Unless I made a stupid mistake, I think that during the time when the scheduler is started the chan_pjsip glue is unfortunately not initialized yet. Does chan_sip works different in this regards?

/*! \pre instance is locked */
 static int rtp_red_init(struct ast_rtp_instance *instance, int buffer_time, int *payloads, int generations)
 {
+       struct ast_rtp_glue *test_glue;
        struct ast_rtp *rtp = ast_rtp_instance_get_data(instance);
        int x;

@@ -9200,6 +9226,19 @@ static int rtp_red_init(struct ast_rtp_instance *instance, int buffer_time, int
                rtp->red->t140red_data[x*4] = rtp->red->pt[x];
        }
        rtp->red->t140red_data[x*4] = rtp->red->pt[x] = payloads[x]; /* primary pt */
+       test_glue = ast_rtp_instance_get_active_glue(instance);
+       ast_log(LOG_WARNING, "scheduler called %p\n", test_glue);

    -- Executing [101@rtt-test:1] NoOp("PJSIP/102-00000000", "Calling 101") in new stack
    -- Executing [101@rtt-test:2] Dial("PJSIP/102-00000000", "PJSIP/101") in new stack
    -- Called PJSIP/101
    -- PJSIP/101-00000001 is ringing
       > 0x7fff7c046c30 -- Strict RTP learning after remote address set to: 127.0.0.1:30004
[Apr 29 17:22:49] WARNING[3903480]: res_rtp_asterisk.c:9231 rtp_red_init: scheduler called (nil)
    -- PJSIP/101-00000001 answered PJSIP/102-00000000
       > 0x7fff7c02cf80 -- Strict RTP learning after remote address set to: 192.168.188.59:1038
[Apr 29 17:22:49] WARNING[3903480]: res_rtp_asterisk.c:9231 rtp_red_init: scheduler called (nil)
    -- Channel PJSIP/101-00000001 joined 'simple_bridge' basic-bridge <4320751b-1277-472a-9dc0-004d26c0ea8b>
    -- Channel PJSIP/102-00000000 joined 'simple_bridge' basic-bridge <4320751b-1277-472a-9dc0-004d26c0ea8b>
       > Bridge 4320751b-1277-472a-9dc0-004d26c0ea8b: switching from simple_bridge technology to native_rtp
       > Locally RTP bridged 'PJSIP/102-00000000' and 'PJSIP/101-00000001' in stack
       > 0x7fff7c02cf80 -- Strict RTP qualifying stream type: text
       > 0x7fff7c02cf80 -- Strict RTP switching source address to 62.228.52.98:1038
       > 0x7fff7c02cf80 -- Strict RTP learning complete - Locking on source address 62.228.52.98:1038
       > 0x7fff7c046c30 -- Strict RTP switching to RTP target address 127.0.0.1:30004 as source
       > 0x7fff7c046c30 -- Strict RTP learning complete - Locking on source address 127.0.0.1:30004
    -- Channel PJSIP/101-00000001 left 'native_rtp' basic-bridge <4320751b-1277-472a-9dc0-004d26c0ea8b>
    -- Channel PJSIP/102-00000000 left 'native_rtp' basic-bridge <4320751b-1277-472a-9dc0-004d26c0ea8b>

[gtjoseph]

Does chan_sip works different in this regards?

Excellent question but it's one I don't have an answer for. :(

Alternatively, you could do the following...

const char *chanid = ast_rtp_instance_get_channel_id(instance);
if (!ast_strlen_zero(chanid)) {
    struct ast_channel *chan = ast_channel_get_by_uniqueid(chanid);
    if (chan && ast_strings_equal(ast_channel_tech(chan)->type, "SIP")) {
        rtp->red->schedid = ast_sched_add(rtp->sched, buffer_time, red_write, instance);
    }
    ao2_cleanup(chan);
}

[henningw]

Thanks. I spend some hours to setup a chan_sip test with asterisk 20. Your proposed second solution seems to work there, see below. As a note, asterisk 20 with chan_sip seems to be not working correctly anyhow for real-time text, as the pjsua does not display any received text messages. This was on a unmodified (besides some added logging) asterisk 20 (73daa96).

The previous dicusssed removal was reverted and the proposed solution was added as another commit, also restoring the write for chan_sip in red_write(..).

  == Spawn extension (rtt-test, 101, 2) exited non-zero on 'SIP/102-00000000'
  == Using SIP RTP CoS mark 5
       > 0x7fff78018370 -- Strict RTP learning after remote address set to: 192.168.188.59:1136
       > 0x7fff7801af50 -- Strict RTP learning after remote address set to: 192.168.188.59:1138
    -- Executing [101@rtt-test:1] NoOp("SIP/102-00000002", "Calling 101") in new stack
    -- Executing [101@rtt-test:2] Dial("SIP/102-00000002", "SIP/101@127.0.0.1:5065") in new stack
  == Using SIP RTP CoS mark 5
    -- Called SIP/101@127.0.0.1:5065
    -- SIP/127.0.0.1:5065-00000003 is ringing
       > 0x7fff44018fb0 -- Strict RTP learning after remote address set to: 127.0.0.1:30032
       > 0x7fff4401bb90 -- Strict RTP learning after remote address set to: 127.0.0.1:30034
[Apr 30 06:33:17] WARNING[309441][C-00000002]: res_rtp_asterisk.c:9215 rtp_red_init: scheduler started for SIP channel
    -- SIP/127.0.0.1:5065-00000003 answered SIP/102-00000002
    -- Channel SIP/127.0.0.1:5065-00000003 joined 'simple_bridge' basic-bridge <62046b1e-949b-49f0-8751-b6e2dd45ef57>
    -- Channel SIP/102-00000002 joined 'simple_bridge' basic-bridge <62046b1e-949b-49f0-8751-b6e2dd45ef57>
       > Bridge 62046b1e-949b-49f0-8751-b6e2dd45ef57: switching from simple_bridge technology to native_rtp
       > Locally RTP bridged 'SIP/102-00000002' and 'SIP/127.0.0.1:5065-00000003' in stack
       > 0x7fff44018fb0 -- Strict RTP switching to RTP target address 127.0.0.1:30032 as source
       > 0x7fff78018370 -- Strict RTP qualifying stream type: audio
       > 0x7fff78018370 -- Strict RTP switching source address to 62.228.52.98:1136
       > 0x7fff78018370 -- Strict RTP learning complete - Locking on source address 62.228.52.98:1136
       > 0x7fff44018fb0 -- Strict RTP learning complete - Locking on source address 127.0.0.1:30032
       > 0x7fff7801af50 -- Strict RTP qualifying stream type: text
       > 0x7fff7801af50 -- Strict RTP switching source address to 62.228.52.98:1138
       > 0x7fff7801af50 -- Strict RTP learning complete - Locking on source address 62.228.52.98:1138
       > 0x7fff4401bb90 -- Strict RTP switching to RTP target address 127.0.0.1:30034 as source
       > 0x7fff4401bb90 -- Strict RTP learning complete - Locking on source address 127.0.0.1:30034

[github-actions]

Workflow Check failed
master-build: FAILED: Alembic Tests

[henningw]

@mariefischer Maybe you want to give it a try with the new version pushed last week. If there are some traces you want to share, feel free to send me an email (visible in my github profile).

@gtjoseph If you are satisified with the chan_sip related changes suggested from your side, please remove the "changes requested" flag, thank you.

@seanbright Can you also review the state from your side? You are also having the "changes requested" flag set, thanks as well.

[henningw]

Presentation with current status and further background from a conference link