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

Eye gaze correction #56

Merged
merged 1 commit into from
Nov 2, 2023
Merged

Conversation

eehakkin
Copy link
Contributor

@eehakkin eehakkin commented Jan 28, 2022

Eye gaze correction corrects the eye gaze then a user is looking at the display and not the camera while the camera is far from the point the user is looking at. This problem is widespread especially with large displays. For an example and background, see #46.

Please see #49 (comment) why a constrainable property is most suitable for the purpose.


Preview | Diff

@eehakkin
Copy link
Contributor Author

@jan-ivar, @aboba: Is it OK to merge this PR?

@eehakkin
Copy link
Contributor Author

@alvestrand: Could you merge this PR?

Copy link
Contributor

@youennf youennf left a comment

Choose a reason for hiding this comment

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

LGTM, we should unify our usage of User Agent, user agent and UA in this document

index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
@eehakkin eehakkin force-pushed the feature/eye-gaze-correction branch from 6c43f7a to 6e12e1e Compare March 17, 2023 12:41
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.

Eye gaze correction is still a rather new and novel feature, which makes me wonder where the web platform should draw the line. Mozilla is still clarifying its position on this API in mozilla/standards-positions#706 so I'd like another week to run this past some of my colleagues, if that's ok.

index.html Outdated Show resolved Hide resolved
@jan-ivar
Copy link
Member

From #46 (comment):

  1. Okay, if your team has conducted user studies with evidence of people feeling "something wrong", we can put this on the back burner and re-visit later when users are more comfortable with this. I thought that if Microsoft and Apple had launched this feature on their native platform, there must be a user need. Do let us know if subsequent user studies suggest the perception of "uncanny valley" among majority of users has changed.

Sorry if I missed the answer to this in a more recent presentation, but has information surfaced that "users are more comfortable with this" now than in 2021?

Also, could you clarify why an application would need to control this feature? For blur, I think a valid argument was made that it was to avoid double-blurring, but that doesn't seem to apply here as much.

Isn't this something users can configure in their user agent or OS today and leave it on? Same question for face framing.

@youennf
Copy link
Contributor

youennf commented Mar 23, 2023

I think a valid argument was made that it was to avoid double-blurring

If face framing is on in the OS, the web page UI might not want to show this option (or might want to show it but unmodifiable).

@eehakkin eehakkin force-pushed the feature/eye-gaze-correction branch from b439256 to bb24084 Compare March 29, 2023 07:57
index.html Outdated Show resolved Hide resolved
Comment on lines +601 to +865
const videoCapabilities = videoTrack.getCapabilities();
if ((videoCapabilities.eyeGazeCorrection || []).includes(true)) {
await videoTrack.applyConstraints({eyeGazeCorrection: {exact: true}});
} else {
// Eye gaze correction is not supported by the platform or by the camera.
// Consider falling back to some other method.
}

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
const videoCapabilities = videoTrack.getCapabilities();
if ((videoCapabilities.eyeGazeCorrection || []).includes(true)) {
await videoTrack.applyConstraints({eyeGazeCorrection: {exact: true}});
} else {
// Eye gaze correction is not supported by the platform or by the camera.
// Consider falling back to some other method.
}
const capabilities = videoTrack.getCapabilities();
if (("eyeGazeCorrection" in capabilities) {
await videoTrack.applyConstraints({eyeGazeCorrection: true});
} else {
// Eye gaze correction is not supported by the platform or by the camera.
// Consider falling back to some other method.
}

Copy link
Contributor Author

@eehakkin eehakkin Mar 31, 2023

Choose a reason for hiding this comment

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

That's not the same thing.

if ((videoCapabilities.eyeGazeCorrection || []).includes(true)) tests if eye gaze correction can be enabled and the else branch can then fall back to some other method.

if ("eyeGazeCorrection" in capabilities) tests only if the UA and the track know about eye gaze correction but the capability can be either [false], [false, true] or [true]. The await videoTrack.applyConstraints({eyeGazeCorrection: true}); will of course always succeed regardless the capability of the lack of it because only optional constraints are used. But now the else branch is reached only if there are no eye gaze correction capabilities but not if the eye gaze correction capability is [false] which contradicts the comment in and the intent of that else branch.

@youennf
Copy link
Contributor

youennf commented Mar 30, 2023

We should fix the test example and probably file an issue about quality of implementation.

@dontcallmedom-bot
Copy link

This issue was mentioned in WEBRTCWG-2023-02-21 (Page 46)

@jan-ivar
Copy link
Member

jan-ivar commented Sep 7, 2023

@eehakkin is there still interest in this one? After reviewing the editors feel this PR should have a note stating the feature lacks consensus if it is to merge ahead of a CfC.

@riju
Copy link

riju commented Sep 14, 2023

@eehakkin is there still interest in this one? After reviewing the editors feel this PR should have a note stating the feature lacks consensus if it is to merge ahead of a CfC.

Yes. We are still interested to continue with this feature.

I have put up an Explainer also. There are 2 modes of Eye Gaze Correction on Windows, so it also an option to have a enum as shown in the Explainer.
If there is a consensus among chairs/editors on having a simple ON/OFF we can start with that also, as in this PR.

Happy to discuss here at TPAC how to move forward on this.

@youennf
Copy link
Contributor

youennf commented Sep 21, 2023

Editor's meeting: @riju could you rebase it and we will plan to merge it next week?

@eehakkin
Copy link
Contributor Author

Editor's meeting: @riju could you rebase it and we will plan to merge it next week?

I rebased it.

@riju
Copy link

riju commented Oct 30, 2023

Gentle ping @jan-ivar @alvestrand

@alvestrand alvestrand merged commit 429fc20 into w3c:main Nov 2, 2023
1 check passed
@aboba
Copy link
Contributor

aboba commented Nov 2, 2023

Should we start a Call for Consensus?

@alvestrand
Copy link
Contributor

alvestrand commented Nov 2, 2023 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants