Created attachment 100 [details]
preserve content type

here's a fix

this patch wont work becuase the artwork routines need the CT defined prior to
the check for artwork.  If CT does not exist, it will lock into deep recursion
and lock the server.

perhaps the proposed fix for bug469 will avoid the infinite loop here as well?
if not, perhaps an assumed CT will work within the readCoverartFiles() and
infoFormat.

sorry...that's bug468.  also, in regards to the patch for this bug, attachment
100 [details], I think the added CT line is redundant since the CT is cached if not
defined only a few lines above where this is proposed to be added.

Created attachment 109 [details]
combined patch

This seems to work for me.  no deep recursion on cover art.  hopefully this
allows flac cue sheets to work and without artwork interference (bug468)

> this patch wont work becuase the artwork routines need the CT defined prior to
> the check for artwork.

CT is still defined before the artwork routines are called. We just don't need
to do it twice.

The combined patch you posted (109) still removes one of the redundant settings
of CT (which is good) but loses the conditional 
$tempCacheEntry->{'CT'} = $type unless defined $tempCacheEntry->{'CT'};
(not so good).

the problem here is that flac files with embedded cuesheets are acting as both a
playlist and as audio data. Part of the process for dealing with this split
personality involves setting their CT as something different than a straight
flac file. This means that in this case CT is defined as something different
than what $type is set to. ($type as its used here is set based solely on the
files .flac suffix.) Blindly using
$tempCacheEntry->{'CT'} = $type;
will break things. It needs the "... unless defined ..." (or something
equivalent) added onto the end to avoid stomping on an existing value.

Please try it with the conditional as used in attachment 100 [details]. I don't believe it
breaks anything. If you find problems with it, let me know and we can look for
an alternate solution.

Thanks,

-michael

odd...in my copy of Info.pm I have 
	if (!defined($tempCacheEntry->{'CT'})) {
		$tempCacheEntry->{'CT'} = $type;
	}
at line 2181, which does exactly the same thing as the unless defined.

yes, that one is just fine. but look at the one on line 2140.

yes, and I already agreed that 2140 needs to be removed, but your patch adds
another place for assigning CT when there is already an if !defined assignment
done at 2181. I'm sure dean will sort it out.

it sounds like we might be arguing the same points and just not communicating it
very well. :)

just to summarize the intent of the original patch, in case I screwed it up
somehow, and to help out whoever gets to sort this all out...
(line numbers based on unmodified current cvs co of Info.pm)

parsing of flacs with embedded cue sheets results in setting a CT that is
different than the file suffix derived $type, and this should be preserved.

on line 2132 there is an unconditional setting of CT that should be removed.

on line 2143 there in an unconditional setting of CT that should be converted
into a conditional (... unless defined ...)

on line 2185 there is a conditional setting of CT that is fine as it is.

I hope this helps clear up some of the confusion.

Thanks!

right, with you now :)

Me confused.  Is this the final patch?

michael's patch is the right one for this bug.

Ok, looks good to me.  Please apply.

righty ho.  I'll take care of this and 468 shortly.

committed 20/08/04. Michael, can you please confirm this works as you exepct and
if so, close this off.

it looks correct (in current cvs). it seems to be behaving properly again.

Thanks!

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.