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

Need something such as rtpSender.reset() #2087

Closed
ibc opened this issue Jan 28, 2019 · 14 comments
Closed

Need something such as rtpSender.reset() #2087

ibc opened this issue Jan 28, 2019 · 14 comments
Labels

Comments

@ibc
Copy link

ibc commented Jan 28, 2019

Use case

  1. Create a transceiver for sending video with simulcast:
const transceiver = pc.addTransceiver(track, {
  direction: 'sendonly',
  sendEncodings: [
    { rid: '111', active: true, maxBitrate: 111111 },
    { rid: '222', active: true, maxBitrate: 222222 },
    { rid: '333', active: true, maxBitrate: 333333 }
  ]
};

// or pass those encodings via transceiver.sender.setParameters()
  1. Later deactivate the video sender:
track.stop();
await transceiver.sender.replaceTrack(null);
transceiver.direction = "inactive";
  1. Reuse the transceiver to send a new video, now without simulcast:
await transceiver.sender.replaceTrack(newTrack);
transceiver.direction = "sendonly";

const parameters = transceiver.sender.getParameters();
const newEncodings = [
  { active: true, maxBitrate: 222222 }
];

await transceiver.sender.setParameters({ ...parameters, encodings: newEncodings });

The problem

According to the spec this is not valid:

6.3 Let N be the number of RTCRtpEncodingParameters stored in sender's internal [[SendEncodings]] slot. If any of the following conditions are met, return a promise rejected with a newly createdInvalidModificationError:

6.4 encodings.length is different from N.

So reusing a transceiver becomes a pain because we must "accept" the previously set encodings and now we can just "mangle" some of them to accomplish with our new and probably unrelated needs.

Yes, of course, weI could initially set 20 encodings and set active: false in most of them, so later we can switch them on/off individually to get the "desired" behavior. This is far from "elegant".

Proposal

Have a new rtpSender.reset() or transceiver.resetSender() method that replaces the previous RtpSender with a fresh one, without dependencies or settings based on previous usages.

Of course, such a reset() method should throw if called while the transceiver has direction "sendonly" or "sendrecv".

@aboba aboba added the question label Jan 29, 2019
@aboba
Copy link
Contributor

aboba commented Jan 29, 2019

@ibc You can set active to false for the last two encodings in order to turn off simulcast. Implementations probably won't support 20 encodings so you'd get some lower number anyway so that the hassle factor is probably not great.

The fundamental problem is that the number of simulcast streams is negotiated in Offer/Answer, so it can't be changed without negotiation and encoding parameters cannot set negotiationneeded. Since a reset method would set negotiationneeded you still couldn't change the number of encodings without negotiation.

@ibc
Copy link
Author

ibc commented Jan 29, 2019

@aboba I don't think that is an elegant approach nor even a workaround. Take into account that some settings such as rid are read-only (or just writable once, the first time). Just because I want to reuse a transceiver (to keep my SDP from growing) it should NOT mean that I have to reuse previous rid values, neither it should mean that I have to manually "reset" all possible encoding values to whichever values they have by default (dtx, maxBitrate, maxFramerate, scaleResolutionBy, etc etc etc). It's a no sense.

The problem is that the number of simulcast streams is negotiated in Offer/Answer, so can't be changed without negotiation

I didn't know that rid values can not change during a renegotiation. Ok, I give up. "Don't fight the SDP". I'll do as I already do: create a new transceiver for sending a new video even if the previous video was already closed/stopped.

@aboba
Copy link
Contributor

aboba commented Jan 29, 2019

The rid values are set during negotiation, which is why they are read-only. Presumably if there is only one encoding that is active, the rid value becomes meaningless since rid header extensions should not be sent (we may need to clarify this).

Changing rid values in a negotiation is difficult because the rid attribute is read-only, and SDP can no longer be changed between createOffer and setLocalDescription. Let us say that a conference server sent an Offer with recv RID values, but the application wants to send a re-offer that changes the values.

It does not seem like there is a way to do this. Therid encoding parameter is read-only so that it cannot be changed so as to modify the output of createOffer and the SDP can no longer be modified between createOffer and setLocalDescription. So the only thing the application can do is to change RID values in the SDP provided by createOffer and then send the modified Offer to the remote peer. However, if it does this, the RIDs configured on the remote peer will not match what the local peer is configure to send, so unless there is an SFU modifying the RID header extensions before forwarding to the remote peer, it won't work.

@ibc
Copy link
Author

ibc commented Jan 29, 2019

In summary: do never reuse the same transceiver for sending a new track unless you want to also reuse the very same sending parameters. So SDP semantics are conditioning the WebRTC 1.0 API much more than in my worst dreams...

@murillo128
Copy link

not sure if I follow, let's imagine that we are in stable state after SDP O/A negotiation.

Then if it could be possible to do something like this, for resetting the transceiver:

transceiver.reset(track,{
  direction: 'sendonly',
  sendEncodings: [
    { rid: '111', active: true, maxBitrate: 111111 },
    { rid: '222', active: true, maxBitrate: 222222 },
    { rid: '333', active: true, maxBitrate: 333333 }
  ]
});
```
This would cause a renegotiation with the remote peer, and the new encodings would apply, right? 

@ibc
Copy link
Author

ibc commented Jan 29, 2019

What @aboba says is that, once first SDP O/A is done, a=rid values cannot be modified/removed in next SDP re-O/, which invalidates any chance to do a real "reset" of the sending parameters.

In short: we chose SDP for WebRTC and now we must di... live with it.

@murillo128
Copy link

murillo128 commented Jan 29, 2019

Since a reset method would set negotiationneeded you still couldn't change the number of encodings without negotiation.

removing the negation, I read that as you could change the number of encodings with negotiation

@alvestrand
Copy link
Contributor

But why do you want to reuse a transceiver like this?

I can see wanting to reuse a media section (it's bits on the wire in an SDP description), but a transceiver is just bits in memory.

Once you have transceiver.stop(), the media section can be reused for a new transceiver (with a different mid).

@murillo128
Copy link

if stop() + addTransceiver(track,params) works, it should be trivial to specify a reset(track,params) method that performs both two operations atomically, keeping the transceiver js object and the mid.

@alvestrand
Copy link
Contributor

stop() + addTransceiver(track, params) will create a new transceiver. It uses code already present.
A reset() that doesn't create a new transceiver is a new code path.

If you keep the MID, then you're telling the other guy to keep his transceiver, even though it's being renegotiated - which means that you can't change # of layers - which was the original point.

again: Why do you think you need this?

@murillo128
Copy link

I think this is the key, is it allowed to change the simulcast rids (add or remove) on a re-offer? Not talking about API now, just SDP O/A.

@ibc
Copy link
Author

ibc commented Jan 31, 2019

stop() + addTransceiver(track, params) will create a new transceiver. It uses code already present.
A reset() that doesn't create a new transceiver is a new code path.

If you keep the MID, then you're telling the other guy to keep his transceiver, even though it's being renegotiated - which means that you can't change # of layers - which was the original point.

again: Why do you think you need this?

In my simplified use case, I've a PeerConnection I just use for sending media. I want to send a video track, "close" it, then send a new video track without having to reuse previous RtpSender parameters (encodings, mid, etc). Well, I don't care much about reusing previous a=mid or not. In addition I'd like that my local SDP does not grow every time I close a transceiver and add a new one.

So if I understand your text above, calling transceiver.stop() (not implemented in Chrome BTW) and later call pc.addTransceiver() I would get a new RtcTransceiver instance (of course) with a fresh RTCRtpSender and the previously stoped m= section would be reused (maybe with a new a=mid). Is that right?

@alvestrand
Copy link
Contributor

That's right. The language that permits this is in JSEP section 5.2.2.

o If any RtpTransceiver has been added, and there exists an m=
section with a zero port in the current local description or the
current remote description, that m= section MUST be recycled by
generating an m= section for the added RtpTransceiver as if the m=
section were being added to the session description (including a
new MID value), and placing it at the same index as the m= section
with a zero port.

@ibc
Copy link
Author

ibc commented Jan 31, 2019

Thanks @alvestrand. Then we just need transceiver.stop() to be implemented in Chrome (Firefox and Safari do implement it) :)

I'll close the issue.

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

No branches or pull requests

4 participants