Bug 4214 - CUE-sheets with multiple FILEs handled incorrectly
: CUE-sheets with multiple FILEs handled incorrectly
Status: RESOLVED WONTFIX
Product: Logitech Media Server
Classification: Unclassified
Component: Playlists
: 6.5.0
: All All
: P2 normal (vote)
: Future
Assigned To: Unassigned bug - please assign me!
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2006-09-24 10:24 UTC by Pete
Modified: 2009-09-08 09:14 UTC (History)
1 user (show)

See Also:
Category: ---


Attachments
Save FILE after encountering TRACK (994 bytes, patch)
2006-09-24 10:25 UTC, Pete
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Pete 2006-09-24 10:24:16 UTC
Slim/Formats/Playlists/CUE.pm is suppose to allow for CUE-sheets with multiple FILE statements (see bug 2126), for example:

FILE "first.wav" WAVE
  TRACK 01 AUDIO
    TITLE "first"
    INDEX 01 00:00:00
FILE "second.wav" WAVE
  TRACK 02 AUDIO
    TITLE "second"
    INDEX 01 00:00:00


However, due to the code:
        if ( $line =~ /^TITLE [...]/i ) {
        [...]
        } elsif (defined $currtrack and defined $filename) {
            # Each track in a cue sheet can have a different
            # filename. See Bug 2126 &
            # http://www.hydrogenaudio.org/forums/index.php?act=ST&f=20&t=4586
            $tracks->{$currtrack}->{'FILENAME'} = $filename;
        }
the filename is updated only if an unused line is encountred in the CUE file (such as "REM IGNORE ME"), otherwise the last FILE is used.

Judging by the examples at http://www.hydrogenaudio.org/forums/index.php?act=ST&f=20&t=4586 I suppose a FILE is suppose to refer to all TRACK statements that follow it, and hence, the update of $tracks->{$currtrack}->{'FILENAME'} should be in the code handling the TRACK-line, like in the attached patch.

Furthermore, it appears as if there are other mistakes: the code relating to bug 2668 ("Also - check the original file for ...") only appears to work with the last encountered file, which is incorrect (different files may have different artists, or whatever). In addition to this, I think the $lastpos calculations are relevant only if the TRACKs refer to the same FILE, otherwise $track->secs() should be used. (I hope I got this right, it's quite tricky to read.)

Perhaps a rewrite of the code, with multiple FILEs in mind from the start would be a good idea?
Comment 1 Pete 2006-09-24 10:25:43 UTC
Created attachment 1583 [details]
Save FILE after encountering TRACK
Comment 2 Pete 2006-09-27 06:58:32 UTC
There's (at least) one other bug in CUE.pm, unrelated to the FILE-handling: when reading attributes from file or from $cuesheet the fields DISC, DISCC and COMMENT are ignored. The "qw(ARTIST ALBUM YEAR ...)" lists should be changed accordingly (two places).

Not sure if it's worth cluttering bugzilla for this one?
Comment 3 Dan Sully 2006-10-01 22:23:54 UTC
This is too large of a change to consider for 6.5.1

I agree, CUE sheet support is fragile.
Comment 4 Pete 2006-10-01 23:57:32 UTC
I'll see if I can manage some kind of rewrite myself, hopefully this weekend.

I just have to figure out what priority to give the tags (read from global cue, file-local cue and file itself), I'm not sure that the current method carries over since it assumes, as far as tags are concerned, only one FILE.

I do think the change in comment #2 can be considered "small and safe" though.
Comment 5 Chris Owens 2007-10-22 09:35:46 UTC
QA to reproduce to see if this still happens after the 7.0 CUE changes
Comment 6 Chris Owens 2008-01-13 14:42:46 UTC
This still appears to be a real bug.  Setting to unassigned so we can review it at the next bug review meeting.
Comment 7 Blackketter Dean 2008-01-14 09:17:06 UTC
Pete if you can come up with a comprehensive patch we'd love to see it.

Also: can you educate us on the importance of multiple-file CUE sheets?  This isn't something we've seen in the wild.
Comment 8 Andy Grundman 2008-01-14 09:19:31 UTC
Please see bug 5735, I put in a patch to ignore cue sheets with multiple FILE tags.  These kind of cue sheets don't make sense to me, why do you want to use them?  Split your cue sheets up to be 1 FILE per cue.
Comment 9 Pete 2008-01-19 02:42:26 UTC
I believe .CUE-sheets with multiple FILE:s are quite uncommon and, as Andy says, they can always be split to one FILE per .CUE, at the price of repeating all albums tags. Hence, Andy's change 15191 is fine by me, so closing this one with resolution WONTFIX makes sense.

Regarding rewrites: I actually did some local changes of a now outdated version to suit my needs, but never got around to submitting them (Logitech got in the way). I'll see how the release of version 7 works for me before I dive in again.