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

Do we need a "trackremoved" event? #1161

Closed
taylor-b opened this issue May 4, 2017 · 40 comments
Closed

Do we need a "trackremoved" event? #1161

taylor-b opened this issue May 4, 2017 · 40 comments
Assignees

Comments

@taylor-b
Copy link
Contributor

taylor-b commented May 4, 2017

In the pre-transceiver world, we had addStream/removeStream methods, and "addstream"/"removestream" events. If you call addStream, then do an offer/answer, the "addstream" event will fire for the PeerConnection on the other side, and same for removeStream/"removestream".

Now, we have addTrack/removeTrack... But we only have a single event, "track". So how are applications expected to know when the other peer removed the track? They would need to inspect the transceiver and see if its direction changed (for example, from "sendrecv" to "recvonly"). This makes the migration from the stream-based API (which is still all that Chrome implements) to the track-based API non-trivial.

So, do we need a "trackremoved" event that fires when a transceiver's direction loses the "send" bit as a result of setting a remote description? This seems like a pretty trivial addition which would be appreciated by existing application developers.

We're also missing a step that will remove the track from any remote streams that were created for it in this step: https://www.w3.org/TR/webrtc/#process-remote-track

I don't know if this was intentional or just an oversight from the RTCRtpTransceiver refactoring.

@Pehrsons
Copy link

Pehrsons commented May 5, 2017

The track's "ended" event achieves this, no?

@taylor-b
Copy link
Contributor Author

taylor-b commented May 5, 2017

Not quite... Because the track only actually ends when the transceiver is stopped or the m= section is rejected. The transceiver direction changing is the only indication that currently exists, as far as I can tell.

@notedit
Copy link

notedit commented May 5, 2017

when we need add/remove track after when we have made a peerconneciton and need renegotiate.
addTack/removeTrack will be much convenient.

@linsir6
Copy link

linsir6 commented May 5, 2017

i meet the same question. it not very friendly for user.

@fippo
Copy link
Contributor

fippo commented May 5, 2017

We have this in http://w3c.github.io/webrtc-pc/#rtcpeerconnection-interface-extensions:

When the other peer stops sending a track in this manner, an ended event 
is fired at the MediaStreamTrack object.

which however does not explain how that happens (referring to jsep)

In the old model when you have a audio m-line and a video m-line and you renegotiate without video, the video tracks ended will be called even though there is no removestream.

Having a trackremoved event will not solve all issues however. Consider the (somewhat hypothetical) case where a track was part of multiple streams and is removed from some streams. Would you expect trackremoved to be called and if yes, with what streams? How does that relate to the mediastreams removetrack event?

@taylor-b
Copy link
Contributor Author

taylor-b commented May 5, 2017

I believe that paragraph is just obsolete. I traced the history and it goes at least as far back as when we had addStream. PR to remove it here: #1168

In the old model when you have a audio m-line and a video m-line and you renegotiate without video, the video tracks ended will be called even though there is no removestream.

This still happens. But removeTrack doesn't cause the implementation to renegotiate without video; only videoTransceiver.stop does.

Consider the (somewhat hypothetical) case where a track was part of multiple streams and is removed from some streams.

