-
Notifications
You must be signed in to change notification settings - Fork 72
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
Choose a standard dict size #14
Comments
Yes, this is definitely reasonable and I'll make this change in lzham_codec_devel (which will be v1.1). |
Hi,
Could you please clarify whether there are any downsides to a large dictionary, besides memory usage? If I understand correctly, unbuffered decompression does not allocate memory for the dictionary, and a PC with virtual memory doesn't commit the parts of the dictionary that the compressor doesn't write to. |
AFAICT there is no guidance in LZHAM as to what the default dictionary size should be. Using 0 results in an error (instead of choosing a sensible default, as happens for most parameters).
The test program will use a dict_size_log2 of 28 on x86_64 and
LZHAM_MAX_DICT_SIZE_LOG2_X86
(aka 26) on x86, but based on the comments in lzham.h:144, "The values of m_dict_size_log2, m_table_update_rate, m_table_max_update_interval, and m_table_update_interval_slow_rate MUST match during compression and decompression." Since 28 >LZHAM_MAX_DICT_SIZE_LOG2_X86
, AFAICT it isn't possible to decompress something on x86 which has been compressed using the default parameters on x86_64.lzham_lzcomp_internal.h seems to like a default value of 22.
I think lzham.h should define a
LZHAM_DEFAULT_DICT_SIZE_LOG2
, which should be <=LZHAM_MAX_DICT_SIZE_LOG2_X86
(somewhere around 22-24 seems reasonable, IMHO). That should then be used by lzhamtest regardless of architecture, unless a different value is specified on the command line.The text was updated successfully, but these errors were encountered: