Skip to content

fix(conference) fix processing device list while initial gUM is pending #16318

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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
+71 −72
Open

fix(conference) fix processing device list while initial gUM is pending #16318

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b873c02
fix(conference) fix processing device list while initial gUM is pending
saghul Aug 7, 2025
63ee428
fix(devices) simplify configureInitialDevices
saghul Aug 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 15 additions & 2 deletions conference.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1922,11 +1922,24 @@ export default {

logDevices(ignoredDevices, 'Ignored devices on device list changed:');

APP.store.dispatch(updateDeviceList(filteredDevices));

/**
* Check if we have gUM pending while the device list has changed. This can happen when joining a meeting
* without the pre-join screen since we don't have the device list while the initial tracks are being created.
*/
const { gumPending: audioGumPending } = state['features/base/media'].audio;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well looking into the code the scenario explained above seems impossible to me.

_onDeviceListChanged seems to be attached as a listener by _initDeviceList only when the passed setDeviceListChangeHandler is true and the only time this method is called with parameter true is here https://github.com/jitsi/jitsi-meet/blob/master/conference.js#L591 . At this point the tracks are already created.

Why do we need this change? Am I missing something?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I saw your commit comment and if the problem is that the tracks are not in Redux yet but they are created I think you should at least fix your comment above.

Also if this is the case I feel like there should be better solution!

Maybe call _initDeviceList(true) only once the tracks are actually stored in redux instead of attaching it too early? WDYT? Or we still want to update the device list?

The scenario you are describing if I understand correctly is the following:

  1. initial tracks are created.
  2. this handler is called and we skip it.
  3. we add the tracks to redux.

In this case shouldn't we call the handler once more to go trough the checks and potentially modify the tracks once the local tracks are added into redux.

I see you are using the gumPending flags. I'm not sure if these flags are the best choice because the condition could be true also during a regular mute/unmute or replace track operation and if I understand correctly we are trying to fix something related only to the initial GUM creation, isn't it? Maybe you can use initialGUMPromise and maybe you can chain the checks for the track change related to the devices changed to the initialGUMPromise (speculating here, not sure exactly when do we resolve those).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also if the problem is that the tracks are not in redux yet maybe we should start putting tracks in redux earlier? When I was looking into the code now we add the into the store once we have connected, the conference have started and we have added the tracks to LJM. WDYT?

const { gumPending: videoGumPending } = state['features/base/media'].video;

if (audioGumPending !== IGUMPendingState.NONE || videoGumPending !== IGUMPendingState.NONE) {
logger.warn('Device list changed while gUM is pending, skipping device change handling.');

return Promise.resolve();
}

const localAudio = getLocalJitsiAudioTrack(state);
const localVideo = getLocalJitsiVideoTrack(state);

APP.store.dispatch(updateDeviceList(filteredDevices));

// Firefox users can choose their preferred device in the gUM prompt. In that case
// we should respect that and not attempt to switch to the preferred device from
// our settings.
Expand Down
89 changes: 40 additions & 49 deletions react/features/base/devices/actions.web.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,71 +72,62 @@ export function addPendingDeviceRequest(request: Object) {
*/
export function configureInitialDevices() {
return (dispatch: IStore['dispatch'], getState: IStore['getState']) => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would add a comment inside the function and also here https://github.com/jitsi/jitsi-meet/blob/master/react/features/conference/actions.web.ts#L56 that we expect getAvailableDevices() to be called before hand. Otherwise I can very well forget about this and remove it by mistake :)

Otherwise kudos for finding this and changing it. I myself was doing a similar cleanup a year ago probably and there was a lot of getAvailableDevices calls happening at the beginning of the call (4-5 IIRC).

const deviceLabels = getDevicesFromURL(getState());
let updateSettingsPromise;
const state = getState();
const deviceLabels = getDevicesFromURL(state);

logger.debug(`(TIME) configureInitialDevices: deviceLabels=${
Boolean(deviceLabels)}, performance.now=${window.performance.now()}`);

if (deviceLabels) {
updateSettingsPromise = dispatch(getAvailableDevices()).then(() => {
const state = getState();

if (!areDeviceLabelsInitialized(state)) {
// The labels are not available if the A/V permissions are
// not yet granted.

Object.keys(deviceLabels).forEach(key => {
dispatch(addPendingDeviceRequest({
type: 'devices',
name: 'setDevice',
device: {
kind: key.toLowerCase(),
label: deviceLabels[key as keyof typeof deviceLabels]
},
// eslint-disable-next-line @typescript-eslint/no-empty-function
responseCallback() {}
}));
});

return;
}

const newSettings: any = {};
if (!areDeviceLabelsInitialized(state)) {
// The labels are not available if the A/V permissions are
// not yet granted.

Object.keys(deviceLabels).forEach(key => {
const label = deviceLabels[key as keyof typeof deviceLabels];
dispatch(addPendingDeviceRequest({
type: 'devices',
name: 'setDevice',
device: {
kind: key.toLowerCase(),
label: deviceLabels[key as keyof typeof deviceLabels]
},
// eslint-disable-next-line @typescript-eslint/no-empty-function
responseCallback() {}
}));
});

// @ts-ignore
const deviceId = getDeviceIdByLabel(state, label, key);
return;
}

if (deviceId) {
const settingsTranslationMap = DEVICE_TYPE_TO_SETTINGS_KEYS[
key as keyof typeof DEVICE_TYPE_TO_SETTINGS_KEYS];
const newSettings: any = {};

newSettings[settingsTranslationMap.currentDeviceId] = deviceId;
newSettings[settingsTranslationMap.userSelectedDeviceId] = deviceId;
newSettings[settingsTranslationMap.userSelectedDeviceLabel] = label;
}
});
Object.keys(deviceLabels).forEach(key => {
const label = deviceLabels[key as keyof typeof deviceLabels];

// @ts-ignore
const deviceId = getDeviceIdByLabel(state, label, key);

dispatch(updateSettings(newSettings));
if (deviceId) {
const settingsTranslationMap = DEVICE_TYPE_TO_SETTINGS_KEYS[
key as keyof typeof DEVICE_TYPE_TO_SETTINGS_KEYS];

newSettings[settingsTranslationMap.currentDeviceId] = deviceId;
newSettings[settingsTranslationMap.userSelectedDeviceId] = deviceId;
newSettings[settingsTranslationMap.userSelectedDeviceLabel] = label;
}
});
} else {
updateSettingsPromise = Promise.resolve();

dispatch(updateSettings(newSettings));
}

return updateSettingsPromise
.then(() => {
const userSelectedAudioOutputDeviceId = getUserSelectedOutputDeviceId(getState());
const userSelectedAudioOutputDeviceId = getUserSelectedOutputDeviceId(getState());

logger.debug(`(TIME) configureInitialDevices -> setAudioOutputDeviceId: performance.now=${
window.performance.now()}`);
logger.debug(`(TIME) configureInitialDevices -> setAudioOutputDeviceId: performance.now=${
window.performance.now()}`);

return setAudioOutputDeviceId(userSelectedAudioOutputDeviceId, dispatch)
.catch(ex => logger.warn(`Failed to set audio output device.
Default audio output device will be used instead ${ex}`));
});
return setAudioOutputDeviceId(userSelectedAudioOutputDeviceId, dispatch)
.catch(ex => logger.warn(`Failed to set audio output device.
Default audio output device will be used instead ${ex}`));
};
}

Expand Down
37 changes: 16 additions & 21 deletions react/features/base/devices/middleware.web.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,18 +1,16 @@
import { AnyAction } from 'redux';

import { IStore } from '../../app/types';
import { processExternalDeviceRequest } from '../../device-selection/functions';
import { processExternalDeviceRequest } from '../../device-selection/functions.web';
import { showNotification, showWarningNotification } from '../../notifications/actions';
import { NOTIFICATION_TIMEOUT_TYPE } from '../../notifications/constants';
import { replaceAudioTrackById, replaceVideoTrackById, setDeviceStatusWarning } from '../../prejoin/actions';
import { isPrejoinPageVisible } from '../../prejoin/functions';
import { replaceAudioTrackById, replaceVideoTrackById, setDeviceStatusWarning } from '../../prejoin/actions.web';
import { isPrejoinPageVisible } from '../../prejoin/functions.web';
import { APP_WILL_MOUNT, APP_WILL_UNMOUNT } from '../app/actionTypes';
import { isMobileBrowser } from '../environment/utils';
import JitsiMeetJS, { JitsiMediaDevicesEvents, JitsiTrackErrors } from '../lib-jitsi-meet';
import { MEDIA_TYPE } from '../media/constants';
import MiddlewareRegistry from '../redux/MiddlewareRegistry';
import { updateSettings } from '../settings/actions';
import { getLocalTrack } from '../tracks/functions';
import { getLocalTrack } from '../tracks/functions.web';

import {
CHECK_AND_NOTIFY_FOR_NEW_DEVICE,
Expand All @@ -27,13 +25,13 @@ import {
removePendingDeviceRequests,
setAudioInputDevice,
setVideoInputDevice
} from './actions';
} from './actions.web';
import {
areDeviceLabelsInitialized,
formatDeviceLabel,
logDevices,
setAudioOutputDeviceId
} from './functions';
} from './functions.web';
import logger from './logger';

