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.

Sign up for GitHub

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

Jump to bottom

Clarify+make fitness distance algorithm agnostic to dictionary member presence. #526

Merged
merged 5 commits into from
Jul 12, 2018
Merged

Clarify+make fitness distance algorithm agnostic to dictionary member presence. #526

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
3dfe450
Clarify, and make fitness distance algorithm agnostic to dictionary m…
jan-ivar Jul 9, 2018
13d9594
typo
jan-ivar Jul 9, 2018
8c26c13
Fix negated logic bug.
jan-ivar Jul 11, 2018
473efc1
Ignore null values from unions and allow for member dictionary-member…
jan-ivar Jul 11, 2018
06fa189
No need to test for null as it gets converted to empty dictionary.
jan-ivar Jul 11, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 16 additions & 12 deletions getusermedia.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4114,15 +4114,19 @@ <h2>Methods</h2>
value being compared against.</p>
<p>We define the <dfn>fitness distance</dfn> between a
<a>settings dictionary</a> and a constraint set <var>CS</var> as
Copy link

Choose a reason for hiding this comment

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

So a constraint set is a webidl dictionary, right? That's not really specified anywhere.

Also, https://w3c.github.io/mediacapture-main/getusermedia.html#dfn-selectsettings should make it clear that it has a constraint set argument, not an ambient constraint set.

the sum, for each constraint provided for a constraint name in
the sum, for each member (represented by a
<var>constraintName</var> and <var>constraintValue</var> pair)
<a href="https://heycam.github.io/webidl/#dfn-present">present</a> in
<var>CS</var>, of the following values:</p>
<ol>
<li>
<p>If the constraint is not supported by the browser, the
fitness distance is 0.</p>
<p>If <var>constraintName</var> is not supported by the
Copy link

Choose a reason for hiding this comment

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

So this is a little odd, actually.

The input to this thing is a WebIDL dictionary, right? If a browser doesn't support some random spec that defines some constraint, it wouldn't know to include that name in the dictionary to start with, so it would not trigger this clause.

Is this clause meant to just apply to constraints that the browser knows about in the sense of having them in its IDL but doesn't actually implement matching for? Or is it meant to apply to constraints that the browser really doesn't know about? In the latter case, the input here obviously can't be a dictionary, because those presuppose a fixed set of known members and ignore everything else.

What do UAs actually implement here?

browser, the fitness distance is 0.</p>
</li>
<li>
<p>If the constraint is required ('min', 'max', or 'exact'),
<p>If the constraint is required (<var>constraintValue</var> either
contains one or more members named 'min', 'max', or 'exact', or is
itself a bare value and bare values are to be treated as 'exact'),
and the <a>settings dictionary</a>'s value for the constraint
does not satisfy the constraint, the fitness distance is
positive infinity.</p>
Expand All @@ -4132,20 +4136,20 @@ <h2>Methods</h2>
this type of device, the fitness distance is 0 (that is, the
constraint does not influence the fitness distance).</p>
</li>
<li>If no ideal value is specified, the fitness distance is
0.</li>
<li>If no ideal value is specified (<var>constraintValue</var>
either contains no member named 'ideal', or, if bare values
are to be treated as 'ideal', isn't a bare value), the
Copy link

Choose a reason for hiding this comment

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

This looks wrong to me. It's saying that if bare values are to be treated as ideal and constraintValue is not a bare value then the ideal value is not specified. That's clearly not the intent, but that's what it says.

I would strongly urge clarity of processing model over brevity here. If we have an "ideal value is specified" boolean with a complicated way to compute it, we should compute it in some substeps instead of trying to jam the computation into a parenthetical. Code that does that is not considered a good idea; neither are specs that do it.

fitness distance is 0.</li>
<li>For all positive numeric non-required constraints (such as
Copy link

Choose a reason for hiding this comment

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

What does "numeric" mean here? Is that a property a constraint can have as part of its definition (in which case please link to the concept's definition) or something else?

height, width, frameRate, aspectRatio, sampleRate and
sampleSize), the fitness distance is the result of the formula
<pre>(actual == ideal) ? 0 : |actual -
ideal|/max(|actual|,|ideal|)
</pre>
<pre>
(actual == ideal) ? 0 : |actual - ideal| / max(|actual|, |ideal|)</pre>
</li>
<li>For all string and enum non-required constraints (e.g.
deviceId, groupId, facingMode, resizeMode, echoCancellation),
the fitness distance is the result of the formula
<pre>(actual == ideal) ? 0 : 1
</pre>
<pre>(actual == ideal) ? 0 : 1</pre>
</li>
</ol>
<p>More definitions:</p>
Expand Down Expand Up @@ -4185,7 +4189,7 @@ <h2>Methods</h2>
<li>Each <a data-lt="constraints">constraint</a> specifies one
or more values (or a range of values) for its property. A
property MAY appear more than once in the list of 'advanced'
ConstraintSets. If an empty object or list has been given as
ConstraintSets. If an empty list has been given as
Copy link

Choose a reason for hiding this comment

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

It's really not clear to me what cases this is supposed to be handling. Is this dealing with constraints that are sequence-valued?

the value for a constraint, it MUST be interpreted as if the
constraint were not specified (in other words, an empty
constraint == no constraint).
Expand Down