Bug#439790: Assumes MSF offset is always 150
Martin Michlmayr [EMAIL PROTECTED] writes: * Martin Michlmayr [EMAIL PROTECTED] [2007-08-29 12:09]: In order words, INDEX 00 00:00:00 INDEX 01 00:00:32 is printed for the first track. So I'd propose: Thanks, Martin, and sorry for not responding earlier. This looks good to me. I'm going to be pretty busy for a while, but probably some time next month I'll be able to get around to testing this and putting out a new release. -- Eric Gillespie * [EMAIL PROTECTED] -- To UNSUBSCRIBE, email to [EMAIL PROTECTED] with a subject of unsubscribe. Trouble? Contact [EMAIL PROTECTED]
Bug#439790: Assumes MSF offset is always 150
* Eric Gillespie [EMAIL PROTECTED] [2007-08-27 14:31]: I suppose this patch should fix it? Not tested, but I think it matches your original patch. I'm not sure why you removed the first TRACK 01 AUDIO line, but neither am I sure why it's there. I removed it but at the same time I decreased the counter, so the for loop would start with track 01 rather than 02. i.e. I simply removed the hard coded TRACK 01 AUDIO line in favour of printing it in the for loop. I did this because the hardcoded line always print 00:00 whereas my new code would take the offset into account (and in my case print TRACK 01... INDEX 00:00:32. Anyway, since the cue sheet specification wasn't clear how to handle this situation, I tried Exact Audio Copy, which is apparantly taken as a standard for cue sheet by many. It generates the following cue sheet: REM DISCID E5108212 REM COMMENT ExactAudioCopy v0.95b4 PERFORMER Unknown Artist TITLE Unknown Title FILE Range.wav WAVE TRACK 01 AUDIO TITLE Track01 PERFORMER Unknown Artist INDEX 00 00:00:00 INDEX 01 00:00:32 TRACK 02 AUDIO TITLE Track02 PERFORMER Unknown Artist INDEX 01 03:05:65 TRACK 03 AUDIO TITLE Track03 PERFORMER Unknown Artist INDEX 01 07:40:35 TRACK 04 AUDIO TITLE Track04 PERFORMER Unknown Artist INDEX 01 12:34:37 TRACK 05 AUDIO TITLE Track05 PERFORMER Unknown Artist INDEX 01 16:38:00 TRACK 06 AUDIO TITLE Track06 PERFORMER Unknown Artist INDEX 01 19:55:67 TRACK 07 AUDIO TITLE Track07 PERFORMER Unknown Artist INDEX 01 25:00:47 TRACK 08 AUDIO TITLE Track08 PERFORMER Unknown Artist INDEX 01 29:29:25 TRACK 09 AUDIO TITLE Track09 PERFORMER Unknown Artist INDEX 01 30:18:50 TRACK 10 AUDIO TITLE Track10 PERFORMER Unknown Artist INDEX 01 34:40:02 TRACK 11 AUDIO TITLE Track11 PERFORMER Unknown Artist INDEX 01 38:19:27 TRACK 12 AUDIO TITLE Track12 PERFORMER Unknown Artist INDEX 01 41:40:00 TRACK 13 AUDIO TITLE Track13 PERFORMER Unknown Artist INDEX 01 45:30:60 TRACK 14 AUDIO TITLE Track14 PERFORMER Unknown Artist INDEX 01 48:35:40 TRACK 15 AUDIO TITLE Track15 PERFORMER Unknown Artist INDEX 01 52:03:52 TRACK 16 AUDIO TITLE Track16 PERFORMER Unknown Artist INDEX 01 57:02:45 TRACK 17 AUDIO TITLE Track17 PERFORMER Unknown Artist INDEX 01 61:46:35 TRACK 18 AUDIO TITLE Track18 PERFORMER Unknown Artist INDEX 01 66:14:35 In order words, INDEX 00 00:00:00 INDEX 01 00:00:32 is printed for the first track. -- Martin Michlmayr http://www.cyrius.com/ -- To UNSUBSCRIBE, email to [EMAIL PROTECTED] with a subject of unsubscribe. Trouble? Contact [EMAIL PROTECTED]
Bug#439790: Assumes MSF offset is always 150
* Martin Michlmayr [EMAIL PROTECTED] [2007-08-29 12:09]: In order words, INDEX 00 00:00:00 INDEX 01 00:00:32 is printed for the first track. So I'd propose: Index: fa-rip === --- fa-rip (revision 2155) +++ fa-rip (working copy) @@ -76,21 +76,21 @@ from org.diplodocus.util import catch_EnvironmentError as c +# http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=439790 +MSF_OFFSET = 150 + def mkcue(disc, trackcount=None): fp = c(file, 'cue', 'w') c(fp.write, 'FILE dummy.wav WAVE\n') -c(fp.write, ' TRACK 01 AUDIO\n') -c(fp.write, 'INDEX 01 00:00:00\n') if trackcount == None: trackcount = disc.lastTrackNum else: trackcount = min(trackcount, disc.lastTrackNum) -pregap = disc.tracks[0][0] -for i in xrange(disc.firstTrackNum, trackcount): -offset = disc.tracks[i][0] -offset -= pregap +for i in xrange(disc.firstTrackNum, trackcount+1): +offset = disc.tracks[i-1][0] +offset -= MSF_OFFSET minutes = seconds = 0 sectors = offset % 75 @@ -100,7 +100,9 @@ minutes = seconds / 60 seconds = seconds % 60 -c(fp.write, ' TRACK %02d AUDIO\n' % (i + 1,)) +c(fp.write, ' TRACK %02d AUDIO\n' % (i,)) +if i == 1 and offset 0: +c(fp.write, 'INDEX 00 00:00:00\n' c(fp.write, 'INDEX 01 %02d:%02d:%02d\n' % (minutes, seconds, sectors)) -- Martin Michlmayr http://www.cyrius.com/ -- To UNSUBSCRIBE, email to [EMAIL PROTECTED] with a subject of unsubscribe. Trouble? Contact [EMAIL PROTECTED]
Bug#439790: Assumes MSF offset is always 150
Package: mkcue Version: 1-2 Severity: important As a comment in mkcue.cc says, mkcue assumes that the first song always starts after 150 frames. However, this assumption is wrong. As I understand it (from investigating this issue as it relates to jack, see #319901), every CD has 150 frames that have to be ignored (something known as the MSF offset). However, there is no guarantee that a song starts exactly after 150 frames - it can start later. When this happens, mkcue generates a bad cue file. As an effect of this, a wrong FreeDB ID will also be generated. In my case this means that the FreeDB ID fb108212 instead of e5108212 will be computed. You can see this pregap e.g. with cdparanoia. cdparanoia -Q shows: tracklength begincopy pre ch === 1.13908 [03:05.33] 32 [00:00.32]no no 2 2.20595 [04:34.45]13940 [03:05.65]no no 2 i.e. cdinfo.FrameOffset[1] will have 182 (150 + 32). The solution is to decrease cdinfo.FrameOffset[track] by 150 rather than by cdinfo.FrameOffset[1]. With this change, my cue sheet looks like this, which is better: | TRACK 01 AUDIO |INDEX 01 00:00:32 (see the patch below.) There is one question though which I have not been able to answer fully yet. How should this pregap of 32 frames be shown in the cue sheet? It seems as if there were two options: 1) use INDEX 00, as in: | TRACK 01 AUDIO |INDEX 00 00:00:00 |INDEX 01 00:00:32 2) use PREGAP, as in: | TRACK 01 AUDIO |PREGAP 00:00:32 |INDEX 01 00:00:32 http://digitalx.org/cuesheetsyntax.php is not clear about which version to favour and I don't know how I could ask. --- mkcue-1.orig/mkcue.cc 2004-10-27 08:20:30.0 +0200 +++ mkcue-1/mkcue.cc2007-08-27 13:38:19.0 +0200 @@ -54,16 +54,13 @@ } puts(FILE \dummy.wav\ WAVE); -puts( TRACK 01 AUDIO); -puts(INDEX 01 00:00:00); #define min(x, y) ((x) (y) ? (x) : (y)) -for (track = cdinfo.FirstTrack + 1; +#define MSF_OFFSET 150 +for (track = cdinfo.FirstTrack; track = min(trackcount, cdinfo.LastTrack); track++) { -/* There is frequently (always?) an offset of 150 sectors, so - * subtract the first track's offset. */ -cdinfo.FrameOffset[track] -= cdinfo.FrameOffset[1]; +cdinfo.FrameOffset[track] -= MSF_OFFSET; minutes = seconds = sectors = 0; sectors = cdinfo.FrameOffset[track] % 75; -- System Information: Debian Release: 4.0 APT prefers stable APT policy: (500, 'stable') Architecture: i386 (i686) Shell: /bin/sh linked to /bin/bash Kernel: Linux 2.6.18-4-686 Locale: LANG=en_US.UTF-8, LC_CTYPE=en_US.UTF-8 (charmap=UTF-8) -- Martin Michlmayr http://www.cyrius.com/ -- To UNSUBSCRIBE, email to [EMAIL PROTECTED] with a subject of unsubscribe. Trouble? Contact [EMAIL PROTECTED]
Bug#439790: Assumes MSF offset is always 150
Martin Michlmayr [EMAIL PROTECTED] writes: Package: mkcue Version: 1-2 Severity: important As a comment in mkcue.cc says, mkcue assumes that the first song always starts after 150 frames. However, this assumption is wrong. As you might guess from that comment, I'm no expert in this area, and am not surprised I got it wrong. I use libmusicbrainz to make cue sheets now, and have abandoned mkcue. It's a dead package, unless someone else wants to take it up. -- Eric Gillespie * [EMAIL PROTECTED] -- To UNSUBSCRIBE, email to [EMAIL PROTECTED] with a subject of unsubscribe. Trouble? Contact [EMAIL PROTECTED]
Bug#439790: Assumes MSF offset is always 150
* Eric Gillespie [EMAIL PROTECTED] [2007-08-27 13:57]: As you might guess from that comment, I'm no expert in this area, and am not surprised I got it wrong. I use libmusicbrainz to make cue sheets now, and have abandoned mkcue. It's a dead package, unless someone else wants to take it up. Out of interest, how does one use libmusicbrainz for cue sheets and does it get it right? I looked at your flac-archive script, and while it uses some libmusicbrainz libraries, the cue sheet generation is quite similar to mkcue (and has the same bug). Martin, looking for a good solution to generate cue sheets. -- Martin Michlmayr http://www.cyrius.com/ -- To UNSUBSCRIBE, email to [EMAIL PROTECTED] with a subject of unsubscribe. Trouble? Contact [EMAIL PROTECTED]
Bug#439790: Assumes MSF offset is always 150
Martin Michlmayr [EMAIL PROTECTED] writes: Out of interest, how does one use libmusicbrainz for cue sheets and does it get it right? I looked at your flac-archive script, and while it uses some libmusicbrainz libraries, the cue sheet generation is quite similar to mkcue (and has the same bug). Heh, I'm not surprised I made the same mistake. I suppose you were looking at fa-rip, but probably not the latest version. See svn://diplodocus.org/projects/audio/flac-archive/main . I really need to make a new release... I suppose this patch should fix it? Not tested, but I think it matches your original patch. I'm not sure why you removed the first TRACK 01 AUDIO line, but neither am I sure why it's there. I wrote that a long time ago, based on something I saw somewhere, without actually understanding this stuff. Thanks. Index: fa-rip === --- fa-rip (revision 2155) +++ fa-rip (working copy) @@ -76,6 +76,9 @@ from org.diplodocus.util import catch_EnvironmentError as c +# http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=439790 +MSF_OFFSET = 150 + def mkcue(disc, trackcount=None): fp = c(file, 'cue', 'w') c(fp.write, 'FILE dummy.wav WAVE\n') @@ -87,10 +90,9 @@ else: trackcount = min(trackcount, disc.lastTrackNum) -pregap = disc.tracks[0][0] for i in xrange(disc.firstTrackNum, trackcount): offset = disc.tracks[i][0] -offset -= pregap +offset -= MSF_OFFSET minutes = seconds = 0 sectors = offset % 75 -- To UNSUBSCRIBE, email to [EMAIL PROTECTED] with a subject of unsubscribe. Trouble? Contact [EMAIL PROTECTED]