Skip to content
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.

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

fix(MediaSettings): sync preferences with devices direct selection #12189

Merged
merged 7 commits into from
Jun 10, 2024
43 changes: 31 additions & 12 deletions src/components/MediaSettings/MediaSettings.vue
Original file line number Diff line number Diff line change
Expand Up @@ -92,12 +92,12 @@
:devices="devices"
:device-id="audioInputId"
@refresh="updateDevices"
@update:deviceId="audioInputId = $event" />
@update:deviceId="handleAudioInputIdChange" />
<MediaDevicesSelector kind="videoinput"
:devices="devices"
:device-id="videoInputId"
@refresh="updateDevices"
@update:deviceId="videoInputId = $event" />
@update:deviceId="handleVideoInputIdChange" />
<MediaDevicesSpeakerTest />
</div>

Expand Down Expand Up @@ -312,7 +312,6 @@ export default {
videoOn: undefined,
silentCall: false,
updatedBackground: undefined,
deviceIdChanged: false,
DorraJaouad marked this conversation as resolved.
Show resolved Hide resolved
audioDeviceStateChanged: false,
videoDeviceStateChanged: false,
isRecordingFromStart: false,
Expand Down Expand Up @@ -422,7 +421,7 @@ export default {
},

showUpdateChangesButton() {
return this.updatedBackground || this.deviceIdChanged || this.audioDeviceStateChanged
return this.updatedBackground || this.audioDeviceStateChanged
|| this.videoDeviceStateChanged
},
},
Expand Down Expand Up @@ -452,14 +451,12 @@ export default {
},

audioInputId(audioInputId) {
this.deviceIdChanged = true
if (this.showDeviceSelection && audioInputId && !this.audioOn) {
this.toggleAudio()
}
},

videoInputId(videoInputId) {
this.deviceIdChanged = true
if (this.showDeviceSelection && videoInputId && !this.videoOn) {
this.toggleVideo()
}
Expand All @@ -474,9 +471,10 @@ export default {
subscribe('talk:media-settings:show', this.showModal)
subscribe('talk:media-settings:hide', this.closeModalAndApplySettings)

const devicesPreferred = BrowserStorage.getItem('devicesPreferred')
if (!devicesPreferred) {
this.tabContent = 'devices'
// FIXME: this is a workaround to remove the old key from the browser storage
// To be removed in the future
if (BrowserStorage.getItem('devicesPreferred')) {
BrowserStorage.removeItem('devicesPreferred')
}
},

Expand All @@ -491,16 +489,22 @@ export default {
if (page === 'video-verification') {
this.isPublicShareAuthSidebar = true
}

if (!BrowserStorage.getItem('audioInputDevicePreferred') || !BrowserStorage.getItem('videoInputDevicePreferred')) {
this.tabContent = 'devices'
}
},

closeModal() {
this.modal = false
this.updatedBackground = undefined
this.deviceIdChanged = false
DorraJaouad marked this conversation as resolved.
Show resolved Hide resolved
this.audioDeviceStateChanged = false
this.videoDeviceStateChanged = false
this.isPublicShareAuthSidebar = false
this.isRecordingFromStart = false
// Update devices preferences
this.updatePreferences('audioinput')
this.updatePreferences('videoinput')
},

toggleAudio() {
Expand Down Expand Up @@ -545,7 +549,6 @@ export default {
emit('local-video-control-button:toggle-video')
}

this.updatePreferences()
Antreesy marked this conversation as resolved.
Show resolved Hide resolved
this.closeModal()
},

Expand Down Expand Up @@ -656,14 +659,25 @@ export default {

setRecordingConsentGiven(value) {
this.$emit('update:recording-consent-given', value)
}
},

handleAudioInputIdChange(audioInputId) {
this.audioInputId = audioInputId
this.updatePreferences('audioinput')
},

handleVideoInputIdChange(videoInputId) {
this.videoInputId = videoInputId
this.updatePreferences('videoinput')
},
},
}
</script>

