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

Lots of FLAC-files can't be loaded #106

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

Lots of FLAC-files can't be loaded #106

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

Comments

@lazka
Copy link
Member

@lazka lazka commented Jul 4, 2014

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


From themineo on February 07, 2012 10:40:39

I'm having trouble loading some FLAC files with mutagen: http://paste.pocoo.org/show/547192/ shows the trace.

The file in question is part of http://blocsonic.com/releases/show/netbloc-vol-36-get-dusted-illness-from-the-dusted-wax-stacks (all other files are affected), http://blocsonic.com/releases/show/netbloc-vol-37-five is affected too, http://blocsonic.com/releases/show/netbloc-vol-35-occupy-music is not.

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


@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 February 07, 2012 02:37:06

first 1.5MB of the file in question

also fails with trunk

Attachment: issue 106 - I Got The Blues.flac

@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 21, 2012 05:56:35

That seems like a reasonable approach.
@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 February 21, 2012 04:45:17

There are lots of places where fileobj.read(x) isn't checked or will silently fail (to_int_be) and lead to even more corrupt files after saving (StreamInfo for example)

What about wrapping the fileobj and raise if the read size doesn't match?
@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 April 09, 2012 09:41:56

- wrap the file object and check returned read size
 - add _distrust_size attribute to MetadataBlock (True for Picture and VCFLACDict) and leave reading to the block if True.

because I didn't write it above: metaflac/libflac also ignores the block size for the Picture block.

tests:
 - 106-short-picture-block-size.flac
   zero size picture block with valid picture
 - 106-invalid-streaminfo.flac
   too short stream info block (missing the checksum), to test read check

Attachment: 106-flac-ignore-size-v1.patch 106-invalid-streaminfo.flac 106-short-picture-block-size.flac

@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 February 21, 2012 16:59:01

Last block has zero size but includes the valid picture block if the size is ignored. 

Offending software (according to the blocsonic guy): Amadeus Pro http://www.hairersoft.com/pro.html original: gstreamer fails, vlc works but no image, easytag shows tags/image
save with writing back invalid block: gstreamer fails, vlc still works, easytag shows only tags
save with ignoring size: gstreamer ok, vlc ok, easytag shows tags/image

So I would suggest to ignore the size for all blocks that have their own size info. Writing back the invalid blocks leaves 1MB of image garbage in there and as can be seen above at least gstreamer doesn't find the audio header, others that work with the original file might fail after that as well.
@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 25, 2012 04:42:59

This issue was closed by revision r124 .

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 reiter.christoph@gmail.com on July 25, 2012 04:44:57

Commited: The above patch unchanged but split up in object-wrapper and picture-block part.
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.