-
Notifications
You must be signed in to change notification settings - Fork 62
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
Fix export of track-muted and set-track-muted. #986
Conversation
Fixes #985 |
We have other uses of |
I briefly considered it, but then realized any export of generic terms like "source" might conflict with other specs. I also considered that maybe we shouldn't export everything by default, and that it might be better to do so on a need-to basis. |
Raising the @dontcallmedom signal for advice. |
Hmm that didn't work either
Trying @youennf's version next |
|
@dontcallmedom ah thanks, will try that next |
looking at fixing the markup, there is an ambiguity in the language of the spec: sometimes "muted" is applied to the concept of source (e.g. "If source is stopped or muted, abort these steps"), sometimes to the concept of track (e.g. "A track sourced by a camera or microphone may be forcibly muted"). The I can propose a fix once this is clarified. |
I think the idea was that "muted" refers to something that happens at the source, and is JS-visible through the "muted" attribute of the track (the source is ~never JS-visible). So "a track is muted" should probably be reworded as "the muted attribute is set on the track". |
I think that's largely an internal inconsistency, unrelated to exporting what to other specs is a track-related concept:
The error seems to come from the desire to add
Which reminds me there's also
This same triad also exists for
...which makes me question moving the muted concept to the source as a solution to the export problem. How do other specs do this? E.g. HTML appears to have done away with internal slots? What's the state of the art here?
The desire to add - [=set a track's muted state=]
+ [=MediaStreamTrack/set a track's muted state=] ...in this spec and all other specs that reference it. Choices here seem to be:
|
Also creates (unexported) definition for muted source
exporting I've gone ahead and:
This will break autolinks in specs that were relying on the definitions with their existing
|
[= source/stopped =].</p> | ||
<p>After the application has invoked the {{MediaStreamTrack/stop()}} | ||
method on a {{MediaStreamTrack}} object, or once the [=source=] of a | ||
{{MediaStreamTrack}} permanently ends production of live samples to its tracks, | ||
whichever is sooner, a {{MediaStreamTrack}} is said to be | ||
<dfn id="track-ended" data-dfn-for="track" data-export>ended</dfn>.</p> | ||
<dfn id="track-ended" data-dfn-for="MediaStreamTrack" data-dfn-type="dfn" data-export>ended</dfn>.</p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<dfn id="track-ended" data-dfn-for="MediaStreamTrack" data-dfn-type="dfn" data-export>ended</dfn>.</p> | |
<dfn data-dfn-for="MediaStreamTrack" class="export">ended</dfn>.</p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the id is used in other specs, I see no reason to break it; the dfn-type
is needed to distinguish it from the IDL attribute of the name and scope
<dfn id="track-ended" data-dfn-for="MediaStreamTrack" data-dfn-type="dfn" data-export>ended</dfn>.</p> | |
<dfn id="track-ended" data-dfn-for="MediaStreamTrack" data-dfn-type="dfn" class=export>ended</dfn>.</p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm... is it defined twice for the two kinds?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
once per kind, yes
<p><dfn class="export" data-dfn-for="MediaStreamTrack" data-dfn-type="dfn" id= | ||
"track-muted">Muted</dfn> refers to the input to the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<p><dfn class="export" data-dfn-for="MediaStreamTrack" data-dfn-type="dfn" id= | |
"track-muted">Muted</dfn> refers to the input to the | |
<p><dfn class="export" data-dfn-for="MediaStreamTrack">Muted</dfn> refers to the input to the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
likewise, dfn-type
is needed to distinguish from the IDL attribute
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I guess... feel free to resolve.
However, I think that might be confusing (even though it's correct)... Maybe there should be a "media stream track" concept?
@@ -954,7 +955,7 @@ <h4>Media Flow</h4> | |||
<var>track</var>.</p> | |||
</li> | |||
</ol> | |||
<p><dfn data-export id="track-enabled" data-lt="track enabled state|enabled" data-lt-noDefault>Enabled/disabled</dfn> on the other hand is | |||
<p><dfn data-export id="track-enabled" data-dfn-for="MediaStreamTrack" data-dfn-type="dfn" data-lt="track enabled state|enabled" data-lt-noDefault>Enabled/disabled</dfn> on the other hand is |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<p><dfn data-export id="track-enabled" data-dfn-for="MediaStreamTrack" data-dfn-type="dfn" data-lt="track enabled state|enabled" data-lt-noDefault>Enabled/disabled</dfn> on the other hand is | |
<p><dfn class="export" data-dfn-for="MediaStreamTrack" data-lt="track enabled state|enabled" data-lt-noDefault>Enabled/disabled</dfn> on the other hand is |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jan-ivar, the data-lt="track enabled state|enabled"
doesn't make much sense. This should really be:
<p><dfn class="export" data-dfn-for="MediaStreamTrack">track state</dfn> can be either "enabled" or "disabled"...
See how HTML handles Document/visibility state:
which is either "hidden" or "visible", initially set to "hidden".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is unchanged by this PR, and I'm not sure what's the current respec way of doing things. @dontcallmedom can I bother you to address the feedback from @marcoscaceres here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<p><dfn data-export id="track-enabled" data-dfn-for="MediaStreamTrack" data-dfn-type="dfn" data-lt="track enabled state|enabled" data-lt-noDefault>Enabled/disabled</dfn> on the other hand is | |
<p>The <dfn data-export id="track-enabled" data-dfn-for="MediaStreamTrack" data-dfn-type="dfn" >enabled</dfn> (resp. disabled) state on the other hand is |
Co-authored-by: Marcos Cáceres <[email protected]>
Thanks @dontcallmedom for the commits! |
Co-authored-by: Marcos Cáceres <[email protected]>
Fixes #985
@marcoscaceres is this correct syntax?
Preview | Diff