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

Set muted before SRD resolves, using new set muted algorithm. #1667

Merged
merged 4 commits into from Dec 7, 2017
Merged

Set muted before SRD resolves, using new set muted algorithm. #1667

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d0326e2
Set muted before SRD resolves, using new set muted algorithm.
jan-ivar Nov 17, 2017
65a5cd8
Missed s/update/set/ in two spots.
jan-ivar Nov 25, 2017
cca53ac
Remove parenthesis to avoid confusion.
jan-ivar Nov 25, 2017
c759665
Collect mute events and fire them right before track events.
jan-ivar Nov 27, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
27 changes: 14 additions & 13 deletions webrtc.html
View file
Open in desktop
Expand Up @@ -5294,7 +5294,7 @@ <h4>Processing Remote MediaStreamTracks</h4>
</li>
<li>
<p>If <var>track.muted</var> is <code>false</code>,
<a>update the muted state</a> of <var>track</var> with the value
<a>set the muted state</a> of <var>track</var> to the value
Copy link
Member Author

Choose a reason for hiding this comment

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

The "set muted state" steps are always queued anyhow

@adam-be Here's the one place we don't queue a task (the purpose of this PR), inside the process the removal of a remote track algorithm which itself is already called from a queued task, to solve #1568.

Copy link
Contributor

Choose a reason for hiding this comment

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

If we fire the event immediately, the event will be fired before all effects of the SRD has occurred - and inspecting the PC or related objects like sender/receiver/transceiver/streams/tracks will yield confusing results sensitive to the order of objects processed by this algorithm.

If we schedule a task to fire the event the event will occur after the SRD promise has resolved (which is inconsistent with ontrack for tracks being added) and could theoretically occur at a time when the muted state has already changed again.

We should do what we do for ontrack:
Here: Set the muted state, and if it changed as a result of this, enqueue mutedEvent on to mutedEvents.
Before SRD is resolved, but still synchronous to the rest of the SRD steps: For each mutedEvent in mutedEvents, fire mutedEvent.

It's important that all events fired in relation to an SRD are fired synchronously, but after the state of all objects has been updated, to ensure inspecting the state of the PC is meaningful.

<code>true</code>.</p>
</li>
</ol>
Expand Down Expand Up @@ -10618,21 +10618,22 @@ <h3>MediaStreamTrack</h3>
<p>A <code>MediaStreamTrack</code> object's reference to its
<code>MediaStream</code> in the non-local media source case (an RTP
source, as is the case for <code>MediaStreamTrack</code>s received over
an <code><a>RTCPeerConnection</a></code> ) is always strong.</p>
an <code><a>RTCPeerConnection</a></code>) is always strong.</p>
<p>When an <code><a>RTCPeerConnection</a></code> receives data on an RTP
source for the first time, it MUST <a>update the muted state</a> of the
corresponding <code><a>MediaStreamTrack</a></code> with the value
<code>false</code>.</p>
source for the first time, it MUST queue a task to
<a>set the muted state</a> of the corresponding
<code><a>MediaStreamTrack</a></code> to <code>false</code>.
</p>
<p>When one of the SSRCs for RTP source media streams received
by an <code><a>RTCPeerConnection</a></code> is removed (either
due to reception of a BYE or via timeout), it MUST <a>update
the muted state</a> of the corresponding
<code><a>MediaStreamTrack</a></code> with the value
<code>true</code>. If and when packets are received again,
the <a data-lt="update the muted state">muted state MUST be
updated</a> with the value <code>false</code>.</p>
<p>The procedure <dfn data-lt="update the muted state" id=
"update-track-muted">update a track's muted state</dfn> is specified in
due to reception of a BYE or via timeout), it MUST queue a task to
<a>set the muted state</a> of the corresponding
<code><a>MediaStreamTrack</a></code> to
<code>true</code>. If and when packets are received again, it must queue a
task to <a data-lt="update the muted state">set the muted state</a> back
to <code>false</code>.</p>
<p>The procedure <dfn data-lt="set the muted state" id=
"update-track-muted">set a track's muted state</dfn> is specified in
Copy link
Contributor

Choose a reason for hiding this comment

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

should this "update" also be changed to "set"?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes thanks for spotting that!

[[!GETUSERMEDIA]].</p>
<p>When a <code><a>MediaStreamTrack</a></code> track produced by
an <code><a>RTCRtpReceiver</a></code> <var>receiver</var> has
Expand Down