Skip to content

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

Merged
achingbrain merged 7 commits intomasterfrom
docs/webrtc-fin-ack
Oct 5, 2023
Merged

webrtc: add FIN_ACK to close datachannels without data loss#582
achingbrain merged 7 commits intomasterfrom
docs/webrtc-fin-ack

Conversation

@achingbrain
Copy link
Copy Markdown
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
Copy Markdown
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.

Co-authored-by: Max Inden <mail@max-inden.de>
Copy link
Copy Markdown
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.

Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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