Bug 1736 - Some tracks have no title after rescan
: Some tracks have no title after rescan
Status: CLOSED FIXED
Product: Logitech Media Server
Classification: Unclassified
Component: Database
: 6.1.0
: PC Other
: P2 normal (vote)
: ---
Assigned To: Dan Sully
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2005-06-30 07:06 UTC by Greg Cannon
Modified: 2011-03-16 04:35 UTC (History)
1 user (show)

See Also:
Category: ---


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Greg Cannon 2005-06-30 07:06:47 UTC
Using 6.1b1, did a rescan with clear library checked and several tracks ended up
with no title in the DB. Many (perhaps all?) of the affected tracks are in saved
playlists, though looking at the playlists themselves there are some entries in
them that do have titles after the rescan.

Setting the playlist folder to null in the Web interface and doing a rescan with
 clear library set produced full set of titles in DB. Adding playlist folder
back via Web interface and doing rescan with clear library not checked produced
the required result of all titles present.
 
Had a trawl through the code, sheesh, Slim/DataStores/DBI/DBIStore.pm is not
pleasant. When newTrack is called readTags is getting the title and other
attributes correctly from the mp3 files, but the title is sometimes being
trashed on return. The hashes are being merged

 $attributeHash = { %{$self->readTags($url)},  %$attributeHash }

and the rh hash sometimes has a key of TITLE in it with no value.
Reversing the order these are merged "solves" the problem BUT.... the playlists
now have %20's in their titles... 

Something is clearly not working the way someone intended here, and its not
clear to me yet what they intended either.

Here's an edited snippet from log with d_info and d_scan plus a few hash dumps
thrown in

2005-06-30 14:06:45.1966 New track for file:///mp3/rock/710aed07_01.mp3
2005-06-30 14:06:45.1974 readTag was 1 forfile:///mp3/rock/710aed07_01.mp3
2005-06-30 14:06:45.1983 dumping attributeHash HASH(0x975be98)
2005-06-30 14:06:45.1992 TITLE=.
2005-06-30 14:06:45.2008 Converting file:///mp3/rock/710aed07_01.mp3 to
/mp3/rock/710aed07_01.mp3
2005-06-30 14:06:45.2020 mp3 file type for file:///mp3/rock/710aed07_01.mp3
2005-06-30 14:06:45.2028 reading tags for: file:///mp3/rock/710aed07_01.mp3
2005-06-30 14:06:45.2104 readTags done, dumping hash HASH(0x975ef58)
2005-06-30 14:06:45.2113 SIZE = 18916831
2005-06-30 14:06:45.2122 TAG = 1
2005-06-30 14:06:45.2130 FS = 18918315
2005-06-30 14:06:45.2138 YEAR = 1977
2005-06-30 14:06:45.2147 OFFSET = 648
2005-06-30 14:06:45.2155 STEREO = 1
2005-06-30 14:06:45.2164 MS = 177.09375000004
2005-06-30 14:06:45.2172 ARTIST = Meat Loaf
2005-06-30 14:06:45.2181 BLOCKALIGN = 1
2005-06-30 14:06:45.2189 COMMENT = 
2005-06-30 14:06:45.2197 SECS = 591.17709375
2005-06-30 14:06:45.2206 PADDING = 0
2005-06-30 14:06:45.2215 GENRE = Rock
2005-06-30 14:06:45.2223 COPYRIGHT = 0
2005-06-30 14:06:45.2231 MM = 9
2005-06-30 14:06:45.2239 CT = mp3
2005-06-30 14:06:45.2248 TAGVERSION = ID3v2.3.0
2005-06-30 14:06:45.2256 TRACKNUM = 1
2005-06-30 14:06:45.2265 SS = 51
2005-06-30 14:06:45.2273 MODE = 0
2005-06-30 14:06:45.2281 LAYER = 3
2005-06-30 14:06:45.2289 FREQUENCY = 44.1
2005-06-30 14:06:45.2298 VBR = 0
2005-06-30 14:06:45.2306 TIME = 09:51
2005-06-30 14:06:45.2315 FRAMES = 241296
2005-06-30 14:06:45.2323 TITLE = Bat Out of Hell
2005-06-30 14:06:45.2332 ALBUM = Bat Out of Hell - Meat Loaf
2005-06-30 14:06:45.2341 AGE = 1074432011
2005-06-30 14:06:45.2350 BITRATE = 256000
2005-06-30 14:06:45.2358 VERSION = 1
2005-06-30 14:06:45.2367 FRAME_LENGTH = 78
2005-06-30 14:06:45.2377 *** returned after readTags, dumping attributeHash
HASH(0x975d68c)
2005-06-30 14:06:45.2386 SIZE=18916831.
2005-06-30 14:06:45.2394 OFFSET=648.
2005-06-30 14:06:45.2404 YEAR=1977.
2005-06-30 14:06:45.2412 FS=18918315.
2005-06-30 14:06:45.2421 TAG=1.
2005-06-30 14:06:45.2429 MS=177.09375000004.
2005-06-30 14:06:45.2438 STEREO=1.
2005-06-30 14:06:45.2446 ARTIST=Meat Loaf.
2005-06-30 14:06:45.2454 COMMENT=.
2005-06-30 14:06:45.2462 BLOCKALIGN=1.
2005-06-30 14:06:45.2471 SECS=591.17709375.
2005-06-30 14:06:45.2479 PADDING=0.
2005-06-30 14:06:45.2487 MM=9.
2005-06-30 14:06:45.2495 COPYRIGHT=0.
2005-06-30 14:06:45.2504 GENRE=Rock.
2005-06-30 14:06:45.2513 CT=mp3.
2005-06-30 14:06:45.2521 TRACKNUM=1.
2005-06-30 14:06:45.2529 TAGVERSION=ID3v2.3.0.
2005-06-30 14:06:45.2538 SS=51.
2005-06-30 14:06:45.2546 LAYER=3.
2005-06-30 14:06:45.2555 MODE=0.
2005-06-30 14:06:45.2563 FREQUENCY=44.1.
2005-06-30 14:06:45.2571 VBR=0.
2005-06-30 14:06:45.2579 TIME=09:51.
2005-06-30 14:06:45.2588 FRAMES=241296.
2005-06-30 14:06:45.2596 ALBUM=Bat Out of Hell - Meat Loaf.
2005-06-30 14:06:45.2605 TITLE=.
2005-06-30 14:06:45.2613 AGE=1074432011.
2005-06-30 14:06:45.2622 BITRATE=256000.
2005-06-30 14:06:45.2630 VERSION=1.
2005-06-30 14:06:45.2638 FRAME_LENGTH=78.
2005-06-30 14:06:45.2650 Adding file:///mp3/rock/710aed07_01.mp3 : SIZE to 18916831
blah blah

