fix: Don't extract unsupported formats by jpirnay · Pull Request #977 · crosspoint-reader/crosspoint-reader

jpirnay · 2026-02-18T19:55:36Z

Summary

During chapter parsing, every tag triggered ZIP decompression and an SD card write regardless of whether the image format was supported. The mandatory delay(50) after each SD write compounded the cost. A chapter with 6 GIF images (a common decorative element in older EPUBs) wasted ~750 ms before any text rendering began.
What changes are included?
Added an ImageDecoderFactory::isFormatSupported() check before any file I/O in the img-handler. Only JPEG and PNG proceed to extraction; all other formats (GIF, SVG, WebP, etc.) fall through immediately to alt-text rendering with no SD card access.

Additional Context

Measured impact on a representative chapter with 6 GIF decorations:

	Before	After
Total parse time	~882 ms	~207 ms
Image handling	~750 ms	~76 ms

AI Usage

While CrossPoint doesn't have restrictions on AI tools in contributing, please be transparent about their usage as it
helps set the right context for reviewers.

Did you use AI tools to help write this code? NO

coderabbitai · 2026-02-18T19:57:37Z

📝 Walkthrough

Walkthrough

Modified HTML parsing in ChapterHtmlSlimParser.cpp to introduce a compile-time buffer size constant, replace hard-coded allocations with this constant, add format-support guards for image processing, and include timing instrumentation to measure parsing duration.

Changes

Cohort / File(s)	Summary
HTML Parser Optimization & Instrumentation `lib/Epub/Epub/parsers/ChapterHtmlSlimParser.cpp`	Introduced `PARSE_BUFFER_SIZE` compile-time constant to replace hard-coded 1024-byte allocations; wrapped image processing with `isFormatSupported()` guard; added timing instrumentation via start timestamp and elapsed time logging; updated buffer allocation and file read operations to use the new constant.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 2

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main change: adding a guard to prevent extraction of unsupported image formats, directly addressing the performance issue described in the changeset.
Description check	✅ Passed	The description is clearly related to the changeset, explaining the performance problem with unsupported image formats, the specific fix implemented (isFormatSupported check), and providing concrete performance metrics.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

🧹 Nitpick comments (1)

lib/Epub/Epub/parsers/ChapterHtmlSlimParser.cpp (1)

182-264: Format support guard looks correct and achieves the PR objective.

The guard properly prevents file I/O for unsupported formats, allowing immediate fallback to alt-text rendering.

Minor observation: isFormatSupported() internally calls getDecoder() (per the factory implementation), and then line 204 calls getDecoder() again. Since decoders are singletons, the overhead is negligible, but you could cache the decoder pointer from a single getDecoder() call and use decoder != nullptr as the guard if you wanted to eliminate the redundant lookup.

♻️ Optional: Single decoder lookup

