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.

Sign up for GitHub

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

Jump to bottom

Added DisplaySurfaceChangeCallback #289

Open
wants to merge 1 commit into
base: gh-pages
Choose a base branch
from
Open

Added DisplaySurfaceChangeCallback #289

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
228 changes: 211 additions & 17 deletions index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -409,25 +409,37 @@ <h2><dfn>MediaDevices</dfn> Additions</h2>

<li>
<p>[=Prompt the user to choose=] a display device, for a
{{PermissionDescriptor}} with its
{{PermissionDescriptor/name}} set to "display-capture",
resulting in a set of provided media.</p>
{{PermissionDescriptor}} with its
{{PermissionDescriptor/name}} set to "display-capture",
resulting in a <dfn>capture-source-set</dfn>,
<var>sources</var>, that meets the following
requirements:</p>


<p>The provided media MUST include precisely one video
track.</p>
<ol>
<li>
<p>The [=capture-source-set=] MUST include precisely one
video track.</p>
</li>


<p>The provided media MUST include at most one audio track.
This audio track MUST NOT be included if audio was not
specified in <var>requestedMediaTypes</var>, or if it was
specified as <code>false</code>.</p>
<li>
<p>The [=capture-source-set=] MUST include at most one
audio track. This audio track MUST NOT be included if
audio was not specified in
<var>requestedMediaTypes</var>, or if it was specified
as <code>false</code>.</p>
</li>


<p>The devices chosen MUST be the ones determined by the
user. Once selected, the source of a {{MediaStreamTrack}}
MUST NOT change, unless the user permits it through their
interaction with the user agent.</p>
<li>
<p>The sources in the [=capture-source-set=] MUST
represent the devices determined by the user. Once
selected, the source of a {{MediaStreamTrack}} MUST NOT
change, unless the user permits it through their
interaction with the user agent.</p>
</li>
</ol>


<p>User agents are encouraged to warn users against sharing
Expand All @@ -439,9 +451,9 @@ <h2><dfn>MediaDevices</dfn> Additions</h2>


<p>If the result of the request is
{{PermissionState/"granted"}}, then for each device that is
sourcing the provided media, using a stable and private id
for the device, <var>deviceId</var>, set
{{PermissionState/"granted"}}, then for each device with a
source in [=capture-source-set=], using a stable and private
id for the device, <var>deviceId</var>, set
[[\devicesLiveMap]]<var>[deviceId]</var> to
<code>true</code>, if it isn’t already <code>true</code>, and
set the [[\devicesAccessibleMap]]<var>[deviceId]</var> to
Expand Down Expand Up @@ -512,7 +524,8 @@ <h2><dfn>MediaDevices</dfn> Additions</h2>
<li>
<p>This invocation of {{MediaDevices/getDisplayMedia()}} is
now considered to have produced a new
<dfn>capture-session</dfn>.</p>
<dfn>capture-session</dfn>, with the associated
[=capture-source-set=], <var>sources</var>.</p>
</li>
Copy link
Member

Choose a reason for hiding this comment

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

