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

Initial support for DTX (EchoTest, VideoRoom) #2789

Merged
merged 2 commits into from
Nov 3, 2021
Merged

Initial support for DTX (EchoTest, VideoRoom) #2789

merged 2 commits into from
Nov 3, 2021

Conversation

lminiero
Copy link
Member

Small patch to make EchoTest and VideoRoom aware of DTX in negotiation, so that it can be enabled in case. Not tested extensively yet, which will need to be done to see if there can be an impact on recordings or audio quality in rooms. Since this patch mostly touched the SDP negotiation those plugins do, I took advantage of this to also change something else, e.g., allow the VideoRoom to negotiate stereo on subscribers if the publisher offers it.

Feedback welcome!

@JanFellner
Copy link
Contributor

Lol i always thought DTX would have been working.
We mundged the missing ;usedtx=1 into the SDP answer from janus since january this year.

I am currently using it for
chrome: ">=56",
safari: ">=12.1",
// firefox: ">=78", Due to audio issues we currently disable DTX for firefox
edge: ">=79", // first Chromium version
opera: ">=43" // Chromium 56

Have only had issues with firefox as always the beginning of audio was like cut off. The subscribe did not get the first 1second audio from the publisher so i disabled it for firefox. I could not really nail it down where it comes from.

You only touched the SDP handshaking or also affected modules inside janus?

@lminiero
Copy link
Member Author

Yes, I realize munging SDPs did the job: the main purpose of this PR was to try and avoid the need to munge the SDPs sent to clients. Munging is all we do, as there's nothing else we need to do with traffic.

@lminiero
Copy link
Member Author

lminiero commented Nov 3, 2021

Merging.

@lminiero lminiero merged commit b94a89d into master Nov 3, 2021
@lminiero lminiero deleted the dtx branch November 3, 2021 09:20
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

Successfully merging this pull request may close these issues.

None yet

2 participants