Bug 16073 - Check for undefined tag values to avoid scanner.log spam; Better Perl style
: Check for undefined tag values to avoid scanner.log spam; Better Perl style
Status: RESOLVED FIXED
Product: Logitech Media Server
Classification: Unclassified
Component: Formats
: 7.5.x
: PC Other
: P1 normal (vote)
: 7.5.1
Assigned To: Andy Grundman
: Audio::Scan
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2010-04-15 04:19 UTC by ast
Modified: 2011-03-16 04:35 UTC (History)
2 users (show)

See Also:
Category: ---


Attachments
Slim/Formats.pm and Slim/Formats/MP3.pm diffs (2.09 KB, patch)
2010-04-15 04:19 UTC, ast
Details | Diff
Example of mp3 with a WXXX header causing slimserver and scanner to crash (4.68 MB, application/octet-stream)
2010-04-15 06:04 UTC, ast
Details
This patch fixes crash (2.22 KB, patch)
2010-04-15 14:45 UTC, ast
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description ast 2010-04-15 04:19:52 UTC
Created attachment 6783 [details]
Slim/Formats.pm and Slim/Formats/MP3.pm diffs

scanner.pl spams lots of "undefined value used in ..." so the log gets bloated

the attached diffs fixes those and also handles arrays and scalars similarly,
avoiding duplicate code

I found this whilst investigating a  problem that perl dumps core in scanner.pl
(and server) when there is a id3 tag of the WOAR, WOAS, WCOM, ... type.

I'm trying to get to the root cause of this. I have example MP# file good/bad
if interested.
Comment 1 Andy Grundman 2010-04-15 05:54:04 UTC
Make sure you're using the right version of Audio::Scan, there was an SSODS release with the wrong version that spewed a lot of warnings.

I would like to see a bad file too, though, thanks.
Comment 2 ast 2010-04-15 06:04:55 UTC
Created attachment 6784 [details]
Example of mp3 with a WXXX header causing slimserver and scanner to crash

id3info LedZeppelinDarlene_NOK.mp3 

*** Tag information for LedZeppelinDarlene_NOK.mp3
=== WCOM (Commercial information): 
=== TIT2 (Title/songname/content description): Darlene
=== TPE1 (Lead performer(s)/Soloist(s)): Led Zeppelin
=== TALB (Album/Movie/Show title): Coda
=== TYER (Year): 1982
=== TRCK (Track number/Position in set): 06/08
=== TCON (Content type): (17)
*** mp3 info
MPEG1/layer III
Bitrate: 128KBps
Frequency: 44KHz

reports a WCOM header, if this gets removed with

id3 -2u -d LedZeppelinDarlene_NOK.mp3 scanner.pl doesn't crash anymore
Comment 3 Andy Grundman 2010-04-15 07:48:13 UTC
Thanks, I will work on fixing the crash.
Comment 4 ast 2010-04-15 14:45:36 UTC
Created attachment 6785 [details]
This patch fixes crash
Comment 5 ast 2010-04-15 14:46:38 UTC
This patch fixes the Perl coredump with SIGSEGV at

    $tags = $tagReaderClass->getTag($filepath, $anchor);

near the two evals in Slim::Format::readTags

	Slim::Formats::readTags('Slim::Formats', 'file:///n/tmp/LedZeppelinDarlene_NOK/LedZeppelinDarlene_NOK.mp3') called at /usr/local/squeezeboxserver/Slim/Schema.pm line 937
	Slim::Schema::newTrack('Slim::Schema', 'HASH(0x9e86710)') called at /usr/local/squeezeboxserver/Slim/Schema.pm line 1248

The solution is to take out the actual $tagReaderClass->getTag call after the class is loaded at runtime.

Now, the following scan runs through cleanly and so does my full library scan :-)

sudo -u slimserv /usr/local/squeezeboxserver/scanner.pl --wipe /n/tmp/LedZeppelinDarlene_NOK
[10-04-15 23:25:11.5372] main::main (180) Starting Squeezebox Server scanner (v7.5.0, r30464, Thu Apr  1 06:30:44 MDT 2010) perl 5.008009
[10-04-15 23:26:06.4005] main::main (271) Removing artwork cache...
[10-04-15 23:26:08.2458] Slim::Utils::Scanner::scanDirectory (320) Found 1 files in /n/tmp/LedZeppelinDarlene_NOK
[10-04-15 23:26:08.2466] Slim::Utils::Scanner::scanDirectory (333) Scanning: /n/tmp/LedZeppelinDarlene_NOK/LedZeppelinDarlene_NOK.mp3
[10-04-15 23:26:08.4175] Slim::Music::Import::runScanPostProcessing (429) Starting merge of various artists albums
[10-04-15 23:26:08.4466] Slim::Music::Import::endImporter (700) Completed mergeVariousAlbums Scan in 0 seconds.
[10-04-15 23:26:08.4476] Slim::Music::Import::runScanPostProcessing (437) Starting artwork scan
[10-04-15 23:26:08.5046] Slim::Music::Import::endImporter (700) Completed findArtwork Scan in 0 seconds.
[10-04-15 23:26:08.5256] Slim::Music::Import::runScanPostProcessing (483) Starting Database optimization.
[10-04-15 23:26:08.8146] Slim::Music::Import::endImporter (700) Completed dbOptimize Scan in 0 seconds.

Further below in Format.pm, I replace $tags->{$tag} with $value where it is used read-only for speed and readability.
Comment 6 Andy Grundman 2010-04-15 17:08:55 UTC
OK, yep, the WCOM frame has a size of 0, this is illegal [1] but we weren't handling it properly.

You might want to get a fix for your tagging program.

[1] "A frame must be at least 1 byte big, excluding the header." http://www.id3.org/id3v2.3.0
Comment 7 SVN Bot 2010-04-15 17:18:01 UTC
 == Auto-comment from SVN commit #662 to the opensource repo by agrundman ==
 == http://svn.slimdevices.com/opensource?view=revision&revision=662 ==

Fixed bug 16073, properly handle illegal zero-byte ID3 frames