refactor: apply JPEGDEC patches via git apply, not string replace
The previous patch_jpegdec.py used in-place string replacement with a single shared marker for two distinct DC writes (main store and successive-approximation update). If only one of the two anchors matched, the file was written half-patched and the shared marker locked the partial state in for every subsequent run. Generate the fixes as `git format-patch` artifacts under scripts/jpegdec_patches/, then apply them in lexical order via `git apply`. Idempotency is decided by git itself: `--check --reverse` succeeds means already applied; `--check` succeeds means appliable; neither aborts the build rather than leaving a half-patched file. No behaviour change to the patched JPEGDEC source: same redirect, same DC guards, same intent. Just stops the pre-build script from doing something it has no business doing.
This commit is contained in:
Jeremy Klein
+59
-99
@@ -1,125 +1,85 @@
|
||||
"""
|
||||
PlatformIO pre-build script: patch JPEGDEC for safe MCU_SKIP handling.
|
||||
PlatformIO pre-build script: apply CrossPoint's JPEGDEC patches via `git apply`.
|
||||
|
||||
When iMCU is MCU_SKIP (-8), JPEGDecodeMCU_P computes pMCU as
|
||||
&sMCUs[iMCU & 0xffffff] = &sMCUs[0xFFFFF8], a wild pointer ~33 MB past
|
||||
the array. EIGHT_BIT_GRAYSCALE decoding of a 3-component progressive
|
||||
JPEG calls JPEGDecodeMCU_P with MCU_SKIP twice per Y MCU (Cb then Cr),
|
||||
so every MCU exercises the wild pointer.
|
||||
The upstream JPEGDEC pin still has the wild-pointer + DC-write bugs in
|
||||
JPEGDecodeMCU_P that surface when EIGHT_BIT_GRAYSCALE decodes a 3-component
|
||||
progressive JPEG (each Y MCU drags two MCU_SKIP calls behind it for Cb/Cr).
|
||||
The patches in `scripts/jpegdec_patches/` carry the fix; this script applies
|
||||
each one against the libdep working tree.
|
||||
|
||||
Two patches, both required:
|
||||
Each patch's idempotency is decided by git itself:
|
||||
* `git apply --check --reverse` succeeds -> already applied, skip
|
||||
* `git apply --check` succeeds -> apply
|
||||
* neither succeeds -> abort the build
|
||||
|
||||
1. Redirect pMCU to &sMCUs[0] when iMCU < 0. Without this, the AC
|
||||
decode loop (`pMCU[iIndex] = ...`) store-faults on any progressive
|
||||
JPEG whose first scan carries AC coefficients (iScanEnd > 0).
|
||||
|
||||
2. Guard the two pMCU[0] DC writes with `if (iMCU >= 0)`. Without
|
||||
this, patch 1 just relocates the corruption: chroma-skip DC writes
|
||||
land at sMCUs[0] and clobber the freshly-decoded Y DC, producing
|
||||
all-black output for progressive JPEGs at JPEG_SCALE_EIGHTH grayscale.
|
||||
|
||||
The AC loop body (`pMCU[iIndex] = ...` writes and matching reads) is
|
||||
not separately guarded. It is unreachable on the JPEG_SCALE_EIGHTH
|
||||
DC-only first-scan path, and guarding the writes without also skipping
|
||||
bit consumption would desync the bitstream for the next MCU.
|
||||
|
||||
Both patches are idempotent.
|
||||
Patches live in `scripts/jpegdec_patches/` as one-commit-per-fix files
|
||||
(see the file headers for context). Applied in lexical order.
|
||||
"""
|
||||
|
||||
Import("env")
|
||||
import os
|
||||
import subprocess
|
||||
import sys
|
||||
|
||||
|
||||
PATCH_DIR = os.path.join(env["PROJECT_DIR"], "scripts", "jpegdec_patches")
|
||||
|
||||
|
||||
def patch_jpegdec(env):
|
||||
libdeps_dir = os.path.join(env["PROJECT_DIR"], ".pio", "libdeps")
|
||||
if not os.path.isdir(libdeps_dir):
|
||||
return
|
||||
for env_dir in os.listdir(libdeps_dir):
|
||||
jpeg_inl = os.path.join(libdeps_dir, env_dir, "JPEGDEC", "src", "jpeg.inl")
|
||||
if os.path.isfile(jpeg_inl):
|
||||
_apply_mcu_skip_pointer_fix(jpeg_inl)
|
||||
_apply_dc_write_guards(jpeg_inl)
|
||||
|
||||
|
||||
def _apply_mcu_skip_pointer_fix(filepath):
|
||||
MARKER = "// CrossPoint patch: safe pMCU for MCU_SKIP"
|
||||
with open(filepath, "r") as f:
|
||||
content = f.read()
|
||||
|
||||
if MARKER in content:
|
||||
patches = _patch_files()
|
||||
if not patches:
|
||||
return
|
||||
for env_dir in os.listdir(libdeps_dir):
|
||||
jpeg_dir = os.path.join(libdeps_dir, env_dir, "JPEGDEC")
|
||||
if not os.path.isdir(os.path.join(jpeg_dir, ".git")):
|
||||
continue
|
||||
for patch in patches:
|
||||
_apply_one(jpeg_dir, patch)
|
||||
|
||||
OLD = " signed short *pMCU = &pJPEG->sMCUs[iMCU & 0xffffff];"
|
||||
|
||||
NEW = (
|
||||
" " + MARKER + "\n"
|
||||
" signed short *pMCU = (iMCU < 0) ? pJPEG->sMCUs\n"
|
||||
" : &pJPEG->sMCUs[iMCU & 0xffffff];"
|
||||
def _patch_files():
|
||||
if not os.path.isdir(PATCH_DIR):
|
||||
return []
|
||||
return sorted(
|
||||
os.path.join(PATCH_DIR, name)
|
||||
for name in os.listdir(PATCH_DIR)
|
||||
if name.endswith(".patch")
|
||||
)
|
||||
|
||||
if OLD not in content:
|
||||
print(
|
||||
"WARNING: JPEGDEC MCU_SKIP pointer patch target not found in %s "
|
||||
"-- library may have been updated" % filepath
|
||||
)
|
||||
|
||||
def _apply_one(jpeg_dir, patch_path):
|
||||
name = os.path.basename(patch_path)
|
||||
if _git_apply_succeeds(jpeg_dir, patch_path, reverse=True):
|
||||
return
|
||||
|
||||
content = content.replace(OLD, NEW, 1)
|
||||
with open(filepath, "w") as f:
|
||||
f.write(content)
|
||||
print("Patched JPEGDEC: safe pMCU for MCU_SKIP in JPEGDecodeMCU_P: %s" % filepath)
|
||||
|
||||
|
||||
def _apply_dc_write_guards(filepath):
|
||||
MARKER = "// CrossPoint patch: guard pMCU DC writes for MCU_SKIP"
|
||||
with open(filepath, "r") as f:
|
||||
content = f.read()
|
||||
|
||||
if MARKER in content:
|
||||
return
|
||||
|
||||
OLD_DC = """\
|
||||
pMCU[0] = (short)*iDCPredictor; // store in MCU[0]
|
||||
}
|
||||
// Now get the other 63 AC coefficients"""
|
||||
|
||||
NEW_DC = """\
|
||||
""" + MARKER + """
|
||||
if (iMCU >= 0)
|
||||
pMCU[0] = (short)*iDCPredictor; // store in MCU[0]
|
||||
}
|
||||
// Now get the other 63 AC coefficients"""
|
||||
|
||||
OLD_SA = """\
|
||||
pMCU[0] |= iPositive;
|
||||
}
|
||||
goto mcu_done; // that's it"""
|
||||
|
||||
NEW_SA = """\
|
||||
if (iMCU >= 0)
|
||||
pMCU[0] |= iPositive;
|
||||
}
|
||||
goto mcu_done; // that's it"""
|
||||
|
||||
if OLD_DC not in content:
|
||||
print(
|
||||
"WARNING: JPEGDEC DC write guard target not found in %s "
|
||||
"-- library may have been updated" % filepath
|
||||
if not _git_apply_succeeds(jpeg_dir, patch_path, reverse=False):
|
||||
# Not applied, not appliable -- the libdep source has diverged from
|
||||
# what the patch expects. Don't write a half-patched file.
|
||||
result = subprocess.run(
|
||||
["git", "apply", "--check", patch_path],
|
||||
cwd=jpeg_dir,
|
||||
capture_output=True,
|
||||
text=True,
|
||||
)
|
||||
return
|
||||
|
||||
content = content.replace(OLD_DC, NEW_DC, 1)
|
||||
if OLD_SA in content:
|
||||
content = content.replace(OLD_SA, NEW_SA, 1)
|
||||
else:
|
||||
print(
|
||||
"WARNING: JPEGDEC successive-approximation DC guard target not found in %s "
|
||||
"-- continuing without that half of the patch" % filepath
|
||||
sys.stderr.write(
|
||||
"ERROR: JPEGDEC patch %s does not apply cleanly:\n%s%s\n"
|
||||
% (name, result.stdout, result.stderr)
|
||||
)
|
||||
raise SystemExit(1)
|
||||
subprocess.run(["git", "apply", patch_path], cwd=jpeg_dir, check=True)
|
||||
print("Applied JPEGDEC patch: %s" % name)
|
||||
|
||||
with open(filepath, "w") as f:
|
||||
f.write(content)
|
||||
print("Patched JPEGDEC: guard pMCU[0] DC writes for MCU_SKIP in JPEGDecodeMCU_P: %s" % filepath)
|
||||
|
||||
def _git_apply_succeeds(jpeg_dir, patch_path, *, reverse):
|
||||
cmd = ["git", "apply", "--check"]
|
||||
if reverse:
|
||||
cmd.append("--reverse")
|
||||
cmd.append(patch_path)
|
||||
return subprocess.run(
|
||||
cmd, cwd=jpeg_dir, capture_output=True, text=True
|
||||
).returncode == 0
|
||||
|
||||
|
||||
patch_jpegdec(env)
|
||||
|
||||
Reference in New Issue
Block a user