Skip to content

Pass estimator options through GCC - #178

Merged
jayli-nuro merged 1 commit into
pion:mainfrom
jayli-nuro:feat/pass-estimator-options
Sep 1, 2026
Merged

jayli-nuro merged 1 commit into
pion:mainfrom
jayli-nuro:feat/pass-estimator-options

Conversation

@jayli-nuro

Copy link
Copy Markdown
Contributor

The GCC option constructs the estimator itself, so a caller can only influence it through the two bitrate arguments:

opts := []gcc.Option{gcc.SendSideBWEInitialBitrate(initialBitrate)}
if maxBitrate > 0 {
    opts = append(opts, gcc.SendSideBWEMaxBitrate(maxBitrate))
}

return gcc.NewSendSideBWE(opts...)

Every other gcc.Option — the pacer, and anything added to that package later — is unreachable. Configuring the estimator at all currently means editing this package.

This accepts a variadic ...gcc.Option on GCC, setupGCC and newGCCFactory, and appends it after the bitrate options:

opts = append(opts, extra...)

Appending last is deliberate, so a caller can also override the two bitrate options rather than only add to them.

Backward compatible. Adding a variadic parameter does not change existing call sites, so the two-argument callers in examples/ and the tests are untouched. TestGCCRemainsCallableWithTwoArguments pins that.

Three tests: that an option reaches NewSendSideBWE, that a later option overrides the initialBitrate argument, and that the two-argument form still compiles and runs. Removing the append fails the first two.

gofmt, go vet and go test ./sender/ are green.

The GCC option builds the estimator itself, so a caller can only set the
initial and maximum bitrate. Any other gcc.Option is unreachable, which
means configuring the estimator at all requires editing this package.

Accept a variadic gcc.Option on GCC, setupGCC and newGCCFactory, and
append it after the bitrate options so a caller can also override those.
Adding a variadic parameter is backward compatible, so the existing
two-argument callers are unchanged.
@codecov

codecov Bot commented Aug 28, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 52.70%. Comparing base (76ac6d1) to head (70e69ea).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #178      +/-   ##
==========================================
+ Coverage   52.59%   52.70%   +0.11%     
==========================================
  Files          21       21              
  Lines        2181     2182       +1     
==========================================
+ Hits         1147     1150       +3     
+ Misses        947      946       -1     
+ Partials       87       86       -1     
Flag Coverage Δ
go 52.70% <100.00%> (+0.11%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jayli-nuro
jayli-nuro merged commit adbd1e9 into pion:main Sep 1, 2026
18 checks passed
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.

2 participants