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

feat(MediaSettings): preferred media devices selection 🎤📹 #11936

Merged
merged 5 commits into from
Apr 11, 2024

Conversation

Antreesy
Copy link
Contributor

@Antreesy Antreesy commented Mar 26, 2024

☑️ Resolves

  • Fix Media settings sometimes reset to a wrong microphone #9902
  • Add button to manually refresh devices list:
    • event listener tracks only change of physical devices (USB-connected)
    • allow to get in the list such inputs as bluetooth, mini-jack, virtual devices
  • feat(media): list device in preferences order
    • device goes on top of the list (above unplugged devices) every time user closes media settings dialog (when joining a call or applying new devices during the call
    • list can be called with window.OCA.Talk.mediaDevicesManager.listDevices() via devtools

Tip

Use

window.OCA.Talk.mediaDevicesManager.listDevices()

to get list in the console

🖌️ UI Checklist

🖼️ Screenshots / Screencasts

🏚️ Before 🏡 After
image image
image image
Screencast.from.11.04.2024.15.18.10.webm

🚧 Tasks

  • Replace fallback id with preferences (or combine)
  • Update selected device based on preferences
  • Add showWarning for changes / fallback / preferences
  • check why audio/video is disabled on device change during call

🏁 Checklist

  • 🌏 Tested with Chrome, Firefox and Safari or should not be risky to browser differences
  • 🖥️ Tested with Desktop client or should not be risky for it
  • 🖌️ Design was reviewed, approved or inspired by the design team
  • ⛑️ Tests are included or not possible
  • 📗 User documentation in https://github.com/nextcloud/documentation/tree/master/user_manual/talk has been updated or is not required

@Antreesy Antreesy added this to the 💙 Next Major (30) milestone Mar 26, 2024
@Antreesy Antreesy requested a review from DorraJaouad March 26, 2024 13:31
@Antreesy Antreesy self-assigned this Mar 26, 2024
@Antreesy Antreesy force-pushed the fix/9902/preffered-media-device branch 2 times, most recently from 7a64764 to 5456dae Compare March 28, 2024 16:03
@DorraJaouad
Copy link
Contributor

event listener tracks only change of physical devices (USB-connected)

Tested on Google chrome and brave, my Bluetooth headset is detected with event listener.

Copy link
Contributor

@DorraJaouad DorraJaouad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Logic is almost correctly implemented, nice work!

src/services/mediaDevicePreferences.ts Outdated Show resolved Hide resolved
src/services/mediaDevicePreferences.ts Outdated Show resolved Hide resolved
@Antreesy Antreesy force-pushed the fix/9902/preffered-media-device branch from 5456dae to fd435b2 Compare April 10, 2024 13:48
@Antreesy
Copy link
Contributor Author

Current state to test (step by step)

  1. Select devices and start a call (they are on top of the list - preffered)
Media devices:
  Audio input:
    1. Family 17h (Models 10h-1fh) HD Audio Controller Analog Stereo (selected)
    2. AUKEY PC-LM1E Camera Analog Stereo

  Video input:
    1. BisonCam,NB Pro (5986:9102) (selected)
    2. AUKEY PC-LM1E Camera (1bcf:0001)
  1. Select other devices and start a call (now they are preffered)
  Media devices:
  Audio input:
    1. AUKEY PC-LM1E Camera Analog Stereo (selected)
    2. Family 17h (Models 10h-1fh) HD Audio Controller Analog Stereo

  Video input:
    1. AUKEY PC-LM1E Camera (1bcf:0001) (selected)
    2. BisonCam,NB Pro (5986:9102)
  1. Disconnect first device and start a call (first is unavilable, but still counts as preffered, if will be connected):
Media devices:
  Audio input:
    1. AUKEY PC-LM1E Camera Analog Stereo (unplugged)
    2. Family 17h (Models 10h-1fh) HD Audio Controller Analog Stereo (selected)

  Video input:
    1. AUKEY PC-LM1E Camera (1bcf:0001) (unplugged)
    2. BisonCam,NB Pro (5986:9102) (selected)
    ```
    

@Antreesy Antreesy force-pushed the fix/9902/preffered-media-device branch 2 times, most recently from f7d04df to a18fdcc Compare April 11, 2024 13:07
- event listener tracks only change of physical devices (USB-connected)
- allow to get in the list such inputs as bluetooth, mini-jack, virtual devices

Signed-off-by: Maksim Sukharev <antreesy.web@gmail.com>
Signed-off-by: Maksim Sukharev <antreesy.web@gmail.com>
Signed-off-by: Maksim Sukharev <antreesy.web@gmail.com>
@Antreesy Antreesy force-pushed the fix/9902/preffered-media-device branch from a18fdcc to 2a87eff Compare April 11, 2024 13:15
@Antreesy Antreesy requested a review from ShGKme April 11, 2024 13:15
@Antreesy Antreesy marked this pull request as ready for review April 11, 2024 13:20
@Antreesy Antreesy requested a review from DorraJaouad April 11, 2024 13:41
Copy link
Contributor

@ShGKme ShGKme left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just awesome 🔥

Tested with @Antreesy with plugging in-out USB devices.

Not tested yet with Bluetooth devices and virtual devices / enabling/disabling devices in the system

src/services/mediaDevicePreferences.ts Show resolved Hide resolved
src/services/mediaDevicePreferences.ts Outdated Show resolved Hide resolved
Comment on lines +48 to +55
// eslint-disable-next-line no-console
console.log(`Media devices:
Audio input:
${audioInputList.map(getDeviceString).join('\n')}

Video input:
${videoInputList.map(getDeviceString).join('\n')}
`)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do not forget to remove the debug log :P

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's needed to list devices in the console (so user can pass some information to admin, e.g)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then it should not be log

src/services/mediaDevicePreferences.ts Show resolved Hide resolved
Signed-off-by: Maksim Sukharev <antreesy.web@gmail.com>
…lable

Signed-off-by: Maksim Sukharev <antreesy.web@gmail.com>
Copy link
Contributor

@DorraJaouad DorraJaouad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

scenario ( devices A (bluetooth headset), B (virtual mic), C(external mic)):

  • [A+B] select A in presence of B
  • [B] unplug A , B was automatically selected
  • [B+C] plug C and use it
  • [A+B+C] plug A, A was automatically selected

@@ -200,14 +212,14 @@ MediaDevicesManager.prototype = {
// Couldn't find device by id
console.debug('Could not find previous audio device, falling back to default/first device in the list', BrowserStorage.getItem('audioInputId'), this.attributes.devices)
}
this.attributes.audioInputId = this._fallbackAudioInputId
this.attributes.audioInputId = getFirstAvailableMediaDevice(devices, this._preferenceAudioInputList, this._fallbackAudioInputId) ?? 'default'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this._fallbackAudioInputId can be omitted given that we have the list now, but let's do it next time.

@Antreesy Antreesy mentioned this pull request Apr 11, 2024
6 tasks
@Antreesy Antreesy merged commit f458624 into main Apr 11, 2024
45 checks passed
@Antreesy Antreesy deleted the fix/9902/preffered-media-device branch April 11, 2024 14:54
@nickvergessen
Copy link
Member

/backport to stable29

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Media settings sometimes reset to a wrong microphone
4 participants