const JITSI_TRACK_ERROR_TO_MESSAGE_KEY_MAP = {
Expand Down Expand Up @@ -176,12 +174,16 @@ MiddlewareRegistry.register(store => next => action => {
}
break;
}
case UPDATE_DEVICE_LIST:
case UPDATE_DEVICE_LIST: {
const result = next(action);

logDevices(action.devices, 'Device list updated');
if (areDeviceLabelsInitialized(store.getState())) {
return _processPendingRequests(store, next, action);
_processPendingRequests(store);
}
break;

return result;
}
case CHECK_AND_NOTIFY_FOR_NEW_DEVICE:
_checkAndNotifyForNewDevice(store, action.newDevices, action.oldDevices);
break;
Expand All @@ -196,20 +198,15 @@ MiddlewareRegistry.register(store => next => action => {
*
* @param {Store} store - The redux store in which the specified {@code action}
* is being dispatched.
* @param {Dispatch} next - The redux {@code dispatch} function to dispatch the
* specified {@code action} to the specified {@code store}.
* @param {Action} action - The redux action {@code CONFERENCE_JOINED} which is
* being dispatched in the specified {@code store}.
* @private
* @returns {Object} The value returned by {@code next(action)}.
* @returns {void}
*/
function _processPendingRequests({ dispatch, getState }: IStore, next: Function, action: AnyAction) {
const result = next(action);
function _processPendingRequests({ dispatch, getState }: IStore) {
const state = getState();
const { pendingRequests } = state['features/base/devices'];

if (!pendingRequests || pendingRequests.length === 0) {
return result;
return;
}

pendingRequests.forEach((request: any) => {
Expand All @@ -220,8 +217,6 @@ function _processPendingRequests({ dispatch, getState }: IStore, next: Function,
request.responseCallback);
});
dispatch(removePendingDeviceRequests());

return result;
}

/**
Expand Down
Loading