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

replaceTrack only works once #634

Open
cuu508 opened this issue Apr 30, 2020 · 16 comments
Open

replaceTrack only works once #634

cuu508 opened this issue Apr 30, 2020 · 16 comments

Comments

@cuu508
Copy link

cuu508 commented Apr 30, 2020

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?

@nazar-pc
Copy link
Collaborator

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.

@cuu508
Copy link
Author

cuu508 commented Apr 30, 2020

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".

@nazar-pc
Copy link
Collaborator

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

@cuu508
Copy link
Author

cuu508 commented Apr 30, 2020

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.

@nazar-pc
Copy link
Collaborator

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().

@cuu508
Copy link
Author

cuu508 commented Apr 30, 2020

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?

@nazar-pc
Copy link
Collaborator

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.

@cuu508
Copy link
Author

cuu508 commented Apr 30, 2020

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.

@nazar-pc
Copy link
Collaborator

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.

@t-mullen
Copy link
Collaborator

t-mullen commented May 3, 2020

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... 🙁

@ForesightImaging
Copy link

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'.

@t-mullen
Copy link
Collaborator

t-mullen commented Aug 9, 2020

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.

@snettah
Copy link

snettah commented Nov 1, 2020

@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.

@Mihai-github
Copy link

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..

@n9lsjr
Copy link

n9lsjr commented Sep 26, 2022

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

@EdoardoEntusiasta
Copy link

EdoardoEntusiasta commented Nov 3, 2022

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"

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

8 participants