rexstout / abcde

Automatically exported from code.google.com/p/abcde
GNU General Public License v2.0
0 stars 0 forks source link

MP3s not tagged (eyeD3 problem) #101

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1. Set CDDBPROTO to 5
2. Rip a CD with accented (latin1) characters
3. See eyeD3 fail, see abcde ignore the trouble
4. End up with MP3 with no tags.

What version of the product are you using? On what operating system?

abcde v2.5.4 on Ubuntu 12.04.3 LTS with kernel 3.5.7.21

Please provide any additional information below.

Googling for the problem, I came across 
https://bugs.launchpad.net/ubuntu/+source/abcde/+bug/1180215 and I added the 
text below to the bug report. Afterwards, I thought that this might be the 
better place for it.

I'm using v2.5.4 and I came across the same problem for a particular CD. One of 
the titles is "Barcarolle op.65 n°6 (extrait du Troisème receuil de chants)".

My rather large collection is old: I started using id3v2.3 with latin1 encoded 
tags about 10 years ago. I currently request latin1 encoded cddb info from the 
server and keep on tagging new music in the same way. I have written scripts 
that extract music, rename files, etc etc. These scripts expect id3v2.3 tags 
(in latin1 of course and convert where needed).

The latest version of this (excellent!) abcde ripper uses eyeD3, which tags 
id3v2.4 in utf8. It chokes on latin1 characters. EyeD3 error message: "Uncaught 
exception: 'utf8' codec can't decode byte 0xb0 in position 45: invalid start 
byte". abcde silently ignores the error condition, leaving me with MP3s without 
tags. 

EyeD3 does not write (old) id3v1 AND id3v2.3, like the program "id3v2" does. I 
like(d) that, because old/cheap MP3 players will always understand old id3v1.

Reading the source for abcde v2.5.4, I did find "ID3TAGV=2", which switches 
abcde to eyeD3. Now, if CDDBPROTO is not set to 6, one might expect trouble. I 
feel that if eyeD3 is selected, abcde should complain when CDDBPROTO is not set 
to 6.

Setting  "ID3TAGV=1" in my ~/.abcde.conf file fixes my problem (for now) 
because that way id3v2 is used for tagging and it will happily tag id2v2.3 like 
it always did.

Original issue reported on code.google.com by acknakx...@gmail.com on 15 Oct 2013 at 3:28

GoogleCodeExporter commented 9 years ago
Looked into the code some more.

It appears (to me) that id3v2 was replaced by eyeD3, using the existing 
ID3V2OPTS (etc) variables, which confuses me. I took the liberty changing the 
meaning of ID3TAGV, so we can make backward compatible MP3s, while not breaking 
newer code. I've introduced EYED3OPTS (etc) variables to keep things separate 
from id3v2. I separated id3 as well to fix a problem with id3, which did not 
like --TPE2.

This means a few changes in abcde and abcde.conf. Please refer to the patchfile 
below.

Original comment by acknakx...@gmail.com on 16 Oct 2013 at 12:14

Attachments:

GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
Newer versions of eyeD3 have the capability of writing different versions of 
id3. I have not experimented with the following:

ID3 options:
  -1, --v1              Only read and write ID3 v1.x tags. By default, v1.x
                        tags are only read or written if there is not a v2 tag
                        in the file.
  -2, --v2              Only read/write ID3 v2.x tags. This is the default
                        unless the file only contains a v1 tag.
  --to-v1.1             Convert the file's tag to ID3 v1.1 (Or 1.0 if there is
                        no track number)
  --to-v2.3             Convert the file's tag to ID3 v2.3

but it would be interesting to see if these options assisted?

Original comment by andrew.d...@gmail.com on 27 Dec 2014 at 11:50

GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
Hi Andrew!

Happy Newyear! And thanks for investigating the problem I submitted.

I may have done a bad job explaining the problem, so let me try again:

1. eyeD3 will choke on international characters NOT in UTF-8 encoding.
Ripping a CD with tags like "Barcarolle op.65 n°6 (extrait du Troisème
receuil de chants)" (IF coded in now-ancient latin1 or iso8859-1) will
make eyeD3 terminate with an error. abcde did NOT pick up that return
value, resulting in MP3s without any tags.

