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

FLAC: Raptor Audio encodes Vorbis comments with bad FLAC block sizes but correct Vorbis field sizes #52

Closed
lazka opened this issue Jul 4, 2014 · 9 comments
Labels

Comments

@lazka
Copy link
Member

@lazka lazka commented Jul 4, 2014

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


From reiter.christoph@gmail.com on January 10, 2010 23:29:19

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/lib/python2.6/dist-packages/mutagen/__init__.py", line 214, in
File
    if score > 0: return Kind(filename)
  File "/usr/lib/python2.6/dist-packages/mutagen/__init__.py", line 73, in

init
self.load(filename, _args, *_kwargs)
File "/usr/lib/python2.6/dist-packages/mutagen/flac.py", line 584, in load
while self.read_metadata_block(fileobj): pass
File "/usr/lib/python2.6/dist-packages/mutagen/flac.py", line 532, in
__read_metadata_block
block = self.METADATA_BLOCKSbyte & 0x7F
File "/usr/lib/python2.6/dist-packages/mutagen/_vorbis.py", line 68, in
__init

self.load(data, _args, *_kwargs)
File "/usr/lib/python2.6/dist-packages/mutagen/flac.py", line 248, in load
super(VCFLACDict, self).load(data, errors=errors, framing=framing)
File "/usr/lib/python2.6/dist-packages/mutagen/_vorbis.py", line 108, in load
raise error("file is not a valid Vorbis comment")
mutagen._vorbis.error: file is not a valid Vorbis comment

Attached is the start of the file.

The file comes from: http://ubuntuforums.org/showthread.php?p=8618319 Summary: He ripped lots of music, some of the files fail to import in
Rhythmbox, QL and Exaile but the tags show up in nautilus properties.

Attachment: invalid_vorbis_comment.flac

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


@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 nondicti...@gmail.com on January 10, 2010 15:19:56

The file is corrupt.

Metadata block #1 (as reported by metaflac) has a length of 48 bytes which I assure
you is too small to encapsulate the vorbis comment block that is contained
nevertheless intact within the file.

I managed to fix the file using a hex editor and writing at address 2D(hex) the value
AF(hex) for 175 bytes, the actual payload size.

I was able to successfully read the metadata using mutagen afterwards.

Fixed file is attached.

Attachment: invalid_vorbis_comment-fixed.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 January 10, 2010 17:09:40

libFLAC's support is not intentional AFAICT. It just slurps data linearly, so
although it reads 48 bytes for the Vorbis comment, it throws that out and assumes
that parsing the Vorbis comment itself will give it the correct size. I checked in a
change to make Mutagen do a synch flag check at what it thinks is the start of the
audio data, which should allow us to back-patch the correct block size after parsing,
once I figure out if there's an API-compatible way to route the original fileobj
through all the MetadataBlock constructors.

Summary: FLAC: Raptor Audio encodes Vorbis comments with bad FLAC block sizes but correct Vorbis field sizes

@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 10, 2010 16:14:32

The FLAC file is absolute garbage; the Vorbis metadata block claims it's 48 bytes
long, which is just wrong. If it was 36 bytes I'd say it wrote out the reference
string header length + reference string + 0 frame count and that's how it got 36, and
then didn't make the tag larger when it added the actual TITLE etc. tags. But it's
48, which doesn't make any sense.

I could probably hack up something that makes sure the FLAC block size matched the
Vorbis comment size (it's really easy in this case - 48 bytes is obviously not enough
to contain 4 byte length + 32 byte vendor + 4 bytes count + min. 6 * 5 bytes tags),
but while it's easy to detect, I'm not sure I want to try to work around it, because
it's harder to fix than to detect.

I'll look at the source for libFLAC, because it seems to have some kind of workaround
for this. Which is probably why EasyTag can load it.
@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 18, 2010 04:27:44

After poking at this for several hours I'm very reluctant to try to code a workaround
into the FLAC loader in Mutagen. I've never seen this bug before and the code would
be tricky given the way the FLAC loader is currently written. A rewrite of the FLAC
metadata loader could support it but until/if we make mutagen.flac2 this is out of
the question, I think.

The good news is that it should be easy to write a standalone script that knows how
to fixup these files just by looking for the FLAC comment header and scanning the
Vorbis comment size, no Mutagen involved.
@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 March 21, 2010 08:58:56

And line_66.flac is again one of those 48 bytes files.
@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 March 21, 2010 08:46:33

Here is another one: https://code.google.com/p/quodlibet/issues/detail?id=465#c5 (line_65.flac)

This one fails now because of the "End of metadata did not start audio" check.
The last block is a padding with a valid length but with garbage after it.

And I would guess that libFLAC ignores last blocks which are padding.
@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 May 24, 2011 09:44:57

This issue was closed by revision r108 .

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 joe.wreschnig@gmail.com on May 19, 2011 06:20:57

I have a mostly-working fix for this, but I think it's getting worse. Someone on IRC showed me this file yesterday, which looks like it was a not-really-48-byte file, and then something tried to tag it as if it was, leaving garbage data after the last metadata block.

ogg123 will still play them, though, so we'll try to parse them. But we can't fix them without risking intruding on the audio data.

Status: Accepted

Attachment: overwrite.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 August 29, 2012 08:58:39

Issue 121 has been merged into this issue.
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.