A fork of https://github.com/crosspoint-reader/crosspoint-reader
fix: Don't extract unsupported formats (#977)
## 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**_
+74
-68
+74
-68
lib/Epub/Epub/parsers/ChapterHtmlSlimParser.cpp
+74
-68
lib/Epub/Epub/parsers/ChapterHtmlSlimParser.cpp
···
17
17
18
18
// Minimum file size (in bytes) to show indexing popup - smaller chapters don't benefit from it
19
19
constexpr size_t MIN_SIZE_FOR_POPUP = 10 * 1024; // 10KB
20
+
constexpr size_t PARSE_BUFFER_SIZE = 1024;
20
21
21
22
const char* BLOCK_TAGS[] = {"p", "li", "div", "br", "blockquote"};
22
23
constexpr int NUM_BLOCK_TAGS = sizeof(BLOCK_TAGS) / sizeof(BLOCK_TAGS[0]);
···
178
179
// Resolve the image path relative to the HTML file
179
180
std::string resolvedPath = FsHelpers::normalisePath(self->contentBase + src);
180
181
181
-
// Create a unique filename for the cached image
182
-
std::string ext;
183
-
size_t extPos = resolvedPath.rfind('.');
184
-
if (extPos != std::string::npos) {
185
-
ext = resolvedPath.substr(extPos);
186
-
}
187
-
std::string cachedImagePath = self->imageBasePath + std::to_string(self->imageCounter++) + ext;
182
+
if (ImageDecoderFactory::isFormatSupported(resolvedPath)) {
183
+
// Create a unique filename for the cached image
184
+
std::string ext;
185
+
size_t extPos = resolvedPath.rfind('.');
186
+
if (extPos != std::string::npos) {
187
+
ext = resolvedPath.substr(extPos);
188
+
}
189
+
std::string cachedImagePath = self->imageBasePath + std::to_string(self->imageCounter++) + ext;
188
190
189
-
// Extract image to cache file
190
-
FsFile cachedImageFile;
191
-
bool extractSuccess = false;
192
-
if (Storage.openFileForWrite("EHP", cachedImagePath, cachedImageFile)) {
193
-
extractSuccess = self->epub->readItemContentsToStream(resolvedPath, cachedImageFile, 4096);
194
-
cachedImageFile.flush();
195
-
cachedImageFile.close();
196
-
delay(50); // Give SD card time to sync
197
-
}
191
+
// Extract image to cache file
192
+
FsFile cachedImageFile;
193
+
bool extractSuccess = false;
194
+
if (Storage.openFileForWrite("EHP", cachedImagePath, cachedImageFile)) {
195
+
extractSuccess = self->epub->readItemContentsToStream(resolvedPath, cachedImageFile, 4096);
196
+
cachedImageFile.flush();
197
+
cachedImageFile.close();
198
+
delay(50); // Give SD card time to sync
199
+
}
198
200
199
-
if (extractSuccess) {
200
-
// Get image dimensions
201
-
ImageDimensions dims = {0, 0};
202
-
ImageToFramebufferDecoder* decoder = ImageDecoderFactory::getDecoder(cachedImagePath);
203
-
if (decoder && decoder->getDimensions(cachedImagePath, dims)) {
204
-
LOG_DBG("EHP", "Image dimensions: %dx%d", dims.width, dims.height);
201
+
if (extractSuccess) {
202
+
// Get image dimensions
203
+
ImageDimensions dims = {0, 0};
204
+
ImageToFramebufferDecoder* decoder = ImageDecoderFactory::getDecoder(cachedImagePath);
205
+
if (decoder && decoder->getDimensions(cachedImagePath, dims)) {
206
+
LOG_DBG("EHP", "Image dimensions: %dx%d", dims.width, dims.height);
205
207
206
-
// Scale to fit viewport while maintaining aspect ratio
207
-
int maxWidth = self->viewportWidth;
208
-
int maxHeight = self->viewportHeight;
209
-
float scaleX = (dims.width > maxWidth) ? (float)maxWidth / dims.width : 1.0f;
210
-
float scaleY = (dims.height > maxHeight) ? (float)maxHeight / dims.height : 1.0f;
211
-
float scale = (scaleX < scaleY) ? scaleX : scaleY;
212
-
if (scale > 1.0f) scale = 1.0f;
208
+
// Scale to fit viewport while maintaining aspect ratio
209
+
int maxWidth = self->viewportWidth;
210
+
int maxHeight = self->viewportHeight;
211
+
float scaleX = (dims.width > maxWidth) ? (float)maxWidth / dims.width : 1.0f;
212
+
float scaleY = (dims.height > maxHeight) ? (float)maxHeight / dims.height : 1.0f;
213
+
float scale = (scaleX < scaleY) ? scaleX : scaleY;
214
+
if (scale > 1.0f) scale = 1.0f;
213
215
214
-
int displayWidth = (int)(dims.width * scale);
215
-
int displayHeight = (int)(dims.height * scale);
216
+
int displayWidth = (int)(dims.width * scale);
217
+
int displayHeight = (int)(dims.height * scale);
216
218
217
-
LOG_DBG("EHP", "Display size: %dx%d (scale %.2f)", displayWidth, displayHeight, scale);
219
+
LOG_DBG("EHP", "Display size: %dx%d (scale %.2f)", displayWidth, displayHeight, scale);
218
220
219
-
// Create page for image - only break if image won't fit remaining space
220
-
if (self->currentPage && !self->currentPage->elements.empty() &&
221
-
(self->currentPageNextY + displayHeight > self->viewportHeight)) {
222
-
self->completePageFn(std::move(self->currentPage));
223
-
self->currentPage.reset(new Page());
224
-
if (!self->currentPage) {
225
-
LOG_ERR("EHP", "Failed to create new page");
221
+
// Create page for image - only break if image won't fit remaining space
222
+
if (self->currentPage && !self->currentPage->elements.empty() &&
223
+
(self->currentPageNextY + displayHeight > self->viewportHeight)) {
224
+
self->completePageFn(std::move(self->currentPage));
225
+
self->currentPage.reset(new Page());
226
+
if (!self->currentPage) {
227
+
LOG_ERR("EHP", "Failed to create new page");
228
+
return;
229
+
}
230
+
self->currentPageNextY = 0;
231
+
} else if (!self->currentPage) {
232
+
self->currentPage.reset(new Page());
233
+
if (!self->currentPage) {
234
+
LOG_ERR("EHP", "Failed to create initial page");
235
+
return;
236
+
}
237
+
self->currentPageNextY = 0;
238
+
}
239
+
240
+
// Create ImageBlock and add to page
241
+
auto imageBlock = std::make_shared<ImageBlock>(cachedImagePath, displayWidth, displayHeight);
242
+
if (!imageBlock) {
243
+
LOG_ERR("EHP", "Failed to create ImageBlock");
226
244
return;
227
245
}
228
-
self->currentPageNextY = 0;
229
-
} else if (!self->currentPage) {
230
-
self->currentPage.reset(new Page());
231
-
if (!self->currentPage) {
232
-
LOG_ERR("EHP", "Failed to create initial page");
246
+
int xPos = (self->viewportWidth - displayWidth) / 2;
247
+
auto pageImage = std::make_shared<PageImage>(imageBlock, xPos, self->currentPageNextY);
248
+
if (!pageImage) {
249
+
LOG_ERR("EHP", "Failed to create PageImage");
233
250
return;
234
251
}
235
-
self->currentPageNextY = 0;
236
-
}
252
+
self->currentPage->elements.push_back(pageImage);
253
+
self->currentPageNextY += displayHeight;
237
254
238
-
// Create ImageBlock and add to page
239
-
auto imageBlock = std::make_shared<ImageBlock>(cachedImagePath, displayWidth, displayHeight);
240
-
if (!imageBlock) {
241
-
LOG_ERR("EHP", "Failed to create ImageBlock");
255
+
self->depth += 1;
242
256
return;
257
+
} else {
258
+
LOG_ERR("EHP", "Failed to get image dimensions");
259
+
Storage.remove(cachedImagePath.c_str());
243
260
}
244
-
int xPos = (self->viewportWidth - displayWidth) / 2;
245
-
auto pageImage = std::make_shared<PageImage>(imageBlock, xPos, self->currentPageNextY);
246
-
if (!pageImage) {
247
-
LOG_ERR("EHP", "Failed to create PageImage");
248
-
return;
249
-
}
250
-
self->currentPage->elements.push_back(pageImage);
251
-
self->currentPageNextY += displayHeight;
252
-
253
-
self->depth += 1;
254
-
return;
255
261
} else {
256
-
LOG_ERR("EHP", "Failed to get image dimensions");
257
-
Storage.remove(cachedImagePath.c_str());
262
+
LOG_ERR("EHP", "Failed to extract image");
258
263
}
259
-
} else {
260
-
LOG_ERR("EHP", "Failed to extract image");
261
-
}
264
+
} // isFormatSupported
262
265
}
263
266
}
264
267
···
638
641
XML_SetElementHandler(parser, startElement, endElement);
639
642
XML_SetCharacterDataHandler(parser, characterData);
640
643
644
+
// Compute the time taken to parse and build pages
645
+
const uint32_t chapterStartTime = millis();
641
646
do {
642
-
void* const buf = XML_GetBuffer(parser, 1024);
647
+
void* const buf = XML_GetBuffer(parser, PARSE_BUFFER_SIZE);
643
648
if (!buf) {
644
649
LOG_ERR("EHP", "Couldn't allocate memory for buffer");
645
650
XML_StopParser(parser, XML_FALSE); // Stop any pending processing
···
650
655
return false;
651
656
}
652
657
653
-
const size_t len = file.read(buf, 1024);
658
+
const size_t len = file.read(buf, PARSE_BUFFER_SIZE);
654
659
655
660
if (len == 0 && file.available() > 0) {
656
661
LOG_ERR("EHP", "File read error");
···
675
680
return false;
676
681
}
677
682
} while (!done);
683
+
LOG_DBG("EHP", "Time to parse and build pages: %lu ms", millis() - chapterStartTime);
678
684
679
685
XML_StopParser(parser, XML_FALSE); // Stop any pending processing
680
686
XML_SetElementHandler(parser, nullptr, nullptr); // Clear callbacks