simple-peer icon indicating copy to clipboard operation
simple-peer copied to clipboard

replaceTrack only works once

Open cuu508 opened this issue 4 years ago • 16 comments

I'm looking at implementing a "switch between front and rear camera" feature.

When user taps a button to switch the camera:

  • I run navigator.mediaDevices.getUserMedia with appropriate constraint
  • when getUserMedia returns a stream I run replaceTrack like so:
navigator.mediaDevices.getUserMedia({
    video: { facingMode: facingMode },
    audio: true
}).then(function(stream) {
    if (p && localStream) {
        p.replaceTrack(localStream.getVideoTracks()[0], stream.getVideoTracks()[0], localStream);
        console.log("Done");
    }

    localStream = stream;
}).catch(() => {})

In the example, p is a Peer instance, and localStream is a local variable that keeps track of the current local stream. The above works, but only once. When I try to switch camera the second time, the replaceTrack doesn't return (the console.logstatement doesn't run), and there are no errors logged in console.

Am I using it wrong? Is there an example of an idiomatic use of replaceTrack?

cuu508 avatar Apr 30 '20 10:04 cuu508

replaceTrack() doesn't return anything either way. If console.log doesn't run it means your p && localStream is false but from the snippet it is not clear why.

nazar-pc avatar Apr 30 '20 10:04 nazar-pc

The above snippet was simplified for clarity. Here's a full one:

        navigator.mediaDevices.getUserMedia({
            video: { facingMode: facingMode },
            audio: true
        }).then(function(stream) {
            console.log("Got stream from browser");
            var video = document.getElementById("self");
            video.srcObject = stream;
            video.onloadedmetadata = function(e) {
                video.play();
            };

            if (p && localStream) {
                console.log("Running replaceTrack");
                console.log("Replace: ", localStream.getVideoTracks()[0]);
                console.log("Old stream: ", localStream);

                console.log("With:    ", stream.getVideoTracks()[0]);
                console.log("New stream:    ", stream);

                p.replaceTrack(localStream.getVideoTracks()[0], stream.getVideoTracks()[0], localStream);
                console.log("Replace done");
            }

            localStream = stream;

        }).catch(() => {})

And here's what I see in console:

image

Note the missing final "Replace done".

cuu508 avatar Apr 30 '20 10:04 cuu508

Then replace catch(() => {}) with logging to console, there was clearly some error during replaceTrack() call, but you intentionally ignore it.

nazar-pc avatar Apr 30 '20 10:04 nazar-pc

Thanks, missed that! Wasn't intentional, it was just me copying samples from README and not knowing how error reporting works.

I'm now getting this:

image

To me it looks like the newTrack in the first switch matches the oldTrack in the second switch. But it's throwing a "Cannot replace a track that was never added." error.

cuu508 avatar Apr 30 '20 10:04 cuu508

I'm afraid the way it is designed right now, you are supposed to always add the same media stream as a third parameter to replaceTrack().

nazar-pc avatar Apr 30 '20 10:04 nazar-pc

Ah I see. I commented out the line that updates localStream and now it indeed works:

navigator.mediaDevices.getUserMedia({
    video: { facingMode: facingMode },
    audio: true
}).then(function(stream) {
    var video = document.getElementById("self");
    video.srcObject = stream;
    video.onloadedmetadata = function(e) {
        video.play();
    };

    if (p && localStream) {
        p.replaceTrack(localStream.getVideoTracks()[0], stream.getVideoTracks()[0], localStream);
    }

    // localStream = stream;

}).catch((e) => {
    console.log(e);
})

Just to get my mental model correct – is replaceTrack modifying the original media stream that was originally passed to Peer's constructor?

If the third argument is always the same, why does it need to be passed in at all?

cuu508 avatar Apr 30 '20 11:04 cuu508

No, there is just a mapping inside of simple-peer that causes this unnecessary otherwise requirement. According to specification it is not required. I can suggest you to not specify media stream initially and use addTrack() instead, then you can use undefined for media stream and it should work that way.

nazar-pc avatar Apr 30 '20 11:04 nazar-pc

Thanks for the suggestion! I had previously tried to use addStream(), but couldn't get it to work – I have a hunch that's because of my super-simplistic messaging layer which doesn't handle bursts of messages well.

But back to the topic, I think it would be great if there was an example of replaceTrack() usage in README, with a note about what can and should go in the third parameter.

cuu508 avatar Apr 30 '20 13:04 cuu508

addStream() is a wrapper that emulates something that is no longer in the spec for simple use cases only. I'd say the way replaceTrack() would be great to actually align with the spec implementation, but that would be a breaking change I'm afraid.

nazar-pc avatar Apr 30 '20 13:04 nazar-pc

The API reflects the old Plan-B WebRTC API, in which "streams" were added to a RTCPeerConnection with a native addStream method and you attached tracks to that outgoing stream. The mediastream you got those tracks from is irrelevant. The relevant stream is the one passed to addStream.

In other words, you're "attaching" the new track onto the old stream.

Now everything is done with tracks+senders, so this is outdated and confusing. But this behaviour of replaceTrack is by design... 🙁

t-mullen avatar May 03 '20 15:05 t-mullen

No, there is just a mapping inside of simple-peer that causes this unnecessary otherwise requirement. According to specification it is not required. I can suggest you to not specify media stream initially and use addTrack() instead, then you can use undefined for media stream and it should work that way.

FWIW I just tried to pass undefined for the stream to addTrack and that doesn't fly:

TypeError: Failed to execute 'addTrack' on 'RTCPeerConnection': parameter 2 is not of type 'MediaStream'.

ForesightImaging avatar Jul 01 '20 22:07 ForesightImaging

You need to pass a stream to addTrack, it's used for track grouping and synchronization: https://developer.mozilla.org/en-US/docs/Web/API/RTCPeerConnection/addTrack

The only method that doesn't require a stream is sender.replaceTrack, because the sender is created during pc.addTrack - the stream is implied. We don't expose senders, so we need the stream in peer.replaceTrack too.

It seems the only thing that can be done is improve documentation of peer.replaceTrack.

t-mullen avatar Aug 09 '20 22:08 t-mullen

@cuu508 Your problem is resolved ? I'm trying to switch de camera on Android but for me also its work only once, first ime it's ok I've the new track on the peers the second time I've this error Error: Cannot replace track that was never added.

const switchCamera = async () => {
  try {
    // Select another device
    const availableVideos = devices.filter(
      (d) => d.kind === 'videoinput' && d.deviceId !== currentVideoDevice.deviceId,
    )

    if (availableVideos.length === 0) return

    currentVideoDevice = availableVideos[0]
    const localStream = getLocalVideoEl().srcObject as MediaStream
    const currentTrack = localStream.getVideoTracks() as MediaStreamTrack[]

    // Stop sending track to peers
    currentTrack.forEach((t) => t.stop())

    // New stream with new deviceId
    const stream = await getUserMedia({
      ...constraints,
      video: {
        deviceId: currentVideoDevice.deviceId,
      },
    })

    if (stream) {
      // Replace video track to all peers
      apply((peer: Peer.Instance) => {
        peer.replaceTrack(currentTrack[0], stream.getVideoTracks()[0], localStream)
      })
      // Set Local video
      getLocalVideoEl().srcObject = stream
    }
  } catch (error) {
    console.error('switchCamera: ', error)
  }
}

EDIT: Solved

    if (stream) {
      localStream.removeTrack(currentTrack[0])
      localStream.addTrack(stream.getVideoTracks()[0])
      // replace track to all peers
      apply((peer: Peer.Instance) => {
        peer.replaceTrack(currentTrack[0], stream.getVideoTracks()[0], localStream)
      })
      // Set Local video
      // getLocalVideoEl().srcObject = stream
    }

If I change the getLocalVideoEl().srcObject = stream its work only once, but if I change only the tracks from the local stream

localStream.removeTrack(currentTrack[0])
localStream.addTrack(stream.getVideoTracks()[0])

It's work.

So do not use addStream at all and only work with addTrack removeTrack and replaceTrack.

snettah avatar Nov 01 '20 13:11 snettah

Hi, I'm having an issue with making replaceTrack work when a peer reconnect. Somehow the track i try to replace is not in the connection because I get an error, but how and why this is happening? I tried several way to fix this nut nothing works..

Mihai-github avatar Jun 15 '22 19:06 Mihai-github

This sequence works for me

Add new track to local stream localStream.addTrack(stream.getVideoTracks()[0]) Replace track on peer peer.replaceTrack(localStream.getVideoTracks()[0], stream.getVideoTracks()[0], localStream) Remove old track from local stream localStream.removeTrack(localStream.getVideoTracks()[0]) Update your local stream myStream = localStream

n9lsjr avatar Sep 26 '22 01:09 n9lsjr

I had a similar issue and managed to solve this way

First I replace the track peer.replaceTrack(self.user.stream.getVideoTracks()[0], stream.getVideoTracks()[0], self.user.stream); Then I remove the track from my current local stream mystream.removeTrack(self.user.stream.getVideoTracks()[0]) And finally add the new track to my visible stream mystream.addTrack(stream.getVideoTracks()[0])

My error was that I was removing the track from the current local stream before replacing it in the peer, receiving the infamous error: "Cannot replace a track that was never added"

EdoardoEntusiasta avatar Nov 03 '22 17:11 EdoardoEntusiasta