Bug 7586 - FLAC.pm should be a little stricter parsing track titles in _getCDDBTags
: FLAC.pm should be a little stricter parsing track titles in _getCDDBTags
Status: CLOSED FIXED
Product: Logitech Media Server
Classification: Unclassified
Component: Tagging
: 7.0.1
: Other Ubuntu Linux
: -- minor (vote)
: 7.x
Assigned To: Andy Grundman
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2008-03-23 17:55 UTC by Manoj Kasichainula
Modified: 2009-07-31 10:18 UTC (History)
0 users

See Also:
Category: ---


Attachments
Patch to tweak artist and title recognition in whole-CD flacs (428 bytes, patch)
2008-06-24 00:59 UTC, Manoj Kasichainula
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Manoj Kasichainula 2008-03-23 17:55:52 UTC
FLAC.pm, in a TTITLE tag, simply looks for a slash before deciding that the title represents an "Artist / Title" combination. This hits tracks with actual slashes in the title though, producing a bogus artist and title.

I'd suggest something a little stricter, like at least looking for " / " in the TTITLE, and/or checking that the album's artist is "Various".
Comment 1 Michael Herger 2008-06-10 08:03:57 UTC
An issue which won't make it for 7.1. To be reviewed for a future release.
Comment 2 Jim McAtee 2008-06-10 12:31:20 UTC
I thought guess tags only used folder and file names, not tag data.  You can't have a forward slash in a file name, can you?
Comment 3 KDF 2008-06-10 13:06:00 UTC
Jim, this doesn't appear to be guessTags.  I believe this is addressing CDDB or Cuesheet tag parsing in Formats::Flac where the title is given as Artist/Title.

I'm not a user of this type of formatting, so I can't speak to how often the format would strictly adhere to " / " vs "/".  I don't think we can guarantee whitespace in there.  Specifying it as a requirement is an option, but then so is specifying a limitation on the use of "/" when using this type of formatting.
Comment 4 Andy Grundman 2008-06-13 09:14:51 UTC
How are these tags added to the files?  Can you give me a CDDB ID example where our code breaks?
Comment 5 Chris Owens 2008-06-20 16:56:43 UTC
Not going to make it for 7.1.  That example data would still be welcome!
Comment 6 Manoj Kasichainula 2008-06-24 00:59:10 UTC
Created attachment 3479 [details]
Patch to tweak artist and title recognition in whole-CD flacs
Comment 7 Manoj Kasichainula 2008-06-24 01:06:16 UTC
Basically, any case where there's a slash not surrounded by spaces. See
http://www.freedb.org/en/faq.3.html#22 , number 6:

6. When submitting a sampler or compilation, you should include the
track-artist in the track-name, using the syntax "artist / track-title" and set
the CD-artist to "Various"

Almost all the time, the cddb entries I've seen have interpreted this as
requiring spaces around the slash.

I've attached a patch that fixed it to my satisfaction. I've had to repair a
few of the cddb entries locally to work with this, but at least this patch
makes it possible, meets the "standard", and required the least rework.

As for an example: pretty much any of the cddb entries for Aphex Twin's
_Richard D. James Album_. "Girl/Boy Song" will break with the current system,
and there's no aesthetically tolerable way to fix it without switching away
from cddb format in the flacs (which I do also plan to do one of these days,
but I can write scripts easily, others aren't so lucky), or changing FLAC.pm to
not recognize that as an Artist/Title.

Another alternative would be to look for something like "Various" as the CD
artist. I actually don't prefer this, because I have some CDs with both their
own artist and individual artists per track. But the cddb FAQ says the artist
in this case should be "Various", so I can deal with it.

Hope this helps.
Comment 8 Andy Grundman 2008-07-28 08:26:22 UTC
Thanks, patch applied as 7.2 change 22152.
Comment 9 James Richardson 2008-10-09 15:25:39 UTC
Verified fixed in

SqueezeCenter 7.2.1-23472
Comment 10 James Richardson 2008-12-15 12:33:40 UTC
This bug has been fixed in the 7.3.0 release version of SqueezeCenter!

Please download the new version from http://www.slimdevices.com/su_downloads.html if you haven't already.  

If you are still experiencing this problem, feel free to reopen the bug with your new comments and we'll have another look.
Comment 11 Chris Owens 2009-07-31 10:18:26 UTC
Reduce number of active targets for SC