-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
feat(multi-stream-support) Add the support for multiple local video streams. #1870
feat(multi-stream-support) Add the support for multiple local video streams. #1870
Conversation
…treams. This feature is behind 'sendMultipleVideoStreams' config.js flag and is currently supported only on clients running in Unified plan mode.
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.
Left few questions/comments. I'm trying to understand what's the final approach going to look like. Now it seems slightly different to what we were talking about.
modules/RTC/TPCUtils.js
Outdated
&& t.currentDirection === MediaDirection.INACTIVE); | ||
|
||
// If a track of a given media type is added for the first time, replace the track on the first sender. This | ||
// will be called when the user joins the call muted but unmutes during the call. |
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.
how do you know it's okay to just pick the first one in the multi track scenario? were we not supposed to generate dedicated mlines for each local track and re-use them as tracks are added/remove/replaced? I thought that the recvonly mlines will not be used for sending local tracks, but dedicate 'sendonly' mlines will be created?
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.
When we add a transceiver by adding a new m-line in the remote description without a ssrc attached, the browser will create a transceiver with the direction as RECVONLY since the local track hasn't been added yet and the current direction will be INACTIVE since there is no media flowing yet. Current direction is the differentiator here. After the track is added and reneg is done, the transceiver direction will change to sendonly.
* @param {Mediatype} mediaType media type of the new source that is being added. | ||
*/ | ||
SDP.prototype.addMlineForNewLocalSource = function(mediaType) { | ||
const mid = this.media.length; |
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 in another comment, I think if we put source names as mid
for the sender mlines that would make it easier to find transceivers and recycle the mlines.
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.
No I don't think this is a good idea. The same mids show up in both the local and remote description and these mids do not make sense when you are trying to generate new m-lines for new remote sources. They wouldn't follow a pattern and recycling them will not be possible.
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.
They would be dedicated to local tracks and can be recycled for that purpose, but again if you think it's easier this way then that's ok. Still I think it will be more error prone. The if
conditions that rely on ordering, finding media type and inactive or recvonly is hard to understand and follow without tons of comments.
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.
The idea is to use the first available sender when you are adding/replacing it on the peerconnection. I still don't get what you mean by dedicated m-lines. Having one fixed per JitsiLocalTrack means there will be many unused ones since every device change for a given source produces a new JitsiLocalTrack and will be a new m-line.
There is no documentation on how to use multistreaming support. |
@zsinba it's not finished yet, we'll post an update once it's ready for testing |
Do you have an estimated release date? @paweldomas |
Has this been released? |
@zsinba the estimated release date is around end of March |
Thanks。 |
HowAbouIt |
Hello, Can We Have a try now? |
Hello all. how about it now? |
Yes, it is possible to start testing it now. I will put more detailed instructions in this thread: The instructions should be there within a day or two, but long story short you need to:
If you have any followup questions please use the community forum and the thread linked above. |
你太棒了。 I have updated the Jitsi of Docker version to the latest version, can it be used for testing? |
I tried it out and it was great. |
Uhm, I'm confused about this part. It is actually not supposed to combine local camera and desktop into one stream anymore.
You could eventually do that, but you'd have to modify jitsi-meet yourself. |
I may not have made myself clear. I downloaded and ran the latest version of Jitsi of docker; I could share My desktop and open my camera at the same time. They will be pushed to the remote end in the same stream. as follows: My question is: Thank you very much for your constant reply. Thank you for your answer too. |
I consulted about this about two years ago. The response: No. You can't push two streams out at the same time. |
Right, but this changes with the multi stream mode supported on the lib-jitsi-meet/JVB level. You should be able to do that, but you need to modify jitsi-meet. About your screenshots, that is not the new presenter mode, that's how it works currently and it's a single stream. Maybe it's not available in the docker release yet. |
This feature is behind 'sendMultipleVideoStreams' config.js flag and is currently supported only on clients running in Unified plan mode.