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

Mutagen returning wrong file length #93

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

Mutagen returning wrong file length #93

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

Comments

@lazka
Copy link
Member

@lazka lazka commented Jul 4, 2014

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


From james.m...@sourcefabric.org on June 29, 2011 20:12:12

Hi,

The file can be downloaded here : http://www.sendspace.com/file/fzhiqi When I put the file into a banshee media player, I see the length as 1:55:46 (6946832ms). However, when I run x=mutagen.File('temp_test.mp3', easy=True) and print x.info.length, I get 6952.something seconds. It is different by 6 secs.

Is this a bug?

Thank you.

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


@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 June 29, 2011 12:44:20

The file is indeed estimated at six seconds too long, but I haven't bothered to find out why yet. Probably a missing, bad, or unsupported VBR header.

Status: Accepted

@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 james.m...@sourcefabric.org on June 30, 2011 10:46:48

Thank you for the reply.

Can you please guide me on how to spot an issue on this?
We need this so bad :(

Thank you.
@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 reiter.christoph@gmail.com on July 01, 2011 17:04:13

CBR length calculation was wrong: It used the rounded frame_length values (for finding the next frame) including the current frame's padding.

I've also removed the hard coded 144/72/48 values for calculating the frame length, since they are just frame_size / 8.

+ adjusted tests (the first two fail with the old calculation, the other two round to the same length)

regarding self.padding/self.protected(frame crc): as far as I understand it they are frame specific, so they could be removed.

Attachment: mpeg_length.patch

@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 June 30, 2011 12:29:28

The file does have some kind of LAME header, but I don't know offhand what information is in it. If you need to fix it I'd suggest starting with https://code.google.com/p/mutagen/issues/detail?id=66 which parses some parts of a LAME/Info header, and then figure out what else in there might be useful to calculate length accurately.
@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 reiter.christoph@gmail.com on July 02, 2011 02:51:37

Related: The following patch removes the code that should have prevented impossible lengths using bitrate/file size, but was never called because the arguments in seek are switched. Also the calculation is wrong, resulting in 1/64 of the real length (if it would have triggered).

Since nobody will miss it and it introduces extra IO, remove it.

Attachment: mpeg_length_remove.patch

@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 reiter.christoph@gmail.com on July 05, 2011 09:41:34

Yes, I've tested with your file. Did you apply the first patch (mpeg_length.patch)?

"MPEG 1 layer 3, 320000 bps, 44100 Hz, 6946.32 seconds"
@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 james.m...@sourcefabric.org on July 05, 2011 08:15:21

Thank you for the patch guys, however, that patch didn't fix the problem. I am still getting 6 secs longer on the file. Have you guys tested on the file that I provided?

Thank you for your support.
@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 reiter.christoph@gmail.com on July 24, 2012 12:48:17

This issue was closed by revision r119 .

Status: Fixed

@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 james.m...@sourcefabric.org on July 06, 2011 11:11:49

Ok sorry about the confusion. Your patch works great.
I have several questions to ask you.

1. What type of files or situations does this bug happen?
2. Is it possible for you guys to release a new version of mutagen with this patch?

Thanks for your great support.
@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 reiter.christoph@gmail.com on July 24, 2012 13:03:14

The simplification in the first patch wasn't correct for layer 1... use trunk.

james: CBR files with no VBRI/Xing header, depending on the file length.
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.