Skip to content
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

Add siprec extension #4132

Open
wants to merge 46 commits into
base: master
Choose a base branch
from
Open

Conversation

sorooshm78
Copy link

I had a Session Record Client (SRC) that utilized the SIPREC module from the OpenSIPS SIP server, and I intended to develop a Session Record Server (SRS) capable of supporting SIPREC. The goal was for the SRS to handle requests effectively. However, since PJSIP does not support the SIPREC extension, I added SIPREC extension support to PJSIP.

image (3)

Senario with siprec extension:

INVITE Request

Request-Line: INVITE sip:192.168.10.20 SIP/2.0

Message Headers:
    Via: SIP/2.0/UDP 192.168.10.20:5060;branch=z9hG4bK656b.7f03f537.0
    To: <sip:192.168.10.20>
    From: <sip:192.168.10.20>;tag=85c717edf7d282027df52a83293e4957-ce21
    User-Agent: OpenSIPS (3.5.0-dev (x86_64/linux))
    Require: siprec  
    Content-Type: multipart/mixed; boundary=OSS-unique-boundary-42
    Contact: <sip:192.168.21.89:5060>; +sip.src
    ...

Session Description Protocol (SDP):
    Version: 0
    Media Description (m): audio 35001 RTP/AVP 8 0 101
    Media Attributes (a): 
        sendonly
        label:0
    ...
    Media Description (m): audio 35002 RTP/AVP 8 101
    Media Attributes (a): 
        sendonly
        label:1
    ... 

420 Bad Extension Response

Status-Line: SIP/2.0 420 Bad Extension

Message Header
    ...
    Unsupported: siprec
    Supported: replaces, 100rel, timer, norefersub, trickle-ice
    Content-Length:  0

Senario with siprec extension:

In the SIPREC module of PJSIP, when an INVITE is received with the header Require: siprec and two media attribute labels in the offer SDP, it can generate an appropriate 200 OK response that includes those two media attribute labels in the SDP answer.

INVITE Request

Request-Line: INVITE sip:192.168.10.20 SIP/2.0

Message Headers:
    Via: SIP/2.0/UDP 192.168.10.20:5060;branch=z9hG4bK656b.7f03f537.0
    To: <sip:192.168.10.20>
    From: <sip:192.168.10.20>;tag=85c717edf7d282027df52a83293e4957-ce21
    User-Agent: OpenSIPS (3.5.0-dev (x86_64/linux))
    Require: siprec  
    Content-Type: multipart/mixed; boundary=OSS-unique-boundary-42
    Contact: <sip:192.168.21.89:5060>; +sip.src
    ...

Session Description Protocol (SDP):
    Version: 0
    Media Description (m): audio 35001 RTP/AVP 8 0 101
    Media Attributes (a): 
        sendonly
        label:0
    ...
    Media Description (m): audio 35002 RTP/AVP 8 101
    Media Attributes (a): 
        sendonly
        label:1
    ... 

200 OK Response

Message Headers:
    Via: SIP/2.0/UDP 192.168.10.20:5060; received=192.168.10.20; branch=z9hG4bK656b.7f03f537.0
    From: <sip:192.168.10.20>; tag=85c717edf7d282027df52a83293e4957-ce21
    To: <sip:192.168.10.20>; tag=7de0bfd1-94b6-4462-929f-53955d386d91
    Allow: PRACK, INVITE, ACK, BYE, CANCEL, UPDATE, INFO, SUBSCRIBE, NOTIFY, REFER, MESSAGE, OPTIONS
    Supported: replaces, 100rel, timer, siprec, norefersub
    ...

Session Description Protocol (SDP):
    Version: 0
    Session Name (s): pjmedia
    Media Description (m): audio 4000 RTP/AVP 8 101
    Media Attributes (a): 
        label:0
        recvonly
    ...

    Media Description (m): audio 4002 RTP/AVP 8 101
    Media Attributes (a): 
        label:1
        recvonly
    ...

@CLAassistant
Copy link

CLAassistant commented Nov 5, 2024

CLA assistant check
All committers have signed the CLA.

@sorooshm78
Copy link
Author

I couldn't debug the Bitrise error and figure out where the issue in my code is. Please guide me to resolve the problem.

@sauwming
Copy link
Member

Thank you for the submission.

The support seems to be pretty basic, i.e. it allows incoming request with siprec to be accepted (instead of declined). There are a few concerns though:

  • The setting enable_multimedia seems misleading. PJSIP already supports multimedia, i.e. audio and video, even multiple audio and multiple video.
  • The patch seems to assume that only audio is supported.

@sorooshm78
Copy link
Author

I have added more checks based on RFC 7866 , but my code lacks appropriate comments, which I will add in the next commit.

@sorooshm78
Copy link
Author

sorooshm78 commented Nov 20, 2024

I added the comment.

@sauwming
Copy link
Member

Just to give you a heads up, we are preparing for 2.15 release within a few weeks time, so we won't be able to merge this until then.
But I will give you some preliminary comments about the current patch.

build.symbian/pjsip_ua.mmp Outdated Show resolved Hide resolved
pjsip/include/pjsip-ua/sip_siprec.h Outdated Show resolved Hide resolved
* set the number of call active streams to the number of media in the SDP offer.
*/
if(options & PJSIP_INV_REQUIRE_SIPREC){
call->opt.aud_cnt = offer->media_count;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is what I mentioned about the patch assuming audio only. For example, if you check the example in the RFC 7866: https://www.rfc-editor.org/rfc/rfc7866.html#section-7.1.1, there are two videos and two audios in the incoming offer, so this code is obviously incorrect.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will fix the issue within the next few days.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resolved the issue

@sorooshm78
Copy link
Author

Resolved the audio count issue and added video support. Successfully tested with a video call using MicroSIP.

@sorooshm78
Copy link
Author

In the SIPREC protocol, it is now possible to send information in the body in XML format with the Content-Type: application/rs-metadata+xml. This feature has been added.

@andreas-wehrmann
Copy link
Contributor

Just out of curiosity: Why didn't you implement this on top of PJSUA? Or do you not use PJSUA but rather pjsip?

I'm asking because I implemented an SRC on top of PJSUA and a little SRS test tool as well.
That way PJSUA handles all the "annoying low level" media stuff and I just have to inspect the media parameters (and the XML in case of an SRS) in the callbacks.
As for setting/manipulating SDP, this can be done in the existing callbacks also.

This also cleanly separates the different tasks and concerns of each module
(i.e. the SIPREC module is able to understand and handle snapshot requests).

@sauwming sauwming added this to the release-2.16 milestone Dec 4, 2024
@sorooshm78
Copy link
Author

Just out of curiosity: Why didn't you implement this on top of PJSUA? Or do you not use PJSUA but rather pjsip?

I'm asking because I implemented an SRC on top of PJSUA and a little SRS test tool as well. That way PJSUA handles all the "annoying low level" media stuff and I just have to inspect the media parameters (and the XML in case of an SRS) in the callbacks. As for setting/manipulating SDP, this can be done in the existing callbacks also.

This also cleanly separates the different tasks and concerns of each module (i.e. the SIPREC module is able to understand and handle snapshot requests).

I initially tried to do the same thing, but it didn’t work. I was using OpenSIPS for the SRC, and in the INVITE message that OpenSIPS sent, there was a "Require: siprec" header, which caused a "bad extension" error since PJSIP didn’t support SIPREC. I didn’t want to make any changes in OpenSIPS.

@andreas-wehrmann
Copy link
Contributor

I see; didn't adding "siprec" with pjsip_endpt_add_capability() work for you?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants