filetype icon indicating copy to clipboard operation
filetype copied to clipboard

Some types of MP3 are not matched

Open maugre opened this issue 9 years ago • 5 comments

Hi,

I have a couple of .mp3 files which are not matched in my testing. It seems the magic bytes can change depending on channels and bitrates of the files. For example, one mono .mp3 has 0xFFFA90 and a stereo file has 0xFFFB90. Those that start 0x494433 are correctly matched.

It seems numerous examples can be seen here: https://github.com/mirror/jdownloader/blob/master/src/org/jdownloader/extensions/extraction/mime.type

I will shortly attempt to fix this in my program and provide a patch in due course.

Is adding to the 'if' statement the preferred way?

maugre avatar Jul 13 '16 17:07 maugre

Thanks for reporting this. What you mention makes absolute sense. The prefered way is using or logical operator.

h2non avatar Jul 13 '16 17:07 h2non

Seeing how it's done in the matcher functions with matching against only the 2 bytes, I've changed mine to this:

Mp3(buf []byte) bool { return len(buf) > 2 && ((buf[0] == 0x49 && buf[1] == 0x44 && buf[2] == 0x33) || (buf[0] == 0xFF && buf[1] == 0xfa) || (buf[0] == 0xFF && buf[1] == 0xfb)) }

The line with 0xfa is added with no other changes. That now works for me on the files that were previously failing.

Thanks!

maugre avatar Jul 13 '16 18:07 maugre

There may be more possibilities for that second byte. See spec here: http://www.mpgedit.org/mpgedit/mpeg_format/mpeghdr.htm

Edit: I've been looking into this further. We only want MPEG-1 Layer III for MP3 audio, which leaves 0xFA and 0xFB depending on whether the 'protection' bit is set, so this is sufficient. Layer II would be MP2 files so another test.

maugre avatar Jul 13 '16 18:07 maugre

I modified my local matcher function in this way to check the first bit only once:

func Mp3(buf []byte) bool { return len(buf) > 2 && ((buf[0] == 0x49 && buf[1] == 0x44 && buf[2] == 0x33) || // ID3v2 // Final bit (has crc32) may be or may not be set. (buf[0] == 0xFF && (buf[1] == 0xFA || buf[1] == 0xFB))) }

Would that be preferable?

maugre avatar Jul 14 '16 11:07 maugre

I would suggest the same. One check is perfect.

h2non avatar Jul 14 '16 22:07 h2non