Fails on CDs with CD-Text

[chirlu]

Here is an extract from a cdrdao TOC (of http://musicbrainz.org/release/f1c048a3-69cc-4906-9ce1-3bfbfc76f951):

// Track 1
TRACK AUDIO
NO COPY
NO PRE_EMPHASIS
TWO_CHANNEL_AUDIO
ISRC "DED811023010"
CD_TEXT {
  LANGUAGE 0 {
    TITLE "Get The Party Started"
    PERFORMER "VIVA VOCE"
    ISRC ""
  }
}
FILE "data.wav" 0 02:46:58

// Track 2
TRACK AUDIO
NO COPY
NO PRE_EMPHASIS
TWO_CHANNEL_AUDIO
ISRC "DED811023020"
CD_TEXT {
  LANGUAGE 0 {
    TITLE "Die Welt, ein Dorf"
    PERFORMER "VIVA VOCE"
    ISRC "DED811023020"
  }
}
FILE "data.wav" 02:46:58 04:00:59

As you’ll see, the CD has CD-Text which also includes ISRCs for each track (though, for some reason, it is not set on track 1). The parser doesn’t expect two lines containing “ISRC” and fails:

found new ISRC for track 1: DED811023010
Traceback (most recent call last):
  File "./isrcsubmit.py", line 658, in <module>
    track = tracks[trackNumber + trackOffset - 1]
TypeError: unsupported operand type(s) for +: 'NoneType' and 'int'

Probably the simplest fix would be to ignore all lines starting with a space.

[chirlu]

I was able to fix it with this change:

--- isrcsubmit.py   2012-08-29 14:08:03.000000000 +0200
+++ isrcsubmit_mod_f_cdtext.py  2012-08-29 16:17:09.000000000 +0200
@@ -313,7 +313,7 @@
             with open(tmpfile, "r") as toc:
                 for line in toc:
                     words = line.split()
-                    if len(words) > 0:
+                    if not line.startswith(" ") and len(words) > 0:
                         if words[0] == "//":
                             trackNumber = int(words[2])
                         elif words[0] == "ISRC":

Note that this is the very first time I “develop” in Python and I found out about the syntax by looking around in the script, so it’s certainly not optimal. (Why first split the line, then test whether to ignore it?)

[JonnyJD]

Sorry, I must have overlooked this issue/notification mail. Thank you for the contribution.

Now there is also pull request #18 and I received a patch per email. I will fix it quite soon.

[Freso]

Heh. I need to remember to check issues on GitHub. :)

Anyway, AFAICT, the proposed edit here simply ignores ISRCs stored in CD-TEXT. If there is a CD which doesn't use the "ISRC" field on its own but stores it in CD-TEXT->ISRC, this code change would prevent it from being used. My pull request just makes it disregard assigning ISRCs to trackNumber None. JonnyJD knows the code best though, so whatever he decides is the right way to fix it, I'll trust that decision.

[JonnyJD]

Jim Patterson sent me another case with ISRC in CD-Text, but with hyphens. http://musicbrainz.org/release/3e0b924d-ccaa-46f1-a29b-94fab98f4f4c

// Track 1
TRACK AUDIO
NO COPY
NO PRE_EMPHASIS
TWO_CHANNEL_AUDIO
ISRC "USSM19806736"
CD_TEXT {
  LANGUAGE 0 {
    TITLE "Ready To Run"
    PERFORMER "DIXIE CHICKS"
    DISC_ID ""
    ISRC "US-SM1-98-06736"
  }
}
FILE "data.wav" 0 03:52:40

// Track 2
TRACK AUDIO
NO COPY
NO PRE_EMPHASIS
TWO_CHANNEL_AUDIO
ISRC "USSM19901547"
CD_TEXT {
  LANGUAGE 0 {
    TITLE "If I Fall You're Going Down With Me"
    PERFORMER "DIXIE CHICKS"
    DISC_ID ""
    ISRC "US-SM1-99-01547"
  }
}
FILE "data.wav" 03:52:40 03:05:02

[JonnyJD]

Even more important: Track 13 on the same disc has an ISRC given only in CD-Text, but is not found "the normal way".

// Track 13
TRACK AUDIO
NO COPY
NO PRE_EMPHASIS
TWO_CHANNEL_AUDIO
CD_TEXT {
  LANGUAGE 0 {
    TITLE "Ain't No Thang But A Chickin' Wang"
    PERFORMER "DIXIE CHICKS"
    ISRC "US-SM1-99-07390"
  }
}
FILE "data.wav" 44:54:67 00:06:00

[Freso]

I've added a fix for the hyphens to my pull request. (Not tested though, so please do to make sure it actually works. :p)

And the case about the ISRC should be handled fine by my cahnge too, unlike the code in comment #1. In the pull request, it should just add the ISRC in the CD-TEXT to the trackNumber without any issue.

[JonnyJD]

Please tell me if there are still issues after my octupus merge.