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

Clarify the cropTarget minting processing model #47

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Clarify the cropTarget minting processing model #47

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1d57516
Clarify the cropTarget minting processing model
yoavweiss May 19, 2022
db20bf1
typos
yoavweiss May 19, 2022
b57b906
Review comments
yoavweiss May 19, 2022
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
154 changes: 84 additions & 70 deletions index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ <h3>Practical Use-Case</h3>
</p>
</section>
</section>
<section id="scope">
<section id="scope" class="informative">
<h2>Scope</h2>
<p>
As presently defined, {{BrowserCaptureMediaStreamTrack/cropTo}}(<var>cropTarget</var>) returns a
Expand All @@ -97,7 +97,7 @@ <h2>Scope</h2>
browsing context=] breaks off the capture, and therefore also the cropping session.
</p>
</section>
<section id="solution-overview">
<section id="solution-overview" class="informative">
<h2>Solution Overview</h2>
<p>The region-capture mechanism comprises two parts:</p>
<ol>
Expand All @@ -119,7 +119,7 @@ <h2>Solution Overview</h2>
</section>
<section id="produce-crop-target">
<h2><dfn>CropTarget Production</dfn></h2>
<section id="crop-target-motivation">
<section id="crop-target-motivation" class="informative">
<h3>CropTarget Motivation</h3>
<p>
The [=cropping mechanism=] presented in this document
Expand Down Expand Up @@ -151,10 +151,9 @@ <h3><dfn>CropTarget</dfn> Definition</h3>
// Intentionally empty; just an opaque identifier.
};
</pre>
<div class="note">
<div class="issue" data-number="18">
<p>
There is no consensus yet on the name for {{CropTarget}}. This is under discussion in
<a href="https://github.com/w3c/mediacapture-region/issues/18">issue #18</a>.
There is no consensus yet on the name for {{CropTarget}}.
</p>
</div>
<dl data-link-for="CropTarget" data-dfn-for="CropTarget"></dl>
Expand All @@ -166,20 +165,43 @@ <h3><dfn>CropTarget</dfn> Definition</h3>
<li>
<p>Let <var>cropTarget</var> be a new object of type {{CropTarget}}.</p>
</li>
<li> <p>Let <var>weakRef</var> be a weak reference to <var>element</var>.</p></li>
<li>
<p>Let <var>weakRef</var> be a weak reference to <var>element</var>.</p>
<p>
[=Create a CropTarget|Create=] <var>cropTarget</var>.[[\Element]] initialized to
<var>weakRef</var>.
</p>
<p> Set <var>cropTarget</var>.[[\Element]] to <var>weakRef</var>. </p>
<div class="note">
<p>
<var>cropTarget</var> keeps a weak reference to the element it represents. In other
words, <var>cropTarget</var> will not prevent garbage collection of its element.
</p>
</div>
</li>
<li>
<p>Set |cropTarget|.[[\top-level browsing context]] to [=this=]'s [=browsing context=]'s
[=top-level browsing context=].</p>
</li>
<li>
<p>Let |promise| be [=a new promise=].</p>
</li>
<li>
<p>Run the following steps in parallel:</p>
<ol>
<li><p>In an [=implementation-defined=] way, propagate state associated with |cropTarget|.</p></li>
<li>
<p>If state propagation was successful, [=resolve=] |promise|. Otherwise, [=reject=] |promise|.</p>
Copy link
Member

@jan-ivar jan-ivar May 19, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

This is under heavy discussion in #17 and there's no concensus to merge this. Web developers aren't going to expect this to fail, and failures are going to be surpassing and browser dependent, which is bad for web compat.

This also cements an inferior approach to this problem, which I've argued is actually slower than the alternative, a claim no one has challenged yet.

Copy link
Author

Choose a reason for hiding this comment

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

This is under heavy discussion in #17

Please note the PR's description.

Web developers aren't going to expect this to fail

