Bug 468 - setting "Artwork" format may cause problems with flac files that contain embedded cuesheets
: setting "Artwork" format may cause problems with flac files that contain embe...
Status: CLOSED FIXED
Product: Logitech Media Server
Classification: Unclassified
Component: Formats
: 5.x or older
: PC All
: P2 normal (vote)
: ---
Assigned To: Blackketter Dean
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2004-08-02 15:40 UTC by michael
Modified: 2008-12-18 11:55 UTC (History)
0 users

See Also:
Category: ---


Attachments
fix with acceptAll (1.26 KB, text/plain)
2004-08-02 23:39 UTC, KDF
Details
patch for %ARTWORK problem w/ fec files (797 bytes, patch)
2004-08-12 15:21 UTC, michael
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description michael 2004-08-02 15:40:10 UTC
Setting a format in Server Settings->Interface->Artwork may confuse parsing of
flac files with embedded cuesheets. The server will endless loop over parsing
the first track of the flac file. 
Clearing the "Artwork" format will cause the server to behave normally again.
(I haven't tested "Artwork Thumbnail" yet, but it may exhibit similar behaviour.)
Comment 1 KDF 2004-08-02 23:39:06 UTC
Created attachment 84 [details]
fix with acceptAll

this will allow an empty value, but doesn't do any validation at all. 
validating before didn't do much either, however.  it only required text.
Comment 2 KDF 2004-08-02 23:39:43 UTC
Comment on attachment 84 [details]
fix with acceptAll

hate that this skips ahead
Comment 3 KDF 2004-08-02 23:42:37 UTC
do you plan to fix this yourself?  if not, you might not want it assigned to
yourself.  Are you able to locate a more specific location where it loops?
Comment 4 michael 2004-08-03 12:42:36 UTC
yeah, this was my chunk of code, so I figured I'd take a first stab at it.
Though if someone else has a fix handy, I certainly won't complain
Comment 5 michael 2004-08-12 14:43:15 UTC
so here's what's happening...

readTags() will open the .flac file, and discover that it contains a cuesheet.
that cuesheet is passed to parseCUE(). then parseCUE() will call readTags for
each fragment (individual song) in the cuesheet (as a file url containing an
anchor to specify the fragment being handled). mmmmm.... recursion. readTags()
will gather the tags for this fragment, then try to readCoverArtFiles(). if a
content based file name format is specified (e.g. %Foo) readCoverArtFiles()
strips the file url down to it's base filename, and tries to lookup info about
it. since the original call to readTags() on the base file name hasn't returned
yet (still in recursion, handling the first song/fragment) you'll get a cache
miss, which then leads to readTags() being called on the base file name. voila!
infinite loop.

so, I think I need to change readCoverArtFiles() to do it's cache check on the
full file url (w/ anchor).
Comment 6 michael 2004-08-12 15:21:06 UTC
Created attachment 99 [details]
patch for %ARTWORK problem w/ fec files

proposed fix
Comment 7 michael 2004-08-12 15:22:36 UTC
sending to dean for applying proposed fix.
Comment 8 KDF 2004-08-20 14:42:39 UTC
seems ok to me.
Comment 9 Blackketter Dean 2004-08-20 15:28:10 UTC
Kevin, could you commit this?
Comment 10 KDF 2004-08-20 17:56:36 UTC
committed 20/08/04.  Please close this off once this is confirmed as working as
expected.
Comment 11 michael 2004-08-20 18:15:59 UTC
This is working now (as of current cvs).

Just for the record, I realize that there are other (less-critical) issues with
%ARTWORK and fec files, but this will at least keep the server from getting
stuck into an infinite loop while I flesh those out. (I'll open seperate tickets
for the others as I get to them.)

Thanks again!
Comment 12 James Richardson 2008-12-15 11:57:02 UTC
This bug has been fixed in the latest release of SqueezeCenter!

Please download the new version from http://www.slimdevices.com/su_downloads.html if you haven't already.  

If you are still experiencing this problem, feel free to reopen the bug with your new comments and we'll have another look.
Comment 13 Chris Owens 2008-12-18 11:55:01 UTC
Routine bug db maintenance; removing old versions which cause confusion.  I apologize for the inconvenience.