Bugzilla – Bug 15105
NewScanner for 7.4+ intolerant of bad FILE value in embedded cuesheets
Last modified: 2010-04-08 17:24:22 UTC
The new 7.4 and up scanner changes behaviour found in SlimServer 6.x and SqueezeCenter 7.x. In the older versions, the scanner didn't care what the FILE value was in embedded cuesheets. The scanner always used the actual filename of the file being scanned in forming the data for the track.url field. With the new scanner, the filename portion for track.url seems to come from the FILE entry in the cuesheet. With unembedded cuesheets, this is necessary, of course. But with embedded cuesheets, this means that a user must extract, fix up and re-embed his/her cuesheet after even a trivial filename change. Potentially, large portions of user music libraries consisting of flacs with embedded cues that scanned and played just fine with SC 7.3.x no longer play (or get artwork extracted) with 7.4 & 7.5.
Please attach a sample of these bad CUE sheets
ANY flac with an embedded cuesheet will exhibit this bug. Just rename the flac file. E.g. good.flac ==> renamed.flac renamed.flac gets it's metadata scanned, but it won't play back and artwork doesn't get extracted. So: download http://www.hegardtfoundation.org/flacstuff/linux_problem_flacs.zip Rename any of the flacs in that zip. Rescan. The renamed flacs don't work.
SBS 7.4/7.5 no longer uses Dan's Audio::FLAC::Header.pm since Andy's new Audio::Scan code fills that function. But if you look at Dan's Audio::FLAC::Header.pm from SqueezeCenter 7.3.4, line 507, you can see where he overrides the embedded cuesheet's FILE value with the real filename: # metaflac uses "dummy.wav" but we're going to use the actual filename # this will help external parsers that have to associate the resulting # cuesheet with this flac file. push (@$cuesheet, "FILE \"" . basename("$self->{'filename'}") ."\" FLAC\n"); I'm guessing that if Andy fixes this in Audio:Scan, he can undo the fix he applied to FLAC.pm in svn 29237. I'm guessing that bug 14386 is actually dependent on this bug.
Well that's a nasty way of fixing it. Better to just ignore the FILE value if the cue sheet is embedded.
I'm not sure I could recognize the difference between nasty vs elegant perl code even if it hit me in the face, wrapped up in newspaper along with a wet fish. For that, I defer to you and your professional colleagues.
I'm going cross-eyed trying to figure out the differences between the SC7.3.x and SBS 7.4/5 code. It seems as if embedded cuesheets are getting parsed at least three times for each file being scanned! It also seems like tracks are being formed by trying to meld together data from the native flac cuesheet metadata block and the embedded cuesheet in the vorbis comments. Anyway, doesn't Andy's AudioScan Flac.c code at line 622: av_push( cue, newSVpvf("FILE \"%s\" FLAC\n", flac->file) ); ..essentially do the same thing as Dan's code from Header.pm that I cited above? I just can't figure out where the real filename FILE value is getting overwritten by the bad FILE value from the cuesheet data. Seemingly, that should be happening in Formats::Playlists::CUE::parse. But there really isn't any substantive difference between the 7.3 & 7.4 code there. This code gives me a headache. I can understand why getting someone to be interested in patching it is like pulling teeth.
Don't worry, I'll fix it. I think we can just rip out all handling of FILE for embedded cuesheets.
I want to amend my last comment: the perl code is giving me the headache. AudioScan flac.c, ogg.c and common.c are a joy to behold.
OK: my ugly sledgehammer approach, but this seems to work: Slim::Formats::FLAC.pm, line 134, change: my $tracks = Slim::Formats::Playlists::CUE->parse($cuesheet, dirname($file), 1); --to-- my $tracks = Slim::Formats::Playlists::CUE->parse($cuesheet, dirname($file), $file); Slim::Formats::Playlists::CUE.pm, line 96, change: $filename = $1; --to-- $filename = $embedded || $1; And at Slim::Formats::Playlists::CUE.pm, line 106, make the same change: $filename = $1; --to-- $filename = $embedded || $1; These fixes seem to solve the problem, testing on Fedora 11, ext3 fs. I don't really have a way to run svn code on windows here, so I don't know if this will break things there. The above fixes also seem to solve bug 14386. So, Slim::Formats::FLAC.pm, line 130 can be commented out.
Additional tests using a mixture of flacs with embedded cuesheets, and external cuesheets pointing to tagless flacs or wav files show that the above fix works 100% of the time with both formats.
== Auto-comment from SVN commit #29282 to the slim repo by andy == == https://svn.slimdevices.com/slim?view=revision&revision=29282 == Fixed bug 15105, patch from Gordon Harris to ignore FILE value in embedded FLAC cue sheets
Thanks, Andy. Any chance that change could make it into 7.4 trunk as well? With my testing in 7.4.2, it fixes the problem without seeming to cause any others.
Sure I can back-port it. I'm not sure we have any plans to release 7.4.2 though.
== Auto-comment from SVN commit #29296 to the slim repo by andy == == https://svn.slimdevices.com/slim?view=revision&revision=29296 == Bug 15105, backport patch from 7.5
Gordon, Andy, thank you for the fix. 7.4.2 - r29296 works for me. And I'm happily back to the 7.4
*** Bug 15381 has been marked as a duplicate of this bug. ***
This bug has been marked fixed in a released version of Squeezebox Server or the accompanying firmware or mysqueezebox.com release. If you are still seeing this issue, please let us know!