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

Stop referring to advanced constraints #252

Closed
jan-ivar opened this issue Aug 24, 2020 · 13 comments · Fixed by #253
Closed

Stop referring to advanced constraints #252

jan-ivar opened this issue Aug 24, 2020 · 13 comments · Fixed by #253
Assignees

Comments

@jan-ivar
Copy link
Member

This spec refers to the overly complex and outdated advanced constraints syntax in:

  1. examples
  2. In issues like Determine how PTZ permission prompts are supposed to work #243 (comment), where apparently one browser infers new meaning from their presence

The former is unnecessary, and the latter is a non-standard.

The differences between the older advanced constraints and the modern ideal constraints are minimal and best forgotten. TL;DR: they allowed for non-linear fallbacks between width x height x frameRate combos when resizeMode is none. They're largely redundant elsewhere, since most other constrainable capabilities are orthogonal to each other.

It's therefore best to not mention them, to discourage use. Maybe one day we'll be able to remove them from the platform.

@jan-ivar jan-ivar changed the title Stop referring advanced constraints Stop referring to advanced constraints Aug 24, 2020
@beaufortfrancois
Copy link
Contributor

How would we differentiate an optional PTZ request without advanced constraints?
See explainer.

(Sorry I'm on mobile and ooo this week)

@jan-ivar
Copy link
Member Author

@beaufortfrancois show me where in the spec advanced constraints let you do that.

@jan-ivar
Copy link
Member Author

jan-ivar commented Aug 24, 2020

@beaufortfrancois note constraints are non-required by default, so this should suffice:

{video: {pan: true, tilt: true, zoom: true}}

@jan-ivar
Copy link
Member Author

Here's a good example of why advanced constraints suck: a likely bug in ptz-explainer.md:

const videoStream = await navigator.mediaDevices.getUserMedia({
  video: {
    advanced: [{
      // [NEW] Website asks to control camera PTZ as well.
      pan: true, tilt: true, zoom: true,
    }],
  },
});

The above will, if the constraints algorithm is implemented correctly, only discover cameras that have all three capabilities.
If none exists, it'll return any regular camera (possibly forgoing cameras that have some of these abilities but not all).

To get cameras with one or more of the pan, tilt and zoom abilities would instead have to be written like this:

  video: {
    advanced: [
      {pan: true},
      {tilt: true},
      {zoom: true}
    ],
  }

...or simply:

  video: {pan: true, tilt: true, zoom: true}

@jan-ivar jan-ivar self-assigned this Aug 24, 2020
@jan-ivar
Copy link
Member Author

MDN has a good constraints explainer.

@reillyeon
Copy link
Member

Based on the above it sounds like we want to define something like this to support mandatory PTZ constraints:

const videoStream = await navigator.mediaDevices.getUserMedia({
  video: {pan: {exact: true}, tilt: {exact: true}, zoom: {exact: true}}
});

I think this makes sense if we make these properties ConstrainBoolean or ConstrainDouble instead of boolean or ConstrainDouble.

@jan-ivar
Copy link
Member Author

That would seem to run counter to #246.

@reillyeon
Copy link
Member

Agreed, though that seems to make the use of ConstrainDouble also problematic as setting 'min' or 'max' would also create a required constraint.

Focusing on the intention of this design, the reason for the true value is to make it possible for a site to declare that it wants to control pan, tilt, and/or zoom without specifying any particular value. This is important as before getUserMedia() returns the site doesn't know what the supported settings range is or what the current position of the camera is and so no value is appropriate to pass in the initial getUserMedia() call. true is used as a sentinel instead to simply indicate the desire to potentially control this setting in the future..

@jan-ivar
Copy link
Member Author

Yes but true is also merely an alias for{} which already works; a nicety we added to promote the pattern you mention.

Using required constraints that way is not a pattern we want to promote (or with #246 even make possible).

@beaufortfrancois
Copy link
Contributor

IIUC, the presence of a pan, tilt or zoom constraint without an explicit min/man/exact should be an optional request of capability/permission whether it is an basic or advanced set; and with an explicit min/man/exact it should be a mandatory request for capability/permission whether it is an basic or advanced set.

Note that in advanced set, the lack of PTZ capability just means that the advanced set will be ignored, not that the PTZ request will fail. This means permission shouldn't be asked when in an advanced set and no capability.

If that looks good to you, we'll update spec and explainer.

@jan-ivar
Copy link
Member Author

jan-ivar commented Sep 3, 2020

IIUC, the presence of a pan, tilt or zoom constraint without an explicit min/man/exact should be an optional request

Not for true it doesn't, as I point out in #256. I think we should fix that with option 3 in that proposal.

Only then would what I wrote earlier actually be true (my bad):

@beaufortfrancois note constraints are non-required by default, so this should suffice:

{video: {pan: true, tilt: true, zoom: true}}

@riju riju closed this as completed in #253 Sep 4, 2020
@riju riju reopened this Sep 4, 2020
@riju
Copy link
Collaborator

riju commented Sep 4, 2020

I think if we go down the path of #256 (comment), we would not need advanced constraints

@beaufortfrancois
Copy link
Contributor

@riju I believe we can close this issue as we chose to only allowing ideal PTZ constraints in the basic set, and discarding mandatory PTZ constraints in advanced sets.
See https://github.com/w3c/mediacapture-image/pulls

@riju riju closed this as completed Sep 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants