Bug 612 - Change "_" to " " in Guessed Tags
: Change "_" to " " in Guessed Tags
Status: CLOSED FIXED
Product: Logitech Media Server
Classification: Unclassified
Component: Formats
: 5.x or older
: PC Debian Linux
: P2 enhancement (vote)
: ---
Assigned To: Blackketter Dean
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2004-10-06 07:27 UTC by Roland Gerlach
Modified: 2008-12-18 11:55 UTC (History)
0 users

See Also:
Category: ---


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Roland Gerlach 2004-10-06 07:27:04 UTC
Please change "_" to " " in guessed tag formats.

Perhaps there should be an option in the SlimServer Formatting setup page to
turn this feature on or off.

BTW: thanks for such a great product.
Comment 1 KDF 2004-10-06 10:01:54 UTC
you should be able to blank out each line of the default GuessTags setting to
turn the feature off.

"_" or " " would also be something you can enter as part of the matching.
Comment 2 Roland Gerlach 2004-10-06 15:10:20 UTC
Doh!  I guess I wasn't clear enough.

I have a few tracks that have no id3 tags.  Some of these tracks contain a "_" 
character instead of a space in the filename.  I have setup a appropriate 
Guess Tag Format and the SlimServer correctly determines the details.  My 
issue is that the "_" characters are displayed on the web interface and on the 
squeezebox.

For example:
I have a file called Electric_Light_Orchestra/Discovery/05-
Last_Train_To_London.mp3 which has no id3 tags.  I've set the guess tag format 
to /ARTIST/ALBUM/TRACKNUM-TITLE and the SlimServer uses 
Electric_Light_Orchestra as the ARTIST, Discovery as the ALBUM, and 
Last_Train_To_London as the TITLE.

Unfortunately those "_" characters look awful on the squeezebox display.  If a 
track has no id3 tags, could the SlimServer display a " " instead of "_" 
characters in the guessed tag details.  In my example, the ARTIST would then 
be "Electric Light Orchestra" - much nicer on the eyes.
Comment 3 Roland Gerlach 2004-10-06 15:46:45 UTC
I thought I'd try to find the appropriate spot in the code.  Luckily the 
word "guess" only appears in a few places.

I'm not good with perl but this works for me:

--- Slim/Music/Info.pm-pre20041007      2004-10-02 07:34:08.000000000 +1000
+++ Slim/Music/Info.pm  2004-10-07 08:43:39.000000000 +1000
@@ -1232,6 +1232,7 @@
                        foreach my $match (@matches) {
                                $::d_info && Slim::Utils::Misc::msg("$tags[$i] 
=> $match\n");
                                $match = int($match) if $tags[$i] 
=~ /TRACKNUM|DISC{1,2}/;
+                               $match =~ s/_/ /g;
                                $taghash->{$tags[$i++]} = $match;
                        }
                        return;

Ideally, this should be an option on the formating page - 'replace "_" 
with " " for guessed tags': Yes/No.
Comment 4 KDF 2004-10-06 15:57:05 UTC
I had misread what you were requesting.  Dean has a patch, apparently.
Comment 5 Roland Gerlach 2004-10-06 16:22:39 UTC
Hi KDF,

You didn't misread what I was requesting, I didn't explain it clearly even 
after reading the "how to write a good bug report" page.  Looking back, even 
the summary wasn't accurate (so I've changed it).  Thanks for pointing out my 
poorly written request.

Cheers,
Roland.
--
Tell me and I'll forget; show me and I may remember;
involve me and I'll understand - Chinese Proverb.
Comment 6 KDF 2004-10-13 01:04:51 UTC
I believe Dean has fixed this in the latest nightly.  however it does result in
the follwoing warnign for some files:
Use of uninitialized value in transliteration (tr///) at
/usr/local/slimserver/Slim/Music/Inf
o.pm line 1237.

perhaps an if (defined) would be of use here.
Comment 7 Blackketter Dean 2004-10-13 15:45:00 UTC
A fix for the warning is in CVS now.  Please verify that the bug is fixed in the next nightly build.
Thanks!
Comment 8 KDF 2004-10-14 01:26:17 UTC
the warning I got was an undefined value.  That is still undefined, so I still
get the warning.  I had to change to "if (defined $match)" and 
Comment 9 Blackketter Dean 2004-10-14 15:30:39 UTC
The latest code has this line, I belive:

				$match = tr/_/ / if ($match);

Roland: does this address your issue?  Is it fixed in the latest nightly?
Comment 10 Roland Gerlach 2004-10-19 04:35:31 UTC
I've downloaded SlimServer_v2004-10-19...

Now all the tracks without id3 tags are showing up on the squeezebox and the web
pages with all tags containing a "0" instead of any useful information.  For
example my normal squeezebox display is ARTIST: TITLE. 
/music/mp3/Mi-Sex/Space_Race/01-Space_Race.mp3 is showing up as "0: 0".

Perhaps the "tr" is returning a result code instead of a modified string?

My .slimserver.pref file contains:
<snip />
00:04:20:05:0b:ae-titleFormat# = 4
00:04:20:05:0b:ae-titleFormat0 = 5
00:04:20:05:0b:ae-titleFormat1 = 1
00:04:20:05:0b:ae-titleFormat2 = 3
00:04:20:05:0b:ae-titleFormat3 = 6
00:04:20:05:0b:ae-titleFormat4 = 10
00:04:20:05:0b:ae-titleFormatCurr = 4
<snip />
guessFileFormats# = 3
guessFileFormats0 = /ALBUM/TRACKNUM-TITLE__ARTIST
guessFileFormats1 = /ARTIST/ALBUM/TRACKNUM-TITLE
guessFileFormats2 = /ALBUM/TITLE__ARTIST
guessFileFormats3 = /ARTIST/ALBUM/TITLE
<snip />
titleFormat# = 10
titleFormat0 = TITLE
titleFormat1 = DISC-TRACKNUM. TITLE
titleFormat2 = TRACKNUM. TITLE
titleFormat3 = TRACKNUM. ARTIST - TITLE
titleFormat4 = TRACKNUM. TITLE (ARTIST)
titleFormat5 = TRACKNUM. TITLE - ARTIST - ALBUM
titleFormat6 = FILE.EXT
titleFormat7 = TRACKNUM. TITLE from ALBUM by ARTIST
titleFormat8 = TITLE (ARTIST)
titleFormat9 = ARTIST - TITLE
titleFormat10 = ARTIST: TITLE
<snip />
Comment 11 Roland Gerlach 2004-10-19 12:50:40 UTC
I've found the problem - the patch was "$match = tr/_/ /".  tr returns the
number of characters replaced.  The patch should be "$match =~ tr/_/ /" and
there's no need for the "if defined...":

diff -u Info.pm-pre20041020 Info.pm
--- Info.pm-pre20041020 2004-10-19 18:05:56.000000000 +1000
+++ Info.pm     2004-10-20 05:32:39.000000000 +1000
@@ -1232,7 +1232,7 @@
                        my $i = 0;
                        foreach my $match (@matches) {
                                $::d_info && Slim::Utils::Misc::msg("$tags[$i]
=> $match\n");
-                               $match = tr/_/ / if (defined $match);
+                               $match =~ tr/_/ /;
                                $match = int($match) if $tags[$i] =~
/TRACKNUM|DISC{1,2}/;
                                $taghash->{$tags[$i++]} = $match;
                        }

The only other thing is whether there should be an option on the Server Settings
/ Formatting page to turn on/off the replacement.  For example:

Guess Tags Formats
...
Replace all "_" characters with spaces? Enabled/Disabled

Your thoughts?
Comment 12 Blackketter Dean 2004-10-19 13:05:56 UTC
Whoops, my bad.  Should be fixed in tomorrow's nightly release.
Comment 13 KDF 2004-10-19 18:02:06 UTC
brilliant! i can't believe how long I stared at that, absolutely certain
something wasn't right....but I never saw the missing ~ :)
Comment 14 Chris Owens 2007-05-24 15:54:12 UTC
This Slimserver bug was fixed a very long time ago, and is now being marked as Closed.  If you're still experiencing this issue, please verify you are running a current version of Slimserver and re-open the bug.
Comment 15 Chris Owens 2008-12-18 11:55:09 UTC
Routine bug db maintenance; removing old versions which cause confusion.  I apologize for the inconvenience.