Skip to content
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

Merged
merged 1 commit into from
Oct 4, 2024
Merged

Conversation

michalek-no
Copy link
Collaborator

filters with conjunction with proper container not working at this point

scripts/imgtool/image.py Outdated Show resolved Hide resolved
scripts/imgtool/image.py Outdated Show resolved Hide resolved
Comment on lines 349 to 367
help='Try image compression. If it deflates image, use '
'compressed one.')
Copy link
Collaborator

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

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},
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lzma2?

Copy link
Collaborator Author

@michalek-no michalek-no Aug 16, 2024

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.

Copy link
Collaborator

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

Copy link
Collaborator

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.

@michalek-no michalek-no force-pushed the mb-imgtool-comp branch 3 times, most recently from f87579e to 6ab0be5 Compare August 16, 2024 08:42
@nordicjm
Copy link
Collaborator

./scripts/imgtool.py 
/tmp/aa/bootloader/mcuboot/scripts/imgtool/main.py:492: SyntaxWarning: invalid escape sequence '\.'
  if re.match('.*\.bin$', infile) is not None :
/tmp/aa/bootloader/mcuboot/scripts/imgtool/main.py:501: SyntaxWarning: invalid escape sequence '\.'
  compressed_file =  re.sub('zephyr\.bin$','zephyr.compressed.bin',infile)

@nordicjm
Copy link
Collaborator

Have used it to compress an image, seemingly dumpinfo does not work:

