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

Adds support for AEAD_AES_256_GCM #238

Merged
merged 3 commits into from
May 16, 2023
Merged

Adds support for AEAD_AES_256_GCM #238

merged 3 commits into from
May 16, 2023

Conversation

digitalix
Copy link
Member

Description

This adds support for AEAD_AES_256_GCM in srtp module. Later webrtc itself will have to be updated as well to enable support for AEAD_AES_256_GCM fully.

@digitalix digitalix requested a review from Sean-Der May 16, 2023 14:00
@codecov
Copy link

codecov bot commented May 16, 2023

Codecov Report

Patch coverage: 87.50% and project coverage change: +0.03 🎉

Comparison is base (32e385b) 76.50% compared to head (b2d8f18) 76.54%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #238      +/-   ##
==========================================
+ Coverage   76.50%   76.54%   +0.03%     
==========================================
  Files          17       17              
  Lines        1243     1245       +2     
==========================================
+ Hits          951      953       +2     
  Misses        200      200              
  Partials       92       92              
Flag Coverage Δ
go 76.54% <87.50%> (+0.03%) ⬆️
wasm 76.06% <87.50%> (+0.03%) ⬆️

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

Impacted Files Coverage Δ
protection_profile.go 80.00% <85.71%> (+0.83%) ⬆️
context.go 88.61% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Member

@Sean-Der Sean-Der left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Great to see you contributing again @digitalix

@Sean-Der
Copy link
Member

@at-wat @stv0g What do you think about removing the go.mod bumping. Maybe I am missing the value add?

Users on old versions of Go will think they can’t use these libraries. We aren’t using any new Go features though so no reason to block them?

Sorry If I am missing the benefit. Just want Pion to work for most people possible

@stv0g
Copy link
Member

stv0g commented May 16, 2023

My motivation for this check has been mainly harmonization between the various Pion modules.
In my opinion, we should also set clear expectations about which Go versions are supported.
Currently, we are only testing Go 1.19 and 1.20 which matches Go's support policy.
Hamonizing the go.mod version also clearly states which language features and standard packages we are allowed to use.
Currently, we have largely varying versions in the different modules which makes it hard to grasp which features we can use.

@digitalix digitalix merged commit 2987553 into master May 16, 2023
16 checks passed
@digitalix digitalix deleted the digitalix/aes-256-gcm branch May 16, 2023 22:28
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

3 participants