Let me know if anyone needs more info.

regards,
Greg.
Comment 1 Vidur Apparao 2005-06-30 14:27:52 UTC
Dan suggests that he may have broken this.

Seems to me that the problem may be in Slim::Formats::Parse::_updateMetadata.
The test for whether to set the TITLE attribute is based on a defined check, as
opposed to a has value one.
Comment 2 Dan Sully 2005-06-30 23:52:10 UTC
Are there any playlists in your music folder tree?
Comment 3 Greg Cannon 2005-07-01 02:28:09 UTC
Re Dans question, no files other than mp3's in the music directory. Music files
are stored in subdirectories based on CDDB genres, and the playlists are stored
in a parallel directory.  viz:
/somewhere/code
   slimserver revs
/somewhere/mp3/
   blues/
       720a140a_01.mp3
       720a140a_02.mp3
       ...
   classical/
   ...
   rock/
   soundtrack/
/somewhere/playlists/
   play list 1
   play list 2

I did notice that it processed several of the playlists before starting on the
music folders. 

Re the %20's in playlist names, not all the lists are affected. All playlists
were created or edited with 5.X slimserver code. It may be down to order of
processing?

Re Vidurs comment, I came across a point in the code when I thought it should be
checking for exists rather than defined, but convinced myself it was harmless. I
can't remember now if it was the same one, I'll check again when I get time.

It was the newTrack code and the way it creates so many anon hashes, seemingly
without need (readTags returns a ref to one for example), that made me think I
hadn't grasped what was supposed to be going on here. 

On my system the attributeHash is always empty or contains the dud TITLE key
before readTags is called, so why merge them? Are there situations where thats
not going to be true?

regards,
Greg.
Comment 4 Greg Cannon 2005-07-01 06:23:51 UTC
To answer my own last question ... after trawling my logs more carefully I see
that the TITLE key on entry to newTrack can contain valid data when processing
playlists. Disregard my earlier comments about the merging :-)

regards,
Greg.
Comment 5 Greg Cannon 2005-07-01 08:25:18 UTC
Vidur's comment appears to be correct - just changed line 91 of
Slim::Formats::Parse.pm from 

if (defined($title) && ... to  
if ($title =~ /\w/ && ... 

and rebuilt the library. All correct now, no missing titles or funky playlist
names. 

A full rescan nails the box though, streams stop and web interface takes about
10 seconds to respond to anything.

regards,
Greg.
Comment 6 Robert Moser II 2005-07-01 11:12:17 UTC
Try it with this instead:
if (defined($title) && $title gt '' && ...

That should be a little bit lighter than $title =~ /\w/
Comment 7 Dan Sully 2005-07-01 11:13:28 UTC
How about just: if ($title) ... ?
Comment 8 Robert Moser II 2005-07-01 14:43:08 UTC
That's fine until you have a track named "0". Try this:
perl -e "my $test='0'; if ($test) {print 'good'} else {print 'bad'};"

($title gt '') could be replaced with ($title ne '') or (length($title)),
whichever among those three is the cheapest.  I believe that length(undef) will
spit a warning just like undef gt '' or undef ne '', so the defined check is
still needed.
Comment 9 Robert Moser II 2005-07-01 15:01:14 UTC
And in case you were wondering, musicbrainz lists nine tracks, all from
different artists and albums with a track title of 0.  See here:
http://musicbrainz.org/newsearch.html?search=0&table=track
Comment 10 Greg Cannon 2005-07-02 03:13:58 UTC
How about leaving the test as "defined $title" and making sure the five places
that call _updateMetaData all set or undef the second arg to the call correctly?

This would allow the callers to set title to "0" or ''. though I'm dubious of
the benefit of that.

As best I can tell its only sub readM3u that blows it with the "my $title = '';
" around line 108. 

Changing that to "my $title = undef;" and setting the _updateMetaData test to
defined($title) as per the original works correctly for me. 

But I only have .m3u playlists so I cant be sure the other callers will do the
right thing. I think they do, but this will need to be proven.

Otherwise if( defined( $title) && $title ne '' && ... is probably the best option.

regards,
Greg.
Comment 11 Dan Sully 2005-07-05 15:04:30 UTC
I've commited a fix in subversion change 3615.

Checking $title ne '', and not setting my $title.
Comment 12 Chris Owens 2008-03-11 11:28:24 UTC
This bug was marked resolved in Slimserver 6.1, which is several versions ago.  If you're still seeing this bug, please re-open it.  Thanks!