Sometimes others don't receive video/screenshare

[psi-4ward]

Hey Mates,
sometimes other participants do not receive the video / screenshare.

• No JS Errors
• WebRTC shows outgoing (on sender) and incoming (receiver) tracks
• Seems the presence messages gets not send in some cases.

We assume a timing problem of the appearance of the tracks in peerconnection and rtc. The tracks should first be in peerconnection cause of the add-source msg so the code in _updateRoomPresence should trigger the presenceChanged but why if (!localVideoTracks?.length) { ?
The ! does kill the logic, doesn't it?

@jallamsetty1 ;-)

[jallamsetty1]

The tracks should first be in peerconnection cause of the add-source msg so the code in _updateRoomPresence should trigger the presenceChanged but why if (!localVideoTracks?.length) { ?

If the local track has already been added to the peerconnection, the presence will be sent if it needs to be updated - https://github.com/jitsi/lib-jitsi-meet/blob/master/JitsiConference.js#L3666
Can you pls add steps for reproduction ?

[psi-4ward]

In L3666 we've iterated localTracks which is this.rtc.getLocalTracks(mediaType); and not jingleSession.peerconnection.getLocalTracks().

To reproduce the issue is hard cause it seems sometimes the precenceChanged is false but it should be true. My users report the issue most if (a smaller window) gets shared. To see the error we need to enable/disable/enable the sharing several times.

But no matter how:

        if (!localVideoTracks?.length) {
            videoMuteStatusChanged = this._setTrackMuteStatus(MediaType.VIDEO, undefined, true);
            videoTypeChanged = this._setNewVideoType();
        }

If not localVideoTracks.length do setTrackMuteStatus() ... Why to set some Track-State if theres no track?

[jallamsetty1]

In L3666 we've iterated localTracks which is this.rtc.getLocalTracks(mediaType); and not jingleSession.peerconnection.getLocalTracks().

I can change that to use the local tracks from the jingle session but they are ideally in sync. Any source that gets removed from the peerconnection gets removed from the rtc local tracks as well. Are you seeing this issue on a custom client using the library or on a jitsi-meet client ?

To reproduce the issue is hard cause it seems sometimes the precenceChanged is false but it should be true. My users report the issue most if (a smaller window) gets shared. To see the error we need to enable/disable/enable the sharing several times.

But no matter how:

        if (!localVideoTracks?.length) {
            videoMuteStatusChanged = this._setTrackMuteStatus(MediaType.VIDEO, undefined, true);
            videoTypeChanged = this._setNewVideoType();
        }

If not localVideoTracks.length do setTrackMuteStatus() ... Why to set some Track-State if theres no track?

The track mute state needs to be updated in presence when a track is removed

lib-jitsi-meet/JitsiConference.js

Line 1472 in eda20c2

videoMuteChanged = this.room.addVideoInfoToPresence(isMuted);

[smilingthax]

Part 1

The basic issue is that in a Conference occasionally when User A starts desktop sharing, but at User B the stream does not appear.
Looking at the JS logs and XMPP messaging, we know that User B does receive the source-add, but does not receive a presence update.
We can see APP.conference._room.participants[...sender-ep-id...]._tracks[0] .muted=true, .videoType='camera' (expected would be false / 'desktop').
When calling .setMute(false) at the JS console of User B the stream is shown.

In the XMPP messaging User A does not send <presence>, when the issue is hit (when the issue is not hit, a <presence> with <videomuted>false</videomuted> and `desktop is sent, and everything works correctly).

From our understanding, after SOURCE_ADD the presence should be sent here:
https://github.com/jitsi/lib-jitsi-meet/blob/master/JitsiConference.js#L3684
At User A we put a breakpoint into the function and could verify that presenceChanged is true in case everything works, but false when the <presence> is missing.
In that latter case we found localTracks.length = 0.

Part 2

We also made the following observations (btw, we're using an only slightly modified jitsi-meet frontend/client):

1. We were not able to trigger the issue before 55131be#diff-b2f6195536d4c09eac93cd5482c9258cae0dc21129fca78a125fe545594eca30L36
2. We'd expect(ed) the issue to be solved with 474b2ec#diff-b2f6195536d4c09eac93cd5482c9258cae0dc21129fca78a125fe545594eca30R3668 , but can still trigger it.

Comparing the code pre-/post commit 55131be we can see that

    const localAudioTracks = jingleSession.peerconnection.getLocalTracks(MediaType.AUDIO);		
    const localVideoTracks = jingleSession.peerconnection.getLocalTracks(MediaType.VIDEO);

was replaced with

    const localTracks = this.getLocalTracks();

which in 474b2ec was further changed to

    const localTracks = this.getLocalTracks();
    const localAudioTracks = jingleSession.peerconnection.getLocalTracks(MediaType.AUDIO);
    const localVideoTracks = jingleSession.peerconnection.getLocalTracks(MediaType.VIDEO);

We therefore know code pre-1. is working, but code post-1/pre-2. and post-2. is not, although pre-1. and post-2. are closer together in that they both use jingleSession.peerconnection.getLocalTracks(...)– and not just getLocalTracks(), which seems to unexpectedly return .length = 0 in some cases (?!?) – but the logic of using them is still different in that pre-55131be9e66991a6cc9f3d910913607edbcb012e presenceChanged was (possibly) set in the if-branch if (localVideoTracks && localVideoTracks.length) { (for localVideoTracks[0] - same for audio) AND in the else part (for undefined).

It might be that 474b2ec tries to fix another issue (related to source-remove?), but at least it touches the same code areas...

Bottom line

We are not entirely sure why this.getLocalTracks() (i.e. this.rtc.getLocalTracks()) seems to be out-of-sync with jingleSession.peerconnection.getLocalTracks(...) at that particular point in time (during the callback of sendIq(...source-add..., callback, ...)), but it seems to happen more frequently with win11 (chrome) – and anecdotally, when sharing smaller windows (?). Also sharing the desktop with audio instead of without seems to significantly reduce (maybe even completely eliminate?) occurrences of the issue. This seems to point to some race condition / timing issue between the time the track gets added to this.getLocalTracks() and the invocation of the callback...

[jallamsetty1]

@smilingthax Thanks for providing the details, is this happening on both p2p and jvb connections ?

[smilingthax]

We only use jvb connections, no p2p.

[jallamsetty1]

@psi-4ward, I have merged a possible fix. Can you let me know if this fixes your issue ? I have no way of reproducing this exact case but it fixes another related issue.

[psi-4ward]

Looks good! Our qa engineer tested screenshare toggling about 50x with no issues so far.

Nice job @jallamsetty1 👍🏻