Created attachment 2067 [details]
This file crashes the scanner

Note that this file contains more tags than in my bug report; I had managed to remove some of them without making the problem go away.  I've left them as they were to show the difference between this file and the one that works.

Created attachment 2068 [details]
This file does not cause a crash for me

I'll take a look.

I believe that having more then one ALBUMARTIST tag per ALBUM may be the culprit.  I do not think SlimServer supports more then one ALBUMARTIST per ALBUM at this time.  SlimServer should fail gracefully if it encounters more then one ALBUMARTIST tag per ALBUM however.

(In reply to comment #4)
> I believe that having more then one ALBUMARTIST tag per ALBUM may be the
> culprit.  I do not think SlimServer supports more then one ALBUMARTIST per
> ALBUM at this time.  SlimServer should fail gracefully if it encounters more
> then one ALBUMARTIST tag per ALBUM however.
> 

I've tested some Ogg Vorbis files which have multiple ALBUMARTIST tags, and they work fine -- it's been a while since I checked though, so I can't say for sure whether the second tag in each case was available for searching etc.  I've tried scanning one of those albums, and I get no problems.

Having multiple ALBUMARTIST tags is the only way to represent certain metadata (for example, "Ballad Of The Broken Seas" is an album by Isobel Campbell and Mark Lanegan, so they get one ALBUMARTIST tag each).  If it's possible to add support for this (while fixing this problem), that would also be great. :-)

I should also note that this problem began after upgrading SlimServer from 6.5.2 to 6.5.3, so that might be an indication of what changed to make this happen.

I think change 12192 for bug #4694 may have caused this issue.  With further investigation I believe the crash or odd behavior is a result of having more then one ALBUMARTIST tag in a single file in addition to having either an ARTIST, BAND or COMPOSER tag with the same value.  I updated the summary to reflect this.

(In reply to comment #6)
> I think change 12192 for bug #4694 may have caused this issue.  With further
> investigation I believe the crash or odd behavior is a result of having more
> then one ALBUMARTIST tag in a single file in addition to having either an
> ARTIST, BAND or COMPOSER tag with the same value.  I updated the summary to
> reflect this.
> 

Thanks for the update.  Do you know why this fails for some files, while succeeding for other files with almost identical tags?  The two files I uploaded have the same ALBUMARTIST and COMPOSER tags, but only one fails.

This is looking like a fairly serious bug, with more posts about it in the forums.

http://forums.slimdevices.com/showthread.php?t=36004
http://forums.slimdevices.com/showthread.php?t=36789
http://forums.slimdevices.com/showthread.php?t=36949

Can anyone tell what the actual SQL bug is?  It says it's a syntax error, but that's hard to see because of the placeholder/prepared statement and the lack of single quotes shown in the log.

> 2007-07-14 14:48:00.4778 ERROR: Error executing 'INSERT INTO contributors
>(musicbrainz_id, name, namesearch, namesort) VALUES (?, Jon Kaplan, Jon
> Kaplan, Jon Kaplan)': DBD::mysql::st execute failed: You have an error in
> your SQL syntax; check the manual that corresponds to your MySQL server
> version for the right syntax to use near 'Kaplan, Jon Kaplan, Jon Kaplan)'
> at line 1 at /usr/share/perl5/DBIx/Class/Storage/DBI.pm line 773.

Is anyone able to see a relevant difference between the two files I've attached?  Can anyone replicate the behaviour that one file crashes the scanner but the other doesn't?  I've compared the tags in fb2k, and as far as I can see they're the same apart from the title, track number and individual ARTIST tags (but ALBUMARTIST and COMPOSER are the same).  I tagged them both at the same time using fb2k (many months ago; this crash has only started with 6.5.3) so they ought to be the same.

Is there a tool I can use to look at the tags at a lower level than fb2k's interpretation?  To see anything weird that fb2k might have missed?

(In reply to comment #9)
> Is anyone able to see a relevant difference between the two files I've
> attached?  Can anyone replicate the behaviour that one file crashes the scanner
> but the other doesn't?  I've compared the tags in fb2k, and as far as I can see
> they're the same apart from the title, track number and individual ARTIST tags
> (but ALBUMARTIST and COMPOSER are the same).  I tagged them both at the same
> time using fb2k (many months ago; this crash has only started with 6.5.3) so
> they ought to be the same.
> 
> Is there a tool I can use to look at the tags at a lower level than fb2k's
> interpretation?  To see anything weird that fb2k might have missed?
> 

They both crash in my setup (on Linux).
I also noted that an INSERT that works seems to be logged like this:
INSERT INTO contributors (musicbrainz_id, name, namesearch, namesort) VALUES (?, ?, ?, ?): 'NULL', 'Al Kaplan', 'AL KAPLAN', 'AL KAPLAN'
And the one that doesn't work is logged like this:
INSERT INTO contributors (musicbrainz_id, name, namesearch, namesort) VALUES (?, Dr Lecter, Dr Lecter, Dr Lecter): 'NULL'

The reason for the syntax error is probably because it says: Dr Lecter 
Instead of: 'Dr Lecter'

I think the crash happens at line 132 in the find_or_create call in Contributor.pm if no one has realised this yet.

Below is the output from id3v2 on Linux for both files.
===========================================================================
id3v2 tag info for /mnt/mp3music_small/special/crash/04 - Quid Pro Quo.mp3:
TRCK (Track number/Position in set): 04
TIT2 (Title/songname/content description): Quid Pro Quo
TPE1 (Lead performer(s)/Soloist(s)): Clarice
TPE1 (Lead performer(s)/Soloist(s)): Dr Lecter
TYER (Year): 2003
COMM (Comments): ()[]: http://www.silencethemusical.com/
TCOM (Composer): Jon Kaplan
TCOM (Composer): Al Kaplan
TCON (Content type): Unknown (255)
TXXX (User defined text information): (ALBUMARTIST): Jon Kaplan
TXXX (User defined text information): (ALBUMARTIST): Al Kaplan
TALB (Album/Movie/Show title): Silence! The Musical
TXXX (User defined text information): (TRACKTOTAL): 9
TXXX (User defined text information): (SOURCEMEDIUM): MP3
TXXX (User defined text information): (SOURCEFORMAT): Album
TXXX (User defined text information): (ALBUMYEAR): 2003
TXXX (User defined text information): (replaygain_track_gain): -3.58 dB
TXXX (User defined text information): (replaygain_track_peak): 0.920881
TXXX (User defined text information): (replaygain_album_gain): -4.36 dB
TXXX (User defined text information): (replaygain_album_peak): 1.083855

id3v2 tag info for /mnt/mp3music_small/special/crash/05 - It's Me!.mp3:
TRCK (Track number/Position in set): 05
TIT2 (Title/songname/content description): It's Me!
TPE1 (Lead performer(s)/Soloist(s)): Dr Lecter
TPE1 (Lead performer(s)/Soloist(s)): The Police
TYER (Year): 2003
COMM (Comments): ()[]: http://www.silencethemusical.com/
TCOM (Composer): Jon Kaplan
TCOM (Composer): Al Kaplan
TCON (Content type): Unknown (255)
TXXX (User defined text information): (ALBUMARTIST): Jon Kaplan
TXXX (User defined text information): (ALBUMARTIST): Al Kaplan
TALB (Album/Movie/Show title): Silence! The Musical
TXXX (User defined text information): (TRACKTOTAL): 9
TXXX (User defined text information): (SOURCEMEDIUM): MP3
TXXX (User defined text information): (SOURCEFORMAT): Album
TXXX (User defined text information): (ALBUMYEAR): 2003
TXXX (User defined text information): (replaygain_track_gain): -2.79 dB
TXXX (User defined text information): (replaygain_track_peak): 0.945196
TXXX (User defined text information): (replaygain_album_gain): -4.36 dB
TXXX (User defined text information): (replaygain_album_peak): 1.083855
========================================================================

The output with --d_sql logging enabled with only the "04" track:
=================================================================
2007-07-20 11:45:59.4275 SELECT me.id, me.name, me.namesort, me.musicmagic_mixable, me.namesearch, me.musicbrainz_id FROM contributors me WHERE ( ( me.namesearch = ? ) ): 'AL KAPLAN'
2007-07-20 11:45:59.4290 INSERT INTO contributors (musicbrainz_id, name, namesearch, namesort) VALUES (?, ?, ?, ?): 'NULL', 'Al Kaplan', 'AL KAPLAN', 'AL KAPLAN'
2007-07-20 11:45:59.4307 SELECT me.role, me.contributor, me.track FROM contributor_track me WHERE ( ( me.contributor = ? AND me.role = ? AND me.track = ? ) OR ( me.contributor = ? AND me.role = ? AND me.track = ? ) ): '49', '5', '87', '49', '5', '87'
2007-07-20 11:45:59.4318 INSERT INTO contributor_track (contributor, role, track) VALUES (?, ?, ?): '49', '5', '87'
2007-07-20 11:45:59.4660 SELECT me.id, me.name, me.namesort, me.musicmagic_mixable, me.namesearch, me.musicbrainz_id FROM contributors me WHERE ( ( me.namesearch = ? ) ): 'JON KAPLAN'
2007-07-20 11:45:59.4680 INSERT INTO contributors (musicbrainz_id, name, namesearch, namesort) VALUES (?, ?, ?, ?): 'NULL', 'Jon Kaplan', 'JON KAPLAN', 'JON KAPLAN'
2007-07-20 11:45:59.5018 SELECT me.role, me.contributor, me.track FROM contributor_track me WHERE ( ( me.contributor = ? AND me.role = ? AND me.track = ? ) OR ( me.contributor = ? AND me.role = ? AND me.track = ? ) ): '50', '2', '87', '50', '2', '87'
2007-07-20 11:45:59.5031 INSERT INTO contributor_track (contributor, role, track) VALUES (?, ?, ?): '50', '2', '87'
2007-07-20 11:45:59.5081 SELECT me.id, me.name, me.namesort, me.musicmagic_mixable, me.namesearch, me.musicbrainz_id FROM contributors me WHERE ( ( ( ( me.namesearch = ? ) ) ) ): 'Al Kaplan'
2007-07-20 11:45:59.5104 SELECT me.role, me.contributor, me.track FROM contributor_track me WHERE ( ( me.contributor = ? AND me.role = ? AND me.track = ? ) OR ( me.contributor = ? AND me.role = ? AND me.track = ? ) ): '49', '2', '87', '49', '2', '87'
2007-07-20 11:45:59.5116 INSERT INTO contributor_track (contributor, role, track) VALUES (?, ?, ?): '49', '2', '87'
2007-07-20 11:45:59.5159 SELECT me.id, me.name, me.namesort, me.musicmagic_mixable, me.namesearch, me.musicbrainz_id FROM contributors me WHERE ( ( me.namesearch = ? ) ): 'CLARICE'
2007-07-20 11:45:59.5201 INSERT INTO contributors (musicbrainz_id, name, namesearch, namesort) VALUES (?, ?, ?, ?): 'NULL', 'Clarice', 'CLARICE', 'CLARICE'
2007-07-20 11:45:59.5226 SELECT me.role, me.contributor, me.track FROM contributor_track me WHERE ( ( me.contributor = ? AND me.role = ? AND me.track = ? ) OR ( me.contributor = ? AND me.role = ? AND me.track = ? ) ): '51', '6', '87', '51', '6', '87'
2007-07-20 11:45:59.5240 INSERT INTO contributor_track (contributor, role, track) VALUES (?, ?, ?): '51', '6', '87'
2007-07-20 11:45:59.5256 SELECT me.id, me.name, me.namesort, me.musicmagic_mixable, me.namesearch, me.musicbrainz_id FROM contributors me WHERE ( ( ( ( me.namesearch = ? ) ) ) ): 'Dr Lecter'
2007-07-20 11:45:59.5602 INSERT INTO contributors (musicbrainz_id, name, namesearch, namesort) VALUES (?, Dr Lecter, Dr Lecter, Dr Lecter): 'NULL'
2007-07-20 11:45:59.5612 ERROR: Error executing 'INSERT INTO contributors (musicbrainz_id, name, namesearch, namesort) VALUES (?, Dr Lecter, Dr Lecter, Dr Lecter)': DBD::mysql::st execute failed: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'Lecter, Dr Lecter, Dr Lecter)' at line 1 at /opt/SlimServer_v6.5.3/CPAN/DBIx/Class/Storage/DBI.pm line 771.
========================================================================

The logging with --d_sql enabled and only the "05" track
========================================================================
2007-07-20 11:54:32.2044 SELECT me.id, me.name, me.namesort, me.musicmagic_mixable, me.namesearch, me.musicbrainz_id FROM contributors me WHERE ( ( me.namesearch = ? ) ): 'AL KAPLAN'
2007-07-20 11:54:32.2063 INSERT INTO contributors (musicbrainz_id, name, namesearch, namesort) VALUES (?, ?, ?, ?): 'NULL', 'Al Kaplan', 'AL KAPLAN', 'AL KAPLAN'
2007-07-20 11:54:32.2081 SELECT me.role, me.contributor, me.track FROM contributor_track me WHERE ( ( me.contributor = ? AND me.role = ? AND me.track = ? ) OR ( me.contributor = ? AND me.role = ? AND me.track = ? ) ): '49', '5', '87', '49', '5', '87'
2007-07-20 11:54:32.2093 INSERT INTO contributor_track (contributor, role, track) VALUES (?, ?, ?): '49', '5', '87'
2007-07-20 11:54:32.2110 SELECT me.id, me.name, me.namesort, me.musicmagic_mixable, me.namesearch, me.musicbrainz_id FROM contributors me WHERE ( ( me.namesearch = ? ) ): 'JON KAPLAN'
2007-07-20 11:54:32.2123 INSERT INTO contributors (musicbrainz_id, name, namesearch, namesort) VALUES (?, ?, ?, ?): 'NULL', 'Jon Kaplan', 'JON KAPLAN', 'JON KAPLAN'
2007-07-20 11:54:32.2143 SELECT me.role, me.contributor, me.track FROM contributor_track me WHERE ( ( me.contributor = ? AND me.role = ? AND me.track = ? ) OR ( me.contributor = ? AND me.role = ? AND me.track = ? ) ): '50', '2', '87', '50', '2', '87'
2007-07-20 11:54:32.2156 INSERT INTO contributor_track (contributor, role, track) VALUES (?, ?, ?): '50', '2', '87'
2007-07-20 11:54:32.2172 SELECT me.id, me.name, me.namesort, me.musicmagic_mixable, me.namesearch, me.musicbrainz_id FROM contributors me WHERE ( ( ( ( me.namesearch = ? ) ) ) ): 'Al Kaplan'
2007-07-20 11:54:32.2191 SELECT me.role, me.contributor, me.track FROM contributor_track me WHERE ( ( me.contributor = ? AND me.role = ? AND me.track = ? ) OR ( me.contributor = ? AND me.role = ? AND me.track = ? ) ): '49', '2', '87', '49', '2', '87'
2007-07-20 11:54:32.2203 INSERT INTO contributor_track (contributor, role, track) VALUES (?, ?, ?): '49', '2', '87'
2007-07-20 11:54:32.2220 SELECT me.id, me.name, me.namesort, me.musicmagic_mixable, me.namesearch, me.musicbrainz_id FROM contributors me WHERE ( ( me.namesearch = ? ) ): 'DR LECTER'
2007-07-20 11:54:32.2267 INSERT INTO contributors (musicbrainz_id, name, namesearch, namesort) VALUES (?, ?, ?, ?): 'NULL', 'Dr Lecter', 'DR LECTER', 'DR LECTER'
2007-07-20 11:54:32.2286 SELECT me.role, me.contributor, me.track FROM contributor_track me WHERE ( ( me.contributor = ? AND me.role = ? AND me.track = ? ) OR ( me.contributor = ? AND me.role = ? AND me.track = ? ) ): '51', '6', '87', '51', '6', '87'
2007-07-20 11:54:32.2303 INSERT INTO contributor_track (contributor, role, track) VALUES (?, ?, ?): '51', '6', '87'
2007-07-20 11:54:32.2322 SELECT me.id, me.name, me.namesort, me.musicmagic_mixable, me.namesearch, me.musicbrainz_id FROM contributors me WHERE ( ( ( ( me.namesearch = ? ) ) ) ): 'The Police'
2007-07-20 11:54:32.2366 INSERT INTO contributors (musicbrainz_id, name, namesearch, namesort) VALUES (?, The Police, The Police, The Police): 'NULL'
2007-07-20 11:54:32.2373 ERROR: Error executing 'INSERT INTO contributors (musicbrainz_id, name, namesearch, namesort) VALUES (?, The Police, The Police, The Police)': DBD::mysql::st execute failed: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'Police, The Police, The Police)' at line 1 at /opt/SlimServer_v6.5.3/CPAN/DBIx/Class/Storage/DBI.pm line 771.
================================================================================

I took a look at the two attached mp3 files and saw exactly the same thing as Erland.  They both crash the scanner on my system.  I also noticed the same thing about how the SQL call is formed in cases where it crashes vs. those where it doesn't.  So it really is a SQL syntax error.  I looked at some of the code but couldn't tell what is causing a different SQL statement to be executed in these cases.

Created attachment 2070 [details]
potential patch

I believe this is due to change 12151 to lib/MP3/Info.pm  This creates inconsitent attributes arrays from this file.

Please test the attached patch.  I will also post to bug 3972 for review from the author of that patch.

Looks like triode has correctly identified my code as the source of the problem; really, really sorry about that.

I've checked the patch and it looks like it's correct, and I've tested it against my reference MP3s here and it seems to be ok as a regression check as well. I'm happy with the patch.

I've made the change to my local copy of the server so it'll get tested out with a full rescan this evening and all being well should still be ok.

I just tried the patch as well and I am no longer seeing the issues with the attached files.  Thanks Triode!  Steve, you may want to submit a feature request for support of more then one ALBUMARTIST tag in SlimServer.

Patch applied in change 12406 and 12407 for 6.5 and trunk.  Please test and close.

Also, if anyone knows what program is generating the tags, could they please report the fault to the author - they will obviously want to comply to the specification as that gets them the widest range of support. Just about anything can be expected to happen if you do things that are outside the spec. The relevant section is ID3v2.3 specification, section 4.2, second sentance of the first paragraph :

"There may only be one text information frame of its kind in an tag" (sic)

Specifically, the TPE1 frames in ID3v2.3 should have alternate text strings separated by a '/' character. As noted in section 4.2.1, subsection 'TPE1'.

If it's reported to the author then at least we've tried to get things done properly by them, despite SlimServer having to work around their broken code.

I've installed nightly build from the weekend and my scanner no longer crashes: thanks all!  Very pleased to have my library back. :-)

(In reply to comment #16)
> Also, if anyone knows what program is generating the tags, could they please
> report the fault to the author - they will obviously want to comply to the
> specification as that gets them the widest range of support. Just about
> anything can be expected to happen if you do things that are outside the spec.
> The relevant section is ID3v2.3 specification, section 4.2, second sentance of
> the first paragraph :
> 
> "There may only be one text information frame of its kind in an tag" (sic)
> 
> Specifically, the TPE1 frames in ID3v2.3 should have alternate text strings
> separated by a '/' character. As noted in section 4.2.1, subsection 'TPE1'.
> 
> If it's reported to the author then at least we've tried to get things done
> properly by them, despite SlimServer having to work around their broken code.

I tagged my files with foobar2000 version 0.8.3.  New versions have been released since I last upgraded, so I'll upgrade again and see how it behaves now.  fb2k's tag editor allows one to enter multiple instances of any given tag (presumably because the editor itself is format-agnostic, and this would be legal in Ogg Vorbis and others).  I suppose, at least in the version I've been using, that it didn't always convert the tags into a legal structure for the file's actual format.

The comments are a tough read but it looks like this should be closed.  Any objections?