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

MediaTrackSettings binding does not account for unsupported values for a given device #151

Closed
navaronbracke opened this issue Jan 26, 2024 · 5 comments

Comments

@navaronbracke
Copy link

navaronbracke commented Jan 26, 2024

Currently the MediaTrackSettings bindings define the properties as non-null.

However, when retrieving the settings of a MediaStreamTrack, not all properties are available,
which results in an error when trying to retrieve these properties.

For example, when creating a MediaStreamTrack on my Macbook in Chrome, the settings do not have a facing mode,
despite the binding being a String.

As an example, calling objectKeys() on the track settings object yields:

[aspectRatio, deviceId, frameRate, groupId, height, resizeMode, width]

We should probably make all these nullable?

@srujzs
Copy link
Contributor

srujzs commented Jan 30, 2024

This is going to ultimately be opinionated, but my 2c:

From a compatibility perspective i.e. "some browsers don't support certain settings", I don't think we should let that affect the Dart nullability. Historically, having taken that approach in dart:html and other web libraries, it leads to a lot of nullability everywhere. This can be useful, but in many cases, users ended up having to do null-checks for APIs that aren't supported in some contrived situation.

From a context perspective i.e. "under certain conditions, this attribute may or may not exist", I don't think the IDL exposes such details (besides stuff like SecureContext) so it'll need to be a manual override in the generation scripts to always assume the attribute can possibly not exist, and I want to avoid hardcoding logic into the scripts as much as possible, as this will inevitably lead to staleness. Assuming nullability because of SecureContext also led to a lot of dart:html members being nullable.

These were both a bigger problem for dart:html as users couldn't declare these members themselves. However, now users have the tools to extend types and declare the needed nullable external members. I think in many cases, those extra members aren't even needed if users query more details about the context like kind.

@navaronbracke
Copy link
Author

navaronbracke commented Jan 31, 2024

I agree that this might be opinionated for many of the API's in the IDL.

However, my specific use case was referring to the MediaTrackSettings of a MediaTrack, which is device specific (and not browser specific?). When I inspected the supported capabilities of the browser under test, the affected properties were supported.
But the specific MediaTrackSettings for the video track on the Macbook that I tested on did not report the unavailable properties in the settings object. In my opinion this was expected, since the Macbook didn't have multiple video input devices (there is only one webcam, so the facing mode would not be supported for that track anyway).

Considering your opinion, I think we have a few options:

  • just catch the Null is not a subtype of X TypeError and deal with it myself
  • write my own MediaTrackSettings interop binding (soon TM, with Dart 3.3 extension types)

But I wanted to see if there were other options.
When we get to #122 , we could also just document that properties might throw a TypeError if it was unexpectedly null?

@srujzs
Copy link
Contributor

srujzs commented Feb 1, 2024

For this API, it looks like the spec is just assuming you check to see if the property is available (either through in or checking if the value is undefined)? The correct option then might be to use has from dart:js_interop_unsafe, but string lookups for members are not my favorite. However, this is more correct than checking if the value is null in Dart since we conflate JS undefined and JS null to Dart null.

just catch the Null is not a subtype of X TypeError and deal with it myself

Ehh, catching these kinds of errors are not great as it may mask another error, and I'd rather you do:

write my own MediaTrackSettings interop binding (soon TM, with Dart 3.3 extension types)

if you want to use the nullability to determine if a property exists. You can do this today FYI, by extending the type e.g.

extension on MediaTrackSettings {
  @JS('facingMode')
  external String? get facingModeNullable;
}

You can write your own interface too with @staticInterop and of course, extension types on JSObject like you've noted.

When we get to #122 , we could also just document that properties might throw a TypeError if it was unexpectedly null?

That's a good idea. The docs will mention the conflation of undefined and null, but it's worth making this clear when we talk about soundness.

@navaronbracke
Copy link
Author

Do we close this issue then, in favor of #122 ? We have enough options to work around the problem today.

@srujzs
Copy link
Contributor

srujzs commented Feb 1, 2024

Ah, my mistake, I thought #122 was dart-lang/site-www#5438. I think the docs on dart.dev are the best location to talk about null-checks. Either way, yes we should close this and document the limitations. Thanks for filing!

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

No branches or pull requests

2 participants