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

Improve BlurHash decoding performance #3753

Merged
merged 3 commits into from
Jul 16, 2024

Conversation

cbeyls
Copy link
Contributor

@cbeyls cbeyls commented Jul 14, 2024

Changes
Replace the Kotlin Multiplatform BlurHash library with a new local implementation of the decoder which is at least 2 times faster while using less memory (no cache is needed).

For details about the optimizations and some benchmarks, see woltapp/blurhash#254.

@nielsvanvelzen nielsvanvelzen added the enhancement New feature or request label Jul 14, 2024
Copy link

@github-advanced-security github-advanced-security bot left a comment

Choose a reason for hiding this comment

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

detekt found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.

Copy link
Member

@nielsvanvelzen nielsvanvelzen left a comment

Choose a reason for hiding this comment

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

Thanks for the pull request. I'm not against using our own implementation as it won't need much maintenance (it's just a single file).

It should use our package name (probably somewhere in utils) and follow our code style (use tabs).

Additionally, the Detekt warnings can be suppressed for this file as the magic numbers are to be expected.

@nielsvanvelzen
Copy link
Member

Awesome, thanks for the contribution! I've tweaked the styling a bit more (adding new lines, moving the CHARS const to the top and using when statements instead of if statements).

@nielsvanvelzen nielsvanvelzen added this to the v0.17.0 milestone Jul 16, 2024
@nielsvanvelzen nielsvanvelzen enabled auto-merge (rebase) July 16, 2024 11:51
@nielsvanvelzen nielsvanvelzen merged commit 801c043 into jellyfin:master Jul 16, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants