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

Define the fitness distance for all required constrains. #722

Merged
merged 1 commit into from
Sep 17, 2020

Conversation

eehakkin
Copy link
Contributor

@eehakkin eehakkin commented Sep 9, 2020

This CL defines the fitness distance also for required constrains whose
value contains a member named 'ideal' in addition to members named
'exact', 'max' and/or 'min'.

Lets take a width constraint {ideal: 1920, min: 320} as an example and lets assume that the browser supports the constraint and that there are settings dictionaries which satisfy the constraint (so fitness distance algorithm steps 1 and 2 can be ignores).
Then,

  • the step 3 does not apply because the constraint is required (has 'min'),
  • the step 4 does not apply because the ideal value is specified, and
  • the steps 5 and 6 do not apply because the constraint is not non-required but required,

thus the fitness distance is undefined.

Clearly, the step 5 should apply and the fitness distance should be positive infinity for settings dictionaries with width less than 320, 0 for settings dictionaries with width of 1920 and |actual - 1920| / max(|actual|, 1920) for other settings dictionaries.

@youennf
Copy link
Contributor

youennf commented Sep 10, 2020

Booleans are also not covered.
It might be good to add a catch-them-all case where the distance would be 0.

@youennf
Copy link
Contributor

youennf commented Sep 10, 2020

Would be good to add some tests and see what implementations are doing.

@henbos
Copy link
Contributor

henbos commented Sep 10, 2020

@guidou Can you add your two cents?

@guidou
Copy link
Contributor

guidou commented Sep 10, 2020

I think this change is correct (wasn't the spec written this way in the past?), but booleans need to be added to step 6.

This CL defines the fitness distance also for boolean constraints and
for required constrains whose value contains a member named 'ideal' in
addition to members named 'exact', 'max' and/or 'min'.
@eehakkin
Copy link
Contributor Author

I think this change is correct (wasn't the spec written this way in the past?), but booleans need to be added to step 6.

Done.

@youennf
Copy link
Contributor

youennf commented Sep 11, 2020

I am pretty sure this is inline with what all implementations are doing but it is worth checking.
Can we write a WPT test checking exactly that something like getUserMedia({ video : { min : 160, ideal : 320 }) actually returns a video track of width 320 (and not 160 or the default 640 that some implementations may use)?

@eehakkin
Copy link
Contributor Author

I am pretty sure this is inline with what all implementations are doing but it is worth checking.
Can we write a WPT test checking exactly that something like getUserMedia({ video : { min : 160, ideal : 320 }) actually returns a video track of width 320 (and not 160 or the default 640 that some implementations may use)?

Done in web-platform-tests/wpt#25493

@alvestrand alvestrand merged commit 1fb480c into w3c:master Sep 17, 2020
@eehakkin eehakkin deleted the feature/fix-fitness-distance branch October 30, 2020 14:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants