Skip to content

feat: implement compression stream in c++ #43

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

Merged
merged 6 commits into from
Nov 12, 2024

Conversation

grabbou
Copy link
Collaborator

@grabbou grabbou commented Nov 12, 2024

cpp zlib is better, writes headers automatically, so we can get rid of that logic.

As a next step, I guess we could implement streams in C++, but unfortunately, we can't pass them as arguments to Swift/Kotlin HybridObjects, so in case there's a platform API that want to consume stream, this would not work.

So maybe in the future, I talk to Marc at the conference about this.

@grabbou grabbou changed the title wip: move streams to cross-platform c++ feat: implement compression stream in c++ Nov 12, 2024
@grabbou grabbou mentioned this pull request Nov 12, 2024
Copy link

@mani3xis mani3xis left a comment

Choose a reason for hiding this comment

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

Looks great 🚀
I tried really hard, but I found only a few nit picks, and one non-standard function. I haven't fully checked for compliance with the spec. Only DecompressionStream is missing ;)

controller.enqueue(new Uint8Array(compressedData))
} catch (error) {
console.error(error)
}
},
flush(controller) {

Choose a reason for hiding this comment

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

[nit] This is not a standard method. Can we get rid of it, and automatically flush when the stream is closed?

@grabbou grabbou merged commit 1411a92 into feat/android-initial Nov 12, 2024
grabbou added a commit that referenced this pull request Nov 12, 2024
* init

* fix

* Revert changes'

* fix: android build

* chore: tweaks

* chore: share const
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