music-metadata
music-metadata copied to clipboard
Add chapter.start & chapter.timescale
Hello, I added the starting time and timescale values of chapters from audiobook file .m4b
.
Unit test updated and matches values using ffprobe.
Issues Found
-
.m4b
withisom/iso2/mp41
container only returns one chapter but all chapter durations are present.- In the
parseChapterTrack
function,chapterTrack.chunkOffsetTable
only contains only one value, andchapterTrack.sampleSizeTable
is empty. Which I believe they're needed to extract the chapter titles.
- In the
-
.m4b
withM4A/isom/iso2
, it doesn't return any chapter lists.
I can provide the sample files I used if there's a way I can send them privately as they are copyrighted material.
Reading the buffer/token(?) values is not my currently strong suit but I hope this PR helps.
Thanks for your contribution! You can send files to GitHub user @xs4all.nl.
Can you please rebase? I hope that triggers the GitHub action integration tests.
Hello! Feature branch rebased to latest master commit and I also fixed linting issues. Audio files were sent to the given email.
Coverage increased (+0.004%) to 91.921% when pulling f38274330c5896abbc003a1b308822dde240989b on jigzahoy:m4b-chapter into d47097b6168f4360e177b5cd2758cd31ca5c16b2 on Borewit:master.
I think something went wrong on your side with rebasing (you need to force push after a rebase). I would not have helped because I failed to fix the CI tests. But now it is fixed, and I rebased the branch.
Now CI tests are running and green, and merge conflicts have been resolved.
Side effect is that the branch on you local dev machine will now be in conflict with origin/m4b-chapter
. Not sure how experienced you are with things, but the easiest way to resolve that is:
- Switch (checkout) to master branch
- Delete
m4b-chapter
branch (not on GitHub, but only on your local machine) - checkout
origin/m4b-chapter
I got the files you send me 🙏, I review your work soon.
So this PR is about adding starting time and timescale values.
Can please convert the issues to actual issues @jigzahoy?
All I had to do was to force push it instead of merging it! Clearly it is my first time to do a PR 😅. Really appreciate the help, thank you very much!
Why are
format.chapter[n].sampleOffset / format.sampleRate
&format.chapter[n].start / format.timeScale
different?
I've notice that format.chapter[n].sampleOffset
and format.chapter[n].start
are relatively close and they have small difference between them. I assumed every first chapters starts at 00:00
and using the files I tested, format.chapter[0].sampleOffset
is a non-zero value (may be small and negligible when divided by format.sampleRate
). Was format.chapter[n].sampleOffset
has the same intended use as format.chapter[n].start
?
Some files I've used sometimes gives format.chapter[n].timeScale
a different value from format.sampleRate
. I only added it just in case. And I am honestly not sure if timeScale
would be the proper name for it. ffpmeg calls it time_base
and it's the reciprocal of timescale
(1/timeScale
). I will update the typing descriptions. But what do you think?
Clearly it is my first time to do a PR 😅.
You are doing just fine. Nice work.
Regarding the timeScale. I suspect there is reason for both. If we make those values part of the music-metadata.common API it should be document what these values represent. Maybe start
and timescale
should replace sampleOffset
, I honestly don't know. If you say that works better, I have no problem doing that. A formula how the timescale
relates the start
is required, otherwise it is not clear what these values represent.