1440598 - peerconnection removeTrack does not remove track

Status: RESOLVED INVALID

(Core :: WebRTC, defect, P4)

Description

[felix.ilbring]

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:59.0) Gecko/20100101 Firefox/59.0
Build ID: 20180219114835

Steps to reproduce:

I created a fiddle to reproduce the issue
https://jsfiddle.net/wuhcdrwk/15/

* add a track to the peer connection 
* do the offer answer
* remove the track from the peer connection

We are adding Firefox support to GotoMeeting and our other Goto products. Starting and stopping is crucial for our user experience.


Actual results:

* the track is not removed  
* there is still a ssrc in the sdp 
* adding a new track results in +1 ssrc 
* the remote never gets a track removed callback 

* I also saw no upstream packets send in about:webrtc once I added a new upstream again. However, that is not reproducible with this fiddle. 


Expected results:

you can add and remove streams from a peer connection

Comment 1

[Nils Ohlmeier [:drno]]

Let's start with the fact that Fx 59 has Transceiver support and the changes quite a bit of behavior.
If I'm not mistaken that track removal no longer results in tracks ending is an intended side effect of the Transceivers.

So with Fx 58 I see the following difference in the SPD offer before and after track removal:

- a=sendrecv changes to a=recvonly
- a=msid disappears
- a=ssrc:xxx the number changes

With Fx 60 I see:

- a=sendrecv changes to a=recvonly
- a=msid stays constant
- a=ssrc:xx the number stays constant

According to spec https://www.w3.org/TR/webrtc/#dom-rtcrtpreceiver-track you now want to listen to the 'muted' event on the given track.

It sounds like you are trying to use a=ssrc as indication for what is going on with tracks. That model will only work with Chrome's Plan B implementation, which Google is in the process of finally switching over to Unified Plan which Firefox supports since a long time. So if you base you implementation on a=ssrc you are sooner or later going to get into trouble.

I would recommend to pay attention to the direction attribute (sendrecv vs recvonly) instead. And look into adding support for the muted event, although I think that even is still broken on the Chrome side.

Byron, any thoughts on this from you side?

Comment 2

[Byron Campen [:bwc]]

(In reply to felix.ilbring from comment #0)

> Actual results:
> 
> * the track is not removed  

   What precisely do you mean by this? Do you mean the stream still contains the track? Or the sender still contains the track? The former is according to the spec, and the expected behavior. The latter would be a bug.

> * there is still a ssrc in the sdp 
> * adding a new track results in +1 ssrc

   Yeah, you don't want to get used to paying attention to ssrc in SDP. That's not technically supposed to be in the SDP anymore. We only do so until enough endpoints support RTP MID (for demuxing bundle).

   And, before you ask, you shouldn't pay attention to the track id in a=msid either, because it doesn't actually mean much in the latest spec.

> * the remote never gets a track removed callback

   Yeah, this is one of those cases where the spec has moved in a way that plays extremely poorly with pre-existing implementations. Old Firefox used a=msid to drive things like track removal callbacks, but the latest transceivers spec outright forbids changing the a=msid on an m-section, even if the track is removed, and even if the track is replaced by another one.

> * I also saw no upstream packets send in about:webrtc once I added a new
> upstream again. However, that is not reproducible with this fiddle. 

   So you're saying you called addTrack, renegotiated, and did not see packets being sent? Can you post steps to reproduce, or at least the SDP offer/answer?

Comment 3

[Alteisen]

Well I am worried here, onremovestream is neither marked deprecated nor is compatibility crossed out for Firefox. 
(https://developer.mozilla.org/en-US/docs/Web/API/RTCPeerConnection/onremovestream)

Moving it from supported to obsolete(+unsupported) without going through deprecated is rather tough for app developers.

Is this really intended? Or am I reading the API documentation wrong here?

Comment 4

[Byron Campen [:bwc]]

Hmm. There might be some value in firing onremovestream when a receive stream has no tracks anymore, and mark this function deprecated. A lot of the code to do this still remains (and is unused). jib, what do you think?

Comment 5

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Thanks Felix for reporting this! We forgot to remove onremovestream from our webidl in 59. Filed as bug 1442385 with dev-doc-needed to make sure there will be a compatibility note posted about it.

This is intentional fallout from our switch to transceivers in 59, and we don't want to resurface stream API bits at this point.

Note you can shim onremovestream like this: https://jsfiddle.net/jib1/71wkLnop/

    remote.ontrack = function(event) {
        console.info('got downstream', event.streams[0]);
        downstreamVideo.srcObject = event.streams[0];
>       downstreamVideo.srcObject.onremovetrack = function() {
>         if (!downstreamVideo.srcObject.getTracks().length) {
>           remote.onremovestream({stream: downstreamVideo.srcObject});
>         }
>       }
    };

Note I also replaced event.streams with event.stream in your onremovestream.

onremovestream was removed from the spec in February 2015, but that's what it used to have.

Comment 6

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

(In reply to Alteisen from comment #3)
> Moving it from supported to obsolete(+unsupported) without going through
> deprecated is rather tough for app developers.

Just wanted to add a mitigating note here that even when I test your fiddle in Firefox 58 it doesn't work. I see:

got downstream undefined
ICE failed, add a STUN server and see about:webrtc for more details

The first problem is the s/event.streams/event.stream/ problem I already mentioned, but the second problem is that removing all tracks from a peer connection in 58 would cause ICE failure (unless you have a data channel or something else being sent), so it's not something Firefox has generally supported well in practice, which matters when we consider backwards compatibility.