Traceback (most recent call last):
  File "/tmp/aa/bootloader/mcuboot/scripts/imgtool.py", line 22, in <module>
    main.imgtool()
  File "/usr/lib/python3.12/site-packages/click/core.py", line 1157, in __call__
    return self.main(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/site-packages/click/core.py", line 1078, in main
    rv = self.invoke(ctx)
         ^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/site-packages/click/core.py", line 1688, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/site-packages/click/core.py", line 1434, in invoke
    return ctx.invoke(self.callback, **ctx.params)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/site-packages/click/core.py", line 783, in invoke
    return __callback(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/tmp/aa/bootloader/mcuboot/scripts/imgtool/main.py", line 250, in dumpinfo
    dump_imginfo(imgfile, outfile, silent)
  File "/tmp/aa/bootloader/mcuboot/scripts/imgtool/dumpinfo.py", line 158, in dump_imginfo
    _tlv_prot_head = struct.unpack(
                     ^^^^^^^^^^^^^^
struct.error: unpack requires a buffer of 4 bytes

And AuTerm cannot find the MCUboot header on the image

Copy link
Collaborator

@nordicjm nordicjm left a 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

@nordicjm
Copy link
Collaborator

Output from using this:

#### Image header (offset: 0x0) ############################
magic:              0x96f3b83d
load_addr:          0x0
hdr_size:           0x200
protected_tlv_size: 0x134
img_size:           0x3bca
flags:              COMPRESSED_LZMA2 (0x400)
version:            0.0.0+0
############################################################
#### Payload (offset: 0x200) ###############################
|                                                          |
|              FW image (size: 0x3bca Bytes)               |
|                                                          |
############################################################
#### Protected TLV area (offset: 0x3dca) ###################
magic:     0x6908
area size: 0x134
        ---------------------------------------------
        type: COMP_SIZE (0x70)
        len:  0x4
        data: 0x64 0x5a 0x00 0x00 
        ---------------------------------------------
        type: COMP_SHA (0x71)
        len:  0x20
        data: 0x5d 0xe8 0xc9 0xea 0x11 0x63 0x28 0x2b 
              0xe1 0x6a 0x1e 0xf3 0xc7 0x76 0xdd 0xc6 
              0x26 0x40 0x67 0x8e 0xdd 0x34 0x5f 0x90 
              0xf2 0xb8 0xe9 0xfb 0x44 0x90 0xe4 0x8e 
        ---------------------------------------------
        type: COMP_SIGNATURE (0x72)
        len:  0x100
        data: 0xc1 0x06 0x66 0x11 0xde 0x70 0x26 0x71 
              0x70 0x13 0xb9 0x17 0xe6 0x52 0xd9 0xb2 
              0x1d 0x61 0x4d 0xdf 0x86 0x03 0xdb 0xe7 
              0x94 0xc2 0xa8 0x20 0x1a 0x11 0xc3 0xcb 
              0x3f 0xc0 0x4f 0xe4 0xd6 0x0a 0xbd 0xd9 
              0x1c 0x8f 0x46 0x25 0x04 0x1a 0x70 0x3b 
              0x5d 0xdc 0x8d 0x95 0xd9 0x99 0xf9 0x1d 
              0x1a 0xbc 0x79 0x91 0x07 0x9e 0x7d 0x7a 
              0xc7 0x0b 0xfa 0x14 0x6a 0x2f 0x8c 0xd9 
              0xd4 0x57 0xc1 0x9b 0x66 0x74 0xb5 0xdb 
              0xd3 0x21 0xf4 0x58 0xb3 0xaa 0xcb 0x80 
              0xbd 0xea 0x55 0x78 0xb7 0x95 0xf2 0x18 
              0x96 0x4b 0x7f 0x01 0xde 0x51 0xa3 0x91 
              0x23 0x22 0x67 0xd1 0xbb 0x20 0x5c 0x55 
              0x34 0xba 0xdb 0x48 0x51 0xef 0x5c 0x76 
              0x3d 0x9d 0xf8 0xea 0x40 0x07 0x21 0xba 
              0xad 0xea 0x00 0xc4 0xfd 0xaf 0x44 0x09 
              0xe4 0xa5 0xfb 0xaa 0xcd 0x63 0x8b 0xab 
              0x94 0x20 0x79 0x73 0xf2 0xfa 0xbe 0x1a 
              0xf1 0x0c 0x33 0x88 0x8a 0xe5 0x5f 0x75 
              0x5e 0x6b 0xd9 0xf1 0x08 0x26 0x54 0x2f 
              0x0c 0x5c 0xb9 0xb1 0xe7 0xf2 0x91 0x3a 
              0xbf 0xe5 0xf1 0x66 0xed 0xee 0x57 0xef 
              0xe6 0x88 0x40 0x18 0xe7 0x54 0x10 0xcb 
              0x6e 0x0b 0x96 0x9d 0x1c 0x37 0x48 0x5d 
              0xfc 0x81 0x7d 0x11 0xd7 0x38 0x9e 0xb9 
              0xbe 0x97 0x68 0xb9 0x8f 0x89 0x60 0xd9 
              0xee 0xf8 0x80 0xd8 0xc6 0xf5 0xfc 0x7c 
              0x19 0x21 0xf6 0x73 0x63 0x58 0x81 0x8d 
              0x33 0x5c 0xaa 0x13 0x1c 0xc0 0x6a 0x7a 
              0x75 0xa8 0x34 0x24 0x12 0xe0 0x2c 0x29 
              0x4e 0xa5 0x94 0x6c 0x6e 0x91 0x8e 0xdf 
############################################################
#### TLV area (offset: 0x3efe) #############################
magic:     0x6907
area size: 0x150
        ---------------------------------------------
        type: SHA256 (0x10)
        len:  0x20
        data: 0xa7 0xbe 0x27 0xe2 0xa0 0x2d 0x28 0x85 
              0x2a 0xc4 0x30 0x2d 0xa1 0x9f 0x30 0xc6 
              0x02 0xf2 0xb5 0xa1 0xb3 0xc8 0x17 0x89 
              0x31 0x56 0xbb 0x17 0x50 0x41 0xa4 0x55 
        ---------------------------------------------
        type: KEYHASH (0x1)
        len:  0x20
        data: 0xfc 0x57 0x01 0xdc 0x61 0x35 0xe1 0x32 
              0x38 0x47 0xbd 0xc4 0x0f 0x04 0xd2 0xe5 
              0xbe 0xe5 0x83 0x3b 0x23 0xc2 0x9f 0x93 
              0x59 0x3d 0x00 0x01 0x8c 0xfa 0x99 0x94 
        ---------------------------------------------
        type: RSA2048 (0x20)
        len:  0x100
        data: 0x89 0xbe 0x84 0x37 0x88 0xd4 0x8f 0x30 
              0xc3 0x51 0x4d 0xdb 0x24 0xb2 0x46 0x02 
              0xae 0xcc 0xb1 0x3e 0x51 0xc1 0x5b 0x23 
              0x53 0xe8 0x49 0x86 0xec 0x6c 0x49 0xa5 
              0xab 0x1f 0x30 0x8b 0x75 0xce 0x5e 0x97 
              0x6b 0x84 0xfc 0x58 0xc6 0x6c 0x68 0x24 
              0x96 0xa2 0x45 0xc0 0xca 0x7b 0xb7 0x9e 
              0x5a 0xca 0x62 0x9c 0x66 0xad 0x8b 0x29 
              0x07 0x75 0x0c 0xd9 0xa5 0x64 0x3b 0x2c 
              0x87 0x25 0xfd 0xde 0x64 0xb9 0x1e 0x97 
              0xc5 0x89 0x69 0x3b 0x2f 0x08 0xfc 0xde 
              0x4f 0xb2 0x8b 0xc2 0x8d 0x7b 0x50 0x1d 
              0xcc 0x86 0xcd 0x1c 0x51 0x7e 0xfc 0xbc 
              0xa6 0xed 0x8f 0xc1 0xea 0x93 0x4d 0xd8 
              0x81 0x01 0x24 0xbb 0xc9 0xbc 0xf4 0x45 
              0x07 0x1f 0xde 0xfc 0x9d 0xd1 0x88 0xfb 
              0x88 0xe0 0xa9 0x0b 0xf0 0x2d 0xcb 0x4d 
              0x0d 0x80 0x5b 0xa9 0xbd 0xfe 0xd6 0x3b 
              0x81 0x30 0x44 0x2c 0x58 0x6c 0x17 0x7c 
              0x64 0x2c 0x6e 0x82 0x4b 0x41 0x22 0xf9 
              0x56 0x28 0x2d 0x57 0xcc 0x15 0x3f 0x72 
              0x86 0x6f 0x92 0xb4 0xba 0xda 0xc8 0x54 
              0x5b 0x1d 0x87 0xbf 0xd4 0x7e 0xa3 0xc3 
              0x71 0x81 0xf2 0x84 0x8f 0x41 0xd9 0x64 
              0x5f 0xe4 0xa6 0x12 0x71 0x30 0xf4 0xb5 
              0x19 0x2c 0x86 0x56 0x6f 0x30 0x18 0xa2 
              0x86 0xed 0x83 0x2a 0x4b 0x68 0x5c 0x91 
              0x17 0x2f 0xff 0xc9 0xa7 0x7e 0x54 0xfb 
              0x57 0xd5 0x2e 0xdf 0xa6 0xad 0xea 0x59 
              0xd9 0x1e 0x8d 0xf2 0xa9 0x17 0x33 0x69 
              0xe3 0x93 0xe3 0x99 0x4f 0xf8 0x20 0xe3 
              0x07 0x0b 0xd2 0xaf 0x85 0x46 0x21 0xe0 
############################################################
#### End of Image  #########################################

Output on same input file using reference implementation of compression system and hacked imgtool with static entries:

#### Image header (offset: 0x0) ############################
magic:              0x96f3b83d
load_addr:          0x0
hdr_size:           0x200
protected_tlv_size: 0x134
img_size:           0x3b0e
flags:              COMPRESSED_LZMA2 (0x400)
version:            0.0.0+0
############################################################
#### Payload (offset: 0x200) ###############################
|                                                          |
|              FW image (size: 0x3b0e Bytes)               |
|                                                          |
############################################################
#### Protected TLV area (offset: 0x3d0e) ###################
magic:     0x6908
area size: 0x134
        ---------------------------------------------
        type: COMP_SIZE (0x70)
        len:  0x4
        data: 0x64 0x5a 0x00 0x00 
        ---------------------------------------------
        type: COMP_SHA (0x71)
        len:  0x20
        data: 0x5d 0xe8 0xc9 0xea 0x11 0x63 0x28 0x2b 
              0xe1 0x6a 0x1e 0xf3 0xc7 0x76 0xdd 0xc6 
              0x26 0x40 0x67 0x8e 0xdd 0x34 0x5f 0x90 
              0xf2 0xb8 0xe9 0xfb 0x44 0x90 0xe4 0x8e 
        ---------------------------------------------
        type: COMP_SIG (0x72)
        len:  0x100
        data: 0x15 0x55 0x7f 0x38 0x4b 0x53 0xa5 0x3d 
              0x30 0x82 0xbd 0x55 0xad 0xcc 0x71 0x27 
              0x39 0xf4 0x1a 0xa3 0xcf 0x8b 0x8f 0x0a 
              0xf7 0xea 0x1b 0x63 0xf8 0xd3 0xe6 0xc4 
              0x08 0xd7 0x7d 0x45 0x2c 0x0b 0x4c 0x73 
              0x5d 0xa2 0x17 0xaf 0x89 0x94 0xdd 0xda 
              0xb3 0xce 0x8e 0xf1 0x7b 0x42 0x3e 0x65 
              0x31 0x1e 0xde 0x52 0x4f 0xea 0x15 0xf5 
              0xec 0xc2 0xf7 0x25 0xf2 0xcc 0x8a 0x65 
              0xdd 0x0c 0xe2 0xaf 0x21 0x4c 0xb5 0x2f 
              0x50 0xb9 0x02 0xd9 0x72 0x39 0xed 0xbd 
              0xb4 0x91 0xd4 0x7f 0x7a 0x56 0x9e 0x5d 
              0xad 0x38 0xc1 0xe1 0xa0 0xa9 0x9f 0xf6 
              0x0d 0xed 0xcf 0xc7 0x4e 0x9e 0xb3 0x9c 
              0x20 0xc0 0x25 0xb6 0xee 0x7f 0x05 0xdc 
              0x1f 0xeb 0x69 0xd3 0xc7 0x68 0x56 0x2b 
              0x39 0xdd 0x59 0xeb 0x67 0x5f 0xd3 0x41 
              0x55 0xc0 0x28 0x6a 0x87 0x11 0xcf 0x14 
              0xe8 0x9a 0xea 0x38 0x3f 0x86 0x2b 0x22 
              0xb2 0x94 0xaf 0xfb 0x8b 0x96 0xd7 0xf6 
              0x57 0x0b 0xcc 0x6b 0xd5 0x29 0x5b 0x3a 
              0x62 0x28 0xc2 0xcd 0xff 0x85 0x24 0x7a 
              0x85 0xf4 0xe2 0x30 0xde 0xbb 0xae 0x90 
              0x54 0x1e 0xfb 0x97 0x38 0x76 0x1a 0xa2 
              0xa5 0x1a 0x15 0xf9 0xe7 0x96 0x1a 0x5c 
              0x4d 0x36 0xac 0xd4 0x24 0x34 0xb0 0x7a 
              0xa1 0xa3 0x02 0xe2 0xf4 0xed 0xf0 0x35 
              0xfd 0x67 0xb4 0x78 0x9d 0x73 0x01 0xc3 
              0xb8 0xf2 0xf9 0xdc 0x9e 0x22 0x37 0xc5 
              0x00 0x24 0x23 0x80 0x06 0xf9 0x49 0x24 
              0x75 0xb2 0x11 0x2d 0xd1 0x11 0x69 0x61 
              0x8d 0x22 0x08 0xa5 0x37 0xc2 0xb8 0x35 
############################################################
#### TLV area (offset: 0x3e42) #############################
magic:     0x6907
area size: 0x150
        ---------------------------------------------
        type: SHA256 (0x10)
        len:  0x20
        data: 0xda 0x22 0x16 0x5a 0xae 0x10 0xe1 0x1b 
              0xe4 0x28 0x37 0x7d 0x09 0x32 0x47 0xbf 
              0x7c 0x38 0xeb 0x36 0xa7 0x4c 0xc8 0x49 
              0x7b 0xb1 0x15 0x39 0x4c 0xdf 0x74 0x38 
        ---------------------------------------------
        type: KEYHASH (0x1)
        len:  0x20
        data: 0xfc 0x57 0x01 0xdc 0x61 0x35 0xe1 0x32 
              0x38 0x47 0xbd 0xc4 0x0f 0x04 0xd2 0xe5 
              0xbe 0xe5 0x83 0x3b 0x23 0xc2 0x9f 0x93 
              0x59 0x3d 0x00 0x01 0x8c 0xfa 0x99 0x94 
        ---------------------------------------------
        type: RSA2048 (0x20)
        len:  0x100
        data: 0xbe 0x88 0xcc 0xea 0x30 0xd9 0x12 0xb5 
              0x40 0xe9 0xd7 0x26 0x2b 0x06 0xdd 0x6d 
              0x1e 0x9a 0xf6 0x98 0x2f 0x79 0x4e 0x01 
              0x79 0x74 0xa9 0xc6 0x19 0x75 0xd8 0x7b 
              0xf3 0x6e 0xee 0xfe 0xc7 0xc4 0xf6 0x08 
              0x12 0x6c 0xb0 0x65 0x0e 0xd3 0x87 0x96 
              0x9b 0x93 0xdd 0x3e 0x5c 0x2f 0xa5 0x6f 
              0xe4 0x4f 0xf8 0x56 0x0c 0x00 0xea 0xfa 
              0xc8 0x9f 0x54 0x90 0xfc 0x1c 0x00 0x9e 
              0xfa 0x9f 0xe3 0xe5 0xdd 0xe6 0x06 0x9b 
              0x93 0x4c 0xa7 0xae 0xbb 0x39 0x04 0x9a 
              0x29 0x34 0x96 0xce 0x9e 0xac 0x62 0x34 
              0x57 0x5a 0xbe 0x46 0xb4 0x20 0xd2 0x8e 
              0x45 0x49 0xd7 0xda 0x72 0x76 0xf3 0xdc 
              0x91 0x27 0xb8 0xed 0x10 0x49 0x5e 0x16 
              0xff 0x6f 0x31 0xe7 0xde 0xe3 0x8c 0xbe 
              0x16 0xe6 0xb0 0x5a 0x61 0x0a 0x82 0x48 
              0xc0 0x05 0xd7 0x9d 0x8a 0xb3 0x0b 0xef 
              0xde 0xe3 0xfe 0xac 0x3e 0x3e 0x2a 0xd3 
              0xcb 0xaf 0x1a 0x6e 0x91 0x25 0x35 0x39 
              0xf0 0x3b 0xe5 0x4b 0x47 0xba 0x70 0xa9 
              0x1f 0xf2 0xef 0x31 0x65 0xb3 0x55 0x8e 
              0xfa 0x19 0x78 0x18 0x6a 0xd8 0xbc 0x9e 
              0x30 0xfc 0x14 0x41 0x4a 0xc7 0x28 0x9e 
              0x15 0x8a 0x1b 0x1d 0x87 0x67 0xff 0x00 
              0x28 0xfb 0x7f 0x00 0xa0 0xe2 0x2b 0x51 
              0xec 0xd6 0xe0 0x62 0xd0 0x20 0xb3 0xf2 
              0x21 0x5f 0xf7 0x90 0x3c 0xeb 0xc2 0x75 
              0xf7 0x4b 0xe4 0xc3 0x4e 0x04 0xc2 0xff 
              0xcb 0x7a 0x09 0x36 0x73 0x9c 0x60 0x9a 
              0xdc 0xba 0x86 0xdc 0x81 0xe5 0x61 0xa5 
              0xe3 0x5c 0x3e 0x98 0x44 0xdb 0x5d 0xc9 
############################################################
#### End of Image ##########################################
  • img_size mismatch by 188 bytes
  • tlv address offset mismatch, should be 0x3e42 but is 0x3efe (188 so due to above issue)

@nordicjm
Copy link
Collaborator

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)

@michalek-no
Copy link
Collaborator Author

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?

@nordicjm
Copy link
Collaborator

nordicjm commented Sep 2, 2024

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

@nvlsianpu
Copy link
Collaborator

@michalek-no Discussed off-channel with Jamie

from an initial test, using the xz program, it does look like raw would work, difference is 2 bytes and lzma2 header is 2 bytes, rest of the data looks the same.
raw with lzma2 header would likely be ok

scripts/imgtool/main.py Outdated Show resolved Hide resolved
boot/bootutil/include/bootutil/image.h Outdated Show resolved Hide resolved
boot/bootutil/include/bootutil/image.h Outdated Show resolved Hide resolved
scripts/imgtool/main.py Outdated Show resolved Hide resolved
scripts/imgtool/main.py Outdated Show resolved Hide resolved
scripts/imgtool/main.py Outdated Show resolved Hide resolved
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)

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.

@michalek-no michalek-no force-pushed the mb-imgtool-comp branch 4 times, most recently from dd299d6 to a0880c4 Compare September 13, 2024 13:50
@michalek-no michalek-no marked this pull request as ready for review September 13, 2024 13:51
scripts/imgtool/main.py Outdated Show resolved Hide resolved
scripts/imgtool/main.py Outdated Show resolved Hide resolved
scripts/imgtool/main.py Outdated Show resolved Hide resolved
scripts/imgtool/main.py Outdated Show resolved Hide resolved
boot/bootutil/include/bootutil/image.h Outdated Show resolved Hide resolved
@nordicjm
Copy link
Collaborator

This completely (silently) ignores compressing images for hex files

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. Can imgtool just return failure on commanding to make *.hex with compression today?

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

@nvlsianpu
Copy link
Collaborator

that's good point.

@michalek-no michalek-no force-pushed the mb-imgtool-comp branch 2 times, most recently from 9136c0e to f83539e Compare September 25, 2024 21:57
@nordicjm
Copy link
Collaborator

Hex output does not work, the file seems to be garbage. Also code does not work if signature is not provided:

compressed image size: 34026 bytes
original image size: 99756 bytes
Traceback (most recent call last):
  File "/tmp/qq/mcuboot/scripts/imgtool.py", line 22, in <module>
    main.imgtool()
  File "/usr/lib/python3.12/site-packages/click/core.py", line 1157, in __call__
    return self.main(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/site-packages/click/core.py", line 1078, in main
    rv = self.invoke(ctx)
         ^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/site-packages/click/core.py", line 1688, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/site-packages/click/core.py", line 1434, in invoke
    return ctx.invoke(self.callback, **ctx.params)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/site-packages/click/core.py", line 783, in invoke
    return __callback(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/tmp/qq/mcuboot/scripts/imgtool/main.py", line 538, in sign
    compression_tlvs["DECOMP_SIGNATURE"] = img.signature
                                           ^^^^^^^^^^^^^
AttributeError: 'Image' object has no attribute 'signature'. Did you mean: 'get_signature'?

@nordicjm
Copy link
Collaborator

nordicjm commented Sep 30, 2024

Also code does not work if signature is not provided:

compressed image size: 34026 bytes
original image size: 99756 bytes
Traceback (most recent call last):
  File "/tmp/qq/mcuboot/scripts/imgtool.py", line 22, in <module>
    main.imgtool()
  File "/usr/lib/python3.12/site-packages/click/core.py", line 1157, in __call__
    return self.main(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/site-packages/click/core.py", line 1078, in main
    rv = self.invoke(ctx)
         ^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/site-packages/click/core.py", line 1688, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/site-packages/click/core.py", line 1434, in invoke
    return ctx.invoke(self.callback, **ctx.params)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/site-packages/click/core.py", line 783, in invoke
    return __callback(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/tmp/qq/mcuboot/scripts/imgtool/main.py", line 538, in sign
    compression_tlvs["DECOMP_SIGNATURE"] = img.signature
                                           ^^^^^^^^^^^^^
AttributeError: 'Image' object has no attribute 'signature'. Did you mean: 'get_signature'?

Issue not addressed

@michalek-no michalek-no force-pushed the mb-imgtool-comp branch 2 times, most recently from 4aedfe0 to e5e5b65 Compare September 30, 2024 08:21
Copy link
Collaborator

@nordicjm nordicjm left a 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!

@michalek-no
Copy link
Collaborator Author

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>
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>
Copy link

@Damian-Nordic Damian-Nordic left a 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

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}

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 ;)

Comment on lines 534 to 535
print("compressed image size:", compressed_size,
"bytes\noriginal image size:", uncompressed_size, "bytes")

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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")

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() != "" :

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if img.get_signature() is not None and img.get_signature() != "" :
if img.get_signature():

@@ -300,19 +317,39 @@ def __repr__(self):
self.__class__.__name__,
len(self.payload))

def load(self, path):
def load(self, path, compression_header=None):

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?

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>
@nvlsianpu nvlsianpu merged commit 63fa7e4 into mcu-tools:main Oct 4, 2024
58 checks passed
rlubos pushed a commit to rlubos/fw-nrfconnect-mcuboot that referenced this pull request Oct 22, 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>
(cherry picked from commit 237b8b9)
rlubos pushed a commit to rlubos/fw-nrfconnect-mcuboot that referenced this pull request Oct 22, 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>
(cherry picked from commit 237b8b9)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants