Skip to content

Commit

Permalink
fix: cover some device selection edge cases (#1604)
Browse files Browse the repository at this point in the history
This PR fixes some edge cases around device selection:

### Selecting the `default` device

Chrome allows requesting user media with device id `default`. For the
purposes of the SDK, we treat this device as if it didn't existed, and
filter it out of device lists. However, it was still possible to select
it manually.

Now device id `default` is filtered out of media requests, so even if it
is selected manually, it is replaced with an actual device id when
device is unmuted.

### Selecting non-existing device id

Previously, selecting non-existing device (e.g.
`call.camera.select('nonsense')`) resulted in the default device being
selected. That could be a nasty surprise for a user.

Now, either `select` method will throw (if device is already unmuted) or
enabling the device will throw later.

This takes us to the final change:

### Error handler for device selectors

`onError` callback added to audio and video device selectors, since
`select` method can now throw in a rare case that the device list is
stale.
  • Loading branch information
myandrienko authored Nov 27, 2024
1 parent 36181d5 commit a8fc0ea
Show file tree
Hide file tree
Showing 6 changed files with 76 additions and 54 deletions.
1 change: 1 addition & 0 deletions packages/client/src/devices/InputMediaDeviceManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,7 @@ export abstract class InputMediaDeviceManager<
await this.applySettingsToStream();
} catch (error) {
this.state.setDevice(prevDeviceId);
await this.applySettingsToStream();
throw error;
}
}
Expand Down
38 changes: 16 additions & 22 deletions packages/client/src/devices/devices.ts
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ export const getAudioStream = async (
const constraints: MediaStreamConstraints = {
audio: {
...audioDeviceConstraints.audio,
...trackConstraints,
...normalizeContraints(trackConstraints),
},
};

Expand All @@ -195,16 +195,6 @@ export const getAudioStream = async (
});
return await getStream(constraints);
} catch (error) {
if (error instanceof OverconstrainedError && trackConstraints?.deviceId) {
const { deviceId, ...relaxedContraints } = trackConstraints;
getLogger(['devices'])(
'warn',
'Failed to get audio stream, will try again with relaxed contraints',
{ error, constraints, relaxedContraints },
);
return getAudioStream(relaxedContraints);
}

getLogger(['devices'])('error', 'Failed to get audio stream', {
error,
constraints,
Expand All @@ -227,7 +217,7 @@ export const getVideoStream = async (
const constraints: MediaStreamConstraints = {
video: {
...videoDeviceConstraints.video,
...trackConstraints,
...normalizeContraints(trackConstraints),
},
};
try {
Expand All @@ -237,16 +227,6 @@ export const getVideoStream = async (
});
return await getStream(constraints);
} catch (error) {
if (error instanceof OverconstrainedError && trackConstraints?.deviceId) {
const { deviceId, ...relaxedContraints } = trackConstraints;
getLogger(['devices'])(
'warn',
'Failed to get video stream, will try again with relaxed contraints',
{ error, constraints, relaxedContraints },
);
return getVideoStream(relaxedContraints);
}

getLogger(['devices'])('error', 'Failed to get video stream', {
error,
constraints,
Expand All @@ -255,6 +235,20 @@ export const getVideoStream = async (
}
};

function normalizeContraints(constraints: MediaTrackConstraints | undefined) {
if (
constraints?.deviceId === 'default' ||
(typeof constraints?.deviceId === 'object' &&
'exact' in constraints.deviceId &&
constraints.deviceId.exact === 'default')
) {
const { deviceId, ...contraintsWithoutDeviceId } = constraints;
return contraintsWithoutDeviceId;
}

return constraints;
}

/**
* Prompts the user for a permission to share a screen.
* If the user grants the permission, a screen sharing stream is returned. Throws otherwise.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,28 +1,35 @@
import { useCallStateHooks } from '@stream-io/video-react-bindings';
import { DeviceSelector } from './DeviceSelector';
import {
createCallControlHandler,
PropsWithErrorHandler,
} from '../../utilities/callControlHandler';

export type DeviceSelectorAudioInputProps = {
export type DeviceSelectorAudioInputProps = PropsWithErrorHandler<{
title?: string;
visualType?: 'list' | 'dropdown';
};
}>;

export const DeviceSelectorAudioInput = ({
title,
visualType,
}: DeviceSelectorAudioInputProps) => {
export const DeviceSelectorAudioInput = (
props: DeviceSelectorAudioInputProps,
) => {
const { useMicrophoneState } = useCallStateHooks();
const { microphone, selectedDevice, devices } = useMicrophoneState();
const handleChange = createCallControlHandler(
props,
async (deviceId: string) => {
await microphone.select(deviceId);
},
);

return (
<DeviceSelector
devices={devices || []}
selectedDeviceId={selectedDevice}
type="audioinput"
onChange={async (deviceId) => {
await microphone.select(deviceId);
}}
title={title}
visualType={visualType}
onChange={handleChange}
title={props.title}
visualType={props.visualType}
icon="mic"
/>
);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,28 +1,33 @@
import {
createCallControlHandler,
PropsWithErrorHandler,
} from '../../utilities/callControlHandler';
import { DeviceSelector } from './DeviceSelector';
import { useCallStateHooks } from '@stream-io/video-react-bindings';

export type DeviceSelectorVideoProps = {
export type DeviceSelectorVideoProps = PropsWithErrorHandler<{
title?: string;
visualType?: 'list' | 'dropdown';
};
}>;

export const DeviceSelectorVideo = ({
title,
visualType,
}: DeviceSelectorVideoProps) => {
export const DeviceSelectorVideo = (props: DeviceSelectorVideoProps) => {
const { useCameraState } = useCallStateHooks();
const { camera, devices, selectedDevice } = useCameraState();
const handleChange = createCallControlHandler(
props,
async (deviceId: string) => {
await camera.select(deviceId);
},
);

return (
<DeviceSelector
devices={devices || []}
type="videoinput"
selectedDeviceId={selectedDevice}
onChange={async (deviceId) => {
await camera.select(deviceId);
}}
title={title}
visualType={visualType}
onChange={handleChange}
title={props.title}
visualType={props.visualType}
icon="camera"
/>
);
Expand Down
27 changes: 21 additions & 6 deletions packages/react-sdk/src/hooks/usePersistedDevicePreferences.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,14 @@ const usePersistDevicePreferences = (
*
* @param key the key to use for local storage.
*/
const useApplyDevicePreferences = (key: string, onApplied: () => void) => {
const useApplyDevicePreferences = (
key: string,
onWillApply: () => void,
onApplied: () => void,
) => {
const call = useCall();
const onWillApplyRef = useRef(onWillApply);
onWillApplyRef.current = onWillApply;
const onAppliedRef = useRef(onApplied);
onAppliedRef.current = onApplied;
useEffect(() => {
Expand Down Expand Up @@ -115,17 +121,22 @@ const useApplyDevicePreferences = (key: string, onApplied: () => void) => {
console.warn('Failed to load device preferences', err);
}
if (preferences) {
await initMic(preferences.mic);
await initCamera(preferences.camera);
await initMic(preferences.mic).catch((err) => {
console.warn('Failed to apply microphone preferences', err);
});
await initCamera(preferences.camera).catch((err) => {
console.warn('Failed to apply camera preferences', err);
});
initSpeaker(preferences.speaker);
}
};

onWillApplyRef.current();
apply()
.then(() => onAppliedRef.current())
.catch((err) => {
console.warn('Failed to apply device preferences', err);
});
})
.then(() => onAppliedRef.current());

return () => {
cancel = true;
Expand All @@ -142,7 +153,11 @@ export const usePersistedDevicePreferences = (
key: string = '@stream-io/device-preferences',
) => {
const shouldPersistRef = useRef(false);
useApplyDevicePreferences(key, () => (shouldPersistRef.current = true));
useApplyDevicePreferences(
key,
() => (shouldPersistRef.current = false),
() => (shouldPersistRef.current = true),
);
usePersistDevicePreferences(key, shouldPersistRef);
};

Expand Down
8 changes: 4 additions & 4 deletions packages/react-sdk/src/utilities/callControlHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,15 @@ export type PropsWithErrorHandler<T = unknown> = T & {
* @param props component props, including the onError callback
* @param handler event handler to wrap
*/
export const createCallControlHandler = (
export const createCallControlHandler = <P extends unknown[]>(
props: PropsWithErrorHandler,
handler: () => Promise<void>,
handler: (...args: P) => Promise<void>,
): (() => Promise<void>) => {
const logger = getLogger(['react-sdk']);

return async () => {
return async (...args: P) => {
try {
await handler();
await handler(...args);
} catch (error) {
if (props.onError) {
props.onError(error);
Expand Down

0 comments on commit a8fc0ea

Please sign in to comment.