fix: support legacy XTC file headers where pageTableOffset=48 (#1816)
## 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**
This commit is contained in:
+20
-12
@@ -84,7 +84,9 @@ XtcError XtcParser::open(const char* filepath) {
|
|||||||
// Defer chapter parsing until actually needed (lazy load).
|
// Defer chapter parsing until actually needed (lazy load).
|
||||||
// Chapter strings can use significant heap; keeping them out of memory
|
// Chapter strings can use significant heap; keeping them out of memory
|
||||||
// during rendering leaves more room for the page bitmap buffer.
|
// 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;
|
m_chaptersLoaded = false;
|
||||||
|
|
||||||
// Close the source file to free its internal SdFat buffers.
|
// 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
|
// 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<uint64_t>(m_header.pageCount) * sizeof(PageTableEntry);
|
const uint64_t pageTableSize = static_cast<uint64_t>(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) {
|
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<unsigned long long>(fileSize), static_cast<unsigned long long>(m_header.pageTableOffset),
|
||||||
|
static_cast<unsigned long long>(pageTableSize), m_header.pageCount,
|
||||||
|
static_cast<unsigned int>(sizeof(PageTableEntry)), static_cast<unsigned long long>(m_header.dataOffset),
|
||||||
|
static_cast<unsigned long long>(XTC_LEGACY_HEADER_SIZE));
|
||||||
return XtcError::CORRUPTED_HEADER;
|
return XtcError::CORRUPTED_HEADER;
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -206,7 +214,7 @@ XtcError XtcParser::readFirstPageInfo() {
|
|||||||
// All other entries are read on-demand via readPageTableEntry()
|
// All other entries are read on-demand via readPageTableEntry()
|
||||||
// This avoids allocating pageCount * 16 bytes (e.g. 65KB for 4000+ pages)
|
// This avoids allocating pageCount * 16 bytes (e.g. 65KB for 4000+ pages)
|
||||||
PageTableEntry entry;
|
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);
|
LOG_DBG("XTC", "Failed to seek to page table at %llu", m_header.pageTableOffset);
|
||||||
return XtcError::READ_ERROR;
|
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
|
// Seek to the specific page table entry on the SD card
|
||||||
const uint64_t entryOffset = m_header.pageTableOffset + static_cast<uint64_t>(pageIndex) * sizeof(PageTableEntry);
|
const uint64_t entryOffset = m_header.pageTableOffset + static_cast<uint64_t>(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);
|
LOG_DBG("XTC", "Failed to seek to page table entry %lu at %llu", pageIndex, entryOffset);
|
||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
@@ -247,7 +255,7 @@ bool XtcParser::readPageTableEntry(uint32_t pageIndex, PageInfo& info) {
|
|||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
|
|
||||||
info.offset = static_cast<uint32_t>(entry.dataOffset);
|
info.offset = entry.dataOffset;
|
||||||
info.size = entry.dataSize;
|
info.size = entry.dataSize;
|
||||||
info.width = entry.width;
|
info.width = entry.width;
|
||||||
info.height = entry.height;
|
info.height = entry.height;
|
||||||
@@ -286,7 +294,7 @@ XtcError XtcParser::readChapters() {
|
|||||||
return XtcError::OK;
|
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) {
|
if (chapterOffset < sizeof(XtcHeader) || chapterOffset >= fileSize || chapterOffset + 96 > fileSize) {
|
||||||
return XtcError::OK;
|
return XtcError::OK;
|
||||||
}
|
}
|
||||||
@@ -310,7 +318,7 @@ XtcError XtcParser::readChapters() {
|
|||||||
return XtcError::OK;
|
return XtcError::OK;
|
||||||
}
|
}
|
||||||
|
|
||||||
if (!m_file.seek(chapterOffset)) {
|
if (!m_file.seek64(chapterOffset)) {
|
||||||
return XtcError::READ_ERROR;
|
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
|
// Seek to page data
|
||||||
if (!m_file.seek(page.offset)) {
|
if (!m_file.seek64(page.offset)) {
|
||||||
LOG_DBG("XTC", "Failed to seek to page %u at offset %lu", pageIndex, page.offset);
|
LOG_DBG("XTC", "Failed to seek to page %u at offset %llu", pageIndex, static_cast<unsigned long long>(page.offset));
|
||||||
m_lastError = XtcError::READ_ERROR;
|
m_lastError = XtcError::READ_ERROR;
|
||||||
return 0;
|
return 0;
|
||||||
}
|
}
|
||||||
@@ -480,7 +488,7 @@ XtcError XtcParser::loadPageStreaming(uint32_t pageIndex,
|
|||||||
}
|
}
|
||||||
|
|
||||||
// Seek to page data
|
// Seek to page data
|
||||||
if (!m_file.seek(page.offset)) {
|
if (!m_file.seek64(page.offset)) {
|
||||||
return XtcError::READ_ERROR;
|
return XtcError::READ_ERROR;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -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_WIDTH = 480;
|
||||||
constexpr uint16_t DISPLAY_HEIGHT = 800;
|
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)
|
#pragma pack(push, 1)
|
||||||
struct XtcHeader {
|
struct XtcHeader {
|
||||||
uint32_t magic; // 0x00: Magic number "XTC\0" (0x00435458)
|
uint32_t magic; // 0x00: Magic number "XTC\0" (0x00435458)
|
||||||
@@ -88,13 +90,13 @@ struct XtgPageHeader {
|
|||||||
|
|
||||||
// Page information (internal use, optimized for memory)
|
// Page information (internal use, optimized for memory)
|
||||||
struct PageInfo {
|
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)
|
uint32_t size; // Data size (bytes)
|
||||||
uint16_t width; // Page width
|
uint16_t width; // Page width
|
||||||
uint16_t height; // Page height
|
uint16_t height; // Page height
|
||||||
uint8_t bitDepth; // 1 = XTG (1-bit), 2 = XTH (2-bit grayscale)
|
uint8_t bitDepth; // 1 = XTG (1-bit), 2 = XTH (2-bit grayscale)
|
||||||
uint8_t padding; // Alignment padding
|
uint8_t padding; // Alignment padding
|
||||||
}; // 16 bytes total
|
};
|
||||||
|
|
||||||
struct ChapterInfo {
|
struct ChapterInfo {
|
||||||
std::string name;
|
std::string name;
|
||||||
|
|||||||
@@ -135,9 +135,11 @@ bool HalStorage::removeDir(const char* path) { HAL_STORAGE_WRAPPED_CALL(removeDi
|
|||||||
|
|
||||||
void HalFile::flush() { HAL_FILE_WRAPPED_CALL(flush, ); }
|
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::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::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::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::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::seekCur(int64_t offset) { HAL_FILE_WRAPPED_CALL(seekCur, offset); }
|
||||||
bool HalFile::seekSet(size_t offset) { HAL_FILE_WRAPPED_CALL(seekSet, offset); }
|
bool HalFile::seekSet(size_t offset) { HAL_FILE_WRAPPED_CALL(seekSet, offset); }
|
||||||
int HalFile::available() const { HAL_FILE_WRAPPED_CALL(available, ); }
|
int HalFile::available() const { HAL_FILE_WRAPPED_CALL(available, ); }
|
||||||
|
|||||||
@@ -76,7 +76,9 @@ class HalFile : public Print {
|
|||||||
size_t getName(char* name, size_t len);
|
size_t getName(char* name, size_t len);
|
||||||
size_t size();
|
size_t size();
|
||||||
size_t fileSize();
|
size_t fileSize();
|
||||||
|
uint64_t fileSize64();
|
||||||
bool seek(size_t pos);
|
bool seek(size_t pos);
|
||||||
|
bool seek64(uint64_t pos);
|
||||||
bool seekCur(int64_t offset);
|
bool seekCur(int64_t offset);
|
||||||
bool seekSet(size_t offset);
|
bool seekSet(size_t offset);
|
||||||
int available() const;
|
int available() const;
|
||||||
|
|||||||
@@ -179,7 +179,8 @@ void XtcReaderActivity::renderPage() {
|
|||||||
// Load page data
|
// Load page data
|
||||||
size_t bytesRead = xtc->loadPage(currentPage, pageBuffer, pageBufferSize);
|
size_t bytesRead = xtc->loadPage(currentPage, pageBuffer, pageBufferSize);
|
||||||
if (bytesRead == 0) {
|
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);
|
free(pageBuffer);
|
||||||
renderer.clearScreen();
|
renderer.clearScreen();
|
||||||
renderer.drawCenteredText(UI_12_FONT_ID, 300, tr(STR_PAGE_LOAD_ERROR), true, EpdFontFamily::BOLD);
|
renderer.drawCenteredText(UI_12_FONT_ID, 300, tr(STR_PAGE_LOAD_ERROR), true, EpdFontFamily::BOLD);
|
||||||
|
|||||||
Reference in New Issue
Block a user