feross / simple-peer

📡 Simple WebRTC video, voice, and data channels
MIT License
7.43k stars 974 forks source link

Cannot remove track that was never added. #407

Closed omardoma closed 5 years ago

omardoma commented 5 years ago

I am using the latest version of the library, so basically what I am trying to do is to switch video on and off in my many to many call, so my approach is:

    // Remove current stream from peers
    this.members
      .filter(member => member.peer)
      .forEach(member => member.peer.removeStream(this.localStream));

    // Stop current stream
    this.localStream.getTracks().forEach(track => track.stop());
    this.localStream = null;

    // Adjust flags
    this.videoMode = !this.videoMode;
    if(this.videoMode) {
      this.speakerMode = true;
    }

    // Init a new stream based on the flags
    const device = this.deviceList.find(device => {
      const label = device.label.toLowerCase();
      return device.kind === 'audioinput' && this.speakerMode
        ? label.includes('speaker') || label.includes('loud')
        : label.includes('earpiece') ||
            label.includes('headset') ||
            label.includes('headphone');
    });
    this.localStream = await navigator.mediaDevices.getUserMedia({
      video: this.videoMode ? { facingMode: 'user' } : false,
      audio: device ? { deviceId: { exact: device.deviceId } } : true
    });

    // Add new stream to peers
    this.members
      .filter(member => member.peer)
      .forEach(member => member.peer.addStream(this.localStream));

But I am getting the following error:

Error: Cannot remove track that was never added.
    at r.removeTrack (simplepeer.min.js:11)
    at simplepeer.min.js:11
    at Array.forEach (<anonymous>)
    at r.removeStream (simplepeer.min.js:11)
    at call.component.ts:498
    at Array.forEach (<anonymous>)
    at CallComponent.<anonymous> (call.component.ts:498)
    at step (tslib.es6.js:97)
    at Object.next (tslib.es6.js:78)
    at tslib.es6.js:71
t-mullen commented 5 years ago

Based on the code you provided, you're removing the stream, before you add the stream:

Adding null isn't valid and will throw the same error.

peer.addStream(null) // not valid

Keep track of which stream was added to the peer.

    this.members
      .filter(member => member.peer)
      .forEach(member => {
           if (!member.localStream) return
           member.peer.removeStream(member.localStream));
      })
    this.members
      .filter(member => member.peer)
      .forEach(member => {
         member.localStream = this.localStream
         member.peer.addStream(member.localStream)
      });
omardoma commented 5 years ago

Based on the code you provided, you're removing the stream, before you add the stream:

Adding null isn't valid and will throw the same error.

peer.addStream(null) // not valid

Keep track of which stream was added to the peer.

    this.members
      .filter(member => member.peer)
      .forEach(member => {
           if (!member.localStream) return
           member.peer.removeStream(member.localStream));
      })
    this.members
      .filter(member => member.peer)
      .forEach(member => {
         member.localStream = this.localStream
         member.peer.addStream(member.localStream)
      });

I believe you got me wrong, this code runs when there is already a localStream that is shared with all peers and a user presses a button to switch on/off his video, so what happens is, I remove the current localStream which is never a null value, then close it, then initialize a new one with the new constraints and add it to the peers, so I am just switching an already existing and not null stream.

t-mullen commented 5 years ago

Can you log these three things before you call removeStream?