Why?

This also cements an inferior approach to this problem

https://www.w3.org/Consortium/cepc/#expected-behavior

which I've argued is actually slower than the alternative, a claim no one has challenged yet.

#17 (comment)

Copy link
Contributor

Choose a reason for hiding this comment

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

#48 is where this discussion is happening. Please comment there.

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
<p>If state propagation was successful, [=resolve=] |promise|. Otherwise, [=reject=] |promise|.</p>
<p>[=resolve=] |promise|.</p>

<p class="note">
The user agent needs to [=resolve=] or [=reject=] |promise| only after it has finished all
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
The user agent needs to [=resolve=] or [=reject=] |promise| only after it has finished all
The user agent needs to [=resolve=] |promise| only after it has finished all

the necessary internal propagation of state associated with the new {{CropTarget}}, at
which point the user agent it is ready to receive the |cropTarget| as a valid
parameter to {{BrowserCaptureMediaStreamTrack/cropTo}}.
</p>
</li>
</ol>
</li>
<li>
<p>Return |promise|.</p>
</li>
</ol>

<p>
{{CropTarget}} objects are serializable. The [=serialization steps=], given
<var>value</var>, <var>serialized</var>, and a boolean <var>forStorage</var>, are:
Expand All @@ -205,6 +227,19 @@ <h3><dfn>CropTarget</dfn> Definition</h3>
</p>
</li>
</ol>

<p> When asked to run <dfn>is crop target valid input</dfn>, given {{cropTarget}} |target| as input, run the following
steps:</p>
<ol>
<li>
<p>
If |target|'s [[\top-level browsing context]] equals [=this=]'s [=browsing context=]'s
[=top-level browsing context=], return true.
</p>
</li>
<li><p>Otherwise, return false.</p></li>
</ol>

</section>
<section id="producecroptarget-method">
<h3>MediaDevices.produceCropTarget</h3>
Expand All @@ -219,48 +254,33 @@ <h3>MediaDevices.produceCropTarget</h3>
<dfn>produceCropTarget()</dfn>
</dt>
<dd>
<p>
<p class="note">
Calling {{MediaDevices/produceCropTarget}} on an {{Element}} of a supported type
associates that {{Element}} with a {{CropTarget}}. This {{CropTarget}} may be used as
input to {{BrowserCaptureMediaStreamTrack/cropTo}}. We define a
<dfn>valid CropTarget</dfn> as one returned by a previous call to
{{MediaDevices/produceCropTarget()}} in the current [=top-level browsing context=] or
any of its
<a data-cite="HTML#list-of-the-descendant-browsing-contexts"
>descendant browsing contexts</a
>.
input to {{BrowserCaptureMediaStreamTrack/cropTo}}.
</p>
<p>
When {{MediaDevices/produceCropTarget}} is called on a given <var>element</var>, the
user agent [=create a CropTarget|creates a CropTarget=] with <var>element</var> as
input. The user agent MUST return a {{Promise}} <var>p</var>. The user agent MUST
resolve <var>p</var> only after it has finished all the necessary internal propagation
of state associated with the new {{CropTarget}}, at which point the user agent MUST be
ready to receive the new {{CropTarget}} as a valid parameter to
{{BrowserCaptureMediaStreamTrack/cropTo}}.
input and returns the result.
</p>
<p>
<p class="note">
When cloning an {{Element}} on which {{MediaDevices/produceCropTarget}} was previously
called, the clone is not associated with any {{CropTarget}}. If
{{MediaDevices/produceCropTarget}} is later called on the clone, a new {{CropTarget}}
will be assigned to it.
</p>
</dd>
</dl>
<div class="note">
<p>There is no consensus yet on the following issues:</p>
<ul>
<li>
Whether <code>produceCropTarget()</code> should be exposed on instances of
{{MediaDevices}} or on instances of {{Element}}. This is under discussion in
<a href="https://github.com/w3c/mediacapture-region/issues/11">issue #11</a>.
</li>
<li>
Whether {{MediaDevices/produceCropTarget()}} should return a {{CropTarget}} or a
{{Promise}}&lt;{{CropTarget}}&gt;. This is under discussion in
<a href="https://github.com/w3c/mediacapture-region/issues/17">issue #17</a>.
</li>
</ul>
<div class="issue" data-number="11">
<p>There is no consensus whether <code>produceCropTarget()</code> should be exposed on instances of
{{MediaDevices}} or on instances of {{Element}}.
</p>
</div>
<div class="issue" data-number="17">
<p>There is no consensus whether {{MediaDevices/produceCropTarget()}} should return a {{CropTarget}} or a
{{Promise}}&lt;{{CropTarget}}&gt;.
</p>
</div>
</section>
</section>
Expand Down Expand Up @@ -309,53 +329,47 @@ <h3>BrowserCaptureMediaStreamTrack</h3>
following algorithm:
</p>
<ol>
<li>Let <var>p</var> be a new {{Promise}}.</li>
<li>
If [=this=] is not a [=self-capture video track=], the user agent MUST return a new
{{Promise}}, [=rejected=] with an {{NotSupportedError}}.
If [=this=] is not a [=self-capture video track=], return |p|, [=rejected=] with an
{{NotSupportedError}}.
</li>
<li>
<p>
The user agent MUST validate <var>cropTarget</var> according to [=this=] track's
current [=crop-state=].
</p>
<ul>
<li>
If [=this=] track is [=uncropped=], the user agent MUST only accept [=valid
CropTargets=].
</li>
<li>
If [=this=] track is [=cropped=], the user agent MUST accept either [=valid
CropTargets=] or {{undefined}}.
</li>
</ul>
<p>
If the user agent does not accept <var>cropTarget</var>, return a {{Promise}}
[=rejected=] with an {{UnknownError}}.
</p>
Let |valid| be the result of running [=is crop target valid input=] on
|cropTarget|.
</li>
<li>
Let |success| be a [=boolean=], initially set to false.
</li>
<li>
If [=this=]'s track is [=uncropped=] and |valid| is true, set |success| to true.
</li>
<li>
If [=this=]'s track is [=cropped=], and |cropTarget| is either {{undefined}} or |valid|
is true, set |success| to true.
</li>
<li>
If |success| is false, return |p| [=rejected=] with an {{UnknownError}}.
</li>
<li>Let <var>p</var> be a new {{Promise}}.</li>
<li>
<p>Run the following steps in parallel:</p>
<ol>
<li>
If <var>cropTarget</var> is neither {{undefined}} nor a [=valid CropTarget=],
reject <var>p</var> with a {{NotAllowedError}} and abort these steps.
</li>
<li>
<p>
If <var>cropTarget</var> is either {{undefined}} or a [=valid CropTarget=],
the user agent MUST update [=this=] video track's [=crop-state=] according to
<var>cropTarget</var>:
[=Assert=]: <var>cropTarget</var> is either {{undefined}} or calling [=is crop
target valid input=] returns false.
</p>
<p>
Update [=this=] video track's [=crop-state=] according to <var>cropTarget</var>:
</p>
<ul>
<li>
If <var>cropTarget</var> is set to {{undefined}}, the user agent MUST stop
cropping. [=This=] video track reverts to the [=uncropped=] state.
</li>
<li>
If <var>cropTarget</var> is a [=valid CropTarget=], the user agent MUST
start cropping [=this=] video track to the contours of the element
referenced by this {{CropTarget}}. This means that for each new frame
Otherwise, the user agent MUST start cropping [=this=] video track to the
contours of the element referenced by this {{CropTarget}}. This means that for each new frame
produced on the track, the user agent calculates the bounding box of the
pixels belonging to the element, and crops the frame to the coordinates of
this bounding box.
Expand Down