From b692bad10dfc1b223e455e7984c6f401be061690 Mon Sep 17 00:00:00 2001 From: Julia Date: Sun, 3 May 2026 21:48:35 -0400 Subject: [PATCH] fix: support legacy XTC file headers where pageTableOffset=48 (#1816) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary **What is the goal of this PR?** Restore compatibility with older XTC files that use the legacy 48-byte header layout, so they open correctly instead of failing header/page-table validation. **What changes are included?** - Updates the XTC parser to accept legacy files where pageTableOffset starts at 0x30 instead of the newer full header size. - Avoids treating legacy header bytes as a valid chapterOffset, so older files are not misclassified as having chapters. - Switches XTC file size and seek operations to 64-bit-safe wrapper calls in HalStorage, which matches the XTC format’s 64-bit offset fields and keeps page/chapter lookups from narrowing offsets. - Keeps the existing bounds checks and improves related logging when XTC page loads fail. ## Additional Context - This is a narrow compatibility fix for XTC handling. The functional change is in the XTC parser; the HalStorage changes are supporting wrapper methods needed by that parser update. - Risk should be low outside XTC reading, since the HAL changes only expose existing underlying file APIs rather than changing storage behavior. --- ### 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? **YES** --- lib/Xtc/Xtc/XtcParser.cpp | 32 +++++++++++++-------- lib/Xtc/Xtc/XtcTypes.h | 8 ++++-- lib/hal/HalStorage.cpp | 6 ++-- lib/hal/HalStorage.h | 2 ++ src/activities/reader/XtcReaderActivity.cpp | 3 +- 5 files changed, 33 insertions(+), 18 deletions(-) diff --git a/lib/Xtc/Xtc/XtcParser.cpp b/lib/Xtc/Xtc/XtcParser.cpp index e30f7796..877d8237 100644 --- a/lib/Xtc/Xtc/XtcParser.cpp +++ b/lib/Xtc/Xtc/XtcParser.cpp @@ -84,7 +84,9 @@ XtcError XtcParser::open(const char* filepath) { // 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); + // Older XTC files start the page table at 0x30, so they do not have the later + // chapterOffset field even if the bytes read into that slot are non-zero. + m_hasChapters = (m_header.hasChapters == 1 && m_header.pageTableOffset >= sizeof(XtcHeader)); m_chaptersLoaded = false; // Close the source file to free its internal SdFat buffers. @@ -194,11 +196,17 @@ XtcError XtcParser::readFirstPageInfo() { } // Verify the file is large enough to contain the full page table - const uint64_t fileSize = m_file.size(); + const uint64_t fileSize = m_file.fileSize64(); const uint64_t pageTableSize = static_cast(m_header.pageCount) * sizeof(PageTableEntry); - if (m_header.pageTableOffset < sizeof(XtcHeader) || m_header.pageTableOffset > fileSize || + if (m_header.pageTableOffset < XTC_LEGACY_HEADER_SIZE || m_header.pageTableOffset > fileSize || pageTableSize > fileSize - m_header.pageTableOffset) { - LOG_DBG("XTC", "Page table exceeds file bounds"); + LOG_DBG("XTC", + "Page table exceeds file bounds: file=%llu tableOffset=%llu tableSize=%llu pages=%u entrySize=%u " + "dataOffset=%llu minTableOffset=%llu", + static_cast(fileSize), static_cast(m_header.pageTableOffset), + static_cast(pageTableSize), m_header.pageCount, + static_cast(sizeof(PageTableEntry)), static_cast(m_header.dataOffset), + static_cast(XTC_LEGACY_HEADER_SIZE)); return XtcError::CORRUPTED_HEADER; } @@ -206,7 +214,7 @@ XtcError XtcParser::readFirstPageInfo() { // 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)) { + if (!m_file.seek64(m_header.pageTableOffset)) { LOG_DBG("XTC", "Failed to seek to page table at %llu", m_header.pageTableOffset); return XtcError::READ_ERROR; } @@ -235,7 +243,7 @@ bool XtcParser::readPageTableEntry(uint32_t pageIndex, PageInfo& info) { // 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)) { + if (!m_file.seek64(entryOffset)) { LOG_DBG("XTC", "Failed to seek to page table entry %lu at %llu", pageIndex, entryOffset); return false; } @@ -247,7 +255,7 @@ bool XtcParser::readPageTableEntry(uint32_t pageIndex, PageInfo& info) { return false; } - info.offset = static_cast(entry.dataOffset); + info.offset = entry.dataOffset; info.size = entry.dataSize; info.width = entry.width; info.height = entry.height; @@ -286,7 +294,7 @@ XtcError XtcParser::readChapters() { return XtcError::OK; } - const uint64_t fileSize = m_file.size(); + const uint64_t fileSize = m_file.fileSize64(); if (chapterOffset < sizeof(XtcHeader) || chapterOffset >= fileSize || chapterOffset + 96 > fileSize) { return XtcError::OK; } @@ -310,7 +318,7 @@ XtcError XtcParser::readChapters() { return XtcError::OK; } - if (!m_file.seek(chapterOffset)) { + if (!m_file.seek64(chapterOffset)) { return XtcError::READ_ERROR; } @@ -405,8 +413,8 @@ size_t XtcParser::loadPage(uint32_t pageIndex, uint8_t* buffer, size_t bufferSiz } // Seek to page data - if (!m_file.seek(page.offset)) { - LOG_DBG("XTC", "Failed to seek to page %u at offset %lu", pageIndex, page.offset); + if (!m_file.seek64(page.offset)) { + LOG_DBG("XTC", "Failed to seek to page %u at offset %llu", pageIndex, static_cast(page.offset)); m_lastError = XtcError::READ_ERROR; return 0; } @@ -480,7 +488,7 @@ XtcError XtcParser::loadPageStreaming(uint32_t pageIndex, } // Seek to page data - if (!m_file.seek(page.offset)) { + if (!m_file.seek64(page.offset)) { return XtcError::READ_ERROR; } diff --git a/lib/Xtc/Xtc/XtcTypes.h b/lib/Xtc/Xtc/XtcTypes.h index 5cee691d..99cf4b11 100644 --- a/lib/Xtc/Xtc/XtcTypes.h +++ b/lib/Xtc/Xtc/XtcTypes.h @@ -31,7 +31,9 @@ constexpr uint32_t XTH_MAGIC = 0x00485458; // "XTH\0" for 2-bit page data constexpr uint16_t DISPLAY_WIDTH = 480; constexpr uint16_t DISPLAY_HEIGHT = 800; -// XTC file header (56 bytes) +constexpr uint64_t XTC_LEGACY_HEADER_SIZE = 0x30; // Original header before chapterOffset was added. + +// XTC file header (56 bytes; legacy files may start the page table at 48 bytes) #pragma pack(push, 1) struct XtcHeader { uint32_t magic; // 0x00: Magic number "XTC\0" (0x00435458) @@ -88,13 +90,13 @@ struct XtgPageHeader { // Page information (internal use, optimized for memory) struct PageInfo { - uint32_t offset; // File offset to page data (max 4GB file size) + uint64_t offset; // File offset to page data uint32_t size; // Data size (bytes) uint16_t width; // Page width uint16_t height; // Page height uint8_t bitDepth; // 1 = XTG (1-bit), 2 = XTH (2-bit grayscale) uint8_t padding; // Alignment padding -}; // 16 bytes total +}; struct ChapterInfo { std::string name; diff --git a/lib/hal/HalStorage.cpp b/lib/hal/HalStorage.cpp index 24395e4f..154e2c34 100644 --- a/lib/hal/HalStorage.cpp +++ b/lib/hal/HalStorage.cpp @@ -135,9 +135,11 @@ bool HalStorage::removeDir(const char* path) { HAL_STORAGE_WRAPPED_CALL(removeDi void HalFile::flush() { HAL_FILE_WRAPPED_CALL(flush, ); } size_t HalFile::getName(char* name, size_t len) { HAL_FILE_WRAPPED_CALL(getName, name, len); } -size_t HalFile::size() { HAL_FILE_FORWARD_CALL(size, ); } // already thread-safe, no need to wrap -size_t HalFile::fileSize() { HAL_FILE_FORWARD_CALL(fileSize, ); } // already thread-safe, no need to wrap +size_t HalFile::size() { HAL_FILE_FORWARD_CALL(size, ); } // already thread-safe, no need to wrap +size_t HalFile::fileSize() { HAL_FILE_FORWARD_CALL(fileSize, ); } // already thread-safe, no need to wrap +uint64_t HalFile::fileSize64() { HAL_FILE_FORWARD_CALL(fileSize, ); } // already thread-safe, no need to wrap bool HalFile::seek(size_t pos) { HAL_FILE_WRAPPED_CALL(seekSet, pos); } +bool HalFile::seek64(uint64_t pos) { HAL_FILE_WRAPPED_CALL(seekSet, pos); } bool HalFile::seekCur(int64_t offset) { HAL_FILE_WRAPPED_CALL(seekCur, offset); } bool HalFile::seekSet(size_t offset) { HAL_FILE_WRAPPED_CALL(seekSet, offset); } int HalFile::available() const { HAL_FILE_WRAPPED_CALL(available, ); } diff --git a/lib/hal/HalStorage.h b/lib/hal/HalStorage.h index d5824bae..5483678b 100644 --- a/lib/hal/HalStorage.h +++ b/lib/hal/HalStorage.h @@ -76,7 +76,9 @@ class HalFile : public Print { size_t getName(char* name, size_t len); size_t size(); size_t fileSize(); + uint64_t fileSize64(); bool seek(size_t pos); + bool seek64(uint64_t pos); bool seekCur(int64_t offset); bool seekSet(size_t offset); int available() const; diff --git a/src/activities/reader/XtcReaderActivity.cpp b/src/activities/reader/XtcReaderActivity.cpp index b24cee18..00dff8f8 100644 --- a/src/activities/reader/XtcReaderActivity.cpp +++ b/src/activities/reader/XtcReaderActivity.cpp @@ -179,7 +179,8 @@ void XtcReaderActivity::renderPage() { // Load page data size_t bytesRead = xtc->loadPage(currentPage, pageBuffer, pageBufferSize); if (bytesRead == 0) { - LOG_ERR("XTR", "Failed to load page %lu", currentPage); + LOG_ERR("XTR", "Failed to load page %lu: bufferSize=%lu bitDepth=%u error=%s", currentPage, pageBufferSize, + bitDepth, xtc::errorToString(xtc->getLastError())); free(pageBuffer); renderer.clearScreen(); renderer.drawCenteredText(UI_12_FONT_ID, 300, tr(STR_PAGE_LOAD_ERROR), true, EpdFontFamily::BOLD);