console.log(peer._senderMap.get(this.localStream.getTracks()[0])
console.log(peer._senderMap.get(this.localStream.getTracks()[0]).get(this.localStream))
console.log(this.localStream)
schirrmie commented 5 years ago

Hi,

i have the same issue. It is not every time but sometimes i'll get the same error.

This is my source code

      console.log("Pause stream");

      try {
        console.log(this.localStream);
        console.log(this.RTCPeer._senderMap.get(this.localStream.getTracks()[0]));
        console.log(this.RTCPeer._senderMap.get(this.localStream.getTracks()[0]).get(this.localStream));
      }
      catch( ex ) {
        console.log( ex );
      }
      this.RTCPeer.removeStream(this.localStream);

This is my console.log output

gbt.js:410 Pause stream gbt.js:413 MediaStream {id: "iFru3oJQanTcCMGcCy80uOc6fCDuEcdSF4sY", active: true, onaddtrack: null, onremovetrack: null, onactive: null, …}active: trueid: "iFru3oJQanTcCMGcCy80uOc6fCDuEcdSF4sY"onactive: nullonaddtrack: nulloninactive: nullonremovetrack: nullproto: MediaStream gbt.js:414 undefined gbt.js:418 TypeError: Cannot read property 'get' of undefined at GBT.PauseStream (gbt.js:415) at HTMLButtonElement. (bmas.html?debug=1:602) at HTMLButtonElement.dispatch (jquery.min.js:3) at HTMLButtonElement.q.handle (jquery.min.js:3) logger.min.js:1 rtc peer error Error: Cannot remove track that was never added.

I think the screenshot is easier to read:

grafik

It is on chrome 71.

Do You need more Informations?

Regards schirrmie

t-mullen commented 5 years ago

Thanks @schirrmie, that's very helpful.

Could you log two more things in browser console? Screenshot is fine.

console.log(this.RTCPeer._senderMap)
console.log(this.localStream.getTracks())
schirrmie commented 5 years ago

Hi, thank You for your quick reply.

This is the source code

      console.log("Pause stream");

      try {
        console.log(this.localStream);
        console.log(this.RTCPeer._senderMap)
        console.log(this.localStream.getTracks())
        console.log(this.RTCPeer._senderMap.get(this.localStream.getTracks()[0]));
        console.log(this.RTCPeer._senderMap.get(this.localStream.getTracks()[0]).get(this.localStream));
      }
      catch( ex ) {
        console.log( ex );
      }
      this.RTCPeer.removeStream(this.localStream);

And here is the console output:

grafik

t-mullen commented 5 years ago

this.RTCPeer._senderMap is empty, which I don't think is possible if you've called addStream(). Can you double-check that addStream is being called before removeStream?

Are you using replaceTrack at all?

schirrmie commented 5 years ago

Hi,

I never call addStream. But all streams are working, sending and receiving. I initialize SimplePeer like this:

    var peer_options = {
      initiator: initiator,
      config: {
        iceTransportPolicy: 'relay',
        //rtcpMuxPolicy: 'negotiate',
        iceServers: [
          { **** },
        ]
      },
    };
   peer_options.stream = this.localStream;

   this.RTCPeer = new SimplePeer( peer_options );

Anything wrong with this? Again video ist working fine, I see both sending and receiving on both sides.

Regards schirrmie

schirrmie commented 5 years ago

Oh and no I don't use replaceTrack.

schirrmie commented 5 years ago

I don't know how to reproduce this issue. It only happens 1 of 4 tries and I always do the same. It is strange :)

t-mullen commented 5 years ago

Do you call removeStream inside of an event, like “connect”?

schirrmie commented 5 years ago

No it is after the connect in an user event. A Button to enable / disable Video sending.

omardoma commented 5 years ago

@t-mullen The issue in my case is, I start the peer connection with a MediaStream in the streams array, then when I need to switch off video for example on a click of a button, I do removeStream -> stop the stream -> addStream, but I get the error mentioned in the initial issue comment.

So it basically doesn't consider the initial connection stream as added before, and thus when you try to remove it even though it is there in the array of streams, it throws the error because you never did call addStream on it.

t-mullen commented 5 years ago

addStream is called internally if you use the constructor option. And it's always called.

I'm not able to reproduce this at all...

Could you log the ID of the peer before you call addStream and before you call removeStream (to make sure it's indeed the same peer object).

console.log(peer.id)

Can you try putting a small timeout before you remove the stream? (To rule out any eventing race conditions).

setTimeout(()=>{
  peer.removeStream(localStream)
}, 1000)
t-mullen commented 5 years ago

Alright, I've been able to reproduce this. Working on a fix.

omardoma commented 5 years ago

Finally! thats great to hear, thanks for your hard work man!

Mihai-github commented 2 years ago

hi, my issue is with reconnecting. After let's say answer peer reconnects the offering peer tries to switch the camera and gets an error, my case is with replace track...

Any solution, please?