-          if (ImageDecoderFactory::isFormatSupported(resolvedPath)) {
+          ImageToFramebufferDecoder* decoder = ImageDecoderFactory::getDecoder(resolvedPath);
+          if (decoder) {
             // Create a unique filename for the cached image
             ...
             if (extractSuccess) {
               // Get image dimensions
               ImageDimensions dims = {0, 0};
-              ImageToFramebufferDecoder* decoder = ImageDecoderFactory::getDecoder(cachedImagePath);
               if (decoder && decoder->getDimensions(cachedImagePath, dims)) {

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@lib/Epub/Epub/parsers/ChapterHtmlSlimParser.cpp` around lines 182 - 264,
Replace the two separate ImageDecoderFactory::isFormatSupported(...) and later
ImageDecoderFactory::getDecoder(...) calls with a single cached decoder pointer:
call ImageToFramebufferDecoder* decoder =
ImageDecoderFactory::getDecoder(resolvedPath) before doing file I/O, use decoder
!= nullptr as the format-support guard, and then reuse that same
decoder->getDimensions(cachedImagePath, dims) after extraction (remove the
second getDecoder(...) call). Update conditionals that currently check
isFormatSupported(...) and the later decoder pointer creation to use this single
cached decoder variable (functions/classes involved:
ImageDecoderFactory::getDecoder, isFormatSupported,
ImageToFramebufferDecoder::getDimensions).

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d02e21a and e1dca4d.

📒 Files selected for processing (1)

lib/Epub/Epub/parsers/ChapterHtmlSlimParser.cpp

🧰 Additional context used

🧬 Code graph analysis (1)

lib/Epub/Epub/parsers/ChapterHtmlSlimParser.cpp (1)

lib/Epub/Epub/converters/ImageDecoderFactory.cpp (4)

isFormatSupported (42-42)

isFormatSupported (42-42)

getDecoder (14-40)

getDecoder (14-14)

🔇 Additional comments (3)

lib/Epub/Epub/parsers/ChapterHtmlSlimParser.cpp (3)

20-20: LGTM!

Good extraction of the magic number into a named constant. This improves readability and maintainability.

644-658: LGTM!

Good addition of timing instrumentation for performance measurement, and consistent use of PARSE_BUFFER_SIZE constant in both buffer allocation and file reads.

683-683: Format specifier is consistent with codebase pattern.

The %lu format with millis() - chapterStartTime is the established pattern throughout this codebase. Multiple similar LOG_DBG calls use %lu with time differences (Epub.cpp:363, 395, 409, 410; main.cpp:207), and other files store millis() results as unsigned long. On the target embedded platform, unsigned long and uint32_t are equivalent, so this is not a portability issue here. No action needed.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@lib/Epub/Epub/parsers/ChapterHtmlSlimParser.cpp`:
- Around line 182-264: Replace the two separate
ImageDecoderFactory::isFormatSupported(...) and later
ImageDecoderFactory::getDecoder(...) calls with a single cached decoder pointer:
call ImageToFramebufferDecoder* decoder =
ImageDecoderFactory::getDecoder(resolvedPath) before doing file I/O, use decoder
!= nullptr as the format-support guard, and then reuse that same
decoder->getDimensions(cachedImagePath, dims) after extraction (remove the
second getDecoder(...) call). Update conditionals that currently check
isFormatSupported(...) and the later decoder pointer creation to use this single
cached decoder variable (functions/classes involved:
ImageDecoderFactory::getDecoder, isFormatSupported,
ImageToFramebufferDecoder::getDimensions).

insunaa · 2026-02-19T07:39:49Z

Does the timing function have to be there in the final code?

Does crosspoint reader code have an equivalent to

#ifdef NDEBUG

to exclude things like this from release builds?

I'm aware that the impact of this is trivial at best, I'm trying to learn how the project works.

## Summary * During chapter parsing, every <img> tag triggered ZIP decompression and an SD card write regardless of whether the image format was supported. The mandatory delay(50) after each SD write compounded the cost. A chapter with 6 GIF images (a common decorative element in older EPUBs) wasted ~750 ms before any text rendering began. * **What changes are included?** Added an ``ImageDecoderFactory::isFormatSupported()`` check before any file I/O in the img-handler. Only JPEG and PNG proceed to extraction; all other formats (GIF, SVG, WebP, etc.) fall through immediately to alt-text rendering with no SD card access. ## Additional Context Measured impact on a representative chapter with 6 GIF decorations: | | Before | After| |-- | -- | --| |Total parse time | ~882 ms | ~207 ms| |Image handling | ~750 ms | ~76 ms| --- ### AI Usage While CrossPoint doesn't have restrictions on AI tools in contributing, please be transparent about their usage as it helps set the right context for reviewers. Did you use AI tools to help write this code? _**NO**_

Dont extract unsupported formats

e1dca4d

coderabbitai bot reviewed Feb 18, 2026

View reviewed changes

znelson approved these changes Feb 18, 2026

View reviewed changes

osteotek approved these changes Feb 18, 2026

View reviewed changes

daveallie approved these changes Feb 19, 2026

View reviewed changes

daveallie merged commit 6be4413 into crosspoint-reader:master Feb 19, 2026
8 checks passed

daveallie mentioned this pull request Feb 19, 2026

chore(release): 1.1.0 Release Candidate #992

Merged

jpirnay deleted the fix-unsupported branch February 19, 2026 18:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: Don't extract unsupported formats#977

fix: Don't extract unsupported formats#977
daveallie merged 1 commit intocrosspoint-reader:masterfrom
jpirnay:fix-unsupported

jpirnay commented Feb 18, 2026 •

edited

Loading

Uh oh!

coderabbitai bot commented Feb 18, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Uh oh!

coderabbitai bot left a comment

Uh oh!

insunaa commented Feb 19, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Uh oh!

Conversation

jpirnay commented Feb 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Additional Context

AI Usage

Uh oh!

coderabbitai bot commented Feb 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

insunaa commented Feb 19, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

jpirnay commented Feb 18, 2026 •

edited

Loading

coderabbitai bot commented Feb 18, 2026 •

edited

Loading