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.

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

On track removal: Should you mute an already muted track? #2506

Closed
henbos opened this issue Apr 3, 2020 · 18 comments
Closed

On track removal: Should you mute an already muted track? #2506

henbos opened this issue Apr 3, 2020 · 18 comments
Assignees

Comments

@henbos
Copy link
Contributor

henbos commented Apr 3, 2020

If a track is negotiated to receive we fire ontrack regardless of packets flowing.

I can't find the relevant spec reference for this, but we should only fire onunmute when we've received RTP packets (and have a decoded frame to render?).

The removeTrack() section says...

When the other peer stops sending a track in this manner, the track is removed from any remote MediaStreams that were initially revealed in the track event, and if the MediaStreamTrack is not already muted, a mute event is fired at the track.

It says "is not already muted", but our SDP processing does this:

For each track in muteTracks, set the muted state of track to the value true.

Which references an algorithm that does not care if the track is already muted, it fires onmute regardless.

So what is the intent? If the track is still muted because we haven't received RTP packets yet, does onmute fire or not?

  • If yes, onmute acts like the legacy "onremovetrack" which is nice, but it would be confusing if this was used both for RTP and for negotiation, rather than as a way to tell if we have media flowing, and muting an already muted track looks like a bug.
  • If no, there are edge cases were onmute doesn't fire which could catch an application by surprise.

Is the intent that an application should manually loop through all transceivers and check its currentDirection before and after the negotiation to detect if a track was removed in a way that does not race with RTP packets?

@alvestrand
Copy link
Contributor

@jan-ivar
Copy link
Member

jan-ivar commented Apr 3, 2020

It says "is not already muted", but our SDP processing does this:

For each track in muteTracks, set the muted state of track to the value true.

Which references an algorithm that does not care if the track is already muted, it fires onmute regardless.

No, because where muteTracks is populated says: "If track.muted is false, add track to muteTracks."

As far as desirable outcome, I think we're doing the right thing. Firing muted on a track that's already muted seems wrong.

Now, there are other places in this spec where we call that mediacapture algorithm (RTCP BYE etc.) where it looks like we're not checking as carefully.

I've filed w3c/mediacapture-main#676 to fix mediacapture.

@jan-ivar
Copy link
Member

jan-ivar commented Apr 3, 2020

If yes, onmute acts like the legacy "onremovetrack"

Is the intent that an application should manually loop through all transceivers and check its currentDirection before and after the negotiation to detect if a track was removed in a way that does not race with RTP packets?

No, people can use stream.onremovetrack instead.

https://stackoverflow.com/questions/60636439/webrtc-how-to-detect-when-a-stream-or-track-gets-removed-from-a-peerconnection/60860584#60860584

@fippo
Copy link
Contributor

fippo commented Apr 4, 2020

No, people can use stream.onremovetrack instead.

That assumes there is a stream but we allow addTrack without a stream.

@jan-ivar
Copy link
Member

jan-ivar commented Apr 4, 2020

That assumes there is a stream but we allow addTrack without a stream.

Yes but

  1. the people using pc.removeTrack model are typically stream people, not track people.
  2. Track-only people people can use a throwaway stream for this, since they don't rely on streams.

@guest271314
Copy link

I can't find the relevant spec reference for this, but we should only fire onunmute when we've received RTP packets (and have a decoded frame to render?).

At both Nightly and Chromium when unmute event is attached within ontrack unmute is only fired on the remote track and muted attribute is set to false - whether packets are sent from local track and received at remote track or not. unmute event is not fired on the original local track passed to addTrack().

@guest271314
Copy link

Actually, mute and unmute are fired on original track. Was using onmute and onunmute instead of addEventListener().

unmute event is still dispatched even when no media packets are being transmitted after addTrack(track).

Chromium in particular can toggle between muted === true and muted === false without any media packets being transmitted whatsoever.

@guest271314
Copy link

One question that have is what is the expected state of a CanvasCaptureMediaStreamTrack where no images have been drawn onto the canvas?

