Add chapter select support to XTC files#145
Add chapter select support to XTC files#145daveallie merged 1 commit intocrosspoint-reader:masterfrom
Conversation
There was a problem hiding this comment.
Nice implementation! The approach of normalizing page indices during parsing (converting 1-based to 0-based) is clean.
A couple of suggestions:
-
Unused include:
#include <SD.h>inXtcReaderChapterSelectionActivity.cppappears unused and can be removed. -
Long chapter names: Consider truncating chapter names that exceed screen width with "..." suffix, similar to
EpubReaderChapterSelectionActivity::renderScreen().
Currently long names could overflow the display.
Otherwise LGTM!
I've also been working on similar features in my branch https://github.com/eunchurn/crosspoint-reader-ko/tree/feature/xtc-chapter-support
- Chapter overlay: Displays current chapter name at bottom center while reading
- V2 metadata parsing: Full
XtcHeaderV2,XtcMetadatastructures forcoverPage,author,title(future use) - Centralized chapter lookup:
getChapterIndexForPage()inXtcParserinstead of Activity - Cover page support: Uses
coverPagefrom metadata for cover image generation
And I've created PR to your repo treetrum#1
| if (!xtc) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
| if (!xtc) { | |
| return; | |
| } | |
| if (!xtc || !xtc->hasChapters()) { | |
| return; | |
| } |
There was a problem hiding this comment.
This avoids creating a FreeRTOS task unnecessarily if chapters are somehow empty when entering the activity.
|
Thanks for the review @eunchurn! Appreciate the feedback 🙂 Given you've already addressed those points you've left in your fork, should we merge this as is and have you open a new PR with those fixes? |
|
Going to merge this in and we can have the remaining improvements to the XTC parser come later |
|
Good work!. I don't think it's a good idea to merge my improvement commit yet. There's some interference with the chapter display. I think we'll need to make this optional later. I'll reorganize the parser and types and submit a PR. |
## Summary - **What is the goal of this PR?** Add chapter selection support to the XTC reader activity, including parsing chapter metadata from XTC files. - **What changes are included?** Implemented XTC chapter parsing and exposure in the XTC library, added a chapter selection activity for XTC, integrated it into XtcReaderActivity, and normalized chapter page indices by shifting them to 0-based. ## Additional Context - The reader uses 0-based page indexing (first page = 0), but the XTC chapter table appears to be 1-based (first page = 1), so chapter start/end pages are shifted down by 1 during parsing.
## Summary - **What is the goal of this PR?** Add chapter selection support to the XTC reader activity, including parsing chapter metadata from XTC files. - **What changes are included?** Implemented XTC chapter parsing and exposure in the XTC library, added a chapter selection activity for XTC, integrated it into XtcReaderActivity, and normalized chapter page indices by shifting them to 0-based. ## Additional Context - The reader uses 0-based page indexing (first page = 0), but the XTC chapter table appears to be 1-based (first page = 1), so chapter start/end pages are shifted down by 1 during parsing.
## Summary - **What is the goal of this PR?** Add chapter selection support to the XTC reader activity, including parsing chapter metadata from XTC files. - **What changes are included?** Implemented XTC chapter parsing and exposure in the XTC library, added a chapter selection activity for XTC, integrated it into XtcReaderActivity, and normalized chapter page indices by shifting them to 0-based. ## Additional Context - The reader uses 0-based page indexing (first page = 0), but the XTC chapter table appears to be 1-based (first page = 1), so chapter start/end pages are shifted down by 1 during parsing.
3 participants
Summary
Additional Context