[python] expose new encoder parameters as kwargs of brotli.compress #105
Conversation
| 'font': brotli.MODE_FONT | ||
| # default values of encoder parameters | ||
| DEFAULT_PARAMS = { | ||
| 'mode': brotli.MODE_TEXT, |
There was a problem hiding this comment.
I just added a MODE_GENERIC compression mode, which is the default. This avoids the confusion that MODE_TEXT could equally mean 'no idea' and 'text for sure' before. Could you change your argument description and default accordingly?
|
sure, I'll do that. |
update brotli.compress docstring accordingly
renamed variables: transform -> enable_transforms, dictionary -> enable_dictionary
modified the help string to include the new parameters.
| 'Range is 16 to 24. If set to 0, the value will be set based ' | ||
| 'on the quality. Defaults to 0.') | ||
| above9 = parser.add_argument_group( | ||
| 'encoder parameters respected only if quality > 9.') |
There was a problem hiding this comment.
I think these low-level parameters should not be exposed to the binary. Their purpose is to find a balance between compression speed and compression density, just like the quality parameter. The reason I still have these here is that I don't have a quality 10 yet, which would be a slow but not extremely slow version. Once I have a reasonable compromise there, I will probably remove these and rely solely on the quality (or maybe the mode as well), just like for qualities < 9.
There was a problem hiding this comment.
I see. Which arguments shall I keep or remove precisely?
There was a problem hiding this comment.
Those under "// These settings will be respected only if quality > 9."
should be removed.
On Mon, May 11, 2015 at 11:46 AM, Cosimo Lupo [email protected]
wrote:
In python/bro.py
#105 (comment):
- params.add_argument('-q', '--quality', metavar="QUALITY", type=int,
choices=list(range(0, 12)),help='Controls the compression-speed vs compression-density ''tradeoff. The higher the quality, the slower the ''compression. Range is 0 to 11. Defaults to 11.')- params.add_argument('--lgwin', metavar="LGWIN", type=int,
choices=list(range(16, 25)),help='Base 2 logarithm of the sliding window size. Range is ''16 to 24. Defaults to 22.')- params.add_argument('--lgblock', metavar="LGBLOCK", type=int,
choices=[0] + list(range(16, 25)),help='Base 2 logarithm of the maximum input block size. ''Range is 16 to 24. If set to 0, the value will be set based ''on the quality. Defaults to 0.')- above9 = parser.add_argument_group(
'encoder parameters respected only if quality > 9.')I see. Which arguments shall I keep or remove precisely?
—
Reply to this email directly or view it on GitHub
https://github.com/google/brotli/pull/105/files#r30025220.
remove additional low-level parameters
|
@szabadka OK, I've rebased on top of master, set MODE_GENERIC as default compression mode, and removed the additional low-level parameters from the command line tool's arguments (but I left them in the module's compress function) |
|
As for quality 0, it is allowed, but currently is the same as quality 1. I found why it is broken now, I will fix it soon. |
[python] expose new encoder parameters as kwargs of brotli.compress
|
thank you! |
I added support for the new BrotliParams to the Python extension, as requested in #72.
I also modified the bro.py script so that we can control these parameters from the command line.
I have a question. In the encode.h, it's written that the parameter "quality" has to be in the range from 0 to 11. I presume this range is inclusive. Now, if I set the quality between 1 and 11, the output changes accordingly. However, if I set it to 0, the output is always truncated to the same 10 bytes. Does this mean that 0 is an invalid value for the 'quality' parameter?
Thanks.
C.