const canvas = document.createElement('canvas');
const ctx = canvas.getContext('2d');
const stream = canvas.captureStream(0);
const [track] = stream.getVideoTracks();
pc1.addTrack(track);

Does a MediaStreamTrack derived from canvas.captureStream() without any image being drawn onto the canvas before passing to addTrack() qualify as a MediaStreamTrack that is in state muted === false or muted === true?

If muted === true because no image has been drawn onto the canvas why would unmute event be fired simply because the local track becomes remote after addTrack()?

If muted === false because the canvas itself is a transparent image, thus media, should unmute be dispatched without any image being drawn onto the canvas locally?

@jan-ivar
Copy link
Member

At both Nightly and Chromium when unmute event is attached within ontrack unmute is only fired ... whether packets are sent from local track and received at remote track or not.

Firefox should be unmuting correctly (only once packets arrive). Chrome has bugs here.

@guest271314
Copy link

Firefox should be unmuting correctly (only once packets arrive). Chrome has bugs here.

Does the canvas itself with only stream = canvas.captureStream(); pc1.addTrack(stream.getVideoTracks()[0]); qualify as "only once packets arrive"? Does a canvas without any image drawn onto it at all quality as "packets"? Or, is "only once packets arrive" the MediaStreamTrack itself, a WebRTC network communications term for making a connection. What is the definition of "packets" used in this case?

@guest271314
Copy link

Note, the proposed workaround

const video = document.createElement("video");
video.srcObject = new MediaStream([track]);
video.onloadedmetadata = () => log("unmuted workaround!");

does not work for this case web-platform-tests/wpt#22779 (comment), where loadedmetadata will never fire - at Chrome or Firefox.

@jan-ivar
Copy link
Member

Does the canvas itself with only stream = canvas.captureStream(); pc1.addTrack(stream.getVideoTracks()[0]); qualify as "only once packets arrive"?

I believe so, because canvas.captureStream says: "a captured stream MUST request capture of a frame when created, even if frameRequestRate is zero."

I've verified this in Firefox with https://jsfiddle.net/jib1/kzsh9wgx/ which produces a black frame (alpha channel does not survive a peer connection AFAIK)

@jan-ivar
Copy link
Member

Hmm, looks like it's intermittent actually. I've filed a bug.

@guest271314
Copy link

Neither Chromium nor Firefox behaviour is consistency re unmute and mute events of MediaStreamTrack, nor with regards to how MediaStreamTrack and MediaStream connect programmatically to HTMLMediaElement, both from run of code to next run of code and comparatively between the browsers.

The filed issue does not address or explain why loadedmetadata of HTMLMediaElement is not fired, if even for only the first "packet" of the when the network connection is established.

@guest271314
Copy link

Attempted to use the same code at Firefox and Chromium to 1) determine what is a minimum requirement for a remote MediaStreamTrack from canvas.captureStream() to fire unmute event; 2) for HTMLVideoElement to fire loadedmetadata event; 3) for HTMLVideoElement to fire timeupdate event.

Observations

Chromium fires unmute irrespective of whether or not a method of the specific context set at the source canvas is executed or the canvas is otherwise changed.

Firefox does not fire unmute event when no canvas context method is executed and the canvas is not changed.

Two canvas 2d context method to execute for unmute of remote track to fire are clearRect() or fillRect(). loadedmetadata of HTMLVideoElement also fires following unmute for each browser.

At Firefox <video> timeupdate is fired and currentTime progresses when autoplay is set, continues streams content ("black frames"), theoretically, infinitely.

At Chromium timeupdate is not fired and currentTime does not progress, similar to the behaviour described at https://bugs.chromium.org/p/chromium/issues/detail?id=1045832. The video display is a "black frame". If the play control is pressed 0 is alerted from timeupdate event.

Given unmute will not be fired at browsers under current specifications and implementations without some value being set or method being called on the remote source canvas a RTCDataChannel can be used to signal the remote peer to perform the required task for the stream to be unmuted.

