[cddb_daemon] slash clash

[ttcoder]

The CDDB daemon parsing code has a feature where a "/" slash in a Track title is interpreted as a delimiter for a track specific artist (i.e. an artist that overrides the disc-wide artist for that particular track): ​http://cgit.haiku-os.org/haiku/tree/src/servers/cddb_daemon/cddb_server.cpp#n259

This causes trouble for track titles that include... a slash! Like track TTITLE0 on this disk: ​http://www.freedb.org/freedb/misc/5d0a5908

The www.freedb.org FAQ does mention that feature, but it says it only applies if the disc-wide artist is set to "Various" : ​http://www.freedb.org/en/faq.3.html#22

For highest compatibility, cddb_server.cpp:259 should test for "Various" before applying the overriden artist feature. If the artist is not "Various" but e.g. "Doug Smith" then the slash in title should be interpreted as-is... with the required defensive programming when using that value for creating files of course. Indeed there are provisions for that elsewhere in the code (look for ReplaceSet("/", " ") calls in several places, including when processing the title and artist fields) so the fact that "Various" is not taken into account seems to be a simple overlook.

[axeld]

Makes perfect sense, patches welcome :-)

[jscipione]

Patch to check for Various before looking for track specific artist info

[umccullough]

I don't think that's what he meant in the bug description - I believe the problem here is that if the CD-artist is "Various", then the track name is supposed to be used to parse the artist name out when it contains a "/"

Your patch seems to just look for "Various /" in the track name instead.

[axeld]

Wow, that was fast. Unfortunately, it doesn't look correct. I assume you did not test this, John? You check for "Various / " in the track title. You won't find it there, though -- you'll only have to consider " / " when the disk title is set to "Various".

[jscipione]

Replying to axeld:

Wow, that was fast. Unfortunately, it doesn't look correct. I assume you did not test this, John? You check for "Various / " in the track title. You won't find it there, though -- you'll only have to consider " / " when the disk title is set to "Various".

No, I didn't test since I don't have the data to test this with. I guess I misinterpreted the bug. I'll try again.

[ttcoder]

Right, if I'm not mistaken it could be a one-line change, from

if (pos != B_ERROR) {

to

if (pos != B_ERROR && diskData->artist == "Various") {

(or even using ICompare() for case-insensitivity) Sorry I was not more explicit in my description...

[jscipione]

Should "Various" be case sensitive or not? Should I match on "VARIOUS"?

[axeld]

I would assume that a band that calls itself VARIOUS and uses " / " in their titles can be safely ignored. Therefore I would suggest to compare case insensitively.

[umccullough]

Replying to jscipione:

Should "Various" be case sensitive or not? Should I match on "VARIOUS"?

Having done my share of queries against cddb/freedb over the years - I would highly suggest making it a case-insensitive comparison... the data coming out of that database is pretty bad sometimes.

[jscipione]

Let's try again. Look for disk title set to Various case-insensitively before parsing track specific artist info using ICompare()

[axeld]

Looks good to me now. Can you test that, ttcoder?

[ttcoder]

Thanks much guys. I'm making a build of the modified daemon for dsuden (he's the one with the oddball Doug Smith CD :-) and will be sure to report back in a few days.

[ttcoder]

We finally had time to test the patched daemon on a few CDs. No regression on 'normal' albums, and the oddball album now works great; jscipione you may please apply the patch..!

[jscipione]

Fixed in hrev45754