- Sponsor
-
Notifications
You must be signed in to change notification settings - Fork 5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add ability for chapters [feature request] #3
Comments
Hey @kour1er, thanks for reaching out. You mean: https://id3.org/id3v2-chapters-1.0? |
Yes exactly that - thanks very much! |
Hello @kour1er, I've created a pre-release with the support of the chapter markers. Would you be so kind to check if the feature works as expected? |
Gosh that was quick work! So I've tested a couple different sets of mp3 files to merge into a single chapterised mp3 file. When I use the --verbose flag I can see the chapter timestamp being generated but in the final file I'm not seeing any chapter data. As a side issue, if I use the --cover flag, that information doesn't appear to be added either, so I wonder if the additional data just isn't being written to the final file? I checked with the previous release and --cover works there for me but not in the new release. For testing, I'm using Bound on iOS, ffprobe on the command line with -show_chapters flag and IINA for macOS (a front end for mpv). Am I missing a setting somewhere? This is with version 4.1.0+pre1 |
re: ffprobe/chapters - please let me know if I can provide further information on that. re: cover - so let's take something popular as an example: how about The Martian. It's a 500x500 pixel image so I don't think that should be outside of spec. If I try using that jpg on the pre-release the cover isn't added to the mp3 on my machine (doesn't work TM?). But using version 4.0.0 it does. I'm validating that using Meta on macOS. I don't know 'magic discovery' and couldn't find an obvious match from a search, could you link to it and I can test here with that tool too? Cheers |
Interesting - I checked with kid3 on my system and it also shows the correct image in both the old and prerelease versions. But other tools (IINA, VLC, Meta, Finder, Bound etc) don't show the cover art on mp3s generated by the prerelease version. If I inspect the hex of an mp3 created by the prerelease and the current release I see the same ID3 information you found too. So that's weird :) Also, on the original matter of the chapter embedding, I noticed in kid3, that although it does show embedded chapter information it doesn't appear to have the "Table of Contents" "toc" tag. I think that's a required element? Haven't been able to find a good link for that. Just wonder if that might be it? |
Baby steps: merging two mp3 files without id3 tags yield the same result in 4.0.0 and head. |
I've changed the order when the id3 tags is written, because I wanted to parse each file once for the duration:
My guess is that the file offset of the output file is not properly set (stateful) and I'm corrupting the file with a random write. |
Mhh, the id3 tag is now written at the end, but still not working. Next check is id3v2.4 (UTF-8) vs id3v2.3 (UTF-8). I've also noticed that any other id3 editor rewrites the file so that the id3 tag is always at the beginning. I know that this was a requirement for id3v1. |
If I write the ID3 tag at the beginning of the file, any editor recognizes the tag. I always thought it would be better so that music players don't need to parse the whole file. But I discarded it with the chapter change as "premature optimization", but I want to keep the concept of parsing the files only once (IO) and not holding the whole files in memory. BTW: the underlying library for ID3 follows the idea of putting the ID3 tag at the beginning of the file. "They" also pay the IO price of copying the data into a temp file: https://github.com/bogem/id3v2/blob/main/v2/tag.go#L321 |
Interesting - the 'root of all evil' strikes again :) |
Thanks for the working title of the next pre-release. You may want to give it a try. I'm not sure if I want to keep this specific implementation (using a temporary file to ensure the frame order) but any other solution this is more complicated and more error prone (e.g. own parser for frame offsets and delay the actual read till the end). Maybe a temporary file is not so bad at all. |
Congrats on the progress! Love the pre-release name :) Seems like a totally reasonable approach to me. I really love how fast your tool is. Crazy how much faster than some other tools I've used. I've now run some tests in my environment. The cover art problem is solved 👍 Chapter information is odd. I can see the chapter information correctly added on the final file in IINA (mpv frontend). But the merged file is not showing the chapter information on iOS in Bound or BookPlayer nor on macOS in the Apple books player. Interestingly, on PocketCast (I know it's a podcast player but I know it supports chapters) chapters do appear. So then I was reading the ID3v2 Chapter Frame Addendum and saw this bit:
Following that, I added by hand in kid3 'FFFFFFFF' to each chapter start offset and end offset. Then I saw this bit in the same spec:
Then also in kid3 I added a Table of Contents. Following the structure of another mp3 with chapter marks, I changed the chapter Identifier from 'chap-0'... to 'ch0'... and used those as the Items of in the TOC. I also made the TOC 'top level' and 'ordered' enabled in kid3. Retransmitted the altered file and chapters work everywhere! So it "appears" that:
This is obviously just experimental - love to know your thoughts |
|
Just to give you quick feedback - progress! Chapters are now working in IINA, Books (Apple desktop app), Bound (iOS) and BookPlayer (iOS). Nice work on the library work. 👍 One regression, the artist, title and album (so author and book as it were) don't appear to be written to the final file. I don't see those tags in Kid3 either. This is only from testing one book running it through your tool a few times. Cover is working fine. |
Now is the time to add those missing tests :) |
Can you post the command line here? I assume it contains spaces or special chars ( UPDATE: In the next beta the value can contain spaces with or without surrounding quotes (as long as there is no
|
With #5 closed, I think it becomes more user-friendly if one uses |
Doh! You're entirely correct. Can't believe I didn't think of that. So I've tested three more books using the most recent beta you sent and all seems to be working correctly! I will continue to test some more and of course with the new version with the more 'user-friendly' quote version too. Congrats on the solution. (I still can't get over how fast the tool is) |
I consider the current head stable enough for new official release. Please feel free to open new issues any time. |
Would it be possible to add the ability to make chapter markers for merging mp3 files when working with audiobooks? For example, if I have a directory of mp3 files that I'm merging, to make a new chapter marker for every file in the final outputted single mp3 file?
Thanks for the consideration
The text was updated successfully, but these errors were encountered: