Every compression algorithm binman drives has a level, and the best one to use depends on what the image is for: a payload sent over USB wants the smallest it can get, one read from fast storage would rather not wait for it. Today only the tool's own default is reachable.
Add a 'compress-level' property, passed through as -<level>, which is how all compressors want it except lzma: the LZMA SDK tool has no such option, and is controlled by the -lc/-lp/-pb values binman already passes. Signed-off-by: Alexey Charkov <[email protected]> --- tools/binman/binman.rst | 21 +++++++++++++ tools/binman/bintool.py | 10 +++++-- tools/binman/btool/lz4.py | 6 ++-- tools/binman/btool/lzma_alone.py | 6 +++- tools/binman/entry.py | 6 +++- tools/binman/ftest.py | 34 ++++++++++++++++++++++ tools/binman/test/entry/compress_level.dts | 12 ++++++++ tools/binman/test/entry/compress_level_gzip.dts | 12 ++++++++ tools/binman/test/entry/compress_level_lzma.dts | 12 ++++++++ .../test/entry/compress_level_no_compress.dts | 11 +++++++ 10 files changed, 124 insertions(+), 6 deletions(-) diff --git a/tools/binman/binman.rst b/tools/binman/binman.rst index 3a860421a697..d14fbe8e5fd7 100644 --- a/tools/binman/binman.rst +++ b/tools/binman/binman.rst @@ -711,6 +711,11 @@ compress: Sets the compression algorithm to use (for blobs only). See the entry documentation for details. +compress-level: + Sets how hard the compression algorithm should try, as the level number + the tool itself uses. Requires 'compress'. See the entry documentation + for details. + missing-msg: Sets the tag of the message to show if this entry is missing. This is used for external blobs. When they are missing it is helpful to show @@ -1182,6 +1187,22 @@ The entry will then contain the compressed data, using the 'lz4' compression algorithm. Currently this is the only one that is supported. The uncompressed size is written to the node in an 'uncomp-size' property, if -u is used. +Where an algorithm offers a choice of how hard to try, 'compress-level' passes +one through:: + + blob { + filename = "datafile"; + compress = "zstd"; + compress-level = <19>; + }; + +The number is the tool's own level, so its meaning and range follow that tool: +1 to 9 for gzip, bzip2, lzop and xz, 1 to 12 for lz4, 1 to 19 for zstd. Asking +for more than an algorithm offers usually gets a warning and its maximum +instead. Leaving the property out uses the tool's default, which is what binman +has always done. It cannot be used with 'lzma', whose LZMA SDK tool has no such +option. + Compression is also supported for sections. In that case the entire section is compressed in one block, including all its contents. This means that accessing an entry from the section requires decompressing the entire section. Also, the diff --git a/tools/binman/bintool.py b/tools/binman/bintool.py index 4cf8d5b165bf..caddb9eb8fc8 100644 --- a/tools/binman/bintool.py +++ b/tools/binman/bintool.py @@ -558,11 +558,14 @@ class BintoolPacker(Bintool): fetch_package = name self.fetch_package = fetch_package - def compress(self, indata): + def compress(self, indata, level=None): """Compress data Args: indata (bytes): Data to compress + level (int): Compression level to use, or None for the tool's own + default. Every packer tool spells this as -<level>; one asked + for a level it does not have generally warns and clamps it. Returns: bytes: Compressed data @@ -570,7 +573,10 @@ class BintoolPacker(Bintool): with tempfile.NamedTemporaryFile(prefix='comp.tmp', dir=tools.get_output_dir()) as tmp: tools.write_file(tmp.name, indata) - args = self.compress_args + ['--stdout', tmp.name] + args = self.compress_args[:] + if level is not None: + args.append(f'-{level}') + args += ['--stdout', tmp.name] return self.run_cmd(*args, binary=True) def decompress(self, indata): diff --git a/tools/binman/btool/lz4.py b/tools/binman/btool/lz4.py index fd520d13a566..92b3dc7410a7 100644 --- a/tools/binman/btool/lz4.py +++ b/tools/binman/btool/lz4.py @@ -78,11 +78,13 @@ class Bintoollz4(bintool.Bintool): def __init__(self, name): super().__init__(name, 'lz4 compression', r'.* (v[0-9.]*),.*') - def compress(self, indata): + def compress(self, indata, level=None): """Compress data with lz4 Args: indata (bytes): Data to compress + level (int): Compression level (1 to 12), or None for the level + binman has always used here Returns: bytes: Compressed data @@ -90,7 +92,7 @@ class Bintoollz4(bintool.Bintool): with tempfile.NamedTemporaryFile(prefix='comp.tmp', dir=tools.get_output_dir()) as tmp: tools.write_file(tmp.name, indata) - args = ['--no-frame-crc', '-B4', '-5', '-c', tmp.name] + args = ['--no-frame-crc', '-B4', f'-{level or 5}', '-c', tmp.name] return self.run_cmd(*args, binary=True) def decompress(self, indata): diff --git a/tools/binman/btool/lzma_alone.py b/tools/binman/btool/lzma_alone.py index 1fda2f68c7b4..d5a7bf3e1495 100644 --- a/tools/binman/btool/lzma_alone.py +++ b/tools/binman/btool/lzma_alone.py @@ -55,15 +55,19 @@ class Bintoollzma_alone(bintool.Bintool): def __init__(self, name): super().__init__(name, 'lzma_alone compression') - def compress(self, indata): + def compress(self, indata, level=None): """Compress data with lzma_alone Args: indata (bytes): Data to compress + level (int): Must be None: the LZMA SDK tool has no -<level> + option, it is steered by the -lc/-lp/-pb/-d values below Returns: bytes: Compressed data """ + if level is not None: + raise ValueError("lzma_alone does not support 'compress-level'") with tempfile.NamedTemporaryFile(prefix='comp.tmp', dir=tools.get_output_dir()) as inf: tools.write_file(inf.name, indata) diff --git a/tools/binman/entry.py b/tools/binman/entry.py index 9b39c7118892..e93b90801113 100644 --- a/tools/binman/entry.py +++ b/tools/binman/entry.py @@ -143,6 +143,7 @@ class Entry(object): self.image_pos = None self.extend_size = False self.compress = 'none' + self.compress_level = None self.missing = False self.faked = False self.external = False @@ -325,6 +326,9 @@ class Entry(object): # This is only supported by blobs and sections at present self.compress = fdt_util.GetString(self._node, 'compress', 'none') + self.compress_level = fdt_util.GetInt(self._node, 'compress-level') + if self.compress_level is not None and self.compress == 'none': + self.Raise("'compress-level' requires 'compress'") self.offset_from_elf = fdt_util.GetPhandleNameOffset(self._node, 'offset-from-elf') @@ -1222,7 +1226,7 @@ features to produce new behaviours. if self.compress != 'none': self.uncomp_size = len(indata) if self.comp_bintool.is_present(): - data = self.comp_bintool.compress(indata) + data = self.comp_bintool.compress(indata, self.compress_level) uniq = self.GetUniqueName() fname = tools.get_output_filename(f'comp.{uniq}') tools.write_file(fname, data) diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 5f0de4c74a70..48adacebb6dc 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -2168,6 +2168,40 @@ class TestFunctional(unittest.TestCase): bintool = self.comp_bintools['lz4'] return bintool.decompress(data) + def testCompressLevel(self): + """Test that a compression level reaches the tool and round-trips""" + self._CheckLz4() + data = self._DoReadFile('entry/compress_level.dts', use_real_dtb=True) + self.assertEqual(COMPRESS_DATA, self._decompress(data)) + + entry = control.images['image'].GetEntries()['blob'] + self.assertEqual(12, entry.compress_level) + + def testCompressLevelPacker(self): + """Test a compression level reaching a tool binman drives generically""" + bintool = self.comp_bintools['gzip'] + self._CheckBintool(bintool) + data = self._DoReadFile('entry/compress_level_gzip.dts', + use_real_dtb=True) + self.assertEqual(COMPRESS_DATA, bintool.decompress(data)) + + def testCompressLevelUnsupported(self): + """Test that lzma rejects a level its tool cannot express""" + bintool = self.comp_bintools['lzma_alone'] + self._CheckBintool(bintool) + with self.assertRaises(ValueError) as e: + self._DoReadFile('entry/compress_level_lzma.dts', + use_real_dtb=True) + self.assertIn("lzma_alone does not support 'compress-level'", + str(e.exception)) + + def testCompressLevelNoCompress(self): + """Test that a compression level without an algorithm is rejected""" + with self.assertRaises(ValueError) as e: + self._DoReadFile('entry/compress_level_no_compress.dts', + use_real_dtb=True) + self.assertIn("'compress-level' requires 'compress'", str(e.exception)) + def testCompress(self): """Test compression of blobs""" self._CheckLz4() diff --git a/tools/binman/test/entry/compress_level.dts b/tools/binman/test/entry/compress_level.dts new file mode 100644 index 000000000000..78c999590469 --- /dev/null +++ b/tools/binman/test/entry/compress_level.dts @@ -0,0 +1,12 @@ +// SPDX-License-Identifier: GPL-2.0+ +/dts-v1/; + +/ { + binman { + blob { + filename = "compress"; + compress = "lz4"; + compress-level = <12>; + }; + }; +}; diff --git a/tools/binman/test/entry/compress_level_gzip.dts b/tools/binman/test/entry/compress_level_gzip.dts new file mode 100644 index 000000000000..d2852fd653dd --- /dev/null +++ b/tools/binman/test/entry/compress_level_gzip.dts @@ -0,0 +1,12 @@ +// SPDX-License-Identifier: GPL-2.0+ +/dts-v1/; + +/ { + binman { + blob { + filename = "compress"; + compress = "gzip"; + compress-level = <9>; + }; + }; +}; diff --git a/tools/binman/test/entry/compress_level_lzma.dts b/tools/binman/test/entry/compress_level_lzma.dts new file mode 100644 index 000000000000..da52497b11a0 --- /dev/null +++ b/tools/binman/test/entry/compress_level_lzma.dts @@ -0,0 +1,12 @@ +// SPDX-License-Identifier: GPL-2.0+ +/dts-v1/; + +/ { + binman { + blob { + filename = "compress"; + compress = "lzma"; + compress-level = <5>; + }; + }; +}; diff --git a/tools/binman/test/entry/compress_level_no_compress.dts b/tools/binman/test/entry/compress_level_no_compress.dts new file mode 100644 index 000000000000..1e58202ce639 --- /dev/null +++ b/tools/binman/test/entry/compress_level_no_compress.dts @@ -0,0 +1,11 @@ +// SPDX-License-Identifier: GPL-2.0+ +/dts-v1/; + +/ { + binman { + blob { + filename = "compress"; + compress-level = <9>; + }; + }; +}; -- 2.55.0
