add rtpengine_subscribe_request rtpengine_subscribe_answer and rtpengine_unsubscribe methods to both th cfg and kemi interfaces
#### Pre-Submission Checklist <!-- Go over all points below, and after creating the PR, tick all the checkboxes that apply --> <!-- All points should be verified, otherwise, read the CONTRIBUTING guidelines from above--> <!-- If you're unsure about any of these, don't hesitate to ask on sr-dev mailing list --> - [x] Commit message has the format required by CONTRIBUTING guide - [x] Commits are split per component (core, individual modules, libs, utils, ...) - [x] Each component has a single commit (if not, squash them into one commit) - [x] No commits to README files for modules (changes must be done to docbook files in `doc/` subfolder, the README file is autogenerated)
#### Type Of Change - [ ] Small bug fix (non-breaking change which fixes an issue) - [x] New feature (non-breaking change which adds new functionality) - [ ] Breaking change (fix or feature that would change existing functionality)
#### Checklist: <!-- Go over all points below, and after creating the PR, tick the checkboxes that apply --> - [ ] PR should be backported to stable branches - [x] Tested changes locally - [x] Related to issue #4273
#### Description new kemi and cfg script commands added. existing internal api modifed in a backwards compatible way (though no means of testing this) You can view, comment on, or merge this pull request online at:
https://github.com/kamailio/kamailio/pull/4274
-- Commit Summary --
* rtpengine: add support for commands needed for siprec
-- File Changes --
M src/modules/rtpengine/rtpengine.c (542)
-- Patch Links --
https://github.com/kamailio/kamailio/pull/4274.patch https://github.com/kamailio/kamailio/pull/4274.diff
@rfuchs commented on this pull request.
An example of the intended usage would be helpful
} else if(str_eq(&key, "all")) {
+ ng_flags->all_legs = 1; + } else
I think this is redundant? Just putting the word "all" in the argument string would already lead to "all" being put in the flags list without the need for special handling, no?
else if(str_eq(&key, "from-tags")) {
+ err = "missing value"; + if(!val.s) + goto error; + + if(!ng_flags->from_tags) { + ng_flags->from_tags = + bencode_list(ng_flags->dict->buffer); + } + bencode_list_add_str(ng_flags->from_tags, &val); + } else
Ok if this is how you want to do it, but some suggestions for alternatives: `subscribe request` can take either a single `from-tag` or multiple `from-tags` as a list, and would treat them the same. So here you could also treat "from-tag" and "from-tags" the same, and making building the list conditional on the opmode.
Or: omit handling altogether and require a modern version of rtpengine which parses the flags internally (`parsed_by_module` false). In this case you can use syntax like `rtpengine_blah("flag flag ... from-tags=[tag1 tag2 tag3] ...")`
Up to you, just a hint.
- {"rtpengine_subscribe_offer", (cmd_function)rtpengine_subscribe_request_wrap_f, 4,
+ fixup_rtpengine_subscribe_request_v, fixup_free_rtpengine_subscribe_request_v, ANY_ROUTE}, + {"rtpengine_subscribe_offer", (cmd_function)rtpengine_subscribe_request_wrap_f, 5, + fixup_rtpengine_subscribe_request_v, fixup_free_rtpengine_subscribe_request_v, ANY_ROUTE}, + {"rtpengine_subscribe_answer", (cmd_function)rtpengine_subscribe_answer_wrap_f, 1, + fixup_spve_null, fixup_free_spve_null, ANY_ROUTE}, + {"rtpengine_subscribe_answer", (cmd_function)rtpengine_subscribe_answer_wrap_f, 2, + fixup_spve_spve, fixup_free_spve_spve, ANY_ROUTE}, + {"rtpengine_unsubscribe", (cmd_function)rtpengine_unsubscribe_wrap_f, 1, + fixup_spve_null, fixup_free_spve_null, ANY_ROUTE}, + {"rtpengine_unsubscribe", (cmd_function)rtpengine_unsubscribe_wrap_f, 2, + fixup_spve_spve, fixup_free_spve_spve, ANY_ROUTE},
These would have to be added to the documentation
@tsearle commented on this pull request.
- {"rtpengine_subscribe_offer", (cmd_function)rtpengine_subscribe_request_wrap_f, 4,
+ fixup_rtpengine_subscribe_request_v, fixup_free_rtpengine_subscribe_request_v, ANY_ROUTE}, + {"rtpengine_subscribe_offer", (cmd_function)rtpengine_subscribe_request_wrap_f, 5, + fixup_rtpengine_subscribe_request_v, fixup_free_rtpengine_subscribe_request_v, ANY_ROUTE}, + {"rtpengine_subscribe_answer", (cmd_function)rtpengine_subscribe_answer_wrap_f, 1, + fixup_spve_null, fixup_free_spve_null, ANY_ROUTE}, + {"rtpengine_subscribe_answer", (cmd_function)rtpengine_subscribe_answer_wrap_f, 2, + fixup_spve_spve, fixup_free_spve_spve, ANY_ROUTE}, + {"rtpengine_unsubscribe", (cmd_function)rtpengine_unsubscribe_wrap_f, 1, + fixup_spve_null, fixup_free_spve_null, ANY_ROUTE}, + {"rtpengine_unsubscribe", (cmd_function)rtpengine_unsubscribe_wrap_f, 2, + fixup_spve_spve, fixup_free_spve_spve, ANY_ROUTE},
I'm adding the documentation as a 2nd commit onto this PR
@tsearle commented on this pull request.
else if(str_eq(&key, "from-tags")) {
+ err = "missing value"; + if(!val.s) + goto error; + + if(!ng_flags->from_tags) { + ng_flags->from_tags = + bencode_list(ng_flags->dict->buffer); + } + bencode_list_add_str(ng_flags->from_tags, &val); + } else
from_tags and from_tag are different keys in rtpengine.
from_tag is used to identify the call, from_tags is used as an alternative to all to specify which participants to for streams for
@tsearle commented on this pull request.
} else if(str_eq(&key, "all")) {
+ ng_flags->all_legs = 1; + } else
I was hoping so too, unknown flags get put into ng_flags->flags
however https://github.com/kamailio/kamailio/blob/7de2b9b7625d9cb856944627a3a5d7c167...
ignores ng_flags->flags when doing the encoding. I was not brave enough to change this behaviour as the effect would be quite global!
@tsearle pushed 1 commit.
793b36f17a7abe96242075f7766f7616e8538b01 rtpengine: add documentation for new siprec methods
@rfuchs commented on this pull request.
} else if(str_eq(&key, "all")) {
+ ng_flags->all_legs = 1; + } else
I don't quite follow. The flags are constructed within `rtpp_function_call` and then put into the dictionary: https://github.com/kamailio/kamailio/blob/7de2b9b7625d9cb856944627a3a5d7c167...
@rfuchs commented on this pull request.
else if(str_eq(&key, "from-tags")) {
+ err = "missing value"; + if(!val.s) + goto error; + + if(!ng_flags->from_tags) { + ng_flags->from_tags = + bencode_list(ng_flags->dict->buffer); + } + bencode_list_add_str(ng_flags->from_tags, &val); + } else
Yes they're different keys, but for a subscribe request putting `from-tag=foobar` is identical to `from-tags=[foobar]`
tsearle left a comment (kamailio/kamailio#4274)
Example usage included in the documentation
@rfuchs commented on this pull request.
</para>
+ <para>The <quote>flags</quote> string is a list of space-separated items. Each item + is either an individual token, or a token in <quote>key=value</quote> format. The + possible tokens are described below. This offer SDP can be manipulated with the same flags as + used in <function>rtpengine_offer</function> additionally, the following extra flags can be used + </para> + <itemizedlist> + <listitem><para> + <emphasis>to-tag=...</emphasis> - value of the to tag returned by + <quote>rtpengine_subscribe_request</quote> + </para></listitem> + <listitem><para> + <emphasis>from-tag=...</emphasis> - value of the from tag of the original call. + </para></listitem> + <listitem><para> + <emphasis>callid=...</emphasis> - value of the call id of the original call.
`call-id` with a dash (also below)
@tsearle pushed 1 commit.
cd59bbd8ed1dbb25ecb8af3bf52e2905683bc8a6 rtpengine: add documentation for new siprec methods
@tsearle commented on this pull request.
</para>
+ <para>The <quote>flags</quote> string is a list of space-separated items. Each item + is either an individual token, or a token in <quote>key=value</quote> format. The + possible tokens are described below. This offer SDP can be manipulated with the same flags as + used in <function>rtpengine_offer</function> additionally, the following extra flags can be used + </para> + <itemizedlist> + <listitem><para> + <emphasis>to-tag=...</emphasis> - value of the to tag returned by + <quote>rtpengine_subscribe_request</quote> + </para></listitem> + <listitem><para> + <emphasis>from-tag=...</emphasis> - value of the from tag of the original call. + </para></listitem> + <listitem><para> + <emphasis>callid=...</emphasis> - value of the call id of the original call.
fixed
@tsearle commented on this pull request.
} else if(str_eq(&key, "all")) {
+ ng_flags->all_legs = 1; + } else
I've re-tested with the "all" flag removed and it still gets put in flags. So the only thing lost is enforcing that either "all" or "from_tags" is sent, but not both. updating the patch to remove this code
@tsearle commented on this pull request.
else if(str_eq(&key, "from-tags")) {
+ err = "missing value"; + if(!val.s) + goto error; + + if(!ng_flags->from_tags) { + ng_flags->from_tags = + bencode_list(ng_flags->dict->buffer); + } + bencode_list_add_str(ng_flags->from_tags, &val); + } else
I looked some more about merging the logic.... from-tag is a string in flags, so it doesn't support specifying it multiple times...
also there is som logic for checking that from-tag is set when no sip_msg is present, so I feel more comfortable keeping it as is
@tsearle pushed 2 commits.
effbd7666830f9963e9668a55e39bf8261bad5b9 rtpengine: add support for commands needed for siprec 54a03c5f191d449257c4714547984510d3a371ca rtpengine: add documentation for new siprec methods
Merged #4274 into master.
-
Richard Fuchs -
tsearle