Add wpt/webrtc/ MediaStreamTrack.onunmute test coverage |
||||
Issue descriptionPart of the setRemoteDescription steps is to unmute remote tracks that become unmuted and fire the onunmute event. This should be tested. The addTrack/removeTrack APIs are not enough to cover this. If a track is removed and then re-added, a new sender will be created for the caller, resulting in a new receiver and track for the callee. As such, the track that was previously muted is not the track that will be unmuted. replaceTrack does not cause mute/unmute either because it does not require renegotiating. In order to test this (without SDP munging etc) we need to have RTCRtpTransceivers whose direction we can change. Adding and passing these tests will be part of https://crbug.com/777617 . For now we are blocked on adding transceivers, later completing transceivers will be blocked on passing this.
,
Oct 24 2017
,
Oct 24 2017
+deadbeef: I believe replaceTrack does not affect renegotiation and therefore does not cause mute/unmute events, correct?
,
Oct 24 2017
Correct. Even in the "replaceTrack(null)" case, I don't think anything will happen aside from an absence of media packets being sent.
,
Oct 25 2017
Hm. I think you're right - there's nowhere in the spec (that I can find offhand) that changes the [[direction]] attribute of the transceiver when the track becomes null. Seems a bit odd, but I'm not going to try change it this week. (I was thinking that it might change the attribute despite not causing renegotiation, so that a subsequent renegotiation would pick it up, but it doesn't, so it won't.)
,
Oct 25 2017
I think anything that would cause SRD-related events to fire should have warranted onnegotiationneeded, if it is decided that direction should change.
,
Oct 26 2017
> Hm. I think you're right - there's nowhere in the spec (that I can find offhand) that changes the [[direction]] attribute of the transceiver when the track becomes null. This is by design; none of the RtpSender API calls should require renegotiation. They just affect what's being sent, within the envelope negotiated by SDP. Sending nothing when you've negotiated "sendrecv" is still allowed, right?
,
Nov 13 2017
FYI this comment was referenced from https://crbug.com/774303#c14 with regards to test coverage.
,
Feb 20 2018
If we fix https://crbug.com/webrtc/8730 we can test this without transceivers; the track is supposed to be muted by default.
,
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 hta@chromium.org
, Oct 24 2017