SetRemoteDescription: Fire events and resolve promises synchronously |
|||
Issue descriptionPer-spec, https://w3c.github.io/webrtc-pc/#set-description, immediately after tracks have been added/removed as part of the SetRemoteDescription steps all ontrack events should fire and then the promise should be resolved. This occurs after SRD has updated all the media states (e.g. if you inspect the pc in the first ontrack event you will be able to find the second track that was added before the second ontrack event fires) but it does happen synchronously. The current implementation uses ScheduleDispatchEvent and/or PostTask to avoid prematurely firing the events (e.g. otherwise inspecting the pc in the first ontrack would not reveal the second track). The delayed event firing/promise resolving has some rarely triggered but observable differences from the spec behavior. To resolve this issue: - Each time a track is added/removed, add the resulting track event to a queue to be fired at the end of the SRD steps. - File spec issue to discuss doing the same with regards to events on MediaStream (onaddstream/onremovestream) and MediaStreamTrack (onmute/onunmute) and implement that too.
,
Nov 26 2017
,
Nov 27 2017
,
Nov 29 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5366645016a98a9c9c69d5b729bb5d0d1d7da6c6 commit 5366645016a98a9c9c69d5b729bb5d0d1d7da6c6 Author: Henrik Boström <hbos@chromium.org> Date: Wed Nov 29 12:48:47 2017 setRemoteDescripton: Unify SRD and events, fire in the correct order. This includes RTCPeerConnection.onaddtrack, MediaStream.onaddtrack/ onremovetrack and MediaStreamTrack.onmute. Instead of updating which tracks belong to which streams in an observer that listens to OnChanged in webrtc and PostTasks to the main thread once per stream, all stream states are updated in the SRD callback along with all the other state changes. Spec: https://w3c.github.io/webrtc-pc/#processing-remote-mediastreamtracks With this CL, https://crbug.com/777999 and https://crbug.com/webrtc/8473 are fixed. All events are currently scheduled to fire instead of firing synchronously (https://crbug.com/788558), so to make the promise resolve after all the events have fired we PostTask to resolve the promise. Bug: chromium:777999 , webrtc:8473 , chromium:788558 Change-Id: I21cce27b4e541ae6a53d6a9f91c73d181f3dd88e Reviewed-on: https://chromium-review.googlesource.com/789843 Commit-Queue: Henrik Boström <hbos@chromium.org> Reviewed-by: Guido Urdaneta <guidou@chromium.org> Cr-Commit-Position: refs/heads/master@{#520084} [modify] https://crrev.com/5366645016a98a9c9c69d5b729bb5d0d1d7da6c6/content/renderer/media/rtc_peer_connection_handler.cc [modify] https://crrev.com/5366645016a98a9c9c69d5b729bb5d0d1d7da6c6/content/renderer/media/webrtc/webrtc_media_stream_adapter.cc [modify] https://crrev.com/5366645016a98a9c9c69d5b729bb5d0d1d7da6c6/content/renderer/media/webrtc/webrtc_media_stream_adapter.h [modify] https://crrev.com/5366645016a98a9c9c69d5b729bb5d0d1d7da6c6/content/renderer/media/webrtc/webrtc_media_stream_adapter_map.h [modify] https://crrev.com/5366645016a98a9c9c69d5b729bb5d0d1d7da6c6/content/renderer/media/webrtc/webrtc_media_stream_adapter_unittest.cc [modify] https://crrev.com/5366645016a98a9c9c69d5b729bb5d0d1d7da6c6/third_party/WebKit/LayoutTests/external/wpt/webrtc/RTCPeerConnection-setRemoteDescription-tracks.https-expected.txt [modify] https://crrev.com/5366645016a98a9c9c69d5b729bb5d0d1d7da6c6/third_party/WebKit/LayoutTests/external/wpt/webrtc/RTCPeerConnection-setRemoteDescription-tracks.https.html [modify] https://crrev.com/5366645016a98a9c9c69d5b729bb5d0d1d7da6c6/third_party/WebKit/LayoutTests/fast/peerconnection/RTCPeerConnection-ontrack.html
,
Sep 26
,
Oct 1
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/92605c16c6f845a8ded3b018ac5adbc511446a5e commit 92605c16c6f845a8ded3b018ac5adbc511446a5e Author: Henrik Boström <hbos@chromium.org> Date: Mon Oct 01 08:39:32 2018 [Unified Plan] Remote MediaStreamTracks should be muted by default. Per-spec, tracks that are created with a receiver (e.g. addTrack or setRemoteDescription) are muted by default. Prior to this CL they were unmuted by default, whether or not they were receiving any packets. A correct implementation should unmute the tracks when RTP packets arrive. We are not quite there yet, this CL assumes that if the receiver becomes active through renegotiation it will unmute. We are careful to make sure that the track is muted on the "ontrack" event so that the application has time to wire up the "onunmute" event. By unmuting as part of processing SDP we fix the Unified Plan bug where a remote track that had previously been muted was not unmuted when becoming active again (transciever.currentDirection == 'sendrecv' or 'recvonly'), https://crbug.com/884023 . This CL also makes "ontrack" fire synchronously per-spec, https://crbug.com/788558. Note that some stream events still fire asynchronously, which means they now fire after the "ontrack" event in the Unified Plan case. This is remaining work on that bug. A new file is created to test muting related behaviors, and some helper functions used this and another file are moved to RTCPeerConnection-helper.js. Bug: 884023 , 777619, 788558 Change-Id: I8dc3e2adf04e72282f085779639edc73bacfc86b Reviewed-on: https://chromium-review.googlesource.com/1249066 Commit-Queue: Henrik Boström <hbos@chromium.org> Reviewed-by: Guido Urdaneta <guidou@chromium.org> Cr-Commit-Position: refs/heads/master@{#595405} [modify] https://crrev.com/92605c16c6f845a8ded3b018ac5adbc511446a5e/third_party/WebKit/LayoutTests/TestExpectations [modify] https://crrev.com/92605c16c6f845a8ded3b018ac5adbc511446a5e/third_party/WebKit/LayoutTests/external/wpt/webrtc/RTCPeerConnection-helper.js [add] https://crrev.com/92605c16c6f845a8ded3b018ac5adbc511446a5e/third_party/WebKit/LayoutTests/external/wpt/webrtc/RTCPeerConnection-remote-track-mute.https.html [modify] https://crrev.com/92605c16c6f845a8ded3b018ac5adbc511446a5e/third_party/WebKit/LayoutTests/external/wpt/webrtc/RTCPeerConnection-transceivers.https-expected.txt [modify] https://crrev.com/92605c16c6f845a8ded3b018ac5adbc511446a5e/third_party/WebKit/LayoutTests/external/wpt/webrtc/RTCPeerConnection-transceivers.https.html [modify] https://crrev.com/92605c16c6f845a8ded3b018ac5adbc511446a5e/third_party/WebKit/LayoutTests/virtual/webrtc-wpt-unified-plan/external/wpt/webrtc/RTCPeerConnection-addTransceiver.https-expected.txt [modify] https://crrev.com/92605c16c6f845a8ded3b018ac5adbc511446a5e/third_party/WebKit/LayoutTests/virtual/webrtc-wpt-unified-plan/external/wpt/webrtc/RTCPeerConnection-transceivers.https-expected.txt [modify] https://crrev.com/92605c16c6f845a8ded3b018ac5adbc511446a5e/third_party/blink/renderer/modules/peerconnection/rtc_peer_connection.cc |
|||
►
Sign in to add a comment |
|||
Comment 1 by hbos@chromium.org
, Nov 26 2017