That does not solve the Chromium issue of unmute being fired on the remote track without any change to thesource canvas however not progressing currentTime or firing timeupdate event at HTMLVideoElement when a change is made to the canvas.

One option would be to update the canvas capture stream specification to stream the contents of the canvas regardless of whether or not a new image was painted onto or modification was made to the canvas.

Alternatively, adjust RTCPeerConnection to not await content to stream "black frames" from a <canvas>, which is already an image, thus already content. This effectively provides a "paused" MediaStreamTrack, which could be useful for certain purposes, though the behaviour does not appear to be by design of at least HTMLVideoElement or RTCPeerConnection, if in fact unmute is based on networking, not content.

<!DOCTYPE html>

<html>
  <head>
    <title>
      unmute remote RTCPeerConnection from canvas.captureStream(), fire
      HTMLVideoElement loadedmetadata and timeupdate events
    </title>
    <script src="RTCPeerConnection-helper.js"></script>
  </head>

  <body>
    <code>
      unmute remote RTCPeerConnection from canvas.captureStream(), fire
      HTMLVideoElement loadedmetadata and timeupdate
    </code>
    <script>
      (async _ => {
        const v = document.createElement('video');
        v.width = v.height = 200;
        v.controls = v.autoplay = true;
        const pc1 = new RTCPeerConnection();
        const pc2 = new RTCPeerConnection();
        // not fired at Chromium for fillRect or clearRect
        v.ontimeupdate = e => {
          v.ontimeupdate = null;
          alert('currentTime:' + e.target.currentTime);
        };
        const dc2 = pc2.createDataChannel('canvas-stream');
        document.body.appendChild(v);
        const metadataToBeLoaded = new Promise(
          resolve => (v.onloadedmetadata = e => resolve(e.type))
        );
        const canvasStream = () => {
          const canvas = document.createElement('canvas');
          canvas.width = canvas.height = v.width;
          const ctx = canvas.getContext('2d');
          const stream = canvas.captureStream();
          const [track] = stream.getVideoTracks();
          const dc1 = pc1.createDataChannel('canvas-stream');
          dc1.onopen = e => {
            dc1.onmessage = e => {
              ctx.clearRect(0, 0, canvas.width, canvas.height);
              dc1.close();
            // do stuff
            };
          };
          return stream;
        };

        let stream = canvasStream();
        let [track] = stream.getVideoTracks();
        const sender = pc1.addTrack(track);

        pc2.ontrack = async e => {
          e.track.onunmute = e => {
            alert(e.type);
            v.srcObject = new MediaStream([e.target]);
          };
          pc2.ondatachannel = e => {
            e.channel.send(null);
            e.channel.close();
          };
        };

        exchangeIceCandidates(pc1, pc2);
        doSignalingHandshake(pc1, pc2);
        let result = await metadataToBeLoaded;
        alert(result);
        // pc1.close();
        // pc2.close();
      })();
    </script>
  </body>
</html>

plnkr https://plnkr.co/edit/9XcS0fbCPfgvPBzd?preview

@guest271314
Copy link

For an "infinite" stream of "black frames" at Chromium with timeupdate progression

      dc1.onopen = e => {
        const handleMessage = _ => {
          // do stuff 
          requestAnimationFrame(function paint() {           
            ctx.fillRect(0, 0, canvas.width, canvas.height);
            requestAnimationFrame(paint);              
          });
          dc1.close();
        };
        dc1.onmessage = handleMessage;
      };

however, since Chromium toggle the muted state of the MediaStreamTrack from canvas.captureStream(), for no clear reason, if

track.onmute = e => alert(e.type)

is included in the code, the user will be face with an endless series of prompts with "mute" and "unmute" from the events being fired, even while currentTime is incrementing - though not on the <video> control.

@jan-ivar
Copy link
Member

jan-ivar commented Apr 23, 2020

The chromium unmute bug is 889487 (go ★ it), so we don't need to discuss that part here.

@jan-ivar
Copy link
Member

Closed by w3c/mediacapture-main#685.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants