Skip to content
Open
Changes from 1 commit
Commits
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
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
Loading