2. id3v2 will tag in id3v2.3 AND id3v1; eyeD3 does not (it does EITHER,
but not both). I have an older player that does not understand id3v2,
but does read id3v1, that's why I want both.

3. I can see why abcde needed support for id3v2.4, which the program
id3v2 cannot do. I understand that eyeD3 is a good way to get id3v2.4
support. Unfortunately, eyeD3 is NOT a drop-in replacement for id3v2. I
want/need to continue the use of the id3v2 tagger program (until I
convert my huge collection).

4. I propose to set "ID3TAGV" to one of three possible values: id3v1,
id3v2.3 or id3v2.4. The taggers would be respectively id3, id3v2 and
eyeD3, and the script can compensate (through $ID3SYNTAX) for slight
differences in syntax. Once id3v2.5 or whatever comes along, we'll add
another value and maybe another tagger, WITHOUT BREAKING code for people
that have tens of thousands of tracks already tagged in a particular
way.

5. For that purpose, I wrote a patch, which is included in the bug
report. Could you please take a look at that code? I have been using it
for a while now and it works. (BTW: it fixes id3v1 support as well.)

Thanks again for taking the time,

Adriaan

Original comment by acknakx...@gmail.com on 1 Jan 2015 at 9:58

GoogleCodeExporter commented 9 years ago
I must apologise I obviously did not look at the patch too closely and now that 
I have I can appreciate that it is a nicely considered piece of work.

I am still having some issues with git access but when I get this sorted out 
would you be happy to reconfigure this patch against current git? On my 
personal git repository I have already added in Matthias's eyeD3 version switch 
in the mp3 tagging so a small amount of shuffling will be required with your 
patch (and I have spread myself somewhat thinly at the moment!).

Thanks again for your work which makes mp3 tagging a lot more logical in abcde 
:)

Original comment by andrew.d...@gmail.com on 6 Jan 2015 at 7:11

GoogleCodeExporter commented 9 years ago
Sure can do! I will have time later this week, so once I'm able to get the 
latest git version I'll redo the patch.

(Cannot test git now as I'm on an Android device)

Adriaan

Original comment by acknakx...@gmail.com on 6 Jan 2015 at 7:37

GoogleCodeExporter commented 9 years ago
Don't rush as most of the changes are on my HDD at the moment :). Still waiting 
for Steve to return from holidays to fix my access...

Original comment by andrew.d...@gmail.com on 6 Jan 2015 at 10:16

GoogleCodeExporter commented 9 years ago
OK I have committed the eyeD3 switch which is all I had planned for mp3 tagging 
so if you get a chance to rework your patch for current head I would be very 
grateful.

One question: where you have:

exit 1 # Really? No cleaning up?

perhaps you would be interested in looking at the cleaning options at the end 
of abcde:

log warning "Use \"abcde -a clean -f etc etc....

and seeing if something like this would be suitable?

Original comment by andrew.d...@gmail.com on 8 Jan 2015 at 8:46

GoogleCodeExporter commented 9 years ago
Hi Andrew,

I have redone the patch, incorporating your eyeD3 syntax switch. I've
also added a test for CDDBPROTO when using eyeD3, but I cannot make it
perfect (for cached files).

As for cleaning up - I used the 'log' routine to report to the user, but
I have not looked into cleaning up any partial directories and files
left behind before executing the 'exit 1'. I see it's used throughout
the script, so my sudden 'exit 1' does not stand out...

I ran a quick test to see all three taggers work and it seems OK (the
change is really not that big).

Regards,

Adriaan

Original comment by acknakx...@gmail.com on 9 Jan 2015 at 1:39

Attachments:

GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
Hi Adriaan,

Love the patch which I have pushed:

git commit c9c509700af37bc2680bc2ca281f530ddd123b9c

Thanks very much for your work on this, please let me know if there are any 
improvements or changes needed here.

Andrew

Original comment by andrew.d...@gmail.com on 10 Jan 2015 at 8:22