meetecho / janus-gateway

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

Send PLI in sip plugin Fixes #3092 enhances #3093 #3094

Closed tvildo closed 1 year ago

tvildo commented 2 years ago

Hello, I added additional option based on pull request to force send PLI if detection fails .

Why that is required? I use asterisk which has only Janus clients(web browsers), problem is that asterisk by default won't generate SDP with rtcp-fb. There is a option in asterisk see link which allows to enable rtcp-fb from pjsip.conf config:

[111@test.com]
auth=111@test.com
aors=111@test.com
type=endpoint
context=test
disallow=all
allow=alaw,ulaw,vp8,vp9,h264
webrtc=yes <--- This option

But when I enable that option in asterisk call from Janus fails, after accepting it is hanged instantly. That option will also enable additional things like see link so maybe there is some incompatibility between Janus <---> Asterisk after that is enabled in asterisk.

So my solution was to bypass automatic detection and allow to force send PLI when recording is started

lminiero commented 2 years ago

But when I enable that option in asterisk call from Janus fails, after accepting it is hanged instantly

I think that's expected. Setting webrtc=yes probably means Asterisk expects to use ICE+DTLS on that call, which would not happen with the SIP plugin (the SIP plugin acts as a "regular" softphone, no WebRTC capabilities), and so should indeed not be enabled.

That said, you say this extends #3093, but then based this PR on master. This is causing many more commits to be displayed, and since you've integrated other commits that came after that, this is hard to follow. Please rebase your branch on top of that PR branch (which hasn't been merged yet) and update the base in this PR accordingly (I tried to do it now but, again, it's displaying unrelated commits as if they were yours).

lminiero commented 2 years ago

I just merged the PR, so rebasing should be easier.

lminiero commented 2 years ago

@tvildo any update on the rebase?

tvildo commented 2 years ago

@lminiero Hi, I did rebase.

lminiero commented 2 years ago

Mh doesn't look like you did a rebase, as I still see my own new commits in there. Maybe it's easier to apply your patch in a new PR?

tvildo commented 2 years ago

@lminiero Can you take a look again ?

tvildo commented 1 year ago

@lminiero Can you take a look again ?

lminiero commented 1 year ago

Thanks, merging! :+1: