Separate getDisplayMedia() algorithm; reintroduce constraints. #73

jan-ivar · 2018-08-27T18:34:36Z

Fix for #49.

jan-ivar · 2018-08-27T18:53:27Z

Some explanation of this PR:

After deliberating with other editors, I gave up trying to preserve the call to the getUserMedia algorithm. A growing number of differences made it hard to deduce exactly what to implement. The lower-level algorithm calls are intact.

I also added Privacy Indicator Requirements, since these were only implied before, and with the explicit call to getUserMedia gone, I wanted to make sure we didn't lose them.

Finally, the content changes:

constraints are supported by getDisplayMedia again, with these caveats:
- Using advanced, min or exact constraints gives TypeError.
- Added note on reason for allowing max constraint.
Use the right permission algorithm (prompt the user to choose)
Disallow persisting "granted" permissions only.
Proposes "screen" as permission descriptor name (do we prefer "display"?)

alvestrand · 2018-08-27T20:46:20Z

index.html

+        permissions, only the "Live" indicators are significant.
+        </p>
+      </div>
+      <p>The User Agent MUST NOT implement the <code><a href=


"MUST NOT implement" sounds a bit strange. Would it be clearer to say "MUST NOT fire the devicechange event based on changes in the set of available sources for getDisplayMedia"?

What Harald said.

martinthomson

This looks pretty good. I think that we should settle on a single permission descriptor rather than two. I suggest "display", if only to match the API name. How browsers manage audio capture will be tricky enough in UX without having to worry about conjoined requests being hampered by permission discrepancies.

martinthomson · 2018-08-27T23:06:18Z

index.html

+                      "!HTML52/browsers.html#top-level-browsing-context">top-level
+                      browsing context</a>'s <a data-cite=
+                        "!HTML52/browsers.html#active-document">active document</a>'s
+                      origin.</p>


Wow, what a mouthful. It's correct, but do we not have a shorthand for this we can use instead?

That's existing prose copied from getUserMedia.

martinthomson · 2018-08-27T23:07:14Z

index.html

+                different from <var>originIdentifier</var>, set
+                <var>originIdentifier</var> to the result of combining
+                <var>originIdentifier</var> and the <a>current settings
+                  object</a>'s origin.</p>


I don't know what this means. (If this is a process defined in Feature Policy, then) you need a citation.

Again, copied from getUserMedia.

That doesn't make it good. It just means that you now have two incomprehensible statements.

@martinthomson As such it makes this a non-change as far as this PR is concerned. Can you open an issue on mediacapture-main, and can we take care of it that way? It

Just trying to unblock this PR on feature policy and w3c/mediacapture-main#434.

martinthomson · 2018-08-27T23:08:55Z

index.html

+                    a display device, with a PermissionDescriptor with its name
+                    member set to the permission name associated with
+                    <var>kind</var> for obtaining display devices (e.g. "screen"
+                    for video), resulting in a set of provided media.</p>


I think that "display" is better, and that we don't want to distinguish between video and audio capture in this context.

That's just my intuition here, so I'm open to arguments that suggest otherwise.

martinthomson · 2018-08-27T23:10:37Z

index.html

+                    <code><a>MediaStreamTrack</a></code> MUST NOT change.</p>
+                    <p>User Agents are encouraged to warn users against sharing
+                    <a>browser</a> display devices, or otherwise try to
+                    steer users away from sharing them.</p>


"or otherwise try to discourage their selection on the basis that these represent a significantly higher risk when shared"

martinthomson · 2018-08-27T23:15:00Z

index.html

+                    and set the
+                    [[\devicesAccessibleMap]]<var>[deviceId]</var> to
+                    <code>true</code>, if it isn’t already
+                    <code>true</code>.</p>


We need to establish what deviceId is here. My suggestion is that while it is valuable to interact with the gUM model in this way, you want to assign a unique deviceId every time a capture is started here, only reusing the identifier while the same source is active. That needs to be part of the preceding steps of the algorithm here.

This per-device permission stuff—which no-one implements btw—has already received (legitimate) flak for being overly complicated, so I worry about adding complexity to this. Since the deviceIds are never exposed, can we perhaps use the raw internal ids which are stable?

If the point here is to support indicators, then sure.

I think this needs clarification too.

martinthomson · 2018-08-27T23:17:28Z

index.html

+            The max constraint type lets a web application provide a maximum envelope
+            for constrainable properties like width and height, should the
+            end-user resize a <a>window</a> or <a>browser</a> surface while it
+            is being captured.


This is a reasonable conclusion, but the choice on min/max doesn't seem particularly principled. Are there other constraints that might need similar treatment?

Note that only max is allowed. min and exact are disallowed.

There's no selection, only downscaling/decimating. We never upscale, so no min needed.

Eliminating min also avoids us having to worry about OverconstrainedError after selection.

I don't anticipate constraints other than width and height needing this.
I don't anticipate needing other constraints types than min, given this repurposing.

You might explicitly state what constraints are allowed/disallowed.

@aboba I expect #31 will address that.

martinthomson · 2018-08-27T23:17:43Z

index.html

+        permissions, only the "Live" indicators are significant.
+        </p>
+      </div>
+      <p>The User Agent MUST NOT implement the <code><a href=


What Harald said.

jan-ivar · 2018-08-28T13:21:17Z

@martinthomson @alvestrand Review feedback incorporated PTAL.

henbos · 2018-08-30T14:09:22Z

index.html

-              values. This prevents an application from influencing the selection of sources, see
-              <a href="#constraints"></a> for details.
+              This method is similar to <code><a>getUserMedia</a></code>, except that it
+              acquires media from one display device chosen by the end-user each time.


"one display device" sounds like single-monitor only?

We might add a definition of "a display device", something like "In this context, a display device consists of all the display surfaces that are presented to the user to choose from."

I think this is covered by "Multiple monitors might also be aggregated into as a single logical monitor."

henbos

I put down several comments, but I think they can be addressed separately. This PR is blocking other stuff, and most of the nits are inherited from GUM.

henbos · 2018-09-06T12:59:06Z

index.html

+              <li>
+                <p>Let <var>requestedMediaTypes</var> be the set of media
+                types in <var>constraints</var> with either a dictionary
+                value or a value of <code>true</code>.</p>


"the set of media types in constraints" sounds like a specific member inside of constraints, can this be clarified? I think its meant to say that requestedMediaTypes is the set of the same members as in constraints but only those whose value is either a dictionary or the value true.

@henbos Copied from mediacapture-main.

"media type" seems defined by extension to be audio or video, referring here to the key names "audio" or "video" in e.g. constraints like {audio: true, video: {}}.

The type of elements in the set requestedMediaTypes doesn't seem super-relevant as long as it distinguishes audio vs. video and future types, though it seems safe to infer string IMHO. Please open an issue on mediacapture-main if you think this isn't clear.

It's really more of a template argument than a string I suppose. We do gymnastics like:

"For each media type T in requestedMediaTypes, ... If the value of the T entry of constraints is "true""

E.g. if unrolled, would read "if the value of the audio entry of constraints" and again for video.

henbos · 2018-09-06T13:22:09Z

index.html

+              <li>
+                <p>If <var>requestedMediaTypes</var> is the empty set, set
+                <var>requestedMediaTypes</var> to a set containing
+                <code>"video"</code>.</p>


Should this be "video:true"?

henbos · 2018-09-06T13:26:31Z

index.html

+                <p>Run the following steps in parallel:</p>
+                <ol>
+                  <li>
+                    <p>For each media type <var>T</var> in


Is "media type" well defined? Should it be or a link, or somewhere clarified that this is "audio" or "video"?

henbos · 2018-09-06T13:33:06Z

index.html

+                    <var>originIdentifier</var>, <a>prompt the user to choose</a>
+                    a display device, with a PermissionDescriptor named
+                    <code>"display"</code>, resulting in a set of
+                    provided media.</p>


'with a PermissionDescriptor named "display",'
I'm sure this was copied from GUM, but I don't know what this means, should we file an issue?

nit: '... or in "rejected".'?

henbos · 2018-09-06T13:36:12Z

index.html

+                    each media type in <var>requestedMediaTypes</var>.
+                    The devices chosen MUST be the ones determined by the user.
+                    Once selected, the source of a
+                    <code><a>MediaStreamTrack</a></code> MUST NOT change.</p>


This sentence may be up for interpretation and debatable. You may share an application, whose set of windows changes or resizes, or where the source is N/A during the time something is minimized, or we may add language about what to do if the set of monitors changes. Or something.

This is OK but we may need to revisit it.

henbos · 2018-09-06T13:37:21Z

index.html

+                    and set the
+                    [[\devicesAccessibleMap]]<var>[deviceId]</var> to
+                    <code>true</code>, if it isn’t already
+                    <code>true</code>.</p>


I think this needs clarification too.

henbos · 2018-09-06T13:39:00Z

index.html

+                    provided media.</p>
+                    <p>The provided media MUST include precisely one track of
+                    each media type in <var>requestedMediaTypes</var>.
+                    The devices chosen MUST be the ones determined by the user.


nit: Language like "devices" should be updated, we're talking about display surfaces.

henbos · 2018-09-06T13:40:05Z

index.html

+                    <a href="#create-permission">create a permission storage entry</a>
+                    with a value of "granted".
+                    </p>
+                    <p>If the result is "denied", jump to the step labeled


nit: I'm not sure its clear what the possible results are.

henbos · 2018-09-06T13:44:12Z

index.html

+                    "https://w3c.github.io/mediacapture-main/#dfn-applyconstraints-algorithm">
+                    <dfn>ApplyConstraints algorithm</dfn></a> on all
+                    tracks in <var>stream</var> with the appropriate
+                    constraints. Should this fail, let <var>failedConstraint</var>


nit: "with the appropriate constraints" should be clarified. As-is, both boolean and constraint set values are allowed (do we need to convert "true" to "{}"?), and it is not explicit about applying "audio" constraints to audio and "video" constraints to video. This may seem obvious but as an algorithm step it is vague.

henbos · 2018-09-06T13:46:28Z

index.html

+                  <li>
+                    <p>If <var>value</var> contains a member which in turn
+                    is a dictionary containing a member named either
+                    <code>min</code> or <code>exact</code>, return a promise


(If it turns out that there are meaningful differences between exact and ideal in the case of boolean constraints we can revisit this separately.)

henbos · 2018-09-06T14:43:04Z

Comments to be addressed separately.

Separate getDisplayMedia() algorithm; reintroduce constraints.

c9f1071

jan-ivar requested review from martinthomson and suhasHere August 27, 2018 18:34

alvestrand reviewed Aug 27, 2018

View reviewed changes

martinthomson reviewed Aug 27, 2018

View reviewed changes

Single "display" permission, leverage internal ids, more notes.

9d646ed

jan-ivar added 2 commits August 28, 2018 09:22

typo

194bef3

minor rewording

07023b5

jan-ivar mentioned this pull request Aug 29, 2018

Constraints for Captured Display Surfaces should move to MediaTrackSettings #66

Closed

henbos mentioned this pull request Aug 30, 2018

Be consistent with error handling #68

Closed

henbos reviewed Aug 30, 2018

View reviewed changes

jan-ivar mentioned this pull request Aug 30, 2018

A new permission for screen-sharing with getDisplayMedia() w3c/permissions#182

Closed

uysalere mentioned this pull request Sep 5, 2018

Extend constraints to constrainable patterns #75

Merged

henbos approved these changes Sep 6, 2018

View reviewed changes

aboba merged commit 5245ceb into w3c:gh-pages Sep 6, 2018

jan-ivar deleted the constraints branch September 6, 2018 14:42

jan-ivar mentioned this pull request Sep 6, 2018

Behavior for getDisplayMedia() and getDisplayMedia({}) should be the same #65

Closed

jan-ivar mentioned this pull request Sep 20, 2021

Reject unless fully-active and has focus. #192

Merged

jan-ivar mentioned this pull request Mar 9, 2022

Initial draft of getViewportMedia() w3c/mediacapture-viewport#3

Merged

Separate getDisplayMedia() algorithm; reintroduce constraints. #73

Separate getDisplayMedia() algorithm; reintroduce constraints. #73

Conversation

jan-ivar commented Aug 27, 2018

jan-ivar commented Aug 27, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

martinthomson left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jan-ivar commented Aug 28, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

henbos left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

henbos commented Sep 6, 2018