feat: Add PNG cover image support for EPUB books#827
feat: Add PNG cover image support for EPUB books#827daveallie merged 3 commits intocrosspoint-reader:masterfrom
Conversation
EPUB books with PNG cover images now display covers on the home screen instead of blank rectangles. Adds PngToBmpConverter library mirroring the existing JpegToBmpConverter pattern, using miniz for streaming zlib decompression of PNG IDAT data. Co-Authored-By: Claude Opus 4.6 <[email protected]>
|
Lol well I just finished this as well. I think your implementation is a bit cleaner but you're missing a key thing from one of the test files which was the cover image being inside of a guide reference. So i'll remove my png conversion code and we can merge both side by side which will make the book that's in #769 work properly |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
📝 WalkthroughWalkthroughThis change adds PNG image support to EPUB cover generation by introducing a new PngToBmpConverter class that handles PNG parsing, decompression, filtering, grayscale conversion, optional scaling, and BMP output in a streaming manner. Changes
Sequence DiagramsequenceDiagram
participant Epub as Epub Cover Gen
participant FsFile as File System (FsFile)
participant PngConv as PngToBmpConverter
participant Parser as PNG Parser
participant Decomp as zlib Decompressor
participant Filter as Defilter Engine
participant BmpOut as BMP Output (Print)
Epub->>FsFile: Open PNG file
Epub->>PngConv: pngFileToBmpStream(file, output, crop)
PngConv->>FsFile: Read file sequentially
PngConv->>Parser: Parse PNG signature & IHDR
Parser->>Parser: Validate color type, bit depth
PngConv->>Parser: Scan for IDAT chunks
PngConv->>Decomp: Initialize zlib decompressor
PngConv->>Decomp: Feed IDAT compressed data
Decomp->>Decomp: Decompress scanline bytes
PngConv->>Filter: Apply per-scanline defilter<br/>(none, Sub, Up, Average, Paeth)
Filter->>Filter: Reconstruct filtered pixels
PngConv->>PngConv: Convert to grayscale<br/>(optional scale/crop)
PngConv->>BmpOut: Write BMP headers & pixel data
PngConv-->>Epub: Success/failure result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/Epub/Epub.cpp (1)
478-510:⚠️ Potential issue | 🟡 MinorPotential out-of-bounds access with very short filenames.
The
substr(coverImageHref.length() - 4)call at lines 478, 510 (and similar at line 479) will throwstd::out_of_rangeor produce undefined behavior ifcoverImageHref.length() < 4. While unlikely for valid cover image paths, a malformed EPUB could trigger this. This is a pre-existing issue in the JPEG path, but the PNG addition continues the pattern.Consider using
ends_with(C++20) or a safe suffix check helper:🛡️ Suggested safer pattern
// Helper function static bool endsWith(const std::string& str, const std::string& suffix) { if (suffix.size() > str.size()) return false; return str.compare(str.size() - suffix.size(), suffix.size(), suffix) == 0; } // Usage if (endsWith(coverImageHref, ".jpg") || endsWith(coverImageHref, ".jpeg")) { // JPEG handling } else if (endsWith(coverImageHref, ".png")) { // PNG handling }
🤖 Fix all issues with AI agents
In `@lib/Epub/Epub.cpp`:
- Line 603: The CI failure is due to clang-format style issues in Epub.cpp
(around the code handling coverImageHref, e.g., the branch checking
coverImageHref.substr(...) == ".png"); run the repository formatter script
(bin/clang-format-fix) and commit the resulting changes so the file matches the
project's clang-format rules, then re-run tests/CI.
In `@lib/PngToBmpConverter/PngToBmpConverter.cpp`:
- Around line 1-21: Clang-format flagged formatting issues in
PngToBmpConverter.cpp (headers, include ordering and spacing around the IMAGE
PROCESSING OPTIONS block and constant declarations like USE_8BIT_OUTPUT,
USE_ATKINSON, TARGET_MAX_WIDTH, TARGET_MAX_HEIGHT); run the formatter script
(bin/clang-format-fix) on that file, review the resulting changes to ensure
includes and the comment block are correctly spaced/ordered, then add and commit
the reformatted file so the CI pipeline passes.
- Around line 563-571: The PLTE chunk handling does an unchecked
pngFile.read(ctx.palette, palBytes) which can return fewer bytes or fail; update
the PLTE branch in PngToBmpConverter.cpp to check the return value (or the
number of bytes read) from pngFile.read, and if it is less than palBytes handle
it by logging/erroring and aborting conversion (or treating it as a malformed
PNG), ensuring you do not use partial/uninitialized ctx.palette; also ensure any
remaining bytes and the CRC are skipped/consumed only after a successful read so
you don’t mis-sync the stream.
🧹 Nitpick comments (2)
lib/PngToBmpConverter/PngToBmpConverter.cpp (2)
66-71: Signed right-shift is implementation-defined.Right-shifting a negative
int32_tis implementation-defined in C++. However, since this function is only called with-height(line 87, 117, 147) for BMP's top-down row order, and modern compilers on ARM (ESP32) use arithmetic shift, this works correctly in practice. Consider using amemcpyor union approach for strict portability, though this is low priority for an embedded target.
394-413: Alpha channel is discarded, transparent pixels render as opaque.For
PNG_COLOR_GRAYSCALE_ALPHAandPNG_COLOR_RGBA, the alpha channel is ignored. Transparent or semi-transparent pixels will render as if fully opaque. For EPUB covers this is usually fine, but images with transparency expecting a white background may appear darker than intended. Consider blending against white (gray = alpha * gray + (255 - alpha) * 255 / 255) if this becomes an issue.
| #include "PngToBmpConverter.h" | ||
|
|
||
| #include <HalStorage.h> | ||
| #include <HardwareSerial.h> | ||
| #include <miniz.h> | ||
|
|
||
| #include <cstdio> | ||
| #include <cstring> | ||
|
|
||
| #include "BitmapHelpers.h" | ||
|
|
||
| // ============================================================================ | ||
| // IMAGE PROCESSING OPTIONS - Same as JpegToBmpConverter for consistency | ||
| // ============================================================================ | ||
| constexpr bool USE_8BIT_OUTPUT = false; | ||
| constexpr bool USE_ATKINSON = true; | ||
| constexpr bool USE_FLOYD_STEINBERG = false; | ||
| constexpr bool USE_PRESCALE = true; | ||
| constexpr int TARGET_MAX_WIDTH = 480; | ||
| constexpr int TARGET_MAX_HEIGHT = 800; | ||
| // ============================================================================ |
There was a problem hiding this comment.
Pipeline failure: Clang-format issues detected.
The CI pipeline reports formatting issues. Run bin/clang-format-fix to resolve them before merging.
🧰 Tools
🪛 GitHub Actions: CI (build)
[error] 1-1: Clang-format formatting issues detected. Please run 'bin/clang-format-fix' to fix formatting in this file.
🤖 Prompt for AI Agents
In `@lib/PngToBmpConverter/PngToBmpConverter.cpp` around lines 1 - 21,
Clang-format flagged formatting issues in PngToBmpConverter.cpp (headers,
include ordering and spacing around the IMAGE PROCESSING OPTIONS block and
constant declarations like USE_8BIT_OUTPUT, USE_ATKINSON, TARGET_MAX_WIDTH,
TARGET_MAX_HEIGHT); run the formatter script (bin/clang-format-fix) on that
file, review the resulting changes to ensure includes and the comment block are
correctly spaced/ordered, then add and commit the reformatted file so the CI
pipeline passes.
| if (memcmp(chunkType, "PLTE", 4) == 0) { | ||
| int entries = chunkLen / 3; | ||
| if (entries > 256) entries = 256; | ||
| ctx.paletteSize = entries; | ||
| size_t palBytes = entries * 3; | ||
| pngFile.read(ctx.palette, palBytes); | ||
| // Skip any remaining palette data | ||
| if (chunkLen > palBytes) pngFile.seekCur(chunkLen - palBytes); | ||
| pngFile.seekCur(4); // CRC |
There was a problem hiding this comment.
Return value of pngFile.read() not checked when reading palette.
At line 568, the return value of pngFile.read(ctx.palette, palBytes) is not verified. If the read fails or returns fewer bytes, the palette will contain uninitialized or partial data, potentially causing incorrect colors in the output.
🛡️ Proposed fix
ctx.paletteSize = entries;
size_t palBytes = entries * 3;
- pngFile.read(ctx.palette, palBytes);
+ if (pngFile.read(ctx.palette, palBytes) != static_cast<int>(palBytes)) {
+ Serial.printf("[%lu] [PNG] Failed to read palette data\n", millis());
+ free(ctx.currentRow);
+ free(ctx.previousRow);
+ return false;
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (memcmp(chunkType, "PLTE", 4) == 0) { | |
| int entries = chunkLen / 3; | |
| if (entries > 256) entries = 256; | |
| ctx.paletteSize = entries; | |
| size_t palBytes = entries * 3; | |
| pngFile.read(ctx.palette, palBytes); | |
| // Skip any remaining palette data | |
| if (chunkLen > palBytes) pngFile.seekCur(chunkLen - palBytes); | |
| pngFile.seekCur(4); // CRC | |
| if (memcmp(chunkType, "PLTE", 4) == 0) { | |
| int entries = chunkLen / 3; | |
| if (entries > 256) entries = 256; | |
| ctx.paletteSize = entries; | |
| size_t palBytes = entries * 3; | |
| if (pngFile.read(ctx.palette, palBytes) != static_cast<int>(palBytes)) { | |
| Serial.printf("[%lu] [PNG] Failed to read palette data\n", millis()); | |
| free(ctx.currentRow); | |
| free(ctx.previousRow); | |
| return false; | |
| } | |
| // Skip any remaining palette data | |
| if (chunkLen > palBytes) pngFile.seekCur(chunkLen - palBytes); | |
| pngFile.seekCur(4); // CRC |
🤖 Prompt for AI Agents
In `@lib/PngToBmpConverter/PngToBmpConverter.cpp` around lines 563 - 571, The PLTE
chunk handling does an unchecked pngFile.read(ctx.palette, palBytes) which can
return fewer bytes or fail; update the PLTE branch in PngToBmpConverter.cpp to
check the return value (or the number of bytes read) from pngFile.read, and if
it is less than palBytes handle it by logging/erroring and aborting conversion
(or treating it as a malformed PNG), ensuring you do not use
partial/uninitialized ctx.palette; also ensure any remaining bytes and the CRC
are skipped/consumed only after a successful read so you don’t mis-sync the
stream.
This partially fixes #769 but is dependent upon PR #827 being merged along side this @daveallie. I removed my PNG conversion code after finding out that PR was already created. Without this PR though that book in #769 will still fail to load because of how it's stored in the file --------- Co-authored-by: Dave Allie <[email protected]>
## Summary - EPUB books with PNG cover images now display covers on the home screen instead of blank rectangles - Adds `PngToBmpConverter` library mirroring the existing `JpegToBmpConverter` pattern - Uses miniz (already in the project) for streaming zlib decompression of PNG IDAT data - Supports all PNG color types (Grayscale, RGB, RGBA, Palette, Gray+Alpha) - Optimized for ESP32-C3: batch grayscale conversion, 2KB read buffer, same area-averaging scaling and Atkinson dithering as the JPEG path ## Changes - **New:** `lib/PngToBmpConverter/PngToBmpConverter.h` — Public API matching JpegToBmpConverter's interface - **New:** `lib/PngToBmpConverter/PngToBmpConverter.cpp` — Streaming PNG decoder + BMP converter - **Modified:** `lib/Epub/Epub.cpp` — Added `.png` handling in `generateCoverBmp()` and `generateThumbBmp()` ## Test plan - [x] Tested with EPUB files using PNG covers — covers appear correctly on home screen - [ ] Verify with various PNG color types (most stock EPUBs use 8-bit RGB) - [ ] Confirm no regressions with JPEG cover EPUBs <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit **New Features** - Added PNG format support for EPUB cover and thumbnail images. PNG files are automatically processed and cached alongside existing supported formats. This enhancement enables users to leverage PNG cover artwork when generating EPUB files, improving workflow flexibility and compatibility with common image sources. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Nik Outchcunis <[email protected]> Co-authored-by: Claude Opus 4.6 <[email protected]> Co-authored-by: Dave Allie <[email protected]>
This partially fixes crosspoint-reader#769 but is dependent upon PR crosspoint-reader#827 being merged along side this @daveallie. I removed my PNG conversion code after finding out that PR was already created. Without this PR though that book in crosspoint-reader#769 will still fail to load because of how it's stored in the file --------- Co-authored-by: Dave Allie <[email protected]>
## Summary - EPUB books with PNG cover images now display covers on the home screen instead of blank rectangles - Adds `PngToBmpConverter` library mirroring the existing `JpegToBmpConverter` pattern - Uses miniz (already in the project) for streaming zlib decompression of PNG IDAT data - Supports all PNG color types (Grayscale, RGB, RGBA, Palette, Gray+Alpha) - Optimized for ESP32-C3: batch grayscale conversion, 2KB read buffer, same area-averaging scaling and Atkinson dithering as the JPEG path ## Changes - **New:** `lib/PngToBmpConverter/PngToBmpConverter.h` — Public API matching JpegToBmpConverter's interface - **New:** `lib/PngToBmpConverter/PngToBmpConverter.cpp` — Streaming PNG decoder + BMP converter - **Modified:** `lib/Epub/Epub.cpp` — Added `.png` handling in `generateCoverBmp()` and `generateThumbBmp()` ## Test plan - [x] Tested with EPUB files using PNG covers — covers appear correctly on home screen - [ ] Verify with various PNG color types (most stock EPUBs use 8-bit RGB) - [ ] Confirm no regressions with JPEG cover EPUBs <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit **New Features** - Added PNG format support for EPUB cover and thumbnail images. PNG files are automatically processed and cached alongside existing supported formats. This enhancement enables users to leverage PNG cover artwork when generating EPUB files, improving workflow flexibility and compatibility with common image sources. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Nik Outchcunis <[email protected]> Co-authored-by: Claude Opus 4.6 <[email protected]> Co-authored-by: Dave Allie <[email protected]>
This partially fixes crosspoint-reader#769 but is dependent upon PR crosspoint-reader#827 being merged along side this @daveallie. I removed my PNG conversion code after finding out that PR was already created. Without this PR though that book in crosspoint-reader#769 will still fail to load because of how it's stored in the file --------- Co-authored-by: Dave Allie <[email protected]>
## Summary - EPUB books with PNG cover images now display covers on the home screen instead of blank rectangles - Adds `PngToBmpConverter` library mirroring the existing `JpegToBmpConverter` pattern - Uses miniz (already in the project) for streaming zlib decompression of PNG IDAT data - Supports all PNG color types (Grayscale, RGB, RGBA, Palette, Gray+Alpha) - Optimized for ESP32-C3: batch grayscale conversion, 2KB read buffer, same area-averaging scaling and Atkinson dithering as the JPEG path ## Changes - **New:** `lib/PngToBmpConverter/PngToBmpConverter.h` — Public API matching JpegToBmpConverter's interface - **New:** `lib/PngToBmpConverter/PngToBmpConverter.cpp` — Streaming PNG decoder + BMP converter - **Modified:** `lib/Epub/Epub.cpp` — Added `.png` handling in `generateCoverBmp()` and `generateThumbBmp()` ## Test plan - [x] Tested with EPUB files using PNG covers — covers appear correctly on home screen - [ ] Verify with various PNG color types (most stock EPUBs use 8-bit RGB) - [ ] Confirm no regressions with JPEG cover EPUBs <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit **New Features** - Added PNG format support for EPUB cover and thumbnail images. PNG files are automatically processed and cached alongside existing supported formats. This enhancement enables users to leverage PNG cover artwork when generating EPUB files, improving workflow flexibility and compatibility with common image sources. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Nik Outchcunis <[email protected]> Co-authored-by: Claude Opus 4.6 <[email protected]> Co-authored-by: Dave Allie <[email protected]>
This partially fixes crosspoint-reader#769 but is dependent upon PR crosspoint-reader#827 being merged along side this @daveallie. I removed my PNG conversion code after finding out that PR was already created. Without this PR though that book in crosspoint-reader#769 will still fail to load because of how it's stored in the file --------- Co-authored-by: Dave Allie <[email protected]>
- EPUB books with PNG cover images now display covers on the home screen instead of blank rectangles - Adds `PngToBmpConverter` library mirroring the existing `JpegToBmpConverter` pattern - Uses miniz (already in the project) for streaming zlib decompression of PNG IDAT data - Supports all PNG color types (Grayscale, RGB, RGBA, Palette, Gray+Alpha) - Optimized for ESP32-C3: batch grayscale conversion, 2KB read buffer, same area-averaging scaling and Atkinson dithering as the JPEG path - **New:** `lib/PngToBmpConverter/PngToBmpConverter.h` — Public API matching JpegToBmpConverter's interface - **New:** `lib/PngToBmpConverter/PngToBmpConverter.cpp` — Streaming PNG decoder + BMP converter - **Modified:** `lib/Epub/Epub.cpp` — Added `.png` handling in `generateCoverBmp()` and `generateThumbBmp()` - [x] Tested with EPUB files using PNG covers — covers appear correctly on home screen - [ ] Verify with various PNG color types (most stock EPUBs use 8-bit RGB) - [ ] Confirm no regressions with JPEG cover EPUBs <!-- This is an auto-generated comment: release notes by coderabbit.ai --> **New Features** - Added PNG format support for EPUB cover and thumbnail images. PNG files are automatically processed and cached alongside existing supported formats. This enhancement enables users to leverage PNG cover artwork when generating EPUB files, improving workflow flexibility and compatibility with common image sources. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Nik Outchcunis <[email protected]> Co-authored-by: Claude Opus 4.6 <[email protected]> Co-authored-by: Dave Allie <[email protected]>
Summary
PngToBmpConverterlibrary mirroring the existingJpegToBmpConverterpatternChanges
lib/PngToBmpConverter/PngToBmpConverter.h— Public API matching JpegToBmpConverter's interfacelib/PngToBmpConverter/PngToBmpConverter.cpp— Streaming PNG decoder + BMP converterlib/Epub/Epub.cpp— Added.pnghandling ingenerateCoverBmp()andgenerateThumbBmp()Test plan
Summary by CodeRabbit
New Features