Skip to content

fix(jbig2): restore pdfium's Group 4 decoder for MMR-coded regions - #444

Open
wittjeff wants to merge 1 commit into
docling-project:mainfrom
wittjeff:fix/jbig2-mmr-fax-g4
Open

wittjeff wants to merge 1 commit into
docling-project:mainfrom
wittjeff:fix/jbig2-mmr-fax-g4

Conversation

@wittjeff

@wittjeff wittjeff commented Nov 3, 2026

Copy link
Copy Markdown
Contributor

Rendering a page that draws a JBIG2 image with an MMR-coded generic region killed the process with SIGTRAP (exit 133), in every release since the JBIG2 decoder landed in #252. Arithmetic-coded JBIG2 was fine; parsing without a render_config was fine.

Resolves #440

Cause. The vendored stub of pdfium's core/fxcodec/fax/faxmodule.cpp had drifted from upstream (diffed against pdfium chromium/7000, the revision the rest of the stub matches):

  • FaxG4GetRow was rewritten without the end-of-row returns and without pass mode, and with the T.6 mode codes shifted by one bit (0001 pass mode read as V±2, 00001x V±2 read as V±3, V±3 read as end of row). A decoded row ran on into the next row's codes, a0 passed the row width, the reference-line lookup ref_buf[a0 / 8] indexed past the stride and the span bounds check trapped. That is the StartDecodeMMR → FaxG4Decode trap in the report. Rows that did not trap decoded wrong. It also explains the size pattern in the report: at 8x8 the stride is 4 bytes and a0 / 8 stays inside it, at 32x32 the width equals the stride in bits and the first lookup past the row traps.
  • kFaxWhiteRunIns was missing one entry, 0x27, 18, 0 (white run 18). The table is a sequence of count-prefixed groups, so every code after the 7-bit group was misread and a lookup could walk off the end of the table into the span bounds check. Any white run coded with 7 or more bits hit this (white run 0 is an 8-bit code).
  • FindBit lacked upstream's clamp of the found position to max_pos.

Fix. Restore the upstream text for all three, keeping the stub's FaxG4Decode signature. The row decoder also takes the run-length-exceeds-row guard from current pdfium main. The white table change is one inserted entry; the rest of that table and kFaxBlackRunIns and kOneLeadPos already matched upstream. Every JBIG2 user of MMR reaches FaxG4Decode through the one call in CJBig2_GRDProc::StartDecodeMMR: generic regions (JBig2_Context.cpp), pattern dictionaries (JBig2_PddProc.cpp), halftone regions (JBig2_HtrdProc.cpp) and the collective bitmaps of Huffman symbol dictionaries (JBig2_SddProc.cpp), so the "not tried" cases in the report are covered by the same change. Nothing else calls it, so arithmetic-coded JBIG2 and /CCITTFaxDecode (which has its own decoder in src/parse/utils/ccitt/) are untouched.

I did not route MMR through the in-tree CCITT decoder as the report suggests: that needs a stride packer and a bit-position handoff back to the JBIG2 stream (symbol dictionaries continue after the collective bitmap), which is more new code than restoring the upstream text. pdfium's remaining CHECKs still abort the process by design; making the vendored tree fail one image instead is a separate policy change and is out of scope here.

Test. tests/test_unit_jbig2_mmr.py writes MMR regions with libtiff through Pillow (a Group 4 TIFF strip holds exactly the T.6 codes a JBIG2 MMR region carries) and compares every rendered sample with the source bitmap. Fixtures: the report's 64x16 box, a 32x32 all-white region, a 33x7 all-black region and a 100x24 bitmap whose rows shift by 0 to 3 samples, add and drop runs and touch the right edge, so pass, horizontal and all vertical modes are exercised. All four fixtures also decode identically in pypdfium2. Skipped where Pillow has no libtiff.

fixture main (7.22.1 wheel) this branch
box 64x16 (issue's repro) SIGTRAP, exit 133 renders, 0 of 1024 samples differ from source
white 32x32 SIGTRAP, exit 133 renders, 0 of 1024 differ
black 33x7 SIGTRAP, exit 133 (white run table) renders, 0 of 231 differ
runs 100x24 SIGTRAP, exit 133 renders, 0 of 2400 differ

After the row decoder alone was restored, the 33x7 and 100x24 fixtures still trapped; that is how the missing table entry was found.

Evidence:

The regression dataset has no MMR-coded JBIG2: a segment-header scan of all 453 documents finds JBIG2 in one document only (complex_jbig2_overlays.pdf, arithmetic symbol dictionaries and text regions), so no existing page reproduces the problem and the corpus-wide pypdfium2 metrics cannot move with this change. The pdfium test corpus does have real MMR-coded JBIG2 (22 files; 21 trap on main, all survive on this branch). Cited pages, rendered at scale 1 with the threaded parser; "before" is the 7.22.1 wheel, "after" this branch, reference pypdfium2 5.13.0:

page before after vs pypdfium2
pdfium/bug_880920.pdf p1 (a 3300x2550 scanned page: Huffman symbol dictionary with MMR collective bitmaps) SIGTRAP, exit 133 mean_abs_error 0.159, changed_pixels_ratio 0.0079
third_party/jbig2/bitmap-mmr.pdf (generic region, MMR) SIGTRAP, exit 133 identical (0.000 / 0.0000)
third_party/jbig2/bitmap-halftone-10bpp-mmr.pdf (pattern dictionary + halftone region, MMR) SIGTRAP, exit 133 identical
third_party/jbig2/bitmap-symbol-symhuff-texthuff.pdf (Huffman symbol dictionary, MMR collective bitmap) SIGTRAP, exit 133 identical

bug_880920
bitmap-mmr
bitmap-halftone-10bpp-mmr
bitmap-symbol-symhuff-texthuff

Over all 52 files of the corpus' third_party/jbig2 set (SerenityOS JBIG2 tests converted to PDF, BSD-2-Clause) plus bug_880920.pdf: the 31 non-MMR files render the same before and after; of the 21 MMR files, 15 now match pypdfium2 pixel for pixel and 6 survive but render blank because a second, Huffman refinement/aggregate symbol dictionary fails after the MMR bitmap decoded (StartDecode returns kError). pypdfium2 renders those 6; that is a separate limitation of the vendored decoder's Huffman refinement path, not touched here. Panels, the per-file table and the corpus scan are on the evidence branch.

I have not opened a dataset pull request. The third_party/jbig2 files are BSD-2-Clause if you want bitmap-mmr.pdf and bitmap-halftone-10bpp-mmr.pdf in the regression dataset.

Ground truth changes:

None. The changed function is only reachable from MMR-coded JBIG2 segments, which no regression page contains.

Checklist:

  • One topic per pull request, rebased on main, no merge commits
  • Superseded pull requests closed and linked (none)
  • Regression dataset documents and pages named, or a dataset pull request that adds a new document — no regression document reproduces the problem; real documents from the pdfium test corpus are cited above, no dataset pull request opened
  • Before/after evidence for each cited page
  • Rendering only: pypdfium2 comparison images, corpus-wide metrics table, and an enforced image regression test — the unit test compares against the source bitmap and the fixtures decode identically in pypdfium2; the corpus has no MMR page, so the corpus-wide table is unchanged by construction
  • Blend2D only: link to the upstream report (not applicable)
  • Changes to existing ground truth listed with the reason for each page (none)

Local checks: tests/test_unit_*.py 381 passed, 1 skipped; ruff and mypy clean.

🤖 Generated with Claude Code

Rendering a page with an MMR-coded JBIG2 generic region killed the
process with SIGTRAP (exit 133). The vendored stub of pdfium's
faxmodule.cpp had drifted from upstream in three places:

- FaxG4GetRow was rewritten without the end-of-row returns and without
  pass mode, and with the T.6 mode codes shifted by one bit. A decoded
  row ran on into the next row's codes, a0 passed the row width, the
  reference-line lookup indexed past the stride and the span bounds
  check trapped. Rows that did not trap decoded wrong.
- kFaxWhiteRunIns was missing the entry for white run 18 (0x27, 18, 0).
  The table is a sequence of count-prefixed groups, so every code after
  the 7-bit group was misread and a lookup could walk off the table.
- FindBit lacked upstream's clamp of the found position to max_pos.

Restore the upstream text (pdfium chromium/7000, the revision the stub
matches elsewhere) for all three, keeping the stub's signature. The row
decoder also takes the run-length-exceeds-row guard from current pdfium
main. Every JBIG2 user of MMR (generic regions, pattern dictionaries,
halftone regions, Huffman symbol dictionaries) goes through this path.

The new unit test writes MMR regions with libtiff through Pillow and
compares every rendered sample with the source bitmap; its fixtures
cover the issue's 64x16 box, a 32x32 all-white region, a 33x7 all-black
region and a 100x24 bitmap that exercises pass, horizontal and all
vertical modes. All four trapped before this change.

Closes docling-project#440

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Signed-off-by: Jeff Witt <1848307+wittjeff@users.noreply.github.com>
@github-actions

github-actions Bot commented Nov 3, 2026

Copy link
Copy Markdown
Contributor

✅ DCO Check Passed

Thanks @wittjeff, all your commits are properly signed off. 🎉

@mergify

mergify Bot commented Nov 3, 2026

Copy link
Copy Markdown
Contributor

Merge Protections

🟢 Merge protection satisfied — ready to merge.

Show 1 satisfied protection

🟢 Enforce conventional commit

Make sure that we follow https://www.conventionalcommits.org/en/v1.0.0/

  • title ~= ^(fix|feat|docs|style|refactor|perf|test|build|ci|chore|revert)(?:\(.+\))?(!)?:

This branch has not been deployed

No deployments
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.

SIGTRAP rendering a page with an MMR-coded JBIG2 image (CJBig2_GRDProc::StartDecodeMMR → FaxModule::FaxG4Decode)

1 participant