Skip to content

Commit 5d63b31

Browse files
committed
fix(video-layout) Possibly fixes auto-pinning of SS in a large call.
When a user joins a very large call with SS, sometime SS is not auto-pinned to stage. This may happen when lot of participant joins are processed at the same time and therefore the state for remoteScreenShares may not get updated in time. Added extra logging to help debug if this issue reproduces.
1 parent 4432f72 commit 5d63b31

File tree

5 files changed

+52
-17
lines changed

5 files changed

+52
-17
lines changed

react/features/base/participants/reducer.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import {
2727
isRemoteScreenshareParticipant,
2828
isScreenShareParticipant
2929
} from './functions';
30+
import logger from './logger';
3031
import { FakeParticipant, ILocalParticipant, IParticipant, ISourceInfo } from './types';
3132

3233
/**
@@ -352,6 +353,8 @@ ReducerRegistry.register<IParticipantsState>('features/base/participants',
352353
sortedRemoteVirtualScreenshareParticipants.sort((a, b) => a[1].localeCompare(b[1]));
353354

354355
state.sortedRemoteVirtualScreenshareParticipants = new Map(sortedRemoteVirtualScreenshareParticipants);
356+
357+
logger.debug('Remote screenshare participant joined', id);
355358
}
356359

357360
// Exclude the screenshare participant from the fake participant count to avoid duplicates.
@@ -436,6 +439,8 @@ ReducerRegistry.register<IParticipantsState>('features/base/participants',
436439
if (sortedRemoteVirtualScreenshareParticipants.has(id)) {
437440
sortedRemoteVirtualScreenshareParticipants.delete(id);
438441
state.sortedRemoteVirtualScreenshareParticipants = new Map(sortedRemoteVirtualScreenshareParticipants);
442+
443+
logger.debug('Remote screenshare participant left', id);
439444
}
440445

441446
if (oldParticipant && !oldParticipant.fakeParticipant && !isLocalScreenShare) {

react/features/base/participants/subscriber.ts

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import {
2121
getRemoteScreensharesBasedOnPresence,
2222
getVirtualScreenshareParticipantOwnerId
2323
} from './functions';
24+
import logger from './logger';
2425
import { FakeParticipant } from './types';
2526

2627
StateListenerRegistry.register(
@@ -69,14 +70,19 @@ function _createOrRemoveVirtualParticipants(
6970
const addedScreenshareSourceNames = difference(newScreenshareSourceNames, oldScreenshareSourceNames);
7071

7172
if (removedScreenshareSourceNames.length) {
72-
removedScreenshareSourceNames.forEach(id => dispatch(participantLeft(id, conference, {
73-
fakeParticipant: FakeParticipant.RemoteScreenShare
74-
})));
73+
removedScreenshareSourceNames.forEach(id => {
74+
logger.debug('Dispatching participantLeft for virtual screenshare', id);
75+
dispatch(participantLeft(id, conference, {
76+
fakeParticipant: FakeParticipant.RemoteScreenShare
77+
}));
78+
});
7579
}
7680

7781
if (addedScreenshareSourceNames.length) {
78-
addedScreenshareSourceNames.forEach(id => dispatch(
79-
createVirtualScreenshareParticipant(id, false, conference)));
82+
addedScreenshareSourceNames.forEach(id => {
83+
logger.debug('Creating virtual screenshare participant', id);
84+
dispatch(createVirtualScreenshareParticipant(id, false, conference));
85+
});
8086
}
8187
}
8288

react/features/video-layout/functions.any.ts

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import { getReceiverVideoQualityLevel } from '../video-quality/functions';
1111
import { getMinHeightForQualityLvlMap } from '../video-quality/selector';
1212

1313
import { LAYOUTS } from './constants';
14+
import logger from './logger';
1415

1516
/**
1617
* A selector for retrieving the current automatic pinning setting.
@@ -113,39 +114,49 @@ export function shouldDisplayTileView(state: IReduxState) {
113114
* Private helper to automatically pin the latest screen share stream or unpin
114115
* if there are no more screen share streams.
115116
*
116-
* @param {Array<string>} screenShares - Array containing the list of all the screen sharing endpoints
117+
* @param {Array<string>} previousScreenShares - Array containing the list of all the screen sharing endpoints
117118
* before the update was triggered (including the ones that have been removed from redux because of the update).
119+
* @param {Array<string>} currentScreenShares - Array containing the current list of screen sharing endpoints.
118120
* @param {Store} store - The redux store.
119121
* @returns {void}
120122
*/
121123
export function updateAutoPinnedParticipant(
122-
screenShares: Array<string>, { dispatch, getState }: IStore) {
123-
const state = getState();
124-
const remoteScreenShares = state['features/video-layout'].remoteScreenShares;
124+
previousScreenShares: Array<string>,
125+
currentScreenShares: Array<string>,
126+
{ dispatch, getState }: IStore) {
125127
const pinned = getPinnedParticipant(getState);
126128

127129
// if the pinned participant is shared video or some other fake participant we want to skip auto-pinning
128130
if (pinned?.fakeParticipant && pinned.fakeParticipant !== FakeParticipant.RemoteScreenShare) {
131+
logger.debug('Skipping auto-pin: pinned participant is non-screenshare virtual participant', pinned.id);
132+
129133
return;
130134
}
131135

132136
// Unpin the screen share when the screen sharing participant leaves. Switch to tile view if no other
133137
// participant was pinned before screen share was auto-pinned, pin the previously pinned participant otherwise.
134-
if (!remoteScreenShares?.length) {
138+
if (!currentScreenShares?.length) {
135139
let participantId = null;
136140

137-
if (pinned && !screenShares.find(share => share === pinned.id)) {
141+
if (pinned && !previousScreenShares.find((share: string) => share === pinned.id)) {
138142
participantId = pinned.id;
139143
}
144+
145+
logger.debug('No more screenshares, unpinning or restoring previous pin', participantId);
140146
dispatch(pinParticipant(participantId));
141147

142148
return;
143149
}
144150

145-
const latestScreenShareParticipantId = remoteScreenShares[remoteScreenShares.length - 1];
151+
const latestScreenShareParticipantId = currentScreenShares[currentScreenShares.length - 1];
146152

147153
if (latestScreenShareParticipantId) {
148-
dispatch(pinParticipant(latestScreenShareParticipantId));
154+
const alreadyPinned = pinned?.id === latestScreenShareParticipantId;
155+
156+
if (!alreadyPinned) {
157+
logger.debug(`Auto pinning latest screen share participant: ${latestScreenShareParticipantId}`);
158+
dispatch(pinParticipant(latestScreenShareParticipantId));
159+
}
149160
}
150161
}
151162

react/features/video-layout/middleware.any.ts

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import { isFollowMeActive } from '../follow-me/functions';
1212
import { SET_TILE_VIEW } from './actionTypes';
1313
import { setTileView } from './actions';
1414
import { getAutoPinSetting, updateAutoPinnedParticipant } from './functions';
15-
15+
import logger from './logger';
1616
import './subscriber';
1717

1818
let previousTileViewEnabled: boolean | undefined;
@@ -27,13 +27,21 @@ MiddlewareRegistry.register(store => next => action => {
2727

2828
// we want to extract the leaving participant and check its type before actually the participant being removed.
2929
let shouldUpdateAutoPin = false;
30+
let oldScreenShares: Array<string> = [];
3031

3132
switch (action.type) {
3233
case PARTICIPANT_LEFT: {
3334
if (!getAutoPinSetting() || isFollowMeActive(store)) {
35+
logger.debug('Auto pinning is disabled or Follow Me is active, skipping auto pinning.');
36+
3437
break;
3538
}
3639
shouldUpdateAutoPin = Boolean(getParticipantById(store.getState(), action.participant.id)?.fakeParticipant);
40+
41+
if (shouldUpdateAutoPin) {
42+
// Capture the old screenshare list before the reducer runs
43+
oldScreenShares = store.getState()['features/video-layout'].remoteScreenShares || [];
44+
}
3745
break;
3846
}
3947
}
@@ -75,9 +83,9 @@ MiddlewareRegistry.register(store => next => action => {
7583
}
7684

7785
if (shouldUpdateAutoPin) {
78-
const screenShares = store.getState()['features/video-layout'].remoteScreenShares || [];
86+
const newScreenShares = store.getState()['features/video-layout'].remoteScreenShares || [];
7987

80-
updateAutoPinnedParticipant(screenShares, store);
88+
updateAutoPinnedParticipant(oldScreenShares, newScreenShares, store);
8189
}
8290

8391
return result;

react/features/video-layout/subscriber.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { isFollowMeActive } from '../follow-me/functions';
44

55
import { virtualScreenshareParticipantsUpdated } from './actions';
66
import { getAutoPinSetting, updateAutoPinnedParticipant } from './functions';
7+
import logger from './logger';
78

89
StateListenerRegistry.register(
910
/* selector */ state => state['features/base/participants'].sortedRemoteVirtualScreenshareParticipants,
@@ -22,14 +23,18 @@ StateListenerRegistry.register(
2223
knownSharingParticipantIds.forEach(participantId => {
2324
if (!newScreenSharesOrder.includes(participantId)) {
2425
newScreenSharesOrder.push(participantId);
26+
logger.debug('Adding new screenshare to list', participantId);
2527
}
2628
});
2729

2830
if (!equals(oldScreenSharesOrder, newScreenSharesOrder)) {
31+
logger.debug('Screenshare order changed, dispatching update');
2932
store.dispatch(virtualScreenshareParticipantsUpdated(newScreenSharesOrder));
3033

3134
if (getAutoPinSetting() && !isFollowMeActive(store)) {
32-
updateAutoPinnedParticipant(oldScreenSharesOrder, store);
35+
updateAutoPinnedParticipant(oldScreenSharesOrder, newScreenSharesOrder, store);
36+
} else {
37+
logger.debug('Auto pinning is disabled or Follow Me is active, skipping auto pinning of screenshare.');
3338
}
3439
}
3540
});

0 commit comments

Comments
 (0)