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

NewAPI: register default codecs and interceptors #2630

Merged
merged 1 commit into from Dec 11, 2023

Conversation

andrein
Copy link
Contributor

@andrein andrein commented Dec 1, 2023

Description

As discussed on Discord with @Sean-Der, NewAPI() should register default codecs and interceptors unless configured otherwise.

Copy link

codecov bot commented Dec 1, 2023

Codecov Report

Attention: 6 lines in your changes are missing coverage. Please review.

Comparison is base (2fcd8a1) 76.46% compared to head (c56dde6) 76.34%.

Files Patch % Lines
api.go 62.50% 4 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2630      +/-   ##
==========================================
- Coverage   76.46%   76.34%   -0.13%     
==========================================
  Files          87       87              
  Lines        9867     9879      +12     
==========================================
- Hits         7545     7542       -3     
- Misses       1855     1865      +10     
- Partials      467      472       +5     
Flag Coverage Δ
go 77.87% <62.50%> (-0.14%) ⬇️
wasm 64.54% <ø> (ø)

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

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

@andrein
Copy link
Contributor Author

andrein commented Dec 5, 2023

Hey @Sean-Der, I've fixed the failing tests. Can you approve the CI run please?

Should I update the examples and other tests to remove the setup code for default codecs / interceptors?

@Sean-Der
Copy link
Member

Full context/justification for this change

  • Users are confused/frustrated when switching from NewAPI -> NewPeerConnection and things don't work
  • Users that still want a empty MediaEngine can enable
  • We are working on a /v4 so API break is ok

@Sean-Der Sean-Der merged commit 21c5a71 into pion:master Dec 11, 2023
15 of 16 checks passed
@andrein andrein deleted the newapi branch December 11, 2023 18:29
andrein added a commit to andrein/webrtc that referenced this pull request Dec 11, 2023
The NewAPI() function already registers them by default  (since pion#2630)
andrein added a commit to andrein/webrtc that referenced this pull request Dec 11, 2023
Skip registering the default codecs and interceptors in
NewPeerConnection.
The NewAPI() function already registers them by default  (since pion#2630)
Sean-Der pushed a commit that referenced this pull request Mar 18, 2024
Skip registering the default codecs and interceptors in
NewPeerConnection.
The NewAPI() function already registers them by default  (since #2630)
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.

None yet

2 participants