quodlibet / mutagen

Python module for handling audio metadata
https://mutagen.readthedocs.io
GNU General Public License v2.0
1.52k stars 159 forks source link

mid3v2: Support AIFF files #346

Open drone1 opened 6 years ago

drone1 commented 6 years ago

Info: Python 2.7..10; Grabbed mutagen from this repo as of today via pip install "git+https://github.com/quodlibet/mutagen.git" --user

Problem: If I run a simple command with my mid3v2 like so:

~/Library/Python/2.7/bin/mid3v2 -v -a "TEST" test.aif

It seems to destroy the test.aif file, making it unplayable. It doesn't find the ID3 tag – it prints this error:

No ID3 header found; creating a new tag

The file is now destroyed and will not play.

Running ~/Library/Python/2.7/bin/mutagen-inspect test.aif yields:

can't sync to MPEG frame

I had to make a few changes to this script in order to get it to write frames to my test AIFF and MP3 files. I'm sorry I do not have time to thoroughly test or submit a PR but wanted to let you know.

I made sure to test with the existing version and with my changes, by editing: ~/Library/Python/2.7/lib/python/site-packages/mutagen/_tools/mid3v2.py

In write_files(), I had to do:

try: id3 = mutagen.File(filename).tags

...rather than of:

try: id3 = mutagen.id3.ID3(filename)

Then, later in write_files() calls id3.save(filename), which works fine. I mention this because I made the same change above (from write_files()) to delete_frames(), but also had to change the id3.save() line to id3.save(filename) – and now editing and deleting frames works well for me.

Let me know if I've done something dumb here, or I can provide any additional info for you.

Thanks for building this out! Very useful.

lazka commented 6 years ago

mid3v2 can only handle "normal" id3 files, not file formats like aiff which embed id3 tags in their own file structure.

if you do mid3v2 --delete-all test.aif the "destroyed" file should work again.

drone1 commented 6 years ago

I see. Any reason not support AIFF as well?

The code above allows the underlying file to instantiate its own tags (for AIFF, a _IFFID3 instance, which works with its structure of course), as opposed to forcing an ID3 to be instantiated.

Is there a good reason not to do this?

On Fri, Jun 8, 2018 at 12:53 AM Christoph Reiter notifications@github.com wrote:

mid3v2 can only handle "normal" id3 files, not file formats like aiff which embed id3 tags in their own file structure.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/quodlibet/mutagen/issues/346#issuecomment-395646611, or mute the thread https://github.com/notifications/unsubscribe-auth/AGCCPIgI-nbiW9Wzv8DS13iPqwJc9cadks5t6gNhgaJpZM4UfCFg .

lazka commented 6 years ago

Is there a good reason not to do this?

I don't think so. Some options don't make sense like id3v1, but that's it I think.

drone1 commented 6 years ago

Hm maybe I will do thorough testing and put up a PR.

I’m not a big python master here so before I dig I figure I will ask: how can I clone the repo and run THAT mutagen’s mid3v2 rather than the one installed (to which I’ve been making my changes since I didn’t see a quick answer to this when I looked). The result I saw when I looked the other day seemed kinda hacky and there must be a non-hacky way! Just tell me what to google and I can find it?

On Fri, Jun 8, 2018 at 2:01 PM Christoph Reiter notifications@github.com wrote:

Is there a good reason not to do this?

I don't think so. Some options don't make sense like id3v1, but that's it I think.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/quodlibet/mutagen/issues/346#issuecomment-395840438, or mute the thread https://github.com/notifications/unsubscribe-auth/AGCCPCEHNfF2tPwV_IJ_njA13QD5l85Zks5t6rvHgaJpZM4UfCFg .

lazka commented 6 years ago

how can I clone the repo and run THAT mutagen’s mid3v2 rather than the one installed

PYTHONPATH=. ./tools/mid3v2

drone1 commented 6 years ago

Hm, you sure?

> PYTHONHOME=. ./tools/mid3v2
ImportError: No module named site
lazka commented 6 years ago

PYTHONPATH != PYTHONHOME

drone1 commented 6 years ago

omg, my bad. Tired. That worked!

On Sat, Jun 9, 2018 at 1:54 AM Christoph Reiter notifications@github.com wrote:

PYTHONPATH != PYTHONHOME

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/quodlibet/mutagen/issues/346#issuecomment-395942652, or mute the thread https://github.com/notifications/unsubscribe-auth/AGCCPD3cYIJKioS-lg_V1ZaLPryD_HSMks5t62L6gaJpZM4UfCFg .