imgtool compression support #2038
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
michalek-no
force-pushed
the
mb-imgtool-comp
branch
from
de70c18
to
1b72985
Compare
michalek-no
force-pushed
the
mb-imgtool-comp
branch
from
1b72985
to
4aef29e
Compare
nordicjm
reviewed
Aug 16, 2024
scripts/imgtool/main.py
Outdated
Comment on lines
349
to
367
| help='Try image compression. If it deflates image, use ' | ||
| 'compressed one.') |
There was a problem hiding this comment.
not sure on what the comment means
scripts/imgtool/main.py
Outdated
Comment on lines
502
to
507
| {"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.
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.
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.
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.
michalek-no
force-pushed
the
mb-imgtool-comp
branch
3 times, most recently
from
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 |
michalek-no
force-pushed
the
mb-imgtool-comp
branch
from
6ab0be5
to
8f5f189
Compare
nordicjm
requested changes
Aug 28, 2024
|
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
|
doublemis1
reviewed
Sep 4, 2024
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.
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.
michalek-no
force-pushed
the
mb-imgtool-comp
branch
4 times, most recently
from
dd299d6
to
a0880c4
Compare
nordicjm
reviewed
Sep 16, 2024
michalek-no
force-pushed
the
mb-imgtool-comp
branch
2 times, most recently
from
efd307f
to
1e9cc82
Compare
Dose it make sense to support compression on hex file.? I do believe that only *.bin files are used for providing update candidates to the devices. |
nvlsianpu
approved these changes
Sep 24, 2024
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. |
michalek-no
force-pushed
the
mb-imgtool-comp
branch
2 times, most recently
from
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: |
michalek-no
force-pushed
the
mb-imgtool-comp
branch
from
f83539e
to
4bbdf5b
Compare
Issue not addressed |
michalek-no
force-pushed
the
mb-imgtool-comp
branch
2 times, most recently
from
4aedfe0
to
e5e5b65
Compare
nordicjm
approved these changes
Sep 30, 2024
michalek-no
force-pushed
the
mb-imgtool-comp
branch
from
e5e5b65
to
fd296d2
Compare
|
rebase |
nordicjm
pushed a commit
to nordicjm/sdk-mcuboot
that referenced
this pull request
Oct 1, 2024
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>
de-nordic
approved these changes
Oct 1, 2024
nordicjm
pushed a commit
to nordicjm/sdk-mcuboot
that referenced
this pull request
Oct 2, 2024
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>
d3zd3z
approved these changes
Oct 2, 2024
Damian-Nordic
approved these changes
Oct 3, 2024
scripts/imgtool/main.py
Outdated
Comment on lines
526
to
528
| {"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.
nit: the indentation seems a bit random in this piece ;)
scripts/imgtool/main.py
Outdated
Comment on lines
534
to
535
| print("compressed image size:", compressed_size, | ||
| "bytes\noriginal image size:", uncompressed_size, "bytes") |
There was a problem hiding this comment.
Suggested change
| 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.
Suggested change
| 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.
This method is never actually called with compression_header. Is this needed?
nordicjm
pushed a commit
to nordicjm/sdk-mcuboot
that referenced
this pull request
Oct 4, 2024
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>
nordicjm
pushed a commit
to nrfconnect/sdk-mcuboot
that referenced
this pull request
Oct 4, 2024
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>
bjarki-andreasen
pushed a commit
to bjarki-andreasen/sdk-mcuboot
that referenced
this pull request
Oct 4, 2024
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>
michalek-no
force-pushed
the
mb-imgtool-comp
branch
from
fd296d2
to
d9aa8f3
Compare
Damian-Nordic
approved these changes
Oct 4, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
filters with conjunction with proper container not working at this point