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

Congestion control API proposal #207

Merged
merged 4 commits into from
Dec 12, 2023
Merged

Congestion control API proposal #207

merged 4 commits into from
Dec 12, 2023

Conversation

alvestrand
Copy link
Contributor

@alvestrand alvestrand commented Oct 4, 2023

Fixes #206


Preview | Diff

index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
@youennf
Copy link
Collaborator

youennf commented Oct 5, 2023

I think there is a use case for FIR/PLI handling.
I initially thought there would be a similar need for congestion control but I wonder how much this could be dealt only by the UA monitoring the change of bandwidth induced by the transform (see also #50 (comment)).

I would tend to split this PR in two if we are not quickly converging on the need for the congestion control API.

@alvestrand
Copy link
Contributor Author

Since we've had the lack of congestion control put forward as a major argument for why moving frames between peerconnections is dangerous (discussion on #201), I hope we can quickly converge on saying "yes, this is needed" and continue to "is this shape appropriate".

@youennf
Copy link
Collaborator

youennf commented Oct 10, 2023

Since we've had the lack of congestion control put forward as a major argument for why moving frames between peerconnections is dangerous

It is an issue for that specific use case, which might need a different API from the current webrtc encoded transform API.
I am unsure how much this congestion API will help developers using the current webrtc encoded transform API.

@alvestrand
Copy link
Contributor Author

It will help for scenarios that move frames between PeerConnections (Low Latency Broadcast with Fanout, requirement N39), even though the initial deployment scenario envisioned at Google (upstream bottleneck, downstream high-capacity LAN) doesn't need it.
Agreed that a different API shape than ScriptTransform may suit this use case better, but it's possible to live within the constraints of ScriptTransform for this use case.

It will also help for the "adding lots of metadata to the encoded frame" use case, where the bandwidth that is sent on the wire is very different from the bandwidth that is produced by the encoder. That functionality can be used to satisfy requirement N23 in webrtc-nv-use-cases section 3.5

@alvestrand
Copy link
Contributor Author

Conclusions after discussion on WEBRTC WG October interim:

  • Keyframe signaling is worthwhile, but should go in a separate PR
  • Signals for bandwidth changes aren't necessary; if the transform needs to look at bandwidth controls, it should just read the bandwidth info attribute every time it sends a frame

@alvestrand
Copy link
Contributor Author

Comment on October interim resolution:
The signal for bandwidth change also served as a control surface for saying "I want to tell upstream about this myself". We should have such a knob somewhere in the API; calling the sendBandwidthInfo might be a good enough signal.

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

Successfully merging this pull request may close these issues.

Expose flow control information to transform
3 participants