meetecho / janus-gateway

Janus WebRTC Server
https://janus.conf.meetecho.com
GNU General Public License v3.0
8.02k stars 2.45k forks source link

SIP plugin audio not working in v0.7.4 #1770

Closed jflasch closed 4 years ago

jflasch commented 4 years ago

SIP audio stopped working in v0.7.4, specifically starting with commit dca6fec91c8c4f1da096bbad295a7725a04c5f00.

This is the line we believe to be causing problems for us: https://github.com/meetecho/janus-gateway/commit/dca6fec91c8c4f1da096bbad295a7725a04c5f00#diff-163984d531b35b047dc3f79fabe0527eL3842

We are only using the SIP plugin for audio. Prior to the above commit, audio with the SIP plugin works fine with our application. We also tried with the most recent version of master and it does not work.

We are using the SIP plugin to communicate with Asterisk. Our register message to Janus looks like this:

janus: "message",
      transaction: transaction,
      session_id: this.sessionId,
      handle_id: this.sipHandle,
      body: {
        request: "register",
        username: 'sip:'+userName+'@127.0.0.1',
        send_register: false,
        secret: "****",
      }

Our call request to Janus looks like this:

janus: "message",
      transaction: transaction,
      session_id: this.sessionId,
      handle_id: this.sipHandle,
      body: {
        request: "call",
        uri: uri,
        secret: "****",
        srtp: "sdes_mandatory",
      },
      jsep: {
        type: "offer",
        sdp: sdpOffer
      }

The Janus logs show us this error:

[plugins/janus_sip.c:janus_sip_sofia_callback:3853]         We asked for mandatory SRTP
but didn't get any in the reply!

The change that we linked above seems like it's now expecting audio and video to be mandatory if you specify sdes_mandatory even though we only want to use audio.

lminiero commented 4 years ago

That error appears when a 200 OK comes in, so please share the remote SDP.

Edit: although you may be right, it checks !session->media.has_srtp_remote_video without checking if video is negotiated at all. Should be easy enough to replicate locally.

lminiero commented 4 years ago

The commit above should fix it. Thanks for spotting the issue!

jflasch commented 4 years ago

The commit above fixed it. Thanks for the support!