From fedcb2f53d109d86d12eaf38e1db007319e84c04 Mon Sep 17 00:00:00 2001 From: Justin Mitchell Date: Sat, 18 Apr 2026 15:22:34 -0400 Subject: [PATCH] fix: boot looping when opening large XTC files (#1648) Opening XTC files with a high page count (e.g. *The Magic Mountain* at 4,187 pages) causes an immediate `abort()` crash and reboot loop. The device becomes unusable until the book is removed from the SD card. **Crash log:** ``` abort() was called at 0x4214a5fb on core 0 ``` ### Root cause During `XtcParser::open()`, the parser calls `m_pageTable.resize(pageCount)` to load the entire page table into RAM. Each `PageInfo` entry is 16 bytes, so: - 4,187 pages x 16 bytes = **66,992 bytes (~65KB)** as a single contiguous heap allocation On the ESP32-C3 with ~380KB total RAM (no PSRAM), this allocation fails after firmware, fonts, and the activity system are already loaded. Because the firmware is compiled with `-fno-exceptions`, the failed `new` inside `std::vector::resize()` calls `abort()` instead of throwing. This affects any XTC file with roughly 3,000+ pages, depending on heap state at the time of loading. ## Solution Replace the bulk page table allocation with on-demand reads from the SD card. Instead of loading all page table entries into a vector at file open, we now: 1. Read only the **first** page table entry at open time (to get default page dimensions) 2. Read a **single** 16-byte entry from the SD card each time a page is loaded This reduces page table memory usage from `pageCount * 16` bytes to **zero bytes**, regardless of how many pages the file contains. ### Changes | File | What changed | |------|-------------| | `XtcParser.h` | Removed `std::vector m_pageTable`. Added `readPageTableEntry()` for on-demand reads. | | `XtcParser.cpp` | Replaced `readPageTable()` with `readFirstPageInfo()`. Updated `getPageInfo()`, `loadPage()`, and `loadPageStreaming()` to seek and read individual entries from the file. | ## Trade-offs ### Performance Each page turn now requires one additional SD card seek + 16-byte read to look up the page table entry before reading the page data itself. - SD card sequential read latency: ~0.1-0.5ms for a 16-byte read - E-ink full display refresh: ~1,000-2,000ms I personally can't see any performance difference while reading and the trade off of not boot looping seems to make this well worth it. ### Memory | Metric | Before | After | |--------|--------|-------| | Page table RAM (4,187 pages) | ~65KB | 0 bytes | | Page table RAM (1,000 pages) | ~16KB | 0 bytes | | Page table RAM (max 65,535 pages) | ~1MB (impossible) | 0 bytes | --- lib/Xtc/Xtc.cpp | 2 +- lib/Xtc/Xtc.h | 2 +- lib/Xtc/Xtc/XtcParser.cpp | 187 ++++++++++++++++++++++++++------------ lib/Xtc/Xtc/XtcParser.h | 17 +++- 4 files changed, 146 insertions(+), 62 deletions(-) diff --git a/lib/Xtc/Xtc.cpp b/lib/Xtc/Xtc.cpp index 893e8169c..5f0388d28 100644 --- a/lib/Xtc/Xtc.cpp +++ b/lib/Xtc/Xtc.cpp @@ -103,7 +103,7 @@ bool Xtc::hasChapters() const { return parser->hasChapters(); } -const std::vector& Xtc::getChapters() const { +const std::vector& Xtc::getChapters() { static const std::vector kEmpty; if (!loaded || !parser) { return kEmpty; diff --git a/lib/Xtc/Xtc.h b/lib/Xtc/Xtc.h index 9b75f5869..3dbd160b4 100644 --- a/lib/Xtc/Xtc.h +++ b/lib/Xtc/Xtc.h @@ -58,7 +58,7 @@ class Xtc { std::string getTitle() const; std::string getAuthor() const; bool hasChapters() const; - const std::vector& getChapters() const; + const std::vector& getChapters(); // Cover image support (for sleep screen) std::string getCoverBmpPath() const; diff --git a/lib/Xtc/Xtc/XtcParser.cpp b/lib/Xtc/Xtc/XtcParser.cpp index 183afea92..e30f77964 100644 --- a/lib/Xtc/Xtc/XtcParser.cpp +++ b/lib/Xtc/Xtc/XtcParser.cpp @@ -21,6 +21,7 @@ XtcParser::XtcParser() m_defaultHeight(DISPLAY_HEIGHT), m_bitDepth(1), m_hasChapters(false), + m_chaptersLoaded(false), m_lastError(XtcError::OK) { memset(&m_header, 0, sizeof(m_header)); } @@ -33,6 +34,8 @@ XtcError XtcParser::open(const char* filepath) { close(); } + m_filepath = filepath; + // Open file if (!Storage.openFileForRead("XTC", filepath, m_file)) { m_lastError = XtcError::FILE_NOT_FOUND; @@ -64,25 +67,29 @@ XtcError XtcParser::open(const char* filepath) { m_file.close(); return m_lastError; } + // Trim excess capacity from metadata strings + m_title.shrink_to_fit(); + m_author.shrink_to_fit(); } - // Read page table - m_lastError = readPageTable(); + // Read first page info for default dimensions (no bulk page table allocation) + m_lastError = readFirstPageInfo(); if (m_lastError != XtcError::OK) { - LOG_DBG("XTC", "Failed to read page table: %s", errorToString(m_lastError)); + LOG_DBG("XTC", "Failed to read first page info: %s", errorToString(m_lastError)); // Explicit close() required: member variable persists beyond function scope m_file.close(); return m_lastError; } - // Read chapters if present - m_lastError = readChapters(); - if (m_lastError != XtcError::OK) { - LOG_DBG("XTC", "Failed to read chapters: %s", errorToString(m_lastError)); - // Explicit close() required: member variable persists beyond function scope - m_file.close(); - return m_lastError; - } + // Defer chapter parsing until actually needed (lazy load). + // Chapter strings can use significant heap; keeping them out of memory + // during rendering leaves more room for the page bitmap buffer. + m_hasChapters = (m_header.hasChapters == 1); + m_chaptersLoaded = false; + + // Close the source file to free its internal SdFat buffers. + // It will be reopened on-demand for page table lookups and bitmap reads. + m_file.close(); m_isOpen = true; LOG_DBG("XTC", "Opened file: %s (%u pages, %dx%d)", filepath, m_header.pageCount, m_defaultWidth, m_defaultHeight); @@ -90,18 +97,29 @@ XtcError XtcParser::open(const char* filepath) { } void XtcParser::close() { - if (m_isOpen) { - // Explicit close() required: member variable persists beyond function scope - m_file.close(); - m_isOpen = false; - } - m_pageTable.clear(); + closeFile(); + m_isOpen = false; + m_chaptersLoaded = false; m_chapters.clear(); m_title.clear(); + m_author.clear(); m_hasChapters = false; memset(&m_header, 0, sizeof(m_header)); } +bool XtcParser::ensureFileOpen() { + if (m_file.isOpen()) { + return true; + } + return Storage.openFileForRead("XTC", m_filepath.c_str(), m_file); +} + +void XtcParser::closeFile() { + if (m_file.isOpen()) { + m_file.close(); + } +} + XtcError XtcParser::readHeader() { // Read first 56 bytes of header size_t bytesRead = m_file.read(reinterpret_cast(&m_header), sizeof(XtcHeader)); @@ -169,50 +187,81 @@ XtcError XtcParser::readAuthor() { return XtcError::OK; } -XtcError XtcParser::readPageTable() { +XtcError XtcParser::readFirstPageInfo() { if (m_header.pageTableOffset == 0) { LOG_DBG("XTC", "Page table offset is 0, cannot read"); return XtcError::CORRUPTED_HEADER; } - // Seek to page table + // Verify the file is large enough to contain the full page table + const uint64_t fileSize = m_file.size(); + const uint64_t pageTableSize = static_cast(m_header.pageCount) * sizeof(PageTableEntry); + if (m_header.pageTableOffset < sizeof(XtcHeader) || m_header.pageTableOffset > fileSize || + pageTableSize > fileSize - m_header.pageTableOffset) { + LOG_DBG("XTC", "Page table exceeds file bounds"); + return XtcError::CORRUPTED_HEADER; + } + + // Read only the first entry to get default page dimensions + // All other entries are read on-demand via readPageTableEntry() + // This avoids allocating pageCount * 16 bytes (e.g. 65KB for 4000+ pages) + PageTableEntry entry; if (!m_file.seek(m_header.pageTableOffset)) { LOG_DBG("XTC", "Failed to seek to page table at %llu", m_header.pageTableOffset); return XtcError::READ_ERROR; } - - m_pageTable.resize(m_header.pageCount); - - // Read page table entries - for (uint16_t i = 0; i < m_header.pageCount; i++) { - PageTableEntry entry; - size_t bytesRead = m_file.read(reinterpret_cast(&entry), sizeof(PageTableEntry)); - if (bytesRead != sizeof(PageTableEntry)) { - LOG_DBG("XTC", "Failed to read page table entry %u", i); - return XtcError::READ_ERROR; - } - - m_pageTable[i].offset = static_cast(entry.dataOffset); - m_pageTable[i].size = entry.dataSize; - m_pageTable[i].width = entry.width; - m_pageTable[i].height = entry.height; - m_pageTable[i].bitDepth = m_bitDepth; - - // Update default dimensions from first page - if (i == 0) { - m_defaultWidth = entry.width; - m_defaultHeight = entry.height; - } + size_t bytesRead = m_file.read(reinterpret_cast(&entry), sizeof(PageTableEntry)); + if (bytesRead != sizeof(PageTableEntry)) { + LOG_DBG("XTC", "Failed to read first page table entry"); + return XtcError::READ_ERROR; } - LOG_DBG("XTC", "Read %u page table entries", m_header.pageCount); + m_defaultWidth = entry.width; + m_defaultHeight = entry.height; + + LOG_DBG("XTC", "Page table validated: %u pages, default %dx%d", m_header.pageCount, m_defaultWidth, m_defaultHeight); return XtcError::OK; } +bool XtcParser::readPageTableEntry(uint32_t pageIndex, PageInfo& info) { + if (pageIndex >= m_header.pageCount) { + return false; + } + + if (!ensureFileOpen()) { + LOG_DBG("XTC", "Failed to reopen file for page table read"); + return false; + } + + // Seek to the specific page table entry on the SD card + const uint64_t entryOffset = m_header.pageTableOffset + static_cast(pageIndex) * sizeof(PageTableEntry); + if (!m_file.seek(entryOffset)) { + LOG_DBG("XTC", "Failed to seek to page table entry %lu at %llu", pageIndex, entryOffset); + return false; + } + + PageTableEntry entry; + size_t bytesRead = m_file.read(reinterpret_cast(&entry), sizeof(PageTableEntry)); + if (bytesRead != sizeof(PageTableEntry)) { + LOG_DBG("XTC", "Failed to read page table entry %lu", pageIndex); + return false; + } + + info.offset = static_cast(entry.dataOffset); + info.size = entry.dataSize; + info.width = entry.width; + info.height = entry.height; + info.bitDepth = m_bitDepth; + return true; +} + XtcError XtcParser::readChapters() { - m_hasChapters = false; m_chapters.clear(); + if (!ensureFileOpen()) { + return XtcError::READ_ERROR; + } + uint8_t hasChaptersFlag = 0; if (!m_file.seek(0x0B)) { return XtcError::READ_ERROR; @@ -242,13 +291,12 @@ XtcError XtcParser::readChapters() { return XtcError::OK; } - uint64_t maxOffset = 0; - if (m_header.pageTableOffset > chapterOffset) { + // Clamp maxOffset to fileSize so bogus header values can't inflate chapterCount + uint64_t maxOffset = fileSize; + if (m_header.pageTableOffset > chapterOffset && m_header.pageTableOffset <= fileSize) { maxOffset = m_header.pageTableOffset; - } else if (m_header.dataOffset > chapterOffset) { + } else if (m_header.dataOffset > chapterOffset && m_header.dataOffset <= fileSize) { maxOffset = m_header.dataOffset; - } else { - maxOffset = fileSize; } if (maxOffset <= chapterOffset) { @@ -266,6 +314,7 @@ XtcError XtcParser::readChapters() { return XtcError::READ_ERROR; } + m_chapters.reserve(chapterCount); std::vector chapterBuf(chapterSize); for (size_t i = 0; i < chapterCount; i++) { if (m_file.read(chapterBuf.data(), chapterSize) != chapterSize) { @@ -315,14 +364,24 @@ XtcError XtcParser::readChapters() { return XtcError::OK; } -bool XtcParser::getPageInfo(uint32_t pageIndex, PageInfo& info) const { - if (pageIndex >= m_pageTable.size()) { - return false; +const std::vector& XtcParser::getChapters() { + // Lazy load chapters on first access + if (!m_chaptersLoaded && m_hasChapters) { + const XtcError err = readChapters(); + if (err != XtcError::OK) { + LOG_ERR("XTC", "Failed to lazy-load chapters: %s", errorToString(err)); + m_hasChapters = false; + m_chapters.clear(); + } + m_chaptersLoaded = true; + // Close file after chapter read to free buffers for rendering + closeFile(); } - info = m_pageTable[pageIndex]; - return true; + return m_chapters; } +bool XtcParser::getPageInfo(uint32_t pageIndex, PageInfo& info) { return readPageTableEntry(pageIndex, info); } + size_t XtcParser::loadPage(uint32_t pageIndex, uint8_t* buffer, size_t bufferSize) { if (!m_isOpen) { m_lastError = XtcError::FILE_NOT_FOUND; @@ -334,7 +393,16 @@ size_t XtcParser::loadPage(uint32_t pageIndex, uint8_t* buffer, size_t bufferSiz return 0; } - const PageInfo& page = m_pageTable[pageIndex]; + PageInfo page; + if (!readPageTableEntry(pageIndex, page)) { + m_lastError = XtcError::READ_ERROR; + return 0; + } + + if (!ensureFileOpen()) { + m_lastError = XtcError::FILE_NOT_FOUND; + return 0; + } // Seek to page data if (!m_file.seek(page.offset)) { @@ -402,7 +470,14 @@ XtcError XtcParser::loadPageStreaming(uint32_t pageIndex, return XtcError::PAGE_OUT_OF_RANGE; } - const PageInfo& page = m_pageTable[pageIndex]; + PageInfo page; + if (!readPageTableEntry(pageIndex, page)) { + return XtcError::READ_ERROR; + } + + if (!ensureFileOpen()) { + return XtcError::FILE_NOT_FOUND; + } // Seek to page data if (!m_file.seek(page.offset)) { diff --git a/lib/Xtc/Xtc/XtcParser.h b/lib/Xtc/Xtc/XtcParser.h index 19f3b75dc..b688d7935 100644 --- a/lib/Xtc/Xtc/XtcParser.h +++ b/lib/Xtc/Xtc/XtcParser.h @@ -23,6 +23,9 @@ namespace xtc { * * Reads XTC files from SD card and extracts page data. * Designed for ESP32-C3's limited RAM (~380KB) using streaming. + * + * The source file is kept closed between reads to free heap for rendering. + * It is reopened on-demand for page table lookups and bitmap data reads. */ class XtcParser { public: @@ -42,7 +45,7 @@ class XtcParser { uint8_t getBitDepth() const { return m_bitDepth; } // 1 = XTC/XTG, 2 = XTCH/XTH // Page information - bool getPageInfo(uint32_t pageIndex, PageInfo& info) const; + bool getPageInfo(uint32_t pageIndex, PageInfo& info); /** * Load page bitmap (raw 1-bit data, skipping XTG header) @@ -72,7 +75,7 @@ class XtcParser { std::string getAuthor() const { return m_author; } bool hasChapters() const { return m_hasChapters; } - const std::vector& getChapters() const { return m_chapters; } + const std::vector& getChapters(); // Validation static bool isValidXtcFile(const char* filepath); @@ -82,9 +85,9 @@ class XtcParser { private: FsFile m_file; + std::string m_filepath; bool m_isOpen; XtcHeader m_header; - std::vector m_pageTable; std::vector m_chapters; std::string m_title; std::string m_author; @@ -92,14 +95,20 @@ class XtcParser { uint16_t m_defaultHeight; uint8_t m_bitDepth; // 1 = XTC/XTG (1-bit), 2 = XTCH/XTH (2-bit) bool m_hasChapters; + bool m_chaptersLoaded; XtcError m_lastError; // Internal helper functions XtcError readHeader(); - XtcError readPageTable(); + XtcError readFirstPageInfo(); XtcError readTitle(); XtcError readAuthor(); XtcError readChapters(); + bool readPageTableEntry(uint32_t pageIndex, PageInfo& info); + + // File handle management — reopen on demand, close after use + bool ensureFileOpen(); + void closeFile(); }; } // namespace xtc