<style lang="scss" scoped>
.media-settings {
padding: calc(var(--default-grid-baseline) * 5);
padding-bottom: 0;

&__title {
text-align: center;
Expand Down Expand Up @@ -713,9 +727,14 @@ export default {

&__call-buttons {
display: flex;
z-index: 1;
align-items: center;
justify-content: center;
gap: var(--default-grid-baseline);
position: sticky;
bottom: 0;
background-color: var(--color-main-background);
padding: 10px 0 20px;
}
}

Expand Down
21 changes: 18 additions & 3 deletions src/components/SettingsDialog/MediaDevicesPreview.vue
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
:devices="devices"
:device-id="audioInputId"
@refresh="updateDevices"
@update:deviceId="audioInputId = $event" />
@update:deviceId="handleAudioInputIdChange" />
<div class="preview preview-audio">
<div v-if="!audioPreviewAvailable"
class="preview-not-available">
Expand Down Expand Up @@ -38,7 +38,7 @@
:devices="devices"
:device-id="videoInputId"
@refresh="updateDevices"
@update:deviceId="videoInputId = $event" />
@update:deviceId="handleVideoInputIdChange" />
<div class="preview preview-video">
<div v-if="!videoPreviewAvailable"
class="preview-not-available">
Expand Down Expand Up @@ -95,6 +95,7 @@ export default {
const {
devices,
updateDevices,
updatePreferences,
currentVolume,
currentThreshold,
audioPreviewAvailable,
Expand All @@ -111,6 +112,7 @@ export default {
video,
devices,
updateDevices,
updatePreferences,
currentVolume,
currentThreshold,
audioPreviewAvailable,
Expand Down Expand Up @@ -176,7 +178,20 @@ export default {

return t('spreed', 'Error while accessing camera')
},
}
},

methods: {
handleAudioInputIdChange(audioInputId) {
this.audioInputId = audioInputId
this.updatePreferences('audioinput')
},

handleVideoInputIdChange(videoInputId) {
this.videoInputId = videoInputId
this.updatePreferences('videoinput')
},

},
}
</script>

Expand Down
5 changes: 3 additions & 2 deletions src/composables/useDevices.js
Original file line number Diff line number Diff line change
Expand Up @@ -155,10 +155,11 @@ export function useDevices(video, initializeOnMounted) {

/**
* Update preference counters for devices (audio and video)
* @param {string} kind the kind of the input stream to update ('audioinput' or 'videoinput')
* @public
*/
function updatePreferences() {
mediaDevicesManager.updatePreferences()
function updatePreferences(kind) {
mediaDevicesManager.updatePreferences(kind)
}

/**
Expand Down
70 changes: 34 additions & 36 deletions src/services/__tests__/mediaDevicePreferences.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {
getFirstAvailableMediaDevice,
listMediaDevices,
populateMediaDevicesPreferences,
updateMediaDevicesPreferences,
promoteMediaDevice,
} from '../mediaDevicePreferences.ts'

describe('mediaDevicePreferences', () => {
Expand Down Expand Up @@ -122,73 +122,71 @@ describe('mediaDevicePreferences', () => {
})
})

describe('updateMediaDevicesPreferences', () => {
describe('promoteMediaDevice', () => {
it('returns null if preference lists were not updated (no id or default id provided)', () => {
const ids = [null, undefined, 'default']

const getOutput = (id) => {
const attributes = { devices: allDevices, audioInputId: id, videoInputId: id }
return updateMediaDevicesPreferences(attributes, audioInputPreferenceList, videoInputPreferenceList)
return promoteMediaDevice({
kind: 'audioinput',
devices: allDevices,
inputList: audioInputPreferenceList,
inputId: id
})
}

// Assert
ids.forEach(id => {
expect(getOutput(id)).toMatchObject({ newAudioInputList: null, newVideoInputList: null })
expect(getOutput(id)).toEqual(null)
})
})

it('returns updated preference lists (device A id provided)', () => {
const attributes = { devices: allDevices, audioInputId: audioInputDeviceA.deviceId, videoInputId: videoInputDeviceA.deviceId }
const output = updateMediaDevicesPreferences(attributes, audioInputPreferenceList, videoInputPreferenceList)
const output = promoteMediaDevice({
kind: 'audioinput',
devices: allDevices,
inputList: audioInputPreferenceList,
inputId: audioInputDeviceA.deviceId
})

// Assert: should put device A on top of default device
expect(output).toMatchObject({
newAudioInputList: [audioInputDeviceA, audioInputDeviceDefault, audioInputDeviceB],
newVideoInputList: [videoInputDeviceA, videoInputDeviceDefault, videoInputDeviceB],
})
expect(output).toEqual([audioInputDeviceA, audioInputDeviceDefault, audioInputDeviceB])
})

it('returns null if preference lists were not updated (device A id provided but not available)', () => {
const attributes = {
const output = promoteMediaDevice({
kind: 'audioinput',
devices: allDevices.filter(device => !['da1234567890', 'da4567890123'].includes(device.deviceId)),
audioInputId: audioInputDeviceA.deviceId,
videoInputId: videoInputDeviceA.deviceId,
}
const output = updateMediaDevicesPreferences(attributes, audioInputPreferenceList, videoInputPreferenceList)
inputList: audioInputPreferenceList,
inputId: audioInputDeviceA.deviceId
})

// Assert
expect(output).toMatchObject({ newAudioInputList: null, newVideoInputList: null })
expect(output).toEqual(null)
})

it('returns null if preference lists were not updated (all devices are not available)', () => {
const attributes = {
const output = promoteMediaDevice({
kind: 'audioinput',
devices: allDevices.filter(device => !['audioinput', 'videoinput'].includes(device.kind)),
audioInputId: audioInputDeviceA.deviceId,
videoInputId: videoInputDeviceA.deviceId,
}
const output = updateMediaDevicesPreferences(attributes, audioInputPreferenceList, videoInputPreferenceList)
inputList: audioInputPreferenceList,
inputId: audioInputDeviceA.deviceId
})

// Assert
expect(output).toMatchObject({ newAudioInputList: null, newVideoInputList: null })
expect(output).toEqual(null)
})

it('returns updated preference lists (device B id provided, but not registered, default device and device A not available)', () => {
const attributes = {
const output = promoteMediaDevice({
kind: 'audioinput',
devices: allDevices.filter(device => !['default', 'da1234567890', 'da4567890123'].includes(device.deviceId)),
audioInputId: audioInputDeviceB.deviceId,
videoInputId: videoInputDeviceB.deviceId,
}
const output = updateMediaDevicesPreferences(
attributes,
[audioInputDeviceDefault, audioInputDeviceA],
[videoInputDeviceDefault, videoInputDeviceA],
)
inputList: [audioInputDeviceDefault, audioInputDeviceA],
inputId: audioInputDeviceB.deviceId
})

// Assert: should put device C on top of device B, but not the device A
expect(output).toMatchObject({
newAudioInputList: [audioInputDeviceDefault, audioInputDeviceA, audioInputDeviceB],
newVideoInputList: [videoInputDeviceDefault, videoInputDeviceA, videoInputDeviceB],
})
expect(output).toEqual([audioInputDeviceDefault, audioInputDeviceA, audioInputDeviceB])
})
})
})
38 changes: 14 additions & 24 deletions src/services/mediaDevicePreferences.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,13 @@ enum DeviceKind {
AudioOutput = 'audiooutput',
}

type PromotePayload = {
kind: DeviceKind,
devices: MediaDeviceInfo[],
inputList: MediaDeviceInfo[],
inputId: InputId
}

/**
* List all registered devices in order of their preferences
* Show whether device is currently unplugged or selected, if information is available
Expand Down Expand Up @@ -90,13 +97,14 @@ function registerNewMediaDevice(device: MediaDeviceInfo, devicesList: MediaDevic
*
* Returns changed preference lists for audio / video devices (null, if it hasn't been changed)
*
* @param kind kind of device
* @param devices list of available devices
* @param inputList list of registered audio/video devices in order of preference
* @param inputId id of currently selected input
* @param data the wrapping object
* @param data.kind kind of device
* @param data.devices list of available devices
* @param data.inputList list of registered audio/video devices in order of preference
* @param data.inputId id of currently selected input
* @return {InputListUpdated} updated devices list (null, if it has not been changed)
*/
function promoteMediaDevice(kind: DeviceKind, devices: MediaDeviceInfo[], inputList: MediaDeviceInfo[], inputId: InputId): InputListUpdated {
function promoteMediaDevice({ kind, devices, inputList, inputId } : PromotePayload) : InputListUpdated {
if (!inputId) {
return null
}
Expand Down Expand Up @@ -164,27 +172,9 @@ function populateMediaDevicesPreferences(devices: MediaDeviceInfo[], audioInputL
}
}

/**
* Update devices preferences. Assuming that preferred devices were selected, should be called after applying the selection:
* so either with joining the call or changing device during the call
*
* Returns changed preference lists for audio / video devices (null, if it hasn't been changed)
*
* @param attributes MediaDeviceManager attributes
* @param audioInputList list of registered audio devices in order of preference
* @param videoInputList list of registered video devices in order of preference
* @return {InputLists} object with updated devices lists (null, if they have not been changed)
*/
function updateMediaDevicesPreferences(attributes: Attributes, audioInputList: MediaDeviceInfo[], videoInputList: MediaDeviceInfo[]): InputLists {
return {
newAudioInputList: promoteMediaDevice(DeviceKind.AudioInput, attributes.devices, audioInputList, attributes.audioInputId),
newVideoInputList: promoteMediaDevice(DeviceKind.VideoInput, attributes.devices, videoInputList, attributes.videoInputId),
}
}

export {
getFirstAvailableMediaDevice,
listMediaDevices,
populateMediaDevicesPreferences,
updateMediaDevicesPreferences,
promoteMediaDevice,
}
Loading
Loading