Well, assuming that setting the remote description removes the track from the correct streams (it doesn't right now), you'd be able to listen for the streams' "removetrack" event.

@jan-ivar
Copy link
Member

jan-ivar commented May 5, 2017

The track's "ended" event achieves this, no?

Agree. I think this issue and #1168 are more than editorial changes, and need discussion.

@jan-ivar
Copy link
Member

jan-ivar commented May 8, 2017

Well, assuming that setting the remote description removes the track from the correct streams (it doesn't right now), you'd be able to listen for the streams' "removetrack" event.

@taylor-b If we did this then we wouldn't need a trackremoved event, right?

@fippo
Copy link
Contributor

fippo commented May 9, 2017

Well, assuming that setting the remote description removes the track from the correct streams (it doesn't right now), you'd be able to listen for the streams' "removetrack" event.
@taylor-b If we did this then we wouldn't need a trackremoved event, right?

Iif we followed that pattern we would get a addstream event for every new stream and could listen for stream.onremovetrack. Surfacing both "a track was added" and "a track was removed" at the peerconnection level makes sense imo. Or is at least consistent if we want the first event.

(I agree that it is quite surprising that SRD doesn't remove the track from streams though... well, that can be fixed)

@fippo
Copy link
Contributor

fippo commented May 9, 2017

a lot of the confusion is coming from the lack of an example that show when to remove a video element that shows a remote track.

In the old stream model this was easy, I have code from 2012 doing this:

pc.onremovestream = (e) => {
    // search for video with v.srcObject === e.stream, remove it
};

or using MST.onended (which was removed)

pc.onaddstream = (e) => {
  // create video element
  e.stream.onended = () => { remove video element }
};

In the track-based model this is somewhat more complex:

v.srcObject.addEventListener('onremovetrack', () => {
  // check if the stream now contains 0 tracks. Or check that the streams active flag is false
})

or alternatively do it it track.onended

pc.ontrack = (e) => {
  // maybe create a video element to attach the stream to
  e.track.onended = () => {
    // for any streams in e.stream check if their active flag is false
  }
}

@taylor-b how would this look like with a trackremoved event?

pc.ontrackremoved = (e) => {
  e.streams.forEach((s) => {
    // check if the stream now contains 0 tracks. Or check that the streams active flag is false
  })
};

(we should have an example for this in the spec!)

@jan-ivar
Copy link
Member

jan-ivar commented May 9, 2017

We're also missing a step that will remove the track from any remote streams that were created for it in this step: https://www.w3.org/TR/webrtc/#process-remote-track

Agreed. I decided to test Chrome and Firefox behavior here using https://jsfiddle.net/jib1/xrwsp7o9/

The good news is that, where available, removeTrack/removeStream followed by addTrack/addStream works reliably in both browsers to replace content, over and over.

  • Both browsers fire ended on remote tracks.
  • Both browsers fire removestream on pc2.

But there were differences in the details that threw me:

  • Only Firefox fires addtrack on the remote stream on adding the second track (likely a limitation of the adapter.js ontrack polyfill, since with addstream the stream comes pre-populated).
  • But neither browser fires addtrack on the existing remote stream on subsequent renegotiation.
  • Only Chrome fires removetrack on the remote stream.
  • Firefox leaves old tracks in the remote stream until a subsequent renegotiation adds tracks to it!

I almost missed the good news because of the last one. I'd say the last three here are bugs.

I don't know if this was intentional or just an oversight from the RTCRtpTransceiver refactoring.

I think it's important to support this model, for legacy reasons if nothing else, to let users pc.removeTrack(sender); pc.addTrack(newTrack, sameStream); and have the remote stream still be playable in a video element. - If we switch to not cleaning up the remote streams anymore, then it'll accumulate ended/muted tracks, which seems undesirable, and might break existing use, causing the video element to not play etc.

@Pehrsons
Copy link

Pehrsons commented May 9, 2017

  • Only Chrome fires removetrack on the remote stream.

Firefox doesn't implement the "removetrack" event on MediaStream. When I implemented "addtrack" the spec didn't mention that tracks should be removed (per [1]), and so "removetrack" didn't have any users. See [2] for the full background.

  • Firefox leaves old tracks in the remote stream until a subsequent renegotiation adds tracks to it!

That's wrong I'm afraid. Firefox passes a new stream on the remote side with the new tracks in it. It happens to have the same id as the old stream. I updated your fiddle to show this: https://jsfiddle.net/pehrsons/xrwsp7o9/7/

[1] https://w3c.github.io/webrtc-pc/archives/20160513/webrtc.html#processing-remote-mediastreamtracks
[2] https://bugzilla.mozilla.org/show_bug.cgi?id=1208328#c2

@jan-ivar
Copy link
Member

jan-ivar commented May 9, 2017

@Pehrsons Ok that explains what I'm seeing in Firefox. Isn't two streams with the same id a violation of the mediacapture spec? It's certainly a violation of common expectations of an id IMHO. E.g. If we could just change id's like this then we wouldn't be talking about giving up on track ids.

But regardless, the net effect is nearly the same: There's a model for content replacement here that sorta works in at least two browsers. That seems worth preserving IMHO.

@Pehrsons
Copy link

Isn't two streams with the same id a violation of the mediacapture spec?

The current draft of mediacapture-main even says that stream ids MUST always be generated. It doesn't allow for the id to be transferred from the remote side as it does for MediaStreamTrack.

Of course webrtc-pc contradicts this by saying

When a MediaStream is created to represent a stream obtained from a remote peer, the id attribute is initialized from information provided by the remote source.

--

This behavior though is probably a legacy bug in Firefox where renegotiations have been implemented for quite long, and at the same time hasn't gotten much attention from users.

@taylor-b
Copy link
Contributor Author

That contradiction seems like an issue in itself. Made a new issue for it.

@aboba
Copy link
Contributor

aboba commented Jun 6, 2017

@fippo Section 5.3 says "When one of the SSRCs for RTP source media streams received by receiver is removed (either due to reception of a BYE or via timeout), the mute event is fired at track. If and when packets are received again, the unmute event is fired at track." So according to this one could argue that when the sender stops sending, a mute event is received on the remote received track.

@jan-ivar
Copy link
Member

jan-ivar commented Jun 7, 2017

@aboba The muted event may fire at other times from overconstrained conditions in mediacapture, or "muted by the User Agent.", so one cannot assume this means a track was removed.

@jan-ivar
Copy link
Member

jan-ivar commented Jun 7, 2017

@taylor-b I did some digging, and I need to correct your initial premise:

In the pre-transceiver world, we had addStream/removeStream methods, and "addstream"/"removestream" events. ... Now, we have addTrack/removeTrack... But we only have a single event, "track". So how are applications expected to know when the other peer removed the track?

The answer since Oct 2014 has been "onremovestream has been removed (now, .onended is fired)." Transceivers weren't added until a year later, and the "ended" language remains to this day (#1168).

For a whole year, the spec was addTrack/removeTrack on one side, and track and ended on the other, and nothing else. That's the year Firefox implemented addTrack/removeTrack.

In other words, we did not go straight from streams to transceivers, and the removestream event was not removed because of transceivers. It was removed because of ended.

So we moved from:

  1. addStream/removeStream firing onaddstream and onremovestream (Chrome), to
  2. addTrack/removeTrack firing track and ended (Firefox), to
  3. addTransceiver, and confusion about what happens remotely (Nobody).

I cover these 3 stages in my recent blog post and webinar.

Also note Chrome fires track and ended for both addStream and addTrack through adapter.js.

@aboba
Copy link
Contributor

aboba commented Jun 7, 2017

@jan-ivar With respect to the transceiver model, the remote effects of transceiver.stop() are described in Section 5.4:

"When a remote description is applied with a zero port in the media description for the corresponding transceiver, as defined in [JSEP] (section 4.2.2), the user agent must run the above steps as if stop had been called. In addition, since transceiver.receiver.track has ended, the steps described in track ended must be followed."

Is this confused/unclear?

@jan-ivar
Copy link
Member

jan-ivar commented Jun 7, 2017

@aboba That part seems clear, but sheds no light on what happens except when stop() is called AFAICT.

@jan-ivar
Copy link
Member

jan-ivar commented Jun 7, 2017

@aboba Oh I see how that was confusing. With the third bullet I just meant:

  1. addTrack/removeTrack firing track and confusion about whether ended still fires (Nobody).

@stefhak
Copy link
Contributor

stefhak commented Jun 7, 2017

@jan-ivar what you write in #1161 (comment) and #1161 (comment) matches my memory.

@henbos
Copy link
Contributor

henbos commented Jul 5, 2017

Is there an update on this?
Now that ended doesn't fire anymore, is there no event one can listen to as a result of the other side doing removeTrack?

The only event we have is track event being fired for added tracks according to remote track processing?

  1. Queue a task to fire an event named track with transceiver, track, and streams at the connection object.

This only happens if the event has not fired for the track before according to the set remote description steps.

  1. If the direction of the media description is sendrecv or sendonly, and transceiver.receiver.track has not yet been fired in a track event, process the remote track for the media description, given transceiver.

I hope we can't re-use a transceiver with an old track whose event will not fire because it has already been fired before.

Assuming remote streams should not be created for stream-less local tracks, the trackremoved event should fire on the peer connection and not on individual streams.

@jan-ivar
Copy link
Member

jan-ivar commented Jul 5, 2017

@henbos I believe we decided on #1402 which is waiting to be merged, so we won't need this. It lets users usestream.onremovetrack like they can today in Chrome. They can also listen to track.onmuted.

I hope we can't re-use a transceiver with an old track whose event will not fire because it has already been fired before.

#1402 fixes this to fire track again on unmute.

@aboba
Copy link
Contributor

aboba commented Sep 13, 2017

Consensus at the September interim was to close this issue.

@aboba aboba closed this as completed Sep 13, 2017
@benbro
Copy link

benbro commented Sep 14, 2017

@youennf How do you detect that a remote track is removed in Safari?

@youennf
Copy link
Contributor

youennf commented Sep 14, 2017

Currently, there is no other way than looking at the SDP.

@benbro
Copy link

benbro commented Sep 14, 2017

@youennf Are you going to add one of the following?
track.onended
stream.onremovetrack
track.onmuted

Is there an attribute on the pc, stream, track, or transceiver I can check after applying the remote offer or answer to know that a track was removed?

@ibc
Copy link

ibc commented Sep 14, 2017

We don't need an event for the pc to tell us what happened due the SDP we passed to it. Instead, what we need is pc.addRemoteTrack(rtpParams). But that's impossible with SDP.

I no longer rely on pc events but instead I check the remote streams/tracks/receivers after sRD and match them with previous values (stored at app level).

@benbro
Copy link

benbro commented Sep 14, 2017

@ibc what do you check to find out that a remote track was removed in Safari?
When do you check it? After the pc.setRemoteDescription callback?

@ibc
Copy link

ibc commented Sep 14, 2017

Actually I produce the remote SDP locally by adding the info (RTP parameters) of the added/removed remote track (whose RTP parameters are signaled). Then I call setRemoteDescription(offer) with the locally built remote offer. And then I iterate the pc.getReceivers() until I find the track with the .id corresponding to the a=msid that I've locally created:

https://github.com/versatica/mediasoup-client/blob/master/lib/handlers/Safari11.js#L352

@benbro
Copy link

benbro commented Sep 14, 2017

So you don't "check the remote streams/tracks/receivers after sRD".

@ibc
Copy link

ibc commented Sep 14, 2017

Not in my personal case (because I don't receive a SDP monster from the remote, but specific API "commands/events" such as "new remote track" with its specific RTP parameters).

However, if I had to receive a remote SDP (not created by me or my app) I would parse it and, assuming Chrome or Safari, I would check the remote SSRCs of audio and video, would match them with the previous values, would realize that there is a new remote track, then would get its a=ssrc:xxxx msid (to get the remote MediaStream.id and remote MediaStreamTrack.id), would call pc.setRemoteDescription(offer) and then would iterate pc.getStreams() or pc.getReceivers() to retrieve the new remote track (the one whose track.id matches the new a=ssrc:xxxx msid line).

I just don't want to rely on peerconnection stream/track events at all. Events happen out of context, I hate that, it makes the application logic ugly.

@jan-ivar
Copy link
Member

@fippo we should consider a shim for this in adapter.

@youennf
Copy link
Contributor

youennf commented Sep 14, 2017

@youennf Are you going to add one of the following?
track.onended
track.onmuted

Plan is to handle these correctly as per spec.

stream.onremovetrack

Plan is to support it also, the infrastructure is there although I am not sure we should push people to rely on that.

@benbro
Copy link

benbro commented Feb 23, 2018

@youennf any news about a way to know when a remote track was removed on Safari?
Any of the following supposed to work?
track.onended
track.onmuted
stream.onremotetrack

@itsthetaste
Copy link

Hi, in Firefox 59+ I'm seeing that the track.id's given to me from pc.ontrack don't match those in the SDP and also the track.onended no longer fires. Is this correct?

@jan-ivar
Copy link
Member

@itsthetaste Yes that is correct. Firefox 59 implements Transceivers in the spec, which break this.

See my blog post for details. I hope to write an updated one soon.

@itsthetaste
Copy link

@jan-ivar thanks, i found track.onmute fires like track.onended used to. hopefully, that will stay.
Your blog post is great 👍

@jan-ivar
Copy link
Member

Thanks! Yes onmute will stay and works in Firefox 59+. Just know mute may additionally fire due to SSRC timeout. Some apps may care about the difference.

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

No branches or pull requests