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

WAV corrupted when tagging with foobar2k #392

Closed
maybeRainH opened this issue Aug 12, 2019 · 8 comments
Closed

WAV corrupted when tagging with foobar2k #392

maybeRainH opened this issue Aug 12, 2019 · 8 comments

Comments

@maybeRainH
Copy link

maybeRainH commented Aug 12, 2019

branch : feature/riff-wave-v2
here is the special case:
start with a wav empty file without LIST INFO chunk and ID3 chunk
--> add album and title info using mutagen(a ID3 chunk is added)
--> add artist info using foobar2k ( the artist info is added to ID3 chunk and foobar2k will also
create a LIST INFO chunk using all info in ID3)
--> delete ID3 using mutagen ( left a empty ID3 chunk with 1034 bytes, LIST INFO not deleted)
--> tagging with foobar2000 again.
--> a corrupted file(can be played by foobar2k but can't be tagged using foobar2k saying
a corrupted wav file)
BUT still everything works fine using mutagen. It can recognize ID3 and can change the existed metadata.

here is the corrupted file: ↓
test.zip

@phw
Copy link
Collaborator

phw commented Oct 15, 2019

I can reproduce this with the latest code from #321. I will take a look.

@phw
Copy link
Collaborator

phw commented Oct 15, 2019

In my tests I don't get any leftover ID3 tag in the file after removing the tags again (see attached file). This is different from what you got with your file. Could you retry if you still get this leftover empty ID3 tag?

After removing the tags the LIST INFO chunk is kept, but other software (Media Monkey, MusicBee) has no trouble reading it. But foobar2000 shows this file corrupted error on saving tags. Also it does not read the LIST INFO data.

Media Monkey produces a INFO LIST chunk only file which foobar2000 has no problem (it will read it and happily write again, adding ID3 in the process).

For reference here are the files I generated:

  • test-id3-deleted.wav: This file was generated according to the procedure @maybeRainH described. Contains a LIST INFO chunk and no ID3, cannot be written by foobar2000.
  • test-no-id3-mm-info.wav: This was the same base file, but tags were added with Media Monkey. Contains a LIST INFO chunk and no ID3, can be written by foobar2000.

test-wave.zip

This is the code I used to add tags to a tagless WAVE file:

import mutagen

f = mutagen.File('test.wav')
f.add_tags()

f.tags.add(mutagen.id3.TIT2(encoding=3, text=['The Title']))
f.tags.add(mutagen.id3.TALB(encoding=3, text=['The Album']))

f.save()

This is the code I used to delete the ID3 tags again:

import mutagen
mutagen.wave.delete('test.wav')

@maybeRainH
Copy link
Author

maybeRainH commented Oct 17, 2019

In my tests I don't get any leftover ID3 tag in the file after removing the tags again (see attached file). This is different from what you got with your file. Could you retry if you still get this leftover empty ID3 tag?

After removing the tags the LIST INFO chunk is kept, but other software (Media Monkey, MusicBee) has no trouble reading it. But foobar2000 shows this file corrupted error on saving tags. Also it does not read the LIST INFO data.

Media Monkey produces a INFO LIST chunk only file which foobar2000 has no problem (it will read it and happily write again, adding ID3 in the process).

For reference here are the files I generated:

  • test-id3-deleted.wav: This file was generated according to the procedure @maybeRainH described. Contains a LIST INFO chunk and no ID3, cannot be written by foobar2000.
  • test-no-id3-mm-info.wav: This was the same base file, but tags were added with Media Monkey. Contains a LIST INFO chunk and no ID3, can be written by foobar2000.

test-wave.zip

This is the code I used to add tags to a tagless WAVE file:

import mutagen

f = mutagen.File('test.wav')
f.add_tags()

f.tags.add(mutagen.id3.TIT2(encoding=3, text=['The Title']))
f.tags.add(mutagen.id3.TALB(encoding=3, text=['The Album']))

f.save()

This is the code I used to delete the ID3 tags again:

import mutagen
mutagen.wave.delete('test.wav')

I tried another file and this time no leftover ID3 tag. maybe just forget about it. But still ended up corrupted. Base on your test. I guess it's a foobar2000's issue?

@phw
Copy link
Collaborator

phw commented Oct 19, 2019

I guess it's a foobar2000's issue?

I am not sure about this, actually this is also something I thought, too. But my tests above show that it is possible to have a file with only the INFO chunk and no ID3 which actually works.

Over the weekend I will try to find out more what the difference is between the two files and see where the issue is.

@phw
Copy link
Collaborator

phw commented Oct 19, 2019

I looked at the files in detail, this is a f2k issue. I can reproduce the basic behavior without using the mutagen implementation first. Just tagging a WAVE file without tags in f2k results in a file where the id3 chunk has a wrong length (1 byte less than what got written, resulting in one stray byte at the end). When you then remove the id3 chunk by clearing exactly the length the chunk indicates this stray byte stays around. But if it is directly after the LIST chunk f2k gets confused by this.

I reported it at https://hydrogenaud.io/index.php?topic=118325.0 , there are some more details and example files there.

@phw
Copy link
Collaborator

phw commented Oct 19, 2019

Turns out the RIFF code here does not handle possible padding added to chunks properly. I will take a look to fix this.

@phw
Copy link
Collaborator

phw commented Oct 20, 2019

The AIFF code is affected by the general issues as well:

  • When adding chunks the current code does not in general take care of adding the padding bytes. It does take care to have even sizes for ID3 tags added, though. So as long as you only use mutagen to add and remove tags it works.
  • Deleting chunks does not consider the padding bytes, leaving the stray bytes behind if a chunk with padding gets deleted
  • Adding / removing /resizing frames does not updated the in memory stored offsets for loaded chunks. To safely add or remove chunks you can only do one add / remove operation at a time and the re-load the modified file

My plan currently is to submit a patch for these issues for the existing AIFF code, then after this has been reviewed adapt this patch for the RIFF code as well. I have fixes for the first two points ready, but fixing the third issue requires some more refactoring in order to be able to updated affected chunks on changes.

@phw
Copy link
Collaborator

phw commented Oct 21, 2019

The fixes for the first two issues are at the following branches:

The basic issues of reading / writing / deleting the padding byte, so basic usage now should work.

What this does not yet address is doing multiple chunk operations without reloading in between. E.g. something like this is currently not save to do and will damage the file:

f = RiffFile(somepath)
f.insert_chunk('TST1')
f['TST1'].resize(200)
f.insert_chunk('TST2')  # Will be inserted at wrong offset, not considering the previously inserted chunk size
f.delete_chunk('TST1')  # Will remove bytes, but not update the offsets

This is the same with AIFF. As I wrote above I will refactor the code to address this.

phw added a commit to phw/mutagen that referenced this issue Oct 22, 2019
This allows to safely peform mutlitple chunk insert / delete / resize operations on loaded RIFF files.

See quodlibet#392
phw added a commit to phw/mutagen that referenced this issue Oct 22, 2019
This allows to safely peform mutlitple chunk insert / delete / resize operations on loaded RIFF files.

See quodlibet#392
phw added a commit to phw/mutagen that referenced this issue Oct 22, 2019
This allows to safely perform multiple chunk insert / delete / resize operations on loaded RIFF files.

See quodlibet#392
phw added a commit to phw/mutagen that referenced this issue Oct 22, 2019
This allows to safely perform multiple chunk insert / delete / resize operations on loaded RIFF files.

See quodlibet#392
phw added a commit to phw/mutagen that referenced this issue Oct 22, 2019
This allows to safely perform multiple chunk insert / delete / resize operations on loaded RIFF files.

See quodlibet#392
phw added a commit to phw/mutagen that referenced this issue Oct 22, 2019
This allows to safely perform multiple chunk insert / delete / resize operations on loaded RIFF files.

See quodlibet#392
phw added a commit to phw/mutagen that referenced this issue Oct 23, 2019
This allows to safely perform multiple chunk insert / delete / resize operations on loaded RIFF files.

See quodlibet#392
phw added a commit to phw/mutagen that referenced this issue Oct 23, 2019
This allows to safely perform multiple chunk insert / delete / resize operations on loaded RIFF files.

See quodlibet#392
phw added a commit to phw/mutagen that referenced this issue Oct 23, 2019
This allows to safely perform multiple chunk insert / delete / resize operations on loaded RIFF files.

See quodlibet#392
phw added a commit to phw/mutagen that referenced this issue Oct 23, 2019
This allows to safely perform multiple chunk insert / delete / resize operations on loaded RIFF files.

See quodlibet#392
phw added a commit to phw/mutagen that referenced this issue Nov 15, 2019
This allows to safely perform multiple chunk insert / delete / resize operations on loaded RIFF files.

See quodlibet#392
phw added a commit to phw/mutagen that referenced this issue Nov 18, 2019
This allows to safely perform multiple chunk insert / delete / resize operations on loaded RIFF files.

See quodlibet#392
phw added a commit to phw/mutagen that referenced this issue Jan 3, 2020
This allows to safely perform multiple chunk insert / delete / resize operations on loaded RIFF files.

See quodlibet#392
phw added a commit to phw/mutagen that referenced this issue Feb 10, 2020
This allows to safely perform multiple chunk insert / delete / resize operations on loaded RIFF files.

See quodlibet#392
phw added a commit to phw/mutagen that referenced this issue Mar 13, 2020
This allows to safely perform multiple chunk insert / delete / resize operations on loaded RIFF files.

See quodlibet#392
phw added a commit to phw/mutagen that referenced this issue May 3, 2020
This allows to safely perform multiple chunk insert / delete / resize operations on loaded RIFF files.

See quodlibet#392
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants