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

Wrong mp3 length #182

Closed
lazka opened this issue Aug 24, 2014 · 5 comments
Closed

Wrong mp3 length #182

lazka opened this issue Aug 24, 2014 · 5 comments
Labels
bug

Comments

@lazka
Copy link
Member

@lazka lazka commented Aug 24, 2014

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


For the following file mutagen finds a xing header which contains garbage. Maybe we should be more picky about the xing position.


@lazka

This comment has been minimized.

Copy link
Member Author

@lazka lazka commented Jul 30, 2015

Original comment by ekulyk (Bitbucket: ekulyk, GitHub: ekulyk):


Hi Christoph,

I've recently encountered the wrong MP3 length being reported when a xing header is present. Could it be that we should be looking for the "Info" string to signify the start of the xing header as well as the "Xing" string, whichever comes first? I suppose the check would be added here.

It seems from this post that the Xing header ID can be either 'Xing' or 'Info'. If we look at the file you've attached, we can see the 'Info' identifier occurs nearer to the start of the file than the 'Xing' string and appears to have the correct information.

Checking for both 'Info' and 'Xing', whichever appears first, should solve this issue without having to be picky about the 'Xing' position.

@lazka

This comment has been minimized.

Copy link
Member Author

@lazka lazka commented Jul 30, 2015

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


Thanks for the additional info. Do you have any additional affected files by any chance and can send one to me?

(btw, did you file this quodlibet/quodlibet#1655 ?)

@lazka

This comment has been minimized.

Copy link
Member Author

@lazka lazka commented Jul 30, 2015

Original comment by ekulyk (Bitbucket: ekulyk, GitHub: ekulyk):


I did not open that other ticket.

The file belongs to a client so I am not allowed to share it, unfortunately. If you want me to test any patches, I can definitely do that.

Do you know if there are any official specs for this information? The best resources I've seemed to find are the link in the previous comment and this one, which states:

In the Info Tag, the "Xing" identification string (mostly at 0x24) of the header is replaced by "Info" in case of a CBR file.
This was done to avoid CBR files to be recognized as traditional Xing VBR files by some decoders. Although the two identification strings "Xing" and "Info" are both valid, it is suggested that you keep the identification string "Xing" in case of VBR bistream in order to keep compatibility

I'm unfamiliar with the MP3 spec, but is the xing header always expected to be in the same location in the file, or can it be anywhere within the first 32764 bytes as the mutagen code suggests?

@lazka

This comment has been minimized.

Copy link
Member Author

@lazka lazka commented Aug 4, 2015

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


mp3: rework Xing/VBRI header parsing to be more strict; parse LAME Xing headers; extract the bitrate info from VBRI headers. (Fixes issue # 182)

Instead of scanning the file from the guessed offset and searching
for the header identifier only look at the correct location based
on the mpeg header location and content. This reduces the risk that
we interpret random data as a xing header.

While at it also read lame Xing headers which start with "Info" but are
compatible besides that and extract the real bitrate info from VBRI
headers.

@lazka

This comment has been minimized.

Copy link
Member Author

@lazka lazka commented Aug 23, 2015

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


A new release is out..

@lazka lazka added minor bug labels Apr 7, 2016
@lazka lazka closed this Apr 7, 2016
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.