CC Interceptor Callback Design Makes Associating BandwidthEstimators with PeerConnections Difficult

[harshabose]

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

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

2. Callback timing:
   Even if the ID is valid, the callback is triggered during peer connection creation (before api.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 of webrtc.PeerConnections.

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

1. Pass the PeerConnection's statsID to interceptorRegistry.Build() instead of an empty string
2. OR modify the callback to receive the actual PeerConnection reference along with the estimator
3. 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.

[mengelbart]

Thanks for reporting this. It is a known issue, and I have some ideas on how to fix it. I created pion/interceptor#300, which would allow us to run the bandwidth estimator outside the interceptors. The interceptors are only needed to aggregate some stats on outgoing packets and incoming feedback reports. The user would then have to start the bandwidth estimator manually and pass feedback received from RTCP to the bandwidth estimator. Would that approach solve your issue?

[harshabose]

Thank you for the response. I looked into the ccfb-receiver, and it looks promising.

To confirm my understanding: the new approach would separate data collection from bandwidth estimation, where:

1. The ccfb-receiver interceptor collects packet data and stores reports
2. Applications can access these reports through interceptor attributes
3. We would implement our own bandwidth estimation logic or use the existing gcc bwe to use this data

// Register the new feedback collector interceptor
feedbackCollector := ccfb.NewInterceptor()
registry.Add(feedbackCollector)

// When creating a peer connection:
pc, err := api.NewPeerConnection(...)
// Store in map
client.peerConnections[pcID] = pc

// Now read RTCP packets and get the feedback reports
// Somewhere in the code...
// Get the feedback report from attributes
attr := interceptor.GetAttributes(packets)
if report, ok := attr.Get("ccfb_report").(*ccfb.Report); ok {
    // Use the report to update bandwidth estimator for this connection
    client.peerConnections[pcID].updateBandwidthEstimate(report)
}

Do you have an estimated timeline for when this might be available? While we wait for this, would you recommend any specific pattern for managing bandwidth estimators with multiple peer connections apart from the channel based approach?
Thanks for your work on this!