-
Notifications
You must be signed in to change notification settings - Fork 508
Add support for simulcast streams. #5535
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
Conversation
| function mungeSdpForSimulcasting(sdp) { | ||
| // Let's munge the SDP to add the attributes for enabling simulcasting | ||
| // (based on https://gist.github.com/ggarber/a19b4c33510028b9c657) | ||
| var lines = sdp.split("\r\n"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have this lib included:
https://www.npmjs.com/package/sdp-transform
Should make your life easier?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe also https://www.npmjs.com/package/@jitsi/sdp-simulcast could be used. For the initial proof-of-concept I was just copying the existing code from Janus.
fabc8b1 to
c31bd29
Compare
danxuliu
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the delay.
As far as I can tell in general simulcast works with video streams (see below for screen shares), also when the streams are disabled or when changing between devices. I have also tested with a signaling server version that does not support simulcast and everything works as expected (that is, the high quality is used).
There was an issue with how the simulcast quality was set; selected participants are not the same as promoted participants (selected are explicitly set by the user, promoted are automatically set based on who is speaking), so in speaker mode the current speaker was shown with low quality.
Independently of that, the video quality is directly related to the call view, so I moved the watchers from the WebRTC code to the CallView component and added methods in the model to select a specific quality. I also fixed the issue with the promoted participants by taking that into account in adjustSimulcastQualityForParticipant. It may be possible to improve it a bit further by setting high quality only to the selected participant rather than to the selected and promoted participants when one is selected, but I have not checked that.
Besides that, sometimes the following message appears in Janus logs: No packet received on substream 2 for a while, falling back to 1. This could be related to how the bandwidth is allocated between the different streams (it seems that there are issues in that regard), but I have no idea.
Regarding the open issues:
- Test with screensharing streams.
Simulcast with screensharing streams does not seem to work. But I have no idea why, because they should work just like regular videos 🤷
The received video resolution does not change when the simulcast stream is changed, neither in Firefox nor in Chromium.
Moreover, in the case of Chromium the following warning is shown in Janus logs (for the sender): [WARN] Incoming RTCP SR SSRC (1192582037) does not match the expected one (4197326786) video=1. I have checked the SDPs for a normal video and a screen share and, besides the IDs, the only difference is in the data channel related fields (because the screen sharing peer has no data channels). But if data channels are disabled in the normal video peer the only differences in the SDP of a normal video and a screen share are the IDs, yet the error is shown only for screen shares.
- Adjust selection of substream / temporal layer.
What is missing here? Could you elaborate? :-)
- Check if
SentVideoQualityThrottlercan be disabled if simulcast is used.
I would keep the quality throttler even if simulcast is used, as it should help to keep the CPU usage low due to not sending a stream with a high bitrate when not needed, and in calls with a high number of participants I think that it is fine that a participant who is not speaking is not sent with high quality even if selected. Having said that I have not actually checked how the low quality version of a throttled quality stream looks...
- Select lower quality for slow clients (e.g. also use "low" for grid mode / selected video).
If this is meant to be done by analyzing the stats reported by the received peer connections I would leave this for a later stage. The stats reported by the received peer connections are updated at irregular intervals, so it is difficult to calculate things like number of packets per second or the ratio of lost packets. The approach used in #5885 could be extended to handle received peer connections too, but I have not checked if that would actually work or not.
Of course if this is going to be done in a different way (or if I am missing something about the stats) it would be a nice addition :-)
Some additional issues:
- Simulcast Firefox bitrates should be based on the maximum bandwidth allowed by the HPB?
- Use rid instead of SDP munging for recent Chromium versions, as that will make possible to configure too the bitrates (see https://www.meetecho.com/blog/simulcast-janus-ssrc/)
|
Thanks for the first review! The API for substream selection in the signaling server was released with 0.3.0 today.
There might be issues in the browser implementations with screensharing streams. A quick search found https://bugs.chromium.org/p/webrtc/issues/detail?id=11310
The selection which substream is subscribed for which video could probably be improved. I wasn't sure if the simple selection with grid, promoted, other was sufficient. Imho we should focus on getting basic simulcast working in a first step and then improve based on feedback from real-world usage / feedback in follow-up PRs.
I would move this also to a follow-up PR if necessary. Bandwidth configurations on the HPB side might not be that easy if proxies are used which could have their own bw limits. |
Given that it does not seem to work neither with Firefox nor Chromium and that with Chromium it also spams Janus logs I would remove the calls to
Maybe we could use something like this:
But this can be done in a follow up.
Fine by me 👍
👍 |
Signed-off-by: Joachim Bauch <[email protected]>
c31bd29 to
262023c
Compare
Simulcast does not currently work for screenshares neither in Firefox nor in Chromium. Moreover, if a screen is shared in Chromium and simulcast is enabled for the peer connection of the screen Janus logs will be spammed with "[WARN] Incoming RTCP SR SSRC (XXXXXXXXX) does not match the expected one (YYYYYYYYYY) video=1" messages. Therefore, for now, simulcast is disabled for screen peers. Signed-off-by: Daniel Calviño Sánchez <[email protected]>
danxuliu
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As mentioned, I have disabled simulcast for screen shares. Other than that in general it works, so fine to address the pending issues in follow up pull requests 👍
|
/backport to stable22 |
Initial version for some testing.
Stream selection:
This has some nice bandwidth savings (receiving side) and with that also CPU usage is reduced:

Should be backwards compatible for mobile clients which will receive the highest layer (the same as before).
Requires strukturag/nextcloud-spreed-signaling#104
Open issues:
SentVideoQualityThrottlercan be disabled if simulcast is used.Fixes #5509