Skip to content
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

Some types of MP3 are not matched #10

Open
maugre opened this issue Jul 13, 2016 · 5 comments
Open

Some types of MP3 are not matched #10

maugre opened this issue Jul 13, 2016 · 5 comments
Labels

Comments

@maugre
Copy link

maugre commented Jul 13, 2016

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?

@h2non
Copy link
Owner

h2non commented Jul 13, 2016

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

@h2non h2non added the bug label Jul 13, 2016
@maugre
Copy link
Author

maugre commented Jul 13, 2016

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
Copy link
Author

maugre commented Jul 13, 2016

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
Copy link
Author

maugre commented Jul 14, 2016

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?

@h2non
Copy link
Owner

h2non commented Jul 14, 2016

I would suggest the same. One check is perfect.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants