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

Replace PTZ boolean normalization with boolean constraints #271

Conversation

eehakkin
Copy link
Contributor

@eehakkin eehakkin commented Nov 4, 2020

Fixes #256. This along with w3c/mediacapture-main#742 is to clarify meaning of PTZ constraints presence.


Preview | Diff

@beaufortfrancois
Copy link
Contributor

@jan-ivar @guidou @youennf Does this look good to you?

Copy link
Member

@jan-ivar jan-ivar left a comment

Choose a reason for hiding this comment

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

Logic seems sound, but once w3c/mediacapture-main#766 is merged, I'm hoping we can reduce the prose even more. PTAL

index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
@jan-ivar jan-ivar requested a review from riju January 19, 2021 12:54
Base automatically changed from master to main February 15, 2021 07:19
@jan-ivar
Copy link
Member

We should get this PR in before wide review of image capture starts, since it takes advantage of stuff we merged in w3c/mediacapture-main#766.

To that end, I'll commit my suggestions above to this PR and solicit reviews.

Copy link
Member

@jan-ivar jan-ivar left a comment

Choose a reason for hiding this comment

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

s/after a possible normalization/with a value other than false/

index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
@jan-ivar jan-ivar requested review from jan-ivar and removed request for jan-ivar March 15, 2021 17:42
@jan-ivar
Copy link
Member

@riju @youennf PTAL.

TL;DR: We no longer need to normalize away boolean inputs to non-boolean constraints as they're handled in fitness distance: "4. If constraintValue is a boolean, but the constrainable property is not, then the fitness distance is based on whether the settings dictionary's constraintName member exists or not, from the formula (constraintValue == exists) ? 0 : 1"

@eehakkin eehakkin requested a review from jan-ivar March 16, 2021 14:37
@eehakkin eehakkin force-pushed the feature/replace-ptz-boolean-normalization-with-boolean-constraints branch from 085ac48 to 3bef8ed Compare March 16, 2021 14:40
Copy link
Collaborator

@riju riju left a comment

Choose a reason for hiding this comment

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

Thanks @eehakkin, I think all of @jan-ivar 's requested changed are now handled.

@eehakkin
Copy link
Contributor Author

Thanks @jan-ivar. As a final touch, I replaced occurrences of {{fitness distance}} with <a>fitness distance</a> so that links work properly.

@dontcallmedom dontcallmedom merged commit 77a4936 into w3c:main Mar 29, 2021
github-actions bot added a commit that referenced this pull request Mar 29, 2021
…malization-with-boolean-constraints

SHA: 77a4936
Reason: push, by @dontcallmedom

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@eehakkin eehakkin deleted the feature/replace-ptz-boolean-normalization-with-boolean-constraints branch March 31, 2021 08:49
blueboxd pushed a commit to blueboxd/chromium-legacy that referenced this pull request Apr 14, 2021
This changes pan, tilt and zoom constrainable properties so that for
each them there is a single constrainable property which can be
constrained either with a boolean constraint (resulting in a fitness
distance of 0 or 1) or with a numeric constraint (resulting in a fitness
distance from 0.0 to 1.0).

Spec:
w3c/mediacapture-image#271
w3c/mediacapture-main#766

Change-Id: Iebb0a4f42c518286cefa6b39b2a42f6dbcd30521
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2814545
Commit-Queue: Guido Urdaneta <[email protected]>
Auto-Submit: Eero Häkkinen <[email protected]>
Reviewed-by: Guido Urdaneta <[email protected]>
Cr-Commit-Position: refs/heads/master@{#872352}
mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this pull request Oct 14, 2022
This changes pan, tilt and zoom constrainable properties so that for
each them there is a single constrainable property which can be
constrained either with a boolean constraint (resulting in a fitness
distance of 0 or 1) or with a numeric constraint (resulting in a fitness
distance from 0.0 to 1.0).

Spec:
w3c/mediacapture-image#271
w3c/mediacapture-main#766

Change-Id: Iebb0a4f42c518286cefa6b39b2a42f6dbcd30521
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2814545
Commit-Queue: Guido Urdaneta <[email protected]>
Auto-Submit: Eero Häkkinen <[email protected]>
Reviewed-by: Guido Urdaneta <[email protected]>
Cr-Commit-Position: refs/heads/master@{#872352}
GitOrigin-RevId: b6a7545796bec4c47a1c2b7504c82932fff7a71d
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.

Clarify meaning of PTZ constraints presence
5 participants