Do we need to refactor? It seems like just alwaysRequireStreamToEnumerate is enough to work properly.

Do we need to refactor? It seems like just alwaysRequireStreamToEnumerate is enough to work properly.

Sure, but by doing this we prevent usePermission and the subsequent query method from being called in Firefox. Just adding alwaysRequireStreamToEnumerate without refactor would still query the permissions in Firefox. If that's not a concern I can remove the refactor.

If we have browser-specific stuff here, we should cover that with browser tests.

If we have browser-specific stuff here, we should cover that with browser tests.

I agree that that would be nice. However, you can't grant permissions to camera and microphone in Playwright (microsoft/playwright#5445), which also ties into their lack of support for testing WebRTC in browsers other than Chromium (microsoft/playwright#2973).

It's possible to mock the involved API's. However, this kind of circumvents the point of a test, since that test would just be testing our mocks.

In my opinion, adding tests here would be too much work for what it's worth (if even possible).

@OrbisK let me know what direction you want to take this (closing vs. merging). If you still want to merge, let me know what needs to be done before this can be considered ready to merge.

It's possible to mock the involved API's. However, this kind of circumvents the point of a test, since that test would just be testing our mocks.

Agreed, let's test it manually as best we can 😄

Sure, but by doing this we prevent usePermission and the subsequent query method from being called in Firefox. Just adding alwaysRequireStreamToEnumerate without refactor would still query the permissions in Firefox. If that's not a concern I can remove the refactor.

@OrbisK What do you think? Do we need to refactor?

Sure, but by doing this we prevent usePermission and the subsequent query method from being called in Firefox. Just adding alwaysRequireStreamToEnumerate without refactor would still query the permissions in Firefox. If that's not a concern I can remove the refactor.

@OrbisK What do you think? Do we need to refactor?

We should definitely not call usePermission inside the function. This will cause a memory leak.

https://github.com/vueuse/vueuse/pull/4674/files#diff-1c6f5be67d3284eb4ebf86f4553255b7271ea803ca1fe5134f397d5c8759933fR77

I wonder if we can detect if we need the stream from something other than the useragent. Like we did with the fallback part of useClipboard.

I wonder if we can detect if we need the stream from something other than the useragent. Like we did with the fallback part of useClipboard.

What I can do is add a check in update that'll check if the value of permission is granted, while the value of the deviceId's are all empty strings. When that holds, we can initiate a stream and call the enumerateDevices function again, like so:

async function update() {
  if (!isSupported.value)
    return

  devices.value = await navigator!.mediaDevices.enumerateDevices()

  const { state, query } = usePermission(deviceName, { controls: true })
  await query()

  if (!stream && state.value === "granted" && devices.value.every(device => device.deviceId === "")) {
    await startStream();
    devices.value = await navigator!.mediaDevices.enumerateDevices()
  }

  onUpdated?.(devices.value)
  stopStream()
}

where the startStream and stopStream functions have to be created. The only downside of this is that 'enumerateDevices' will be called twice in FF, but I don't see a way of avoiding that without checking the useragent explicitly.

EDIT: this way the ensurePermissions function can also remain unchanged

I am not against relying on the useragent if it solves the problem. Is FF the only browser with this behaviour? And why?

A general thing with your PR and proposed changes:

You should not call composables within functions other than composables. Composables should always be called at setup level.

Example

// useFoo
function useFoo(s: MaybeRef<string>){
  const lowerCase = computed(()=>{
    return toValue(s).toLowerCase()
  })
  
  watch(()=>s,()=>{
    console.log("s changed")
  }

  return {
    lowerCase
  }
} 

// bad usage
const test = ref("abc")

function doSomeThing(){
  const { lowerCase } = useFoo(test)
  /* ... */ 
  return /* ... */ 
}

doSomeThing() // this will call useFoo the first time -> creating a computed and a watcher
test.value = "123" // this will call console.log once -> expected

doSomeThing() // this will call useFoo the first time -> creating a computed and a watcher
doSomeThing() // this will call useFoo the second time -> creating a computed and a watcher again, but does not  dispose the first

test.value = "123" // this will call console.log twice 

Alex has a great video on this: https://www.youtube.com/watch?v=njsGVmcWviY (its about useFetch, but can be applied to any composable)

I am not against relying on the useragent if it solves the problem. Is FF the only browser with this behaviour? And why?

From what I can gather, this only happens in FF. And while the issue is described on SO, the only other documentation I could find is the spec which states that you need a live MediaStreamTrack in order to be able to enumerate devices. If I'm reading that correctly, FF is the only browser properly adhering to the spec. Meanwhile Chrome and Safari allow enumeration of devices without an active mediastream. In that light, checking useragent might seem like a good workaround.

Another solution is to always create a MediaStream before trying to enumerate the devices. This does adhere to the spec, and will work in each browser, but has two important downsides:

• It takes significant time to initiate a MediaStream
• Visual camera-active indicators become active on creation of a MediaStream (eg. my MacBook will show a green indicator next to the camera).

My preference goes out the solution I proposed earlier, first trying to enumerate the devices, and if that fails retry with an active MediaStream. This has the benefit of not impacting the majority of users (on Chrome and Safari) with the downsides mentioned above, no explicit useragent checking, still functioning if behavior of any of the browsers change, with the only downside being having to call enumerateDevices twice in FF.

You should not call composables within functions other than composables. Composables should always be called at setup level.

I'll make sure to update the PR accordingly.

I have found a discussion on w3c mediacapture w3c/mediacapture-main#1019

web-platform-tests/interop#849 (comment) > https://bugzilla.mozilla.org/show_bug.cgi?id=1928216

@OrbisK @ilyaliao let's get this PR finished up. If I change the update method in the way I describe here, would you merge? If so, I'll get right on it.

@OrbisK @ilyaliao let's get this PR finished up. If I change the update method in the way I describe here, would you merge? If so, I'll get right on it.

Thank you for your commitment. But I think we need to investigate the intended solution for this. From the w3c thread it looks like there should be a proper way to solve this (it also looks like FF is doing it the correct way). I have not had time to look into it yet. Maybe I can do it tomorrow.

TLDR: Let me check the Threads. I will get back to you!

Hey everyone! Just want to propose a solution with adding some flag that we pass to useDevicesList options, for exmaple, forceGUMToEnumerate or smth similiar with default false. And we just modify ensurePermissions this way:

async function ensurePermissions() {
    const deviceName = constraints.video ? 'camera' : 'microphone'
    if (!isSupported.value)
      return false
    if (permissionGranted.value && !forceGUMToEnumerate)
      return true
    const { state, query } = usePermission(deviceName, { controls: true })
    await query()
    if (state.value !== 'granted' || forceGUMToEnumerate) {
      let granted = true
      try {
        stream = await navigator!.mediaDevices.getUserMedia(constraints)
      }
      catch {
        stream = null
        granted = false
      }
      update()
      permissionGranted.value = granted
    }