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

Optimize doResize method with binary search #6137

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

JustSNguyen
Copy link
Contributor

@JustSNguyen JustSNguyen commented Nov 7, 2024

This pull request addresses issues discussed in #6112 . Currently, if an uploaded image exceeds a certain size threshold, BlueSky reduces the image quality by 10% and checks if the new size falls within the threshold. If not, BlueSky repeats this step until the image quality reaches 0. There are two issues with this approach:

  1. As mentioned in Proposal: Be smarter about quality level when converting images to JPGs #6112, even a 1-2% decrease in quality can significantly reduce the image size, so the 10% decrement is somewhat overkill.
  2. The current solution could be optimized, especially for larger images. For these, the current approach requires multiple iterations to reach the target size, which is not ideal.

My proposed solution is to use binary search to find the maximum quality level that keeps the image size within the limit. This approach will prevent too much loss in image quality. Not only that, I also ran some benchmarks, and it appears that the current solution may perform slightly better for smaller images. However, for larger images, the binary search method does much better. That said, I only conducted a few tests here, so additional testing may be needed to confirm the difference.

Image Size (KB) Current Method (ms) Binary Search Method (ms)
1148 844 1040
1911 5454 6477
3011 5530 3396
3230 6079 5402
10609 12210 4141
14765 26390 10213

@gaearon
Copy link
Collaborator

gaearon commented Nov 7, 2024

This makes sense. Let's do it on all platforms?

@JustSNguyen JustSNguyen force-pushed the optimize/optimize-doResize branch from 08a8d4a to 1829c68 Compare November 7, 2024 15:19
@JustSNguyen
Copy link
Contributor Author

I have updated the PR to also support this for the mobile version @gaearon.

@JustSNguyen JustSNguyen changed the title Optimize doResize method with binary search (web version only) Optimize doResize method with binary search Nov 7, 2024
@auroursa
Copy link
Contributor

Excellent! Uploading photos taken with an iPhone(obviously compression is needed) clearly shows that the new algorithm improves image quality noticeably, with a shorter processing time as well.

@gaearon
Copy link
Collaborator

gaearon commented Nov 25, 2024

This is changing lib/media/manip but I'm seeing some code go through a similar path in src/state/gallery. Would you mind checking how it works today? It seems like maybe it's forked between the image picker vs inline camera, but I'm not sure.

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.

3 participants