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

feat: 5952 - better image compression for Prices #6048

Merged
merged 3 commits into from
Dec 20, 2024

Conversation

monsieurtanuki
Copy link
Contributor

What

  • The Prices team (@raphael0202 @raphodn) regularly complains about the size of the proof files we send them.
  • Creating .webp files being only partially supported in flutter, the solution was to keep .jpeg as a format, but with different compression parameters.
  • From now on, the proof images will be compressed with a 80% quality (which seems more than enough from what I read on the web) (and that's the current quality used by the Prices team on their app for .webp), and all images will be compressed (including images that are not cropped and that were sent as-is)
  • The compression for standard OFF images remains the same: 100% quality (which may be too much), and not compressing images when they're not cropped (which is questionable too)

Fixes bug(s)

Impacted files

  • image_compute_container.dart: added the compression quality parameter
  • background_task_image.dart: added the compression quality and force quality parameters; same parameters as before for OxF images (quality 100, no forced compression for non cropped images)
  • background_task_add_price.dart: always compress the proof image, with quality 80

Impacted files:
* `image_compute_container.dart`: added the compression quality parameter
* `background_task_image.dart`: added the compression quality and force quality parameters; same parameters as before for OxF images (quality 100, no forced compression for non cropped images)
* `background_task_add_price.dart`: always compress the proof image, with quality 80
@codecov-commenter
Copy link

codecov-commenter commented Dec 13, 2024

Codecov Report

Attention: Patch coverage is 0% with 7 lines in your changes missing coverage. Please review.

Project coverage is 6.46%. Comparing base (4d9c7fc) to head (8211b0d).
Report is 571 commits behind head on develop.

Files with missing lines Patch % Lines
...ooth_app/lib/background/background_task_image.dart 0.00% 6 Missing ⚠️
...mooth_app/lib/helpers/image_compute_container.dart 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           develop   #6048      +/-   ##
==========================================
- Coverage     9.54%   6.46%   -3.09%     
==========================================
  Files          325     443     +118     
  Lines        16411   25140    +8729     
==========================================
+ Hits          1567    1625      +58     
- Misses       14844   23515    +8671     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@raphodn
Copy link
Member

raphodn commented Dec 13, 2024

The Prices team (@raphael0202 @raphodn) regularly complains about the size of the proof files we send them.

sorry, what ?
let me reformulate : to reduce bandwidth, speed up contribution and improve the overall user experience, images can be compressed by a factor of x5 before being sent, without any impact on quality and post-processing (AI image detection)

@monsieurtanuki
Copy link
Contributor Author

@raphodn It looks like you assumed I meant "complain" in a negative way, like "whine" or "groan" (e.g. "French people always complain").
I've just double-checked on a dictionary, and it doesn't have to be negative.

The title of the PR is about "better image compression", and in the description I also question our current settings for OFF images that would probably be improved in the same way.

Just a couple of "constructive" use of "complain" from https://www.merriam-webster.com/sentences/complain, and both about IT:

  • "Some reviewers also complain of a sometimes frustrating setup process."
  • "Drivers reportedly complained about touchscreen systems adding in at times annoying levels of friction to simple functions like climate control and music."

Copy link
Collaborator

@g123k g123k left a comment

Choose a reason for hiding this comment

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

👌

@monsieurtanuki monsieurtanuki merged commit a7a9990 into openfoodfacts:develop Dec 20, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

Price proof: convert images to webp before upload ?
4 participants