-
Notifications
You must be signed in to change notification settings - Fork 589
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
New rtp.io module (embedded RTPProxy) #3501
base: master
Are you sure you want to change the base?
Conversation
0464fea
to
26f9391
Compare
Need to work around issue with pushing to ghcr, so pipeline works in the PRs. |
26f9391
to
f3bbb64
Compare
Any updates here? No progress has been made in the last 30 days, marking as stale. |
Any updates here? No progress has been made in the last 30 days, marking as stale. |
Hey @sobomax ! I tried out the build & startup phase, feedback as follows (from least to most problematic):
Cheers! |
What rtp.io module does is it starts up a RTP handling threads in the main process and let rtpproxy module access those threads via a 1:1 socketpair, thus providing usual media relaying functionality without using any external relay process. The module requires RTPProxy 3.1 or higher, compiled with --enable-librtpproxy. When rtpproxy module is loaded without arguments and rtp.io is loaded as well, the sockets exported by the rtp.io will be used automatically in the set 0. Otherwise those sockets can be used as part of other set(s) by using "rtp.io:auto" moniker. Provided is also a CI job that builds rtp.io in a rtpproxy container for all supported architectures and tests it using voiptests package. TODO list: o hook up notification socket directly into appropriate handler on the OpenSIPS side; o generate -l / -6 option(s) based on OpenSIPS's own socket list; o more documentation.
Use rtp.io when no explicit rtpproxy configuration is provided. It is also possible to mix internal RTP functionality with externals proxies by adding it up as "rtp.io:auto" into some of the existing set.
This fixes build on systems that have no backtrace API in the libc (e.g. BSDs).
CI: handle debian.
error: format '%lu' expects argument of type 'long unsigned int', but argument 12 has type 'uint64_t'
Well, edge cases handling is somewhat obviously not very well handled since the module at the moment has only one user (us) and we know how to cook it. :) Just open a PR against respective README.md in question as user #2. :)
Well, again, this is more of a call for an action against rtpproxy repository. I build against latest ubuntu and debian on like 6 different architectures (see the appropriate CI pipeline attached). If one of libraries installed on your system @liviuchircu deviates from that and it bothers you, then just open a PR in the https://github.com/sippy/rtpproxy and we'll handle that edge case gracefully. :)
Fixed that, thanks for the reporting! We just never use any of the modules in question, so I put it as a safeguard since I did not know what negative value might mean in that somewhat critical code path. Regarding WIP mode, no. The code is somewhat experimental, yes, but not not a work in progress. The PR is in fact comes with the full working CI pipeline (this is the first module to provide that and the first CI to actually run a call scenario against OpenSIPS in fact) that builds against latest ubuntu and debian releases across 6 architectures and runs against 2 of them (amd64 and riscv).
Since it's not impeding any of the functionality this should be more of a feature request than anything else. The last time I saw rtpproxy crashing in production is some 5 years ago across like 300+ systems deployed. And I went to the great deal to make it reality with the glitching and fuzzing CI runs. But no, moving processing to a separate PID is not impossible and is not that difficult if you have time. So, again, good feature request. Thanks for a review, regards! -Max |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @sobomax -- sure, I can PR against rtpproxy
project, or adjacent README.md
files, to clarify the build hiccups I ran into. Small problems, at the end of the day -- not that time-consuming.
So we're only left with the threads started by "Attendant" which should get moved into a dedicated process for isolation purposes, before merging this PR.
goto e1; | ||
|
||
OPT_RESTORE(); | ||
rpi_descp->rtpp_cfsp = rtpp_main(argc, argv); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This rtpp_main()
function call is the entrypoint into the librtpproxy.a
embedded RTPProxy daemon. The default logic proceeds to fork 12
worker threads which will be part of the "Attendant" OpenSIPS worker.
This logic needs to be moved as part of a proc_export_t
extra process of the module, to prevent any scenario where a thread aborts and prevents OpenSIPS shutdown cleanup procedure. I hate to be pedantic here, but not only there are 188
modules following this rule and we shouldn't start making exceptions, but also it is very likely that rtp.io
will gain a good amount of traction on release, which further reinforces the need of a clean design from the start. This is not a feature request as I see it, it is a design requirement.
Cheers,
Summary
This PR includes a new rtp.io module which allows to provide media relaying and other RTP manipulations without external media relay.
Details
Traditionally, OpenSIPS only handles signalling, offloading media operation to the external relay. However in some situations using external relay may be undesirable from system complexity point of view and others, so having an option to handle it internally may be useful.
Solution
What rtp.io module does is it starts up a RTP handling threads in the main OpenSIPS process and let rtpproxy module access those threads via a 1:1 socketpair.
The module requires RTPProxy 3.1 or higher, compiled with --enable-librtpproxy.
When rtpproxy module is loaded without arguments and rtp.io is loaded as well, the sockets exported by the rtp.io will be used automatically in the set 0. Otherwise those sockets can be used as part of other set(s) by using "rtp.io:auto" moniker.
Provided is also a CI job that builds rtp.io in a rtpproxy container for all supported architectures and tests it using voiptests package.
Compatibility
None
Closing issues
None