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

webrtc: add FIN_ACK to close datachannels without data loss #582

Merged
merged 7 commits into from
Oct 5, 2023

Conversation

achingbrain
Copy link
Member

Specify closing datachannels by mutual agreement to ensure all data has been received by the remote before closing.

Refs: #575

Specify closing datachannels by mutual agreement to ensure all data has been received by the remote before closing.

Refs: #575
Copy link
Member

@mxinden mxinden left a comment

Choose a reason for hiding this comment

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

I find the introduction of a FIN_ACK a clean solution to the problem. A couple of suggestions, mostly editorial.

webrtc/README.md Outdated Show resolved Hide resolved
webrtc/README.md Outdated Show resolved Hide resolved
webrtc/README.md Outdated Show resolved Hide resolved
webrtc/README.md Outdated Show resolved Hide resolved
webrtc/README.md Outdated Show resolved Hide resolved
webrtc/README.md Outdated Show resolved Hide resolved
webrtc/README.md Outdated Show resolved Hide resolved
webrtc/README.md Outdated Show resolved Hide resolved
Co-authored-by: Max Inden <mail@max-inden.de>
webrtc/README.md Outdated Show resolved Hide resolved
webrtc/README.md Outdated Show resolved Hide resolved
webrtc/README.md Outdated Show resolved Hide resolved
Copy link
Contributor

@thomaseizinger thomaseizinger left a comment

Choose a reason for hiding this comment

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

Great work! Some final comments.

webrtc/README.md Outdated Show resolved Hide resolved
webrtc/README.md Outdated Show resolved Hide resolved
webrtc/README.md Outdated Show resolved Hide resolved
webrtc/README.md Outdated Show resolved Hide resolved
webrtc/README.md Outdated Show resolved Hide resolved
Copy link
Member

@sukunrt sukunrt left a comment

Choose a reason for hiding this comment

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

This looks great! 🚀

Copy link
Contributor

@thomaseizinger thomaseizinger left a comment

Choose a reason for hiding this comment

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

Great work!

No more concerns from my end.

@achingbrain
Copy link
Member Author

Awesome stuff everyone!

@achingbrain achingbrain changed the title webrtc: add FIN_ACK to spec to close datachannels without data loss webrtc: add FIN_ACK to close datachannels without data loss Oct 5, 2023
@achingbrain achingbrain merged commit 87c684e into master Oct 5, 2023
@achingbrain achingbrain deleted the docs/webrtc-fin-ack branch October 5, 2023 08:20
@sukunrt sukunrt mentioned this pull request Feb 21, 2024
19 tasks
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.

4 participants