-
Notifications
You must be signed in to change notification settings - Fork 674
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
imgtool compression support #2038
Conversation
de70c18
to
1b72985
Compare
1b72985
to
4aef29e
Compare
scripts/imgtool/main.py
Outdated
help='Try image compression. If it deflates image, use ' | ||
'compressed one.') |
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.
not sure on what the comment means
scripts/imgtool/main.py
Outdated
{"id": lzma.FILTER_LZMA1, "dict_size": 131072}, | ||
{"id": lzma.FILTER_LZMA1, "lp": 1}, | ||
{"id": lzma.FILTER_LZMA1, "lc": 3}, | ||
{"id": lzma.FILTER_LZMA1, "preset": 9}, |
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.
lzma2?
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.
Tried in the first place and it also didn't work. I will look into it down the line.
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 doc. the last filter must be a compression filter (probably lzma.FILTER_LZMA2), I expect syntax as:
{"id": lzma.FILTER_LZAM2, "preset": 9, "dict_size": 131072, "lp": 1, "lc": 3}
Regard compression filters:
FORMAT_ALONE can be use along with FILTER_LZMA1
FILTER_LZMA2 can use FORMAT_XZ and FORMAT_RAW
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.
Also other quote:
When opening a file for writing, the settings used by the
compressor can be specified either as a preset compression
level (with the preset argument), or in detail as a custom
filter chain (with the filters argument). For FORMAT_XZ and
FORMAT_ALONE, the default is to use the PRESET_DEFAULT preset
level. For FORMAT_RAW, the caller must always specify a filter
chain; the raw compressor does not support preset compression
levels.
f87579e
to
6ab0be5
Compare
|
Have used it to compress an image, seemingly dumpinfo does not work:
And AuTerm cannot find the MCUboot header on the image |
6ab0be5
to
8f5f189
Compare
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 does not produce a valid image
Output from using this:
Output on same input file using reference implementation of compression system and hacked imgtool with static entries:
|
Reference PC tool cannot understand file format of file created using this, additionally the header is not valid for lzma2, it looks to be like a lzma1 header (though the tool supports both versions and says the file is valid for neither so there are more issues than that) |
You mean 'raw' zephyr.compressed.bin is neither lzma1 nor lzma2? |
It's lzma1, the tool seemingly does not work with auto detection and gives a false error. The file needs to be lzma2 |
@michalek-no Discussed off-channel with Jamie
|
scripts/imgtool/main.py
Outdated
endian=endian, load_addr=load_addr, rom_fixed=rom_fixed, | ||
erased_val=erased_val, save_enctlv=save_enctlv, | ||
security_counter=security_counter, max_align=max_align) | ||
compressed_file = re.sub('zephyr\.bin$','zephyr.compressed.bin',infile) |
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.
You might use tempfile.TemporaryFile
to store the compressed image not to pollute the build directory. Also, I don't think you should assume zephyr.bin
file name here.
dd299d6
to
a0880c4
Compare
efd307f
to
1e9cc82
Compare
If a user want to create a compressed hex file, they should be able to create a compressed hex file. Without supporting compressed hex there, there can be no test that this feature works |
that's good point. |
9136c0e
to
f83539e
Compare
Hex output does not work, the file seems to be garbage. Also code does not work if signature is not provided:
|
f83539e
to
4bbdf5b
Compare
Issue not addressed |
4aedfe0
to
e5e5b65
Compare
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.
Thanks for getting it to the finish line!
e5e5b65
to
fd296d2
Compare
rebase |
Adds LZMA2 compression to imgtool. Python lzma library is unable to compress with proper parameters while using "ALONE" container, therefore 2 header bytes are calculated and added to payload by imgtool. Upstream PR: mcu-tools/mcuboot#2038 Signed-off-by: Mateusz Michalek <mateusz.michalek@nordicsemi.no>
Adds LZMA2 compression to imgtool. Python lzma library is unable to compress with proper parameters while using "ALONE" container, therefore 2 header bytes are calculated and added to payload by imgtool. Upstream PR: mcu-tools/mcuboot#2038 Signed-off-by: Mateusz Michalek <mateusz.michalek@nordicsemi.no>
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.
some optional readability nits but looks good overall
scripts/imgtool/main.py
Outdated
{"id": lzma.FILTER_LZMA2, "preset": comp_default_preset, | ||
"dict_size": comp_default_dictsize, "lp": comp_default_lp, | ||
"lc": comp_default_lc} |
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: the indentation seems a bit random in this piece ;)
scripts/imgtool/main.py
Outdated
print("compressed image size:", compressed_size, | ||
"bytes\noriginal image size:", uncompressed_size, "bytes") |
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.
print("compressed image size:", compressed_size, | |
"bytes\noriginal image size:", uncompressed_size, "bytes") | |
print(f"compressed image size: {compressed_size} bytes") | |
print(f"original image size: {uncompressed_size} bytes") |
scripts/imgtool/main.py
Outdated
compression_tlvs["DECOMP_SHA"] = img.image_hash | ||
compression_tlvs_size = len(compression_tlvs["DECOMP_SIZE"]) | ||
compression_tlvs_size += len(compression_tlvs["DECOMP_SHA"]) | ||
if img.get_signature() is not None and img.get_signature() != "" : |
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.
if img.get_signature() is not None and img.get_signature() != "" : | |
if img.get_signature(): |
scripts/imgtool/image.py
Outdated
@@ -300,19 +317,39 @@ def __repr__(self): | |||
self.__class__.__name__, | |||
len(self.payload)) | |||
|
|||
def load(self, path): | |||
def load(self, path, compression_header=None): |
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 method is never actually called with compression_header
. Is this needed?
Adds LZMA2 compression to imgtool. Python lzma library is unable to compress with proper parameters while using "ALONE" container, therefore 2 header bytes are calculated and added to payload by imgtool. Upstream PR: mcu-tools/mcuboot#2038 Signed-off-by: Mateusz Michalek <mateusz.michalek@nordicsemi.no>
Adds LZMA2 compression to imgtool. Python lzma library is unable to compress with proper parameters while using "ALONE" container, therefore 2 header bytes are calculated and added to payload by imgtool. Upstream PR: mcu-tools/mcuboot#2038 Signed-off-by: Mateusz Michalek <mateusz.michalek@nordicsemi.no>
Adds LZMA2 compression to imgtool. Python lzma library is unable to compress with proper parameters while using "ALONE" container, therefore 2 header bytes are calculated and added to payload by imgtool. Upstream PR: mcu-tools/mcuboot#2038 Signed-off-by: Mateusz Michalek <mateusz.michalek@nordicsemi.no>
Adds LZMA2 compression to imgtool. Python lzma library is unable to compress with proper parameters while using "ALONE" container, therefore 2 header bytes are calculated and added to payload by imgtool. Signed-off-by: Mateusz Michalek <mateusz.michalek@nordicsemi.no>
fd296d2
to
d9aa8f3
Compare
Adds LZMA2 compression to imgtool. Python lzma library is unable to compress with proper parameters while using "ALONE" container, therefore 2 header bytes are calculated and added to payload by imgtool. Upstream PR: mcu-tools/mcuboot#2038 Signed-off-by: Mateusz Michalek <mateusz.michalek@nordicsemi.no> (cherry picked from commit 237b8b9)
Adds LZMA2 compression to imgtool. Python lzma library is unable to compress with proper parameters while using "ALONE" container, therefore 2 header bytes are calculated and added to payload by imgtool. Upstream PR: mcu-tools/mcuboot#2038 Signed-off-by: Mateusz Michalek <mateusz.michalek@nordicsemi.no> (cherry picked from commit 237b8b9)
filters with conjunction with proper container not working at this point