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

MP3: MPEG-2/2.5 Layer III frame headers parsed incorrectly #46

Closed
lazka opened this issue Jul 4, 2014 · 5 comments
Closed

MP3: MPEG-2/2.5 Layer III frame headers parsed incorrectly #46

lazka opened this issue Jul 4, 2014 · 5 comments
Labels

Comments

@lazka
Copy link
Member

@lazka lazka commented Jul 4, 2014

Originally reported by: Christoph Reiter (Bitbucket: lazka, GitHub: lazka)


From brodie....@gmail.com on December 28, 2009 01:58:23

Given VBR MP3s made with LAME as follows:

flac -d -c silence-44-s.flac | lame --resample 24 -V9 --vbr-old - silence-44-s-mpeg2.mp3
flac -d -c silence-44-s.flac | lame --resample 12 -V9 --vbr-old - silence-44-s-mpeg25.mp3

Mutagen fails to extract correct header information from the resulting MP3:

>>> import mutagen, os
>>> a1 = mutagen.File(os.path.join('tests', 'data', 'silence-44-s-mpeg2.mp3'))
>>> a2 = mutagen.File(os.path.join('tests', 'data', 'silence-44-s-mpeg25.mp3'))
>>> vars(a1.info)
{'bitrate': 288000, 'layer': 1, 'length': 0.23799999999999999, 'mode': 1, 'padding': False, 'protected': True, 
'sample_rate': 48000, 'version': 1}
>>> vars(a2.info)
{'bitrate': 5101, 'layer': 3, 'length': 7, 'mode': 1, 'padding': False, 'protected': False, 'sample_rate': 12000, 'sketchy': 
False, 'version': 2.5}

Because it calculates the frame lengths incorrectly, it's essentially pulling out random header-like data when it can't 
find a second frame.

Going on the documentation about frame length calculation here[1], I'm able to get the headers correctly:

>>> import mutagen, os
>>> a1 = mutagen.File(os.path.join('tests', 'data', 'silence-44-s-mpeg2.mp3'))
>>> a2 = mutagen.File(os.path.join('tests', 'data', 'silence-44-s-mpeg25.mp3'))
>>> vars(a1.info)
{'bitrate': 18191, 'layer': 3, 'length': 3.7679999999999998, 'mode': 1, 'padding': False, 'protected': False, 
'sample_rate': 24000, 'sketchy': False, 'version': 2}
>>> vars(a2.info)
{'bitrate': 9300, 'layer': 3, 'length': 3.8399999999999999, 'mode': 1, 'padding': False, 'protected': False, 
'sample_rate': 12000, 'sketchy': False, 'version': 2.5}

I'm also including a prerequisite patch that fixes integer division being used to calculate MP3 lengths for VBR files.

[1] http://www.codeproject.com/KB/audio-video/mpegaudioinfo.aspx

Attachment: silence-44-s-mpeg2.mp3 silence-44-s-mpeg25.mp3 mpeg2-header.patch

Original issue: http://code.google.com/p/mutagen/issues/detail?id=46


@lazka

This comment has been minimized.

Copy link
Member Author

@lazka lazka commented Jul 4, 2014

Original comment by Christoph Reiter (Bitbucket: lazka, GitHub: lazka):


From joe.wreschnig@gmail.com on January 06, 2010 11:27:29

This issue was closed by revision r72 .

Status: Fixed
Mergedinto: -

@lazka

This comment has been minimized.

Copy link
Member Author

@lazka lazka commented Jul 4, 2014

Original comment by Christoph Reiter (Bitbucket: lazka, GitHub: lazka):


From brodie....@gmail.com on January 06, 2010 11:31:39

Thanks for merging my patch!

The MP3s seem to have been missed during the commit though.
@lazka

This comment has been minimized.

Copy link
Member Author

@lazka lazka commented Jul 4, 2014

Original comment by Christoph Reiter (Bitbucket: lazka, GitHub: lazka):


From kilian.l...@gmail.com on February 25, 2010 16:45:17

in svn 84 this still isnt working properly:
mutagen calculates the length of mp3 files _slightly_ too long. Example:

A mp3 file is shown in vlc, mplayer and gstreamer having a length of 6:09 so around
369 seconds
mutagen gives its length as  370.378 seconds - too long. Picard (which also uses
mutagen) also shows 6:10 as duration.

This occurs with every mp3 i checked, vbr and cbr.
@lazka

This comment has been minimized.

Copy link
Member Author

@lazka lazka commented Jul 4, 2014

Original comment by Christoph Reiter (Bitbucket: lazka, GitHub: lazka):


From joe.wreschnig@gmail.com on January 06, 2010 19:23:11

Sorry about that, fixed in r73 . Another reason for me to finish setting up an
automatic run-tests-on-server-on-commit script...
@lazka

This comment has been minimized.

Copy link
Member Author

@lazka lazka commented Jul 4, 2014

Original comment by Christoph Reiter (Bitbucket: lazka, GitHub: lazka):


From joe.wreschnig@gmail.com on February 25, 2010 16:58:32

If you want 100% accurate length calculation, use a sane format.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
1 participant
You can’t perform that action at this time.