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

incorrect SDP answer in the video room #2547

Closed
B-R-Bender opened this issue Feb 3, 2021 · 5 comments
Closed

incorrect SDP answer in the video room #2547

B-R-Bender opened this issue Feb 3, 2021 · 5 comments

Comments

@B-R-Bender
Copy link
Contributor

Hello guys!

I, recently, was looking for a way to set the audio bitrate in the videoroom higher (by default, it was about 32kbps). The way this could be achieved is to mungle local SDP before creating an offer to tweak audio fmtp line. For instance from this:
a=fmtp:111 minptime=10;useinbandfec=1\r\n
to this:
a=fmtp:111 minptime=10;useinbandfec=1;cbr=1;stereo=1;maxaveragebitrate=160000\r\n
So I've tried to createOffer.customizeSdp callback and surprisely it has no effect. So I found this #1430. And it seems to be that when we receiving an answer from janus along with remote description it has no fmtp line for an audio source at all. Hence I've experienced no changes on audio quality (monitoring over webrtc-internals).
But, if I've mungled answers SDP with this dirty hack:
jsep.sdp = jsep.sdp.replace("a=rtpmap:111 opus/48000/2\r\n", "a=rtpmap:111 opus/48000/2\r\na=fmtp:111 minptime=10;useinbandfec=1;cbr=1;stereo=1;maxaveragebitrate=160000\r\n"); I've got the expected behaviour.

So from my perspective it's an issue. And it could be fixed in a 2 different ways:

  1. make janus to send correct answer with requested Optional parameters as listed in 6.1 of RFC7587
  2. provide a way to customize answers SDP just like createOffer does

second option is what I've done to my code, so I've ended up with this gist

I can do a PR if it's ok, so please give some feedback on this.
And thank you for all your time.

@lminiero
Copy link
Member

lminiero commented Feb 3, 2021

customizeSdp is available both for createOffer and createAnswer. We use it in the Streaming demo already:

https://github.com/meetecho/janus-gateway/blob/master/html/streamingtest.js#L161

@lminiero lminiero closed this as completed Feb 3, 2021
@B-R-Bender
Copy link
Contributor Author

B-R-Bender commented Feb 3, 2021

@lminiero I'm talking not about createAnswer but handleRemoteJsep instead, which is prepareWebrtcPeer which is actually is RTCPeerConnection.setRemoteDescription()
a way to mungle not a local SDP but a remote one

@lminiero
Copy link
Member

lminiero commented Feb 3, 2021

Then yes, a PR would be welcome: as long as you use customizeSdp for consistency, and not the weird function name I see in your gist 🙂

@lminiero lminiero reopened this Feb 3, 2021
@B-R-Bender
Copy link
Contributor Author

I'm using such kind of naming to self describe methods as something dangerous and better not to touch right away in the method signature. But from the consistency perspective sure I will use customizeSdp. Give me a couple minutes I'll prepare a PR.

B-R-Bender pushed a commit to B-R-Bender/janus-gateway that referenced this issue Feb 3, 2021
 - customizeSdp callback added to the handleRemoteJsep(prepareWebrtcPeer) to be able to mangle remote SDP if needed (meetecho#2547)
@B-R-Bender
Copy link
Contributor Author

@lminiero you can check out the PR
and thank you for a quick response!

@lminiero lminiero closed this as completed Feb 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants