Bug#439790: Assumes MSF offset is always 150

2007-09-05 Thread Eric Gillespie
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

2007-08-29 Thread Martin Michlmayr
* 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

2007-08-29 Thread Martin Michlmayr
* 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

2007-08-27 Thread Martin Michlmayr
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

2007-08-27 Thread Eric Gillespie
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

2007-08-27 Thread Martin Michlmayr
* 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

2007-08-27 Thread Eric Gillespie
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]