From a3b972f6e7a1a069ce3a30d969454164024fc6f1 Mon Sep 17 00:00:00 2001 From: Miriam Zimmerman Date: Mon, 7 Oct 2024 16:32:31 -0400 Subject: [PATCH] Fix device selection persistence bug --- ts/calling/findBestMatchingDevice.ts | 23 ++- ts/services/calling.ts | 23 ++- .../calling/findBestMatchingDevice_test.ts | 149 ++++++++++++------ ts/windows/settings/preload.ts | 29 +++- 4 files changed, 157 insertions(+), 67 deletions(-) diff --git a/ts/calling/findBestMatchingDevice.ts b/ts/calling/findBestMatchingDevice.ts index 57043a1762f0..0da510bd4432 100644 --- a/ts/calling/findBestMatchingDevice.ts +++ b/ts/calling/findBestMatchingDevice.ts @@ -3,20 +3,27 @@ import type { AudioDevice } from '@signalapp/ringrtc'; -export function findBestMatchingAudioDeviceIndex({ - available, - preferred, -}: Readonly<{ - available: ReadonlyArray; - preferred: undefined | AudioDevice; -}>): undefined | number { +export function findBestMatchingAudioDeviceIndex( + { + available, + preferred, + }: Readonly<{ + available: ReadonlyArray; + preferred: undefined | AudioDevice; + }>, + isWindows: boolean +): undefined | number { if (!preferred) { return available.length > 0 ? 0 : undefined; } + // On Linux and Mac, the default device is at index 0. + // On Windows, there are two default devices, as presented by RingRTC: + // * The default communications device (for voice calls, at index 0) + // * the default device (for, e.g., media, at index 1) if ( preferred.index === 0 || - (preferred.index === 1 && available.length >= 2) + (isWindows && preferred.index === 1 && available.length >= 2) ) { return preferred.index; } diff --git a/ts/services/calling.ts b/ts/services/calling.ts index f9f54893e6b8..755ab65911fa 100644 --- a/ts/services/calling.ts +++ b/ts/services/calling.ts @@ -156,6 +156,7 @@ import { sendCallLinkUpdateSync } from '../util/sendCallLinkUpdateSync'; import { createIdenticon } from '../util/createIdenticon'; import { getColorForCallLink } from '../util/getColorForCallLink'; import { getUseRingrtcAdm } from '../util/ringrtc/ringrtcAdm'; +import OS from '../util/os/osMain'; const { wasGroupCallRingPreviouslyCanceled } = DataReader; const { @@ -2330,20 +2331,26 @@ export class CallingClass { await this.getAvailableIODevices(); const preferredMicrophone = window.Events.getPreferredAudioInputDevice(); - const selectedMicIndex = findBestMatchingAudioDeviceIndex({ - available: availableMicrophones, - preferred: preferredMicrophone, - }); + const selectedMicIndex = findBestMatchingAudioDeviceIndex( + { + available: availableMicrophones, + preferred: preferredMicrophone, + }, + OS.isWindows() + ); const selectedMicrophone = selectedMicIndex !== undefined ? availableMicrophones[selectedMicIndex] : undefined; const preferredSpeaker = window.Events.getPreferredAudioOutputDevice(); - const selectedSpeakerIndex = findBestMatchingAudioDeviceIndex({ - available: availableSpeakers, - preferred: preferredSpeaker, - }); + const selectedSpeakerIndex = findBestMatchingAudioDeviceIndex( + { + available: availableSpeakers, + preferred: preferredSpeaker, + }, + OS.isWindows() + ); const selectedSpeaker = selectedSpeakerIndex !== undefined ? availableSpeakers[selectedSpeakerIndex] diff --git a/ts/test-both/calling/findBestMatchingDevice_test.ts b/ts/test-both/calling/findBestMatchingDevice_test.ts index 7ed4f845e933..a2fed2f98996 100644 --- a/ts/test-both/calling/findBestMatchingDevice_test.ts +++ b/ts/test-both/calling/findBestMatchingDevice_test.ts @@ -14,10 +14,13 @@ describe('"find best matching device" helpers', () => { { name: 'Big Microphone', index: 1, uniqueId: 'abc123' }, ].forEach(preferred => { assert.isUndefined( - findBestMatchingAudioDeviceIndex({ - available: [], - preferred, - }) + findBestMatchingAudioDeviceIndex( + { + available: [], + preferred, + }, + false + ) ); }); }); @@ -26,14 +29,17 @@ describe('"find best matching device" helpers', () => { const itReturnsTheFirstAvailableDeviceIfNoneIsPreferred = () => { it('returns the first available device if none is preferred', () => { assert.strictEqual( - findBestMatchingAudioDeviceIndex({ - available: [ - { name: 'A', index: 123, uniqueId: 'device-A' }, - { name: 'B', index: 456, uniqueId: 'device-B' }, - { name: 'C', index: 789, uniqueId: 'device-C' }, - ], - preferred: undefined, - }), + findBestMatchingAudioDeviceIndex( + { + available: [ + { name: 'A', index: 123, uniqueId: 'device-A' }, + { name: 'B', index: 456, uniqueId: 'device-B' }, + { name: 'C', index: 789, uniqueId: 'device-C' }, + ], + preferred: undefined, + }, + false + ), 0 ); }); @@ -41,28 +47,34 @@ describe('"find best matching device" helpers', () => { const testUniqueIdMatch = () => { assert.strictEqual( - findBestMatchingAudioDeviceIndex({ - available: [ - { name: 'A', index: 123, uniqueId: 'device-A' }, - { name: 'B', index: 456, uniqueId: 'device-B' }, - { name: 'C', index: 789, uniqueId: 'device-C' }, - ], - preferred: { name: 'Ignored', index: 99, uniqueId: 'device-C' }, - }), + findBestMatchingAudioDeviceIndex( + { + available: [ + { name: 'A', index: 123, uniqueId: 'device-A' }, + { name: 'B', index: 456, uniqueId: 'device-B' }, + { name: 'C', index: 789, uniqueId: 'device-C' }, + ], + preferred: { name: 'Ignored', index: 99, uniqueId: 'device-C' }, + }, + false + ), 2 ); }; const testNameMatch = () => { assert.strictEqual( - findBestMatchingAudioDeviceIndex({ - available: [ - { name: 'A', index: 123, uniqueId: 'device-A' }, - { name: 'B', index: 456, uniqueId: 'device-B' }, - { name: 'C', index: 789, uniqueId: 'device-C' }, - ], - preferred: { name: 'C', index: 99, uniqueId: 'ignored' }, - }), + findBestMatchingAudioDeviceIndex( + { + available: [ + { name: 'A', index: 123, uniqueId: 'device-A' }, + { name: 'B', index: 456, uniqueId: 'device-B' }, + { name: 'C', index: 789, uniqueId: 'device-C' }, + ], + preferred: { name: 'C', index: 99, uniqueId: 'ignored' }, + }, + false + ), 2 ); }; @@ -71,14 +83,17 @@ describe('"find best matching device" helpers', () => { () => { it('returns the first available device if the preferred device is not found', () => { assert.strictEqual( - findBestMatchingAudioDeviceIndex({ - available: [ - { name: 'A', index: 123, uniqueId: 'device-A' }, - { name: 'B', index: 456, uniqueId: 'device-B' }, - { name: 'C', index: 789, uniqueId: 'device-C' }, - ], - preferred: { name: 'X', index: 123, uniqueId: 'Y' }, - }), + findBestMatchingAudioDeviceIndex( + { + available: [ + { name: 'A', index: 123, uniqueId: 'device-A' }, + { name: 'B', index: 456, uniqueId: 'device-B' }, + { name: 'C', index: 789, uniqueId: 'device-C' }, + ], + preferred: { name: 'X', index: 123, uniqueId: 'Y' }, + }, + false + ), 0 ); }); @@ -89,28 +104,64 @@ describe('"find best matching device" helpers', () => { itReturnsTheFirstAvailableDeviceIfNoneIsPreferred(); - [0, 1].forEach(index => { - it(`returns ${index} if that was the previous preferred index (and a device is available)`, () => { - assert.strictEqual( - findBestMatchingAudioDeviceIndex({ + it('returns 0 if that was the previous preferred index (and a device is available)', () => { + assert.strictEqual( + findBestMatchingAudioDeviceIndex( + { available: [ { name: 'A', index: 123, uniqueId: 'device-A' }, { name: 'B', index: 456, uniqueId: 'device-B' }, { name: 'C', index: 789, uniqueId: 'device-C' }, ], - preferred: { name: 'C', index, uniqueId: 'device-C' }, - }), - index - ); - }); + preferred: { name: 'C', index: 0, uniqueId: 'device-C' }, + }, + false + ), + 0 + ); + }); + it('(windows) returns 1 if that was the previous preferred index (and a device is available)', () => { + assert.strictEqual( + findBestMatchingAudioDeviceIndex( + { + available: [ + { name: 'A', index: 123, uniqueId: 'device-A' }, + { name: 'B', index: 456, uniqueId: 'device-B' }, + { name: 'C', index: 789, uniqueId: 'device-C' }, + ], + preferred: { name: 'C', index: 1, uniqueId: 'device-C' }, + }, + true + ), + 1 + ); + }); + it('(non-windows) returns 2 if the preferred device was at index 1 and is now at index 2', () => { + assert.strictEqual( + findBestMatchingAudioDeviceIndex( + { + available: [ + { name: 'A', index: 123, uniqueId: 'device-A' }, + { name: 'B', index: 456, uniqueId: 'device-B' }, + { name: 'C', index: 789, uniqueId: 'device-C' }, + ], + preferred: { name: 'C', index: 1, uniqueId: 'device-C' }, + }, + false + ), + 2 + ); }); it("returns 0 if the previous preferred index was 1 but there's only 1 audio device", () => { assert.strictEqual( - findBestMatchingAudioDeviceIndex({ - available: [{ name: 'A', index: 123, uniqueId: 'device-A' }], - preferred: { name: 'C', index: 1, uniqueId: 'device-C' }, - }), + findBestMatchingAudioDeviceIndex( + { + available: [{ name: 'A', index: 123, uniqueId: 'device-A' }], + preferred: { name: 'C', index: 1, uniqueId: 'device-C' }, + }, + false + ), 0 ); }); diff --git a/ts/windows/settings/preload.ts b/ts/windows/settings/preload.ts index 35d9a678f6f1..2d922765d190 100644 --- a/ts/windows/settings/preload.ts +++ b/ts/windows/settings/preload.ts @@ -15,6 +15,7 @@ import { import { awaitObject } from '../../util/awaitObject'; import { DurationInSeconds } from '../../util/durations'; import { createSetting, createCallback } from '../../util/preload'; +import { findBestMatchingAudioDeviceIndex } from '../../calling/findBestMatchingDevice'; function doneRendering() { ipcRenderer.send('settings-done-rendering'); @@ -239,6 +240,30 @@ async function renderPreferences() { const preferredSystemLocales = MinimalSignalContext.getPreferredSystemLocales(); + const selectedMicIndex = findBestMatchingAudioDeviceIndex( + { + available: availableMicrophones, + preferred: selectedMicrophone, + }, + OS.isWindows() + ); + const recomputedSelectedMicrophone = + selectedMicIndex !== undefined + ? availableMicrophones[selectedMicIndex] + : undefined; + + const selectedSpeakerIndex = findBestMatchingAudioDeviceIndex( + { + available: availableSpeakers, + preferred: selectedSpeaker, + }, + OS.isWindows() + ); + const recomputedSelectedSpeaker = + selectedSpeakerIndex !== undefined + ? availableSpeakers[selectedSpeakerIndex] + : undefined; + const props = { // Settings availableCameras, @@ -279,8 +304,8 @@ async function renderPreferences() { preferredSystemLocales, resolvedLocale, selectedCamera, - selectedMicrophone, - selectedSpeaker, + selectedMicrophone: recomputedSelectedMicrophone, + selectedSpeaker: recomputedSelectedSpeaker, sentMediaQualitySetting, themeSetting, universalExpireTimer: DurationInSeconds.fromSeconds(universalExpireTimer),