MP3 song length and position calculation are off

[MLanghof]

Preface: MP3 files consist of a series of frames, each one with a 4 byte header that determines sample- and bitrate as well as mono/stereo. All of this is possibly surrounded by e.g. ID3 tags.

---

It appears that minim (or an underlying library) looks only at the very first frame header and assumes that the information found there applies to all frames in the file, and that the entire file is nothing but MP3 frames. Both of these assumptions are generally incorrect.

I am basing this on the observation that the following file has a reported length of a bit over 2 minutes (122854 milliseconds), even though its real length is 3:52. If you play it back with minim, player.position() will reach player.length() halfway through the audio and then stop increasing, even though audio is still being played for well over a minute.

https://cdn.discordapp.com/attachments/333199901406003230/574418777194627082/32.mp3

This is the content of the first MP3 header (notice how it starts at byte 247 of the file):

This is what all subsequent MP3 headers look like (frame length alternates between 104 and 105 bytes):

This file was created via ffmpeg -i 01\ Sehnsucht.mp3 -b:a 32k 32.mp3. I did not specifically craft this file to be pathological, it's what came out of this innocent encoder command.

I assume that somewhere within minim, the first MP3 header is parsed, the total file size (here 978,271 bytes) is divided by the frame length in bytes implied by that first header (here 208) and the result multiplied by 26 milliseconds (the length of each decoded MP3 frame). Indeed, doing this by hand I arrive at exactly the same incorrect song length as reported by minim: 122283.875 milliseconds.

This is wrong for multiple reasons:

• MP3 files can (and in almost all cases do) contain non-frame data, e.g. ID3 tags. This leads to the reported length being longer than the actual duration, which I would see as the cause of sound.position() never reaches sound.length() #70.

• There is no guarantee that all frames have the same length in bytes, as both bitrate and sample rate can change each frame. This allows variable bit rate encodings for MP3, but requires parsing all frame headers to determine the true song length.

• Even if all frames have identical sample- and bitrates, their lengths in bytes still occasionally differ by one. This is because the MP3 standard specifically allows an optional padding byte at the end of each frame so that the number of audio bytes per second matches the purported bitrate. While it's only a small deviation, this would still lead to noticeable errors over the length of a song (for the above song it would be about 0.5%).

Due to this, all minim operations that are based on song position in milliseconds are generally inaccurate to unusable.

As part of https://github.com/MLanghof/MP3ROR I've done the header parsing myself already and I should be able to provide a standalone small, fast and robust version of this. I'd be happy to contribute here if you tell me how/where to best integrate this into your code base.

[ddf]

Yes, you are correct about all of the above. The mp3 metadata information that Minim provides, which includes the length in milliseconds, is based on the ID3 tag parsing code from the mp3spi lib that is used for playing back mp3s. That file is MpegAudioFileReader.java. It includes some additional ID3 tag parsing that I added a while ago, like handling comment tags correctly, but is largely identical to same file in mp3spi 1.9.4.

I've just committed a change that removes inclusion of the initial header size when getting the length (I've been sitting on this change for a while and don't recall why I didn't commit it sooner). Which more or less matches the "fix" they added in mp3spi 1.9.5 (see: line 380), but as you've pointed out and I was also discovering, this is not always sufficient.

As far as I can tell, the duration calculated is an approximation by design, presumably because churning through an entire mp3 file just to determine an accurate length can be time-consuming, particularly if the file is quite long or is streaming from the internet. So while I would welcome a contribution that enabled accurate calculation of the duration of an mp3 file, I'm not sure I'd want that to be the default behavior, since that will likely lead to complaints about Minim being slow to load mp3 files, even if they are being streamed from disk instead of loaded into memory.

Also, this is not the only problem with mp3spi that I've run across, but there seem to be precious few mp3 reading libraries for Java out there, so I've stuck with the library even though it is not being actively developed because for the most part it seems to work OK. And since I'm not exactly actively developing Minim either, it seems unwise to swap out the library with something new.

If you wanted to contribute something that could calculate accurate duration for an mp3 file as well as parse all tags currently being parsed by Minim, then a way to do that could be to provide a class (or set of classes) that can accept a file path and return an AudioMetaData object. Ideally the new code would be pure Java and not rely on Javasound, Processing, or other extra libraries. This class could then be used from Minim.loadMetaData instead of calling down to the JavaSound code.