-
-
Notifications
You must be signed in to change notification settings - Fork 8.4k
extmod/uzlib: Add gzip compression support. #5613
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
Conversation
memset(comp, 0, sizeof(*comp)); | ||
|
||
comp->dict_size = 32768; | ||
comp->hash_bits = 12; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This and previsou line could use a comment stating why this is chosen I think (I know nothing about gzip though)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it make sense to expose these settings in the API?
extmod/moduzlib.c
Outdated
uzlib_compress(comp, bufinfo.buf, len); | ||
zlib_finish_block(&comp->out); | ||
|
||
printf("compressed from %u to %u raw bytes\n", len, comp->out.outlen); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use DEBUG_printf
?
extmod/moduzlib.c
Outdated
|
||
printf("compressed from %u to %u raw bytes\n", len, comp->out.outlen); | ||
|
||
mp_uint_t dest_buf_size = (comp->out.outlen + 6 + (3*sizeof(int))); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same remark as earlier, what are these magic constants?
There's MICROPY_PY_UZLIB already, but I guess splitting that up in separate compress/decompress doesn't hurt and can be used to keep the original behavior (i.e. not enabling compression by default). A bunch of tests would be nice indeed, and fairly easy to write (assuming asserting that decompress(compress(x)) == x is sufficient for proving it works) |
extmod/moduzlib.c
Outdated
int mtime = 0; | ||
memcpy(&dest_buf[i], &mtime, sizeof(mtime)); | ||
i += sizeof(mtime); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
According to Wikipedia, mtime
must be 4 bytes. Use uint32_t
here?
extmod/moduzlib.c
Outdated
struct uzlib_comp *comp = m_new_obj(struct uzlib_comp); | ||
memset(comp, 0, sizeof(*comp)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Merge into m_new0(struct uzlib_comp, 1);
?
extmod/moduzlib.c
Outdated
dest_buf[i++] = 0x1f; | ||
dest_buf[i++] = 0x8b; | ||
dest_buf[i++] = 0x08; | ||
dest_buf[i++] = 0x00; // FLG |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Can you add a few more comments here and below where there are more magic numbers?
extmod/moduzlib.c
Outdated
unsigned int crc = ~uzlib_crc32(bufinfo.buf, len, ~0); | ||
memcpy(&dest_buf[i], &crc, sizeof(crc)); | ||
i += sizeof(crc); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
uint32_t
?
extmod/moduzlib.c
Outdated
mp_obj_t data = args[0]; | ||
mp_buffer_info_t bufinfo; | ||
mp_get_buffer_raise(data, &bufinfo, MP_BUFFER_READ); | ||
int len = bufinfo.len; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is written to the output; should this be uint32_t
(or whatever the right size is)?
hello, |
Hi @QAMU I think it worked like |
Hi @andrewleech, |
yep the two should work together well |
It seems compression and decompression don't work with each other:
|
They do if one removes the gzip header, crc and length fields and uses the correct wbits:
|
Updated attempt here. |
to compress use:
to decompress:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to decompress:
import uzlib import uio def decompress(buffer): file = uio.BytesIO(buffer) decoded = uzlib.DecompIO(file, 31) return decoded.read()
Why not:
def decompress(buffer):
return uzlib.decompress(buffer[10:-8], -15)
Seems more lightweight for an embedded solution.
memset(comp->hash_table, 0, hash_size); | ||
|
||
zlib_start_block(&comp->out); | ||
uzlib_compress(comp, bufinfo.buf, len); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
uzlib_compress
as well as zlib_start_block
internally (re-)allocate a buffer at comp->out
which is never free'd. As a result the system runs out of memory after a few compression runs.
A free(comp->out.outbuf);
after the subsequent memcpy solved this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
uzlib_compress
as well aszlib_start_block
internally (re-)allocate a buffer atcomp->out
which is never free'd. As a result the system runs out of memory after a few compression runs.A
free(comp->out.outbuf);
after the subsequent memcpy solved this.
Im trying to encode a string to get into the shorter sms data possible.,
questio is : how can I add this gzip uzlib extension to my current 1.14 toolchain for esp32?
f69e550
to
106f843
Compare
@harbaum Thanks for your additions in #6972, there is some great work there. I've rebased your changes on my original branch as a separate commit to keep your attribution. In addition, I've added support for gzip header in the The docs have been updated and I've added a basic compress unit test. |
Ok I've broken the unit tests with the last additions of gzip decompress and rebase onto current master... and it's too big for tiny ports - so probably does need a new feature flag. Either that, or just build & distribute in the dynamic C module version of the uzlib. |
26c07f9
to
5fff6e9
Compare
4da7fe2
to
8378ac1
Compare
clear out interrupt when freeing the timer
Superseded by #11905. |
Exposes basic compression support from uzlib. I originally wrote this nearly a year ago so don't really remember too much about it.
Pushed up to support #5590
Doesn't have any unit tests written... probably needs some first. Possibly some #define's to enable/disable the compress functionality?