Zeugma440 / atldotnet

Fully managed, portable and easy-to-use C# library to read and edit audio data and metadata (tags) from various audio formats, playlists and CUE sheets
MIT License
460 stars 61 forks source link

MP3/ID3v2 chapters and long files (plus "comment" tag in v2.3 vs v2.4) #86

Closed audiamus closed 3 years ago

audiamus commented 3 years ago

The problem

It appears that for relatively long audiobooks (roughly > 10h) ATL.net wipes existing chapters when saving meta data.

Environment

Details

As discussed in #73 I am still adding the chapters to MP3 conversions with FFmpeg itself, for this strange duration phenomenon.

Now, in the process of replacing TagLibSharp with ATL.net throughout, I add all the other meta data with ATL.net. For shorter files this works. Meta data is there and chapters are still there. Checked with FFmpeg. ffmpeg.exe -i "file.mp3". However, with a longer book, the chapters somehow disappear.

Interestingly, the existing chapters are not listed in the ATL.Track properties after constructing the instance.

(There are pretty long books out there, 40 hours and more. While I encourage users to apply one of the multiple file options because of much higher throughput, some still prefer a monolith.)

Not sure whether it's related or not: FFmpeg complains about the "comment" tag, once written: "Incorrect BOM value". Some UTF8 thing? Should I provide it?

Code To Reproduce Issue

I am uploading sample data, link via Reddit.

Two examples, shorter and longer, each before and after the ATL.Track.Save() operation. When analyzing the embedded meta data you will see that it worked for the shorter one, but not for the longer one.

Zeugma440 commented 3 years ago

First thing that can be seen is ATL is unable to even read the chapters in full before.mp3. If it can't read them at the beginning, it can't maintain them in the end.

That being said, the issue seems to be an obscure spec compliance issue. According to specs, the size of the CTOC frame (table of contents) should be encoded as a synch-safe integer... but it's not done like that on full before.mp3, which confuses the library.

That issue doesn't happen with short files because the size of the frame doesn't go beyond 255 bytes. However, when the file is long, the frame lengthens and eventually uses more than 1 byte to code its size. This is where troubles start...

Which piece of software did you use to add chapters to that file ? Is it FFMpeg ?

audiamus commented 3 years ago

Which piece of software did you use to add chapters to that file ? Is it FFMpeg ?

Yes, release FFmpeg 4.3.1. Chapters are added from a separate meta data text file in FFmpeg's proprietary format, via the -map_chapters option.

This method also seems to overcome the 255 chapters limit I experienced with some other methods.

Zeugma440 commented 3 years ago

Just reviewed their code : looks like a bug on their end to me.

I'll post an issue on their project and try to find a clean workaround on my end....

Edit : Jesus Christ, they still work like in the 90's, with their very own mailing list and have closed github issues and PRs πŸ€¦β€β™‚οΈ Not sure I'd like to go that far.....

audiamus commented 3 years ago

try to find a clean workaround on my end

That would be very nice. However, I have a few doubts now, particularly if the FFmpeg output does not comply with the specs.

The reason I add the MP3 chapters with FFmpeg was to create duration info that is shown correctly in VLC right from the start, and not to derive it from the bit rate and then adapt (#73). That did work in the past. But it no longer does now. I guess VLC has changed because I have used the same FFmpeg and TagLibSharp versions for a year now.

If instant MP3 duration for VLC no longer works then there is no need to add the chapters with FFmpeg. I could do that with ATL.net as well. Therefore I have just let ATL.net add more than 255 chapters to an MP3 book. And ATL.net seems to support it. To overcome this 255 chapter limit (wherever that is located) is essential.

There are not many tools that can read ID3v2 chapters. MediaInfo does not seem to be able to, neither is VLC. But MPC-HC does. And MPC-HC shows the chapters in the examples for this issue for both the shorter and the longer file, even if the longer violates the specs.

they still work like in the 90's, with their very own mailing list and have closed github issues and PRs

I believe the FFmpeg project started in the 90s indeed. It's around for so long. And a mailing list doesn't need any moderation which is an advantage if you don't have a "community liaison officer" or whatever they call a forum moderator these days.

BTW, you have not yet opened the new GitHub "Discussions" feature for this repo. It could be a place for posting the positive feedback. :smiley:

Zeugma440 commented 3 years ago

You know what ? I've been thinking about this, and came to the conclusion that the way FFMpeg tags chapters has likely become the de facto standard, even if it's non spec-compliant. It's not the first thing I've seen in ID3v2 that has taken that path.

The "fix" will simply be reading a plain integer ^^

The 255 chapters limit you're talking about is probably another effect of software implementing specs approximately.

I believe the FFmpeg project started in the 90s indeed. It's around for so long. And a mailing list doesn't need any moderation which is an advantage if you don't have a "community liaison officer" or whatever they call a forum moderator these days.

Sure, you need moderating / community management everywhere now, but that's also called living with one's time. But if they aren't ready for this, there's no reason to force it. Their project is mature enough not to crave for more participants, anyway πŸ˜‰

Believe it or not, I completely missed the "Discussions" feature. Enabling it as we speak ! Thanks man

Zeugma440 commented 3 years ago

Done ! Could you test the new commit and check if everything works for you ?

audiamus commented 3 years ago

OK, did that, checked the MP3 chapters. I can see them now after instantiating ATL.Track. And they are preserved when saving the modified properties, for both the longer and the shorter test file. Very good. :satisfied:

So, the other complaint by FFmpeg about the comment tag is unrelated then?

Incorrect BOM value
Error reading comment frame, skipped
Zeugma440 commented 3 years ago

So, the other complaint by FFmpeg about the comment tag is unrelated then?

Can you confirm it still happens now ? I thought it was a side effect of the 1st bug.

audiamus commented 3 years ago

Yes, still there. Chapters are there now, but this BOM message remains, with the comment skipped, in the FFmpeg analysis. VLC, MPC-HC and MediaInfo ran read the comment, though.

Zeugma440 commented 3 years ago

I just tried adding the same comment as yours on full before.mp3 and analyze it by using ffmpeg -i but I can't repro that issue on my side.

image (don't mind the truncated comment, it's truncated on display only)

The same ffmpeg command does detect a BOM issue in the comments when analyzing full after.mp3

Are you certain you're analyzing the right file ?

Zeugma440 commented 3 years ago

Just realized that my initial assumption was incorrect :

=> The only actual issue was ATL not being able to read chapters in full before.mp3

My only option is to adapt ATL so that it :

I've submitted new code that does that job properly. Please tell me if it works for you.

audiamus commented 3 years ago

I've submitted new code that does that job properly. Please tell me if it works for you.

OK , tested that.

audiamus commented 3 years ago

I think I figured out the cause of the BOM issue: ID3v2.4 vs ID3v2.3. I like to downgrade to ID3v2.3 when running on Win7 systems to be backward compatible. Win7 does not understand ID3v2.4. So the "Length" field in Windows Explorer remains blank. (My lab computer is still a Win7 machine.)

Zeugma440 commented 3 years ago

Alright, that one's on me. I was missing a BOM on the Comments description field.

As ID3v2.4 works with UTF-8 by default, and UTF-8 has no BOM, that was undetected when using ID3v2.4 default settings. On ID3v2.3, default encoding is UTF-16, and FFMpeg expected the corresponding BOM to be written.

Now it looks fine on my end. Can you confirm ? πŸ˜„

audiamus commented 3 years ago

I can indeed. Thanks a lot! :satisfied:

(I have amended the issue title with the "comment" part.)

Zeugma440 commented 3 years ago

Nice ! Thanks to you for helping me investigate these πŸ‘ πŸ‘

I'm waiting for a new issue to be clarified. I'll publish the changes once it's done !

Zeugma440 commented 3 years ago

Published in today's v3.18