Should the part below, which dealt with controller.[[Source]], be updated to reference the video source from capture-source-set rather than (as it does now) the "video track's source"?
(GitHub didn't let me attach this comment in the desired location.)



Expand Down Expand Up @@ -668,6 +681,110 @@ <h2 id="constraints">Unconstrained Display Surface Selection</h2>
</section>


<section>
<h2 id="changing-display-surfaces">Changing Display Surfaces</h2>


<p>
The user agent MUST NOT change the [=display surface=]
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think we can validate this, I would tend to reword so that we do not use MUST NOT.

associated with a [=capture-session=] unless the user has
explicitly indicated that they want this change to be
performed by interacting with user agent or operating system.
Copy link
Member

Choose a reason for hiding this comment

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

nit: Could you make editorial changes here so as to discourage anyone from misreading "by interacting" as binding to the wrong part of the sentence? Maybe "...performed. Users can do this by interacting with..."


If the user indicates a change of [=display surface=], the
Copy link
Collaborator

Choose a reason for hiding this comment

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

We probably need to specify the order of the callback execution and the configurationchange event at MediaStreamTrack.
I would tend to have the event be executed after the callback.

user agent MUST run the following steps:
</p>


<ol>
<li>
<p>If the [=capture-session=] does not have an associated
{{CaptureController}}, <var>controller</var>, abort these steps.</p>
</li>


<li>
<p>If <var>controller.{{CaptureController/[[DisplaySurfaceChangeCallback]]}}</var>
is null, abort these steps.</p>
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we should do this check later inside a task queued since CaptureController/[DisplaySurfaceChangeCallback] can be changed at any point in time by JS, so the checks should be done within the context event loop.

</li>


<li>
<p>Stop delivering frames from all sources in the
[=capture-source-set=] associated with the [=capture-session=].
Copy link
Collaborator

Choose a reason for hiding this comment

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

If audio source is not changing, should we allow audio to flow?

</p>
</li>


<li>
<p>Let <var>newSources</var> be the set of sources selected by the
user when the user chose a new [=display surface=].</p>


<p>
<var>newSources</var> MUST fulfill the same requirements as the
Copy link
Collaborator

Choose a reason for hiding this comment

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

It is not really clear to me what this MUST is about.

[=capture-source-set=] formed when {{MediaDevices/getDisplayMedia}}
is called.</p>
</li>


<li>
<p>Queue a task to:</p>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Will it have a link to the definition?
We should probably start using a task source as well.

<ol>
<li>
<p>
Set the [=capture-source-set=]
associated with the [=capture-session=] to <var>newSources</var>.
</p>
</li>


<li>
<p>Let <var>stream</var> be a new {{MediaStream}} with a new
{{MediaStreamTrack}} for each source in <var>newSources</var>.
</li>


<li>
<p>Run the [=ApplyConstraints algorithm=] on all tracks in
<var>stream</var> with the appropriate constraints. Should
this fail, abort these steps.</p>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why should we fail, user wants to switch, it is ok for the web page to update its constraints as part of the switch.

</li>


<li>
<p>Set
<var>controller</var>.{{CaptureController/[[Source]]}} to
the <var>stream</var>'s video track's
<a data-cite="GETUSERMEDIA#dfn-source-0">[[\Source]]</a>.</p>
</li>


<li>
<p>Set
<var>controller</var>.{{CaptureController/[[DisplaySurfaceType]]}}
to the <var>stream</var>'s video track's
{{DisplayCaptureSurfaceType}}.</p>
</li>


<li>
<p>Invoke
<var>controller</var>.{{CaptureController/[[DisplaySurfaceChangeCallback]]}}
with <var>stream</var> as the argument.</p>
</li>
</ol>
</li>

<li>
<p>Queue tasks to end all tracks connected to sources in the
[=capture-source-set=] associated with the [=capture-session=].
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think the default should be to leave the tracks alive, the future option being for the web page to ask for tracks to be ended.
The principle is that a web page can easily implement ending of the tracks, but it cannot implement the reverse.

</p>
</li>
</ol>
</section>


<section>
<h2 id="constrainable-properties">Constrainable Properties for Captured
Display Surfaces</h2>
Expand Down Expand Up @@ -1099,6 +1216,30 @@ <h2><dfn>CaptureStartFocusBehavior</dfn>
</div>
</section>

<section>
<h2><dfn>DisplaySurfaceChangeCallback</dfn></h2>
<p>
Used to receive a new {{MediaStream}} if the [=display surface=]
associated with a [=capture-session=] changes.
</p>
<div>
<pre class="idl" >callback DisplaySurfaceChangeCallback = undefined
(MediaStream stream);</pre>
<section>
<h2>Callback <a class="idlType">DisplaySurfaceChangeCallback</a>
Parameters</h2>
<dl data-link-for="NavigatorUserMediaSuccessCallback" data-dfn-for=
"NavigatorUserMediaSuccessCallback" class="callback-members">
<dt>stream of type {{MediaStream}}</dt>
<dd>
A new {{MediaStream}} with a new {{MediaStreamTrack}} for each
source in the [=capture-source-set=] associated with the
[=display surface=] that the user has selected.
</dd>
</dl>
</section>
</div>
</section>

<section>
<h2><dfn>CaptureController</dfn>
Expand All @@ -1124,6 +1265,7 @@ <h2><dfn>CaptureController</dfn>
interface CaptureController : EventTarget {
constructor();
undefined setFocusBehavior(CaptureStartFocusBehavior focusBehavior);
undefined setDisplaySurfaceChangeCallback(DisplaySurfaceChangeCallback callback);
};
</pre>

Expand Down Expand Up @@ -1226,6 +1368,19 @@ <h2><dfn>CaptureController</dfn>

<td>The focus behavior desired by the application.</td>
</tr>


<tr>
<td><dfn>[[\DisplaySurfaceChangeCallback]]</dfn>
</td>

<td><code>null</code>
</td>

<td>A callback called when the [=display surface=] of the
associated [=capture-session=] changes.</td>
</tr>

</tbody>
</table>

Expand Down Expand Up @@ -1294,6 +1449,45 @@ <h2><dfn>CaptureController</dfn>
</li>
</ol>
</dd>


<dt><dfn>setDisplaySurfaceChangeCallback</dfn>
</dt>


<dd>
<p>Run the following steps:</p>


<ol>
<li>
<p>Let <var>callback</var> be the method's first argument.</p>
</li>


<li>
<p>If [=this=].{{CaptureController/[[IsBound]]}} is
Copy link
Collaborator

Choose a reason for hiding this comment

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

It seems restrictive to disallow setting the callback more dynamically.
Why should we add this restriction?

<code>true</code>,
[=exception/throw=] an "{{InvalidStateError}}"
{{DOMException}}.</p>
</li>


<li>
<p>If
[=this=].{{CaptureController/[[DisplaySurfaceChangeCallback]]}}
is not <code>null</code>, [=exception/throw=] an
"{{InvalidStateError}}" {{DOMException}}.</p>
Copy link
Collaborator

Choose a reason for hiding this comment

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

I am not sure why we should not allow to overwrite the callback either.

Copy link
Member

@jan-ivar jan-ivar Oct 3, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

This seems different from w3c/mediacapture-screen-share-extensions#4 (comment):

Something like captureController.processSourceSwitch(stream => { ... }); or captureController.processSourceSwitch(null);

I’m fine with this.

</li>


<li>
<p>Set
[=this=].{{CaptureController/[[DisplaySurfaceChangeCallback]]}}
to <var>callback</var>.</p>
</li>
</ol>
</dd>
</dl>


Expand Down