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

Should pc.removeTrack(sender) "reset" sending parameters in the transceiver? #2025

Closed
ibc opened this issue Nov 5, 2018 · 12 comments
Closed

Comments

@ibc
Copy link

ibc commented Nov 5, 2018

Let's have a video transceiver and apply simulcast to its sender via sender.setParameters(...) or initially via pc.addTransceiver(track, { sendEncodings: [ ... ] }).

Later we disable the sending video by calling pc.removeTrack(transceiver.sender).

And later let's attach the previous or a new video track into the same transceiver by using transceiver.sender.replaceTrack(newTrack).

Question: Should newTrack be sent with simulcast or not? The current spec doesn't clarify it IMHO and it's something important to know when it comes to reuse a transceiver for sending new tracks.

@ibc
Copy link
Author

ibc commented Nov 6, 2018

Firefox keeps previous sender parameters after calling pc.removeTrack(transceiver.sender) (reported here).

@alvestrand
Copy link
Contributor

we made the sender parameters parameters of the sender, not parameters of the sender/track association.
Making removeTrack() affect send parameters seems like it makes the model more complicated, and I can't see a benefit from doing that.

@ibc
Copy link
Author

ibc commented Nov 7, 2018

Making removeTrack() affect send parameters seems like it makes the model more complicated, and I can't see a benefit from doing that.

Use case:

  1. Send webcam with simulcast (by app design others will see you in full screen or in a thumbnail).
  2. Later stop webcam and start screen sharing. No simulcast desired but just HD (others will see it in fullscreen so high quality is needed).
  3. And I don't want to artificially generate a new m=video section but, instead, stop the webcam and reuse its transceiver to send screen sharing without simulcast.

I could keep original sender parameters before applying simulcast the first time, and then apply those original params later when reusing the sender for screen sharing. But, not sure why, I expect a way to just "reset" it.

we made the sender parameters parameters of the sender, not parameters of the sender/track association.

Could that be clarified in the spec?

@alvestrand
Copy link
Contributor

  1. send webcam with simulcast
  2. removeTrack(); setParameters(encodings:[{enabled: true, enabled: false...}] ); replaceTrack()
  3. Presentation gets transmitted with no simulcast.

If you want to know what the original parameters were, you have to keep them. But more likely, you have a specific set of parameters in mind.

FWIW, the screencast teams tend to want simulcast too. And neither the video folks nor the screencast folks seem to be at all eager to stop fiddling with the parameters; "reset" to an unknown state is not likely to be useful to them.

@ibc
Copy link
Author

ibc commented Nov 8, 2018

Sure @alvestrand, I agree. The problem is that, for now and regarding sender parameters, there is little to play with in current implementations.

@aboba aboba added the question label Nov 8, 2018
@aboba
Copy link
Contributor

aboba commented Nov 20, 2018

@ibc Have we answered this question? If so, can we close this issue?

@ibc
Copy link
Author

ibc commented Nov 20, 2018

@aboba the answer is clear to me. But IMHO the spec is unclear (that's why I had to ask it here for clarification). I think that some clarification is needed in the spec.

@jan-ivar
Copy link
Member

I don't see where the spec is unclear here. removeTrack says what it does in detail—sets [[SenderTrack]] and [[Direction]] and triggers negotiationneeded—and it doesn't say anything about resetting sender parameters. I don't think we need to call out what it doesn't do, so I'm closing this.

@ibc
Copy link
Author

ibc commented Nov 26, 2018

I'll explain the "issue" in a different way:

  • pc.addTrack(track) creates (or may create) a RTCRtpSender.
  • pc.removeTrack(sender) is a bit... well, idiomatically strange. If addTrack() creates a RTCRtpSender, one may expect that removeTrack() removes a RTCRtpSender.

Now let's do this:

  • const sender1 = pc.addTrack(track1)
  • sender1.setParameters(params1)
  • pc.removeTrack(sender1)
  • const sender2 = pc.addTrack(track2)

...and it happens that sender2 keeps the same params1 as before.

Yes, we know that sender1 and sender2 are (or may be) the same RTCRtpSender object. And we know it BECAUSE THE TRANSCEIVERS, but it sounds strange anyway...

@jan-ivar
Copy link
Member

You probably meant pc.removeTrack(sender1) ;)

If your point is that the addTrack/removeTrack API names are terrible, and set up a false symmetry, then 100% agree. This is why I voted to move at least removeTrack to the legacy section in #1758.

Btw, thanks for a rockin' WPT test!

const pc = new RTCPeerConnection();
const sender1 = pc.addTrack(track, stream);
pc.removeTrack(sender1);
const sender2 = pc.addTrack(track2, stream);
console.log(sender1 === sender2); // true

@ibc
Copy link
Author

ibc commented Nov 26, 2018

You probably meant pc.removeTrack(sender1) ;)

Yes, fixed :)

Btw, thanks for a rockin' WPT test!

Well, it's true in this simple usage. It may not false if more transceivers are created between sender1 and sender2.

@jan-ivar
Copy link
Member

Yeah, but at least it's deterministic.

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

No branches or pull requests

4 participants