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

Updated Readme.md to make quality parameter better documented. #119

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

Kiricon
Copy link

@Kiricon Kiricon commented Mar 22, 2017

Added example of numbers that can be used for quality parameter and explained the current limit of numbers above the 83% quality threshold.

"equivalent to libjpeg quality" does not have inherent meaning to users without the context of the documentation.

Added example of numbers that can be used for quality parameter and explained the current limit of numbers above the 83% quality threshold.
@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.


  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If you signed the CLA as a corporation, please let us know the company's name.

@Kiricon
Copy link
Author

Kiricon commented Mar 22, 2017

I signed it!

@googlebot
Copy link

CLAs look good, thanks!

@kornelski
Copy link

"require modifying the code to go bellow the set threshold." - that can easily be interpreted as "require hacking the argument check to allow lower number" rather than "rewriting the compressor to tune it to lower values".

The check isn't there to annoy people and make them compile source with a lower number. It's there to stop people from wasting their time on using quality that this compressor isn't good at.

@Kiricon
Copy link
Author

Kiricon commented Mar 22, 2017

@pornel That's a very great point I had not thought of. Is there by chance a way we can change that sentence so that we can still give a user heads up about the limit, and inform them that the compressor should not be bellow that threshold for the reasons you outlined? Or Perhaps we should remove that section from the read me and leave in the example "(0 - 100)"?

@kornelski
Copy link

Perhaps something like this:

You can pass a --quality Q parameter to set quality in units equivalent to libjpeg quality (84 - 100). Guetzli works best with high qualities. 84 is the lowest supported quality.

Readme.md text has been updated to give examples of passable values to the `--quality` parameter and also explains why the quality has an artificial limit of 84.
@Kiricon
Copy link
Author

Kiricon commented Mar 23, 2017

@pornel Thanks for the example. I think that the wording is much more elegantly in these terms.

I updated my pull request to include the text in your example.

Thanks again for the feedback!

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