BookPlayer icon indicating copy to clipboard operation
BookPlayer copied to clipboard

Bug: Wrong number of chapters displayed for mp3 with more then 255 chapters (possible integer overflow)

Open jasLogic opened this issue 1 year ago • 7 comments

Hi, thanks for creating this great app. Here is a small bug I just encountered:

Environment

  • OS Version: 16.6.1
  • App Version: 5.0.9-11

Description

When I create a mp3 file with more than 255 chapters the wrong number of chapters is displayed in the player:

  • For 255 chapters everything still works fine.
  • When having exactly 256 chapters the file is displayed has having zero chapters and no chapter title is displayed at all.
  • For more than 256 chapters there are exactly n mod 256 chapters displayed. But you can still listen past the (n mod 256)-th "last" chapter although the player will then just show the title of the "last" chapter and a full progress bar and start increasing the time still left in the chapter.

I have tested that this issue does not arise when using a m4b file.

Steps to reproduce the behavior

  1. Create a mp3 file with more than 255 chapters.
  2. Import the file into the app.
  3. Observe wrong number of chapters.

jasLogic avatar Oct 09 '23 09:10 jasLogic

Thanks for the report!

This is kinda tricky because ID3v2, the header metadata that provides information about stuff including the chapter table of contents, specifies a single unsigned eight bit integer is used for the number of chapters in a file. I'm guessing this is where the issue starts.

This may be an issue with whatever you're using as a tag editor to create the chapters. I think there's a way to cheat this because you can include a table of contents inside a table of contents, but in general there's no way to specify an unlimited number of chapters, because the header has a maximum size.

But it's interesting that the chapter titles are still included during playback. I think this means we could process the header and count the actual number of chapters in the header.

Anyway, thanks, this is a fun issue. If using m4b is possible for you without too much effort and that works for you, I'd recommend that for now.

harrisi avatar Oct 09 '23 17:10 harrisi

Interesting! Thats probably where the problem comes from. But I don't think that the chapters are tagged incorrectly because I write them with ffmpeg directly and I can view the chapters with no problem in mpv.

But yeah you are right, I should really be using m4b instead.

jasLogic avatar Oct 10 '23 15:10 jasLogic

Interesting. Well mpv is open source so I should hopefully be able to figure out where the issue is. Thanks!

harrisi avatar Oct 10 '23 16:10 harrisi

Can I ask how you're creating the chapters with ffmpeg?

harrisi avatar Oct 12 '23 01:10 harrisi

Sure, I usually use ffmetadata files to write the chapters (which is a little bit cursed).

To for example create an audiobook with many chapter for testing purposes I did the following:

$ printf ";FFMETADATA1\ntitle=Chapter Test\n" > metadata.txt
$ for i in $(seq 257); do printf "[CHAPTER]\nTIMEBASE=1/1\nSTART=$((60*i))\nEND=$((60*(i+1)))\ntitle=Part $i\n"; done >> metadata.txt

This creates a ffmetadata file with 257 chapters which are each a minute long. To then write the chapters to a (long enough) audio file I use

$ ffmpeg -i metadata.txt -i in.mp3 -map_metadata 0 -c copy out.mp3

When viewing this file in the app only the first chapter should be displayed.

jasLogic avatar Oct 13 '23 09:10 jasLogic

Thanks, that's what I expected.

So I've looked into this and how other players and tag readers/writers deal with this. The ID3 specification actually isn't clear about what to do here. The issue is based on what I mentioned before, with the chapter table of contents being limited to 255 entries, mostly*.

I've talked to some people and a maintainer of a library for reading and writing ID3 metadata, as well as looked at various implementations, and there isn't much of an agreement on how this should be handled. I think the solution is to update the spec, but ID3 as an org doesn't really exist anymore, as far as I can tell. There are ways to make it work, but I'm not sure how easy it is on iOS.

I think for now, if you need more than 255 chapters for a single file and mp4s work, I'd suggest doing that. Hopefully this can be improved at some point, but, again, it's really an issue with the spec.

*: It's a bit more complicated, actually, because the table of contents can be a tree structure, but I've never seen that be used - I don't think FFmpeg has a way to even create metadata like that for mp3s.

harrisi avatar Nov 08 '23 20:11 harrisi

Ok, thanks, that's interesting, thank you for investigating. As you recommended, I now use m4b files instead which work without any problems.

jasLogic avatar Nov 09 '23 11:11 jasLogic