Fix device selection persistence bug

ts/calling/findBestMatchingDevice.ts

@@ -3,20 +3,27 @@
 
 import type { AudioDevice } from '@signalapp/ringrtc';
 
-export function findBestMatchingAudioDeviceIndex({
+export function findBestMatchingAudioDeviceIndex(
+  {
     available,
     preferred,
-}: Readonly<{
+  }: Readonly<{
     available: ReadonlyArray<AudioDevice>;
     preferred: undefined | AudioDevice;
-}>): undefined | number {
+  }>,
+  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;
   }

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({
+    const selectedMicIndex = findBestMatchingAudioDeviceIndex(
+      {
         available: availableMicrophones,
         preferred: preferredMicrophone,
-    });
+      },
+      OS.isWindows()
+    );
     const selectedMicrophone =
       selectedMicIndex !== undefined
         ? availableMicrophones[selectedMicIndex]
         : undefined;
 
     const preferredSpeaker = window.Events.getPreferredAudioOutputDevice();
-    const selectedSpeakerIndex = findBestMatchingAudioDeviceIndex({
+    const selectedSpeakerIndex = findBestMatchingAudioDeviceIndex(
+      {
         available: availableSpeakers,
         preferred: preferredSpeaker,
-    });
+      },
+      OS.isWindows()
+    );
     const selectedSpeaker =
       selectedSpeakerIndex !== undefined
         ? availableSpeakers[selectedSpeakerIndex]

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({
+            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({
+          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({
+        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({
+        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({
+            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 0 if that was the previous preferred index (and a device is available)', () => {
-        it(`returns ${index} if that was the previous preferred index (and a device is available)`, () => {
         assert.strictEqual(
-            findBestMatchingAudioDeviceIndex({
+          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' },
+              preferred: { name: 'C', index: 0, uniqueId: 'device-C' },
-            }),
+            },
-            index
+            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({
+          findBestMatchingAudioDeviceIndex(
+            {
               available: [{ name: 'A', index: 123, uniqueId: 'device-A' }],
               preferred: { name: 'C', index: 1, uniqueId: 'device-C' },
-          }),
+            },
+            false
+          ),
           0
         );
       });

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,
+    selectedMicrophone: recomputedSelectedMicrophone,
-    selectedSpeaker,
+    selectedSpeaker: recomputedSelectedSpeaker,
     sentMediaQualitySetting,
     themeSetting,
     universalExpireTimer: DurationInSeconds.fromSeconds(universalExpireTimer),