-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Description
Environment
- Pion WebRTC v4
- Go 1.23
- Using CC interceptor from github.com/pion/interceptor/pkg/cc
Description
The current design of the Congestion Control (CC
) interceptor's OnNewPeerConnection
callback makes it difficult to properly associate bandwidth estimators with their respective peer connections in applications managing multiple WebRTC connections.
The CC
interceptor factory provides the OnNewPeerConnection
function to register a callback:
// from [email protected]/pkg/cc/intercepter.go
// NewPeerConnectionCallback returns the BandwidthEstimator for the
// PeerConnection with id
type NewPeerConnectionCallback func(id string, estimator BandwidthEstimator)
// OnNewPeerConnection sets a callback that is called when a new CC interceptor
// is created.
func (f *InterceptorFactory) OnNewPeerConnection(cb NewPeerConnectionCallback) {
f.addPeerConnection = cb
}
According to the NewPeerConnectionCallback type definition and the comment, this callback is expected to provide a BandwidthEstimator for a new PeerConnection, identified by an id string.
Problem
- Empty ID parameter:
// from webrtc/v4/api.go:
i, err := api.interceptorRegistry.Build("") // Empty string passed as ID
This results in the callback receiving an empty ID, making it impossible to identify which peer connection the estimator belongs to.
- Callback timing:
Even if the ID is valid, the callback is triggered during peer connection creation (beforeapi.NewPeerConnection()
returns), meaning the application doesn't yet have a reference to the peer connection when it needs to associate the estimator, making it difficult to manage group ofwebrtc.PeerConnection
s.
congestionController.OnNewPeerConnection(func(id string, estimator cc.BandwidthEstimator) {
client.peerConnections[id].estimator = estimator // does not exists
}
Code Example
// Set up callback
congestionController.OnNewPeerConnection(func(id string, estimator cc.BandwidthEstimator) {
fmt.Printf("got bitrate estimator for peer connection with label: '%s'\n", id) // Prints empty string
// This fails because NewPeerConnection() hasn't returned yet
if pc, exists := client.peerConnections[id]; !exists {
fmt.Println("peer connection does not exist in client map")
}
})
// Later...
pc, err := api.NewPeerConnection(...) // This triggers the callback before returning
Expected vs. Actual Behavior
Expected:
Callback receives a meaningful ID that can be used to associate the estimator with a specific peer connection
OR callback is called after the peer connection is constructed and made available to the application
Actual:
Callback receives an empty string as ID
Callback is called before the peer connection is available to the application
Current Workaround
The examples/bandwidth-estimation-from-disk
example uses a channel-based approach for a single connection:
estimatorChan := make(chan cc.BandwidthEstimator, 1)
congestionController.OnNewPeerConnection(func(id string, estimator cc.BandwidthEstimator) {
estimatorChan <- estimator
})
// ...create peer connection...
estimator := <-estimatorChan // Wait for estimator
However, this pattern doesn't scale well to multiple connections.
Suggested Improvements
- Pass the PeerConnection's statsID to interceptorRegistry.Build() instead of an empty string
- OR modify the callback to receive the actual PeerConnection reference along with the estimator
- OR provide a way to retrieve the estimator after the peer connection is fully created
Any of these changes would make it much easier to properly implement bandwidth estimation in multi-connection applications.