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

Bug 25298 - VideoFacingMode "other" for UAs to coerse unknown strings into. #1

Closed
wants to merge 19 commits into from

Conversation

jan-ivar
Copy link
Member

May be seen by users in constraints returned to them.

… into. May be seen by users in constraints returned to them.
@jan-ivar
Copy link
Member Author

Older discussion is here fluffy/webrtc-w3c#34

@fluffy
Copy link
Contributor

fluffy commented Aug 25, 2014

So I think just a littel bit more might be needed here. Let me check my understanding first.

If an unknown string say "insideOutUpsideDown" gets passed in as say an optional constraint, then the system would change that to "oher" and pass on down and perhaps select the environment facing camera. Later if the system read back the current constraints, would it get back "insideOutUpsideDown", "other", or "environment".

I would have expected it to get back "insideOutUpsideDown" as the constraint and "environment" as the setting. Now if their is the system selected a camera that is not user or environment or any other implemented enum, it seems like it would be reasonable for the settings to return "unknown" or NULL, or empty string. But is seems like what we need here is not other but a unknown value for the setting to return.

Is this the sort of use case you were thinking about ?

@jan-ivar
Copy link
Member Author

Two problems are being mixed up here:

  1. VideoFacingMode throws, which is sad - https://www.w3.org/Bugs/Public/show_bug.cgi?id=25793
  2. Kiran's unknown camera - https://www.w3.org/Bugs/Public/show_bug.cgi?id=25298

This pull request really belongs to the former, but I incorrectly got Harald to close it as a dup of the latter.

The conclusion to Kiran's problem is in https://www.w3.org/Bugs/Public/show_bug.cgi?id=25298#c19 which was to use JS undefined for unknown cameras, so nothing to do there.

This pull request relates instead to the DOMString kludge to the "enums throw" problem. The webidl issue https://www.w3.org/Bugs/Public/show_bug.cgi?id=19936 offers hope of going back to using WebIDL enums someday (enums are quite useless to us otherwise), so we should leave the door open by not straying too far away from being an enum, which is what this patch does.

If bug 19936 succeeds, VideoFacingMode proper can produce WebIDL bindings that coerce "insideOutUpsideDown" into "other" (instead of throwing) as soon as it enters the browser object, where it can be implemented as a c++ enum in a c++ implementation. This means the original value is lost, and getConstraints would show "other".

To answer your other questions:

  • getCapabilities and getSettings of a environment-facing camera show "environment".
  • If the camera faces an unknown direction or does not implement facingMode, then getCapabilities and getSettings would return a dictionary without the facingMode member, i.e. undefined in JS.

So "other" and unknown are different:

  • "other" is a none-of-the-above value known to the webpage but unknown to the UA, whereas
  • undefined represents the UA not knowing what the camera supports.

Semantically "other" means none of the above (important for failing on required), whereas undefined means it could be a known value or not, the UA just doesn't know what the direction is.

Worth noting: getSettings and getCapabilities must never return "other", as that would satisfy constraints known only to the webpage.

I concede that the naming is unfortunate, since users may go looking for "other" in settings and capabilities, but at the same time I think that cementing expectations that rely on the type being DOMString forever would be a mistake. Perhaps a better name for "other" would be "unsupported"?

@fluffy
Copy link
Contributor

fluffy commented Aug 26, 2014

Well we have agreed that we are solving the throw problem by moving all the enums to strings for any thing that is extensible (like this). So if webidl enum change like we hope they will, I'm still not getting what other means particular after some adional types had been added. Imagine that some extension added "up" and "down" (not really sure what these mean but best thing I can think of).

I think I am just having a hard time grocking

"other" is a none-of-the-above value known to the webpage but unknown to the UA,

so if the webpage set it to "up" and the UA understood that and the camera was that, it would get up. If the webpage set it to "up" and the UA did not understand that, it would get "other" or "unknown". I'm just not getting what use case this solves to have an "other" that is different from "unknown".

Walk me through this a bit more, I'm not getting it yet.

@jan-ivar
Copy link
Member Author

It's not about a new use-case, it's about implementations reserving the right to use enums later once they're fixed.

I don't know how rigid we expect to be on this later - maybe I'm over-analyzing it and it's fine as is - but without this patch I worry we're implicitly saying that track.ApplyConstraints({facingMode: "up"}); dump(track.getConstraints()); MUST produce { facingMode: "up" }, which would close the door on implementations ever using enum, since enums can't hold arbitrary strings (they're not strings in implementations). Using an enum - once it has been fixed - means the binding code parsing "up" would need to encode it into some ordinal value, like a reserved OTHER value, so track.ApplyConstraints({facingMode: "up"}); dump(track.getConstraints()); would necessarily produce { facingMode: "other" }; If the spec says that users may see "other" instead of an unsupported facingMode string, then the door to implementations using enum is left open, and I'm happy.

Walking through your example: If the webpage set it to "up" and the UA did not understand that, it would get "other". There is no "unknown". If you mean {facingMode: undefined } then undefined becomes "undefined" in a DOMString. If you mean absence of a dictionary member, then this already has meaning (unconstrained constraint or unknown capability). Since users need a way to require "up" or fail, JS undefined wont do as it means unconstrained and never fails. This isn't a problem with DOMString since it preserves arbitrary values, but it's a problem with enums.

@jan-ivar
Copy link
Member Author

I should also mention that this pull request makes the webidl in the spec agree with the language (right now the webidl says ConstrainVideoFacingMode i.e. enum, while the IANA table says ConstrainDOMString).

@fluffy
Copy link
Contributor

fluffy commented Sep 11, 2014

Hi Jan-Ivar, could we have a short phone call about this.I just want to figure out what to do next

@jan-ivar
Copy link
Member Author

Sure! Not sure I have your email. Pm me your # or url on #media (I'm jib)

@fluffy
Copy link
Contributor

fluffy commented Sep 12, 2014

jiv and I synced up and have a good plan. jiv is going to form some new pull requests for this

@jan-ivar
Copy link
Member Author

See new pull requests. Retiring this one.

@jan-ivar jan-ivar closed this Sep 12, 2014
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.

3 participants