w3c / mediacapture-screen-share

Media Capture Screen Capture specification
https://w3c.github.io/mediacapture-screen-share/
Other
86 stars 28 forks source link

getSupportedOptions() #260

Open eladalon1983 opened 1 year ago

eladalon1983 commented 1 year ago

We have mediaDevices.getSupportedConstraints() for MediaTrackSupportedConstraints, but we don't have a mediaDevices.getSupportedOptions() that'd return the DisplayMediaStreamOptions the user agent supports. I think we should have that.

CC @coread

Coread commented 1 year ago

This is a useful method for feature detection. Is it due to the lack of this/a mistake that Chrome returns displaySurface from getSupportedConstraints?

eladalon1983 commented 1 year ago

This is a useful method for feature detection. Is it due to the lack of this/a mistake that Chrome returns displaySurface from getSupportedConstraints?

I don't see a mistake here. That displaySurface should be returned by getSupportedConstraints() is specified here. Note that there is one spec that defines MediaTrackSupportedConstraints and another spec that extends it.

Coread commented 1 year ago

I think I was being confused by the naming here. getUserMedia takes an argument called "constraints" which is NOT the same as a the constraints returned from getSupportedConstraints. getDisplayMedia takes options, which is similar to the getUserMedia "constraints" but has extra parameters for various things.

So "displaySurface" is a valid MediaTrackConstraint (althought only in the context of getDisplayMedia) and so appears in getSupportedConstraints. It is interesting how things that relate to either audio or video are instead added at the options e.g. systemAudio, instead of being added as a media track constraint. Is that something to do with this line:

"Constraints serve a different purpose in getDisplayMedia than they do in getUserMedia(). They do not aid discovery, instead they are applied only after user-selection."

anyway, with regards to getSupportedOptions, yes it is does seem like a useful idea, especially since things that act in a constraint like way are included in the options and so it is hard to feature detect them otherwise.

eladalon1983 commented 1 year ago

It is interesting how things that relate to either audio or video are instead added at the options e.g. systemAudio, instead of being added as a media track constraint.

Options like selfBrowserSurface, systemAudio, etc., affect the capture-session, not the track. One way to think of it is - would it ever make sense to applyConstraints({...selfBrowserSurface: "exclude", ...}). It wouldn't, so these aren't MediaStreamTrack constraints.

Coread commented 1 year ago

Options like selfBrowserSurface, systemAudio, etc., affect the capture-session, not the track. One way to think of it is - would it ever make sense to applyConstraints({...selfBrowserSurface: "exclude", ...}). It wouldn't, so these aren't MediaStreamTrack constraints.

Thanks for the explanation of the distinction between constraints and options.

I think displaySurface is an odd case. Because it merely suggests which surface is to be displayed, it does not have the effect of a constraint. However it is a property of the resulting media track.

To me it feels slightly strange to have displaySurface as a constraint, but then have something that controls which display surface e.g. the proposed monitorTypeSurfaces be an option. What do you think?

eladalon1983 commented 1 year ago

I think displaySurface is an odd case. Because it merely suggests which surface is to be displayed, it does not have the effect of a constraint. However it is a property of the resulting media track.

To me it feels slightly strange to have displaySurface as a constraint, but then have something that controls which display surface e.g. the proposed monitorTypeSurfaces be an option. What do you think?

I think displaySurface is a constraint for historical reasons. Namely, I think that it was originally intended to report what the user chose through track.getSettings().displaySurface.

We have since also specified a meaning for displaySurface as a constraint. This has since been implemented by some user agents (Chromium-based browsers) and is likely relied upon by Web developers. So I think we should stay away from changing this meaning, lest we unnecessarily break existing Web applications.

Instead, I think it would be good to follow the pattern set forth by selfBrowserSurface, which is similar enough. That's my opinion, anyway. But perhaps we should discuss this on #261, and leave the current issue for discussions of getSupportedOptions().

youennf commented 1 year ago

Feature detection can be used for these preferences, for instance https://jsfiddle.net/vayrLp85/ is using the fact getDisplayMedia should reject with InvalidStateError outside of a user gesture in Safari. Other methods could be used by relying on either error name or error message.

youennf commented 1 year ago

There are other errors that could be used, which might be more robust, capture controller being already bound for instance, or the use of advanced constraints.

eladalon1983 commented 1 year ago

Feature detection can be used for these preferences, for instance https://jsfiddle.net/vayrLp85/ is using the fact getDisplayMedia should reject with InvalidStateError outside of a user gesture in Safari.

Thanks for the jsfiddle. I'm afraid that it raises more questions for me than it answers.

And if we do find a half-way robust way to feature-detect - which we haven't so far - why would we force Web developers to resort to such brittle hacks? Why should we not define an elegant, ergonomic, standard way of detecting which options are supported?

There are other errors that could be used, which might be more robust, capture controller being already bound for instance, or the use of advanced constraints.

All of these make needless assumptions about the implementation. Assumptions which break in novel and exciting ways on older versions of some browsers, yet miraculously somehow "work" on even older versions of the same browser. Why would we subject Web developers to this torture?

youennf commented 1 year ago

This is just a quick and simple example that illustrates the trick that libraries like adapter.js could use. Here is a 100% reliable version based on WebIDL dictionary conversion algorithm: https://jsfiddle.net/j6xgc59k/ This is honestly much better that I thought it would be.

eladalon1983 commented 1 year ago

Thanks for answering the "how?" Could you please answer "why?"

youennf commented 1 year ago

If an API can be done in JavaScript in a simple manner, as is the case here, why should the UA bother to implement it?

eladalon1983 commented 1 year ago

~(Btw, it's not "100% reliable". For instance, it requires a transient activation. What if the app wants to feature-detect on load, before deciding which UX to show to the user?)~ Edit: I was probably wrong here.

eladalon1983 commented 1 year ago

If an API can be done in JavaScript in a simple manner, as is the case here, why should the UA bother to implement it?

  1. This is not that simple.
  2. ~See previous comment - it still requires transient activation.~ Edit: I was probably wrong here.
youennf commented 1 year ago

This shim does not require transient activation to work as per spec. The way it works is as follows:

  1. When JS calls getDisplayMedia, the UA will convert the JS parameters using WebIDL.
  2. If the conversion fails, the promise will be rejected with the conversion error.
  3. If the conversion does not fail, the getDisplayMedia algorithm will start to be executed. This is where transient activation will be checked and may fail or not. The jsfiddle will never get to step 3 since we ensure the conversion to WebIDL will fail.
jan-ivar commented 1 year ago

Thanks for the jsfiddle! That seems sufficient to me.

Regarding "why", the bar for custom feature detection APIs should be extremely high IMHO, because it's a problem that disappears over time. IOW, if it can be shimmed in JS at all, then no new API should be needed.

I believe this opinion is generally shared, as MDN is filled with examples like addEventListener's passive option.

We have mediaDevices.getSupportedConstraints()

It exists specifically to solve the narrow problem where non-failure of an exact "constraint being ignored due to lack of support in a user agent is not tolerated by the application". Since exact constraints are largely deprecated in new specs, there should be no need to extend this pattern.

youennf commented 1 year ago

It exists specifically to solve the narrow problem where non-failure of an exact "constraint being ignored due to lack of support in a user agent is not tolerated by the application". Since exact constraints are largely deprecated in new specs, there should be no need to extend this pattern.

Now that MediaStreamTrack can be created outside of getUserMedia in all browsers (say via peer connection), mediaDevices.getSupportedConstraints() is no longer strictly needed to solve this narrow problem.

Also, this raises the question whether we should continue to extend MediaTrackSupportedConstraints for new constraints. We are doing this in mediacapture-extensions currently.

eladalon1983 commented 1 year ago

Regarding "why", the bar for custom feature detection APIs should be extremely high IMHO, because it's a problem that disappears over time.

This problem will NOT disappear over time:

I do not anticipate WebKit implementing it, at least in a foreseeing future. [sic]
- Youenn, speaking of surfaceSwitching

I don't think the code in the JS fiddle is a reasonable burden to place on Web developers. It's a non-trivial hack that relies on highly involved details about execution order and the implementation of WebIDL bindings.

jan-ivar commented 1 year ago

It seems no worse than the MDN example I gave.

eladalon1983 commented 1 year ago

It seems no worse than the MDN example I gave.

~You linked to multiple pages. The MDN one has 16 pages. Could you please be more specific?~ (Edited)

~Examining the link, it seems like it was a deep-link whose target changed. Was it here you were linking? (Hope it does not change again by the time you read this.)~ (Edited)

What do you know? The link works correctly on Firefox, but not on Chrome. (28th edit: Works for François but not me. Super weird. [29th update: Resetting zoom fixes it.]) For future reference, here's what you pointed out:

/* Feature detection */
let passiveIfSupported = false;

try {
  window.addEventListener(
    "test",
    null,
    Object.defineProperty({}, "passive", {
      get() {
        passiveIfSupported = { passive: true };
      },
    })
  );
} catch (err) {}

window.addEventListener(
  "scroll",
  (event) => {
    /* do something */
    // can't use event.preventDefault();
  },
  passiveIfSupported
);

Back on topic - that looks horrible, and I don't see why we'd want to replicate this pattern.

jan-ivar commented 6 months ago

Back on topic - that looks horrible, and I don't see why we'd want to replicate this pattern.

The point of showing the worst one is showing how high the bar is for adding a dedicated feature detection API.

The bar here is only that it needs to be detectable. Youenn's fiddle shows that it is, so I suggest we close this.