Tone.js icon indicating copy to clipboard operation
Tone.js copied to clipboard

Feature Request: Support custom MediaTrackConstraints in Tone.UserMedia.open

Open rodw opened this issue 4 years ago • 2 comments

Currently the open function in Tone.UserMedia accepts a single parameter - the device label or ID.

This value is added (as audio.deviceId) to a small collection of MediaTrackConstraints that are hard-coded as a local variable in the Tone.UserMedia.open method:

const constraints = {
  audio: {
    echoCancellation: false,
    sampleRate: this.context.sampleRate,
    noiseSuppression: false,
    mozNoiseSuppression: false,
  }
}

which are then provided to the navigator.mediaDevices.getUserMedia method.

Would it be possible to add a second optional parameter to this method that would allow one to specify a set of constraints to merge into (or simply replace) the default constraints?

In theory one could use MediaTrack.applyConstraints to change some of these values after the fact but as far as I can tell many of those constraints cannot be changed after the stream/track is created. For example (based on experimentation with multiple browsers) the only reliable way to set echoCancellation, noiseSuppression or autoGainControl is by specifying the constraint in the initial call to mediaDevices.getUserMedia. If you try to change those values via applyConstraints the request is ignored (or if you specify exact the attempt fails with an OverconstrainedError).

In short what I'm proposing is to change the signature of UserMedia.open from open(labelOrID) to open(labelOrID, constraints) where constraints is an optional map of constraints to add to or replace the set currently defined in lines 120 - 127 of UserMedia.ts.

If you're open to this change and it would make things easier for you I'm happy to create a PR for it. (To be honest I'll probably need to create some type of local monkey-patch implementation of this to address my immediate problem anyway.)

For what it is worth I think the simplest approach would be to allow the caller to specify the entire constraints dictionary as an optional parameter, falling back to the current set of constraints as a default (and for backward compatibility). But if you think it is important or valuable to be piecemeal about it another option would be "merge" the caller-provided constraints on top of the existing defaults (such that if the called didn't specify a value for echoCancellation for example, the existing value (false) would be used instead). Given the current set of default constraints that seems straightforward but I'm a little concerned that that behavior could be brittle or complicated if sub-attributes like exact or max/min start showing up in both the default and caller-specified constraints. The "all or none" approach isn't very sophisticated but it is at least easy to understand.

rodw avatar Jan 01 '21 04:01 rodw

i think this makes sense. I'd be happy to accept a PR with this change. I might suggest that you could keep the parameters to to open just a single argument with something like this open(labelOrIdOrConstraints) and then in the function check if the argument is a string or a number and use the current route, or check if its an object in which case pass it in as a constraint like you're suggesting. If you add it at the second parameter, there's a chance that the deviceId would be different in the first argument and in the constraint in which case the code would have to pick between the two like in this scenario

userMedia.open("a-device-id", {
	audio: {
		deviceId: "a-different-device-id"
		echoCancellation: false,
		noiseSuppression: false,
		}
	}
})

But instead maybe around this line, if you checked isObject you could assume that the object is the constraints and pass it into navigator.mediaDevices.getUserMedia(constraints);

Would that work?

tambien avatar Jan 04 '21 15:01 tambien

Thanks for your insight. I think the open(labelOrIdOrConstraints) approach should work for me.

If I understand you correctly I think you are suggesting that in the "constraints" (isObject()===true) case we just skip the logic in lines 106-118?

Looking at the current implementation more closely I guess ultimately that whole block after UserMedia.enumerateDevices (line 106) is just identifying a deviceId to add to the constraints (even silently falling back to whatever the first device in the list if whatever label or ID the caller provided doesn't match). So I imagine there's nothing wrong with skipping that outright - even if there's no deviceId specified in the constraints I think.

The only difference in that case is that the UserMedia._device attribute wouldn't be populated when L106-118 are skipped. I'm not sure it really matters either way but I think we should be able to work backwards from _mediaStream.getSettings().deviceId to populate that if needed.

Assuming no one beats me to it (and I won't be offended if they do, so if anyone reading wants to take a stab at it feel free) I'll experiment with that approach and submit a PR along those lines.

rodw avatar Jan 05 '21 19:01 rodw