diff --git a/README.md b/README.md index 3265bc3..0eff6b1 100644 --- a/README.md +++ b/README.md @@ -188,7 +188,8 @@ The benchmark below was run on **macOS ARM64 (Apple M-series)** using the includ | **isal** | 277.30 MiB | 0.2708 | 446.9 | **4.02x** | 1142.9 | - **zlib-ng** compresses **1.66x faster** than stdlib zlib with nearly identical compression ratio — the best all-rounder for maximum compression with speed. -- **ISAL** compresses **4.02x faster** than stdlib zlib, trading a slight ~1% larger output for dramatically better throughput — ideal for large game images. +- **ISAL** compresses **4.02x faster** than stdlib zlib, but is **not auto-selected** because its output can trigger PS5 hardware decompressor errors and kernel panics for some games. Use only with `--compression-backend isal` at your own risk; see [issue #132](https://github.com/PSBrew/MkPFS/issues/132). +- **auto** selects `zlib-ng`, then `zlib`. - **stdlib zlib** is always available as a fallback; it produces the smallest output but is the slowest to compress. - Decompress speeds are excellent across all backends (1142–2287 MB/s), so compressed images load fast regardless of which backend created them. diff --git a/mkpfs/__init__.py b/mkpfs/__init__.py index 0e7f62d..cb769c9 100644 --- a/mkpfs/__init__.py +++ b/mkpfs/__init__.py @@ -3,4 +3,4 @@ MkPFS is a toolkit for building, verifying, browsing, and managing PlayStation PFS images. """ -__version__: str = "1.0.0" +__version__: str = "1.0.1" diff --git a/mkpfs/cli.py b/mkpfs/cli.py index 74b3b35..5b02305 100644 --- a/mkpfs/cli.py +++ b/mkpfs/cli.py @@ -9,7 +9,7 @@ import sys import tempfile import time -from collections.abc import Iterator +from collections.abc import Generator from contextlib import contextmanager, suppress from dataclasses import dataclass @@ -88,6 +88,14 @@ "WARNING: The inner file was renamed to a safer file name to improve compatibility." ) +_ISAL_BACKEND_WARNING_TEXT: str = ( + "WARNING: The ISA-L compression backend is faster at compressing, but it has a known bug " + "that produces PFSC blocks the PS5 hardware decompressor rejects for some games, " + "causing the console to kernel panic shortly after launch.\n" + "Only use it if you accept this risk. Prefer zlib-ng or zlib instead.\n" + "See: https://github.com/PSBrew/MkPFS/issues/132\n" +) + def _emit_game_folder_compression_warning() -> None: """Emit the red warning about compressing direct game-folder images.""" @@ -122,6 +130,12 @@ def _emit_single_file_verify_name_mismatch_warning(*, external_name: str, intern ) +def _emit_isal_backend_warning() -> None: + """Emit the red warning when the compression backend resolves to ISA-L.""" + info("") # Adds an empty line before the warning. + warning(_ISAL_BACKEND_WARNING_TEXT, icon_name="warning") + + def _resolve_single_file_internal_name(*, source_file: Path, rename_inner_image: bool) -> str: """Resolve the internal file name used for single-file pack and verify flows. @@ -801,7 +815,10 @@ def cli_mkpfs_add_create_args( "--compression-backend", choices=("auto", "zlib-ng", "zlib", "isal"), default="auto", - help="Compression backend to use for PFSC block compression (default: auto — isal > zlib-ng > zlib)", + help=( + "Compression backend to use for PFSC block compression " + "(default: auto — zlib-ng > zlib; isal is never auto-selected, see issue #132)" + ), ) parser.add_argument( @@ -1286,7 +1303,7 @@ def _run_pack_build( @contextmanager def _stage_single_file_source_root( *, source_file: Path, temp_folder: Path | None = None, staged_file_name: str | None = None -) -> Iterator[Path]: +) -> Generator[Path, None, None]: """Yield a temporary source root exposing one file, avoiding data copies when possible. The staged file is created as a hard link when possible, with a symlink @@ -2419,7 +2436,7 @@ def cli_mkpfs_main_parsers() -> argparse.ArgumentParser: "--compression-backend", choices=("auto", "zlib-ng", "zlib", "isal"), default="auto", - help="Compression backend (default: auto)", + help="Compression backend (default: auto — zlib-ng > zlib; isal is never auto-selected)", ) batch_parser.add_argument( "--max-compressed-ratio", @@ -2520,6 +2537,8 @@ def cli_mkpfs_main(argv: list[str] | None = None) -> int: except Exception as exc: error(f"Unable to select compression backend '{getattr(args, 'compression_backend', None)}': {exc}") return 2 + if comp.get_backend_name() == "isal": + _emit_isal_backend_warning() return int(args.func(args)) diff --git a/mkpfs/compression.py b/mkpfs/compression.py index 8d2553b..ce7c65e 100644 --- a/mkpfs/compression.py +++ b/mkpfs/compression.py @@ -6,7 +6,7 @@ API:: - set_backend(name: str) -> None # pick "zlib-ng", "isal", or "zlib" + set_backend(name: str) -> None # pick "auto", "zlib-ng", "isal", or "zlib" get_backend_name() -> str # returns current backend name init_worker(backend_name=None) # per-process bootstrap for multiprocessing compress_block(data, level=6) # compress bytes (level 1-9, std zlib scale) @@ -24,6 +24,8 @@ PFSC in mkpfs.pfs. The PFSC encoder/decoder lives entirely in pfs.py. * Decompression is backend-agnostic: any backend can decompress output from any other because the zlib stream format is interoperable. +* ISA-L is never auto-selected because its streams can trigger PS5 hardware + inflater errors (issue #132); manual selection still works. """ from __future__ import annotations @@ -83,15 +85,16 @@ def set_backend(name: str) -> None: global _backend_name, _backend normalised = name.lower() - # Auto-selection tries backends in preferred order: isal > zlib-ng > zlib + # Auto-selection tries zlib-ng > zlib; isal is excluded due to PS5 ZCN + # incompatibility (issue #132). if normalised == "auto": - for candidate in ("isal", "zlib-ng", "zlib"): + for candidate in ("zlib-ng", "zlib"): try: set_backend(candidate) return except ImportError: continue - raise ImportError("no available compression backend (isal, zlib-ng, zlib)") + raise ImportError("no available compression backend (zlib-ng, zlib)") if normalised == "zlib-ng": loader = _load_zlib_ng @@ -122,10 +125,9 @@ def init_worker(backend_name: str | None = None) -> None: Call this as ``Pool(initializer=init_worker, initargs=(name,))`` so each spawned process loads the correct backend before compressing data. - Args: backend_name: Name of the backend to load (e.g. ``"zlib-ng"``). If *None*, re-uses whatever was last set on the parent side or falls - back to ``"isal"`` / ``"zlib-ng"`` / ``"zlib"`` in that order. + back to ``"zlib-ng"`` / ``"zlib"`` in that order. """ global _backend, _backend_name @@ -139,7 +141,7 @@ def init_worker(backend_name: str | None = None) -> None: # If nothing was passed (or explicit load failed) and default wasn't loaded, attempt preferred fallbacks. if _backend is None: - for fallback in ("isal", "zlib-ng", "zlib"): + for fallback in ("zlib-ng", "zlib"): try: set_backend(fallback) break diff --git a/tests/mkpfs/test_cli.py b/tests/mkpfs/test_cli.py index 6cffd34..05ad782 100644 --- a/tests/mkpfs/test_cli.py +++ b/tests/mkpfs/test_cli.py @@ -263,6 +263,22 @@ def test_pack_parser_uses_zero_as_default_threshold_gain(self) -> None: ) self.assertEqual(threshold_action.default, 0) + def test_pack_folder_compression_backend_defaults_to_auto_and_documents_order(self) -> None: + """The pack folder parser should default to safe automatic backend selection.""" + parser: argparse.ArgumentParser = cli.cli_mkpfs_main_parsers() + pack_parser: argparse.ArgumentParser = next( + action.choices["pack"] for action in parser._actions if isinstance(action, argparse._SubParsersAction) + ) + pack_choices: dict[str, argparse.ArgumentParser] = next( + action.choices for action in pack_parser._actions if isinstance(action, argparse._SubParsersAction) + ) + folder_parser: argparse.ArgumentParser = pack_choices["folder"] + compression_action: argparse.Action = next( + action for action in folder_parser._actions if getattr(action, "dest", "") == "compression_backend" + ) + self.assertEqual(compression_action.default, "auto") + self.assertIn("zlib-ng > zlib", compression_action.help or "") + def test_pack_parser_uses_thirty_two_as_default_inode_bits(self) -> None: """The pack parser should expose 32 as the default inode width.""" parser: argparse.ArgumentParser = cli.cli_mkpfs_main_parsers() @@ -2609,6 +2625,46 @@ def test_pack_file_streaming_dry_run_writes_nothing(self) -> None: self.assertEqual(rc, 0) self.assertFalse(out.exists()) + def test_pack_folder_warns_when_isal_backend_is_selected(self) -> None: + """Folder packing should warn when ISA-L is explicitly selected.""" + import pytest + + import mkpfs.compression as comp + + original_backend_name: str = comp._backend_name + original_backend: object | None = comp._backend + self.addCleanup(setattr, comp, "_backend_name", original_backend_name) + self.addCleanup(setattr, comp, "_backend", original_backend) + + try: + comp.set_backend("isal") + except ImportError: + pytest.skip("isal backend is not available in this environment") + + tmp_path: Path = self.make_temp_path() + source: Path = self.make_valid_source(tmp_path) + output: Path = tmp_path / "output.ffpfs" + stdout_buffer: StringIO = StringIO() + stderr_buffer: StringIO = StringIO() + with redirect_stdout(stdout_buffer), redirect_stderr(stderr_buffer): + return_code: int = cli_mkpfs_main( + [ + "pack", + "folder", + str(source), + str(output), + "--compression-backend", + "isal", + "--dry-run", + "--no-adjust-output-file-extension", + ] + ) + + self.assertEqual(return_code, 0) + output_text: str = stdout_buffer.getvalue() + stderr_buffer.getvalue() + self.assertIn("ISA-L", output_text) + self.assertIn("issues/132", output_text) + class TestCliTreeStructureOnly(CliTestCase): """The tree command should render structure without decoding file payloads.""" diff --git a/tests/mkpfs/test_compression_backends.py b/tests/mkpfs/test_compression_backends.py index 8ef89e8..1305676 100644 --- a/tests/mkpfs/test_compression_backends.py +++ b/tests/mkpfs/test_compression_backends.py @@ -37,6 +37,25 @@ def test_set_backend_updates_name(self) -> None: comp.set_backend("zlib-ng") assert comp.get_backend_name() == "zlib-ng" + def test_auto_never_selects_isal_even_when_available(self) -> None: + import mkpfs.compression as comp + + def fake_load_isal() -> object: + class FakeIsal: + def compress(self, data: bytes, level: int = 2) -> bytes: + return b"fake" + + return FakeIsal() + + original = comp._load_isal + comp._load_isal = fake_load_isal # type: ignore[assignment] + try: + comp.set_backend("auto") + assert comp.get_backend_name() != "isal" + assert comp.get_backend_name() in ("zlib-ng", "zlib") + finally: + comp._load_isal = original # type: ignore[assignment] + def test_compress_decompress_roundtrip(self) -> None: import mkpfs.compression as comp