Fix issue #1335: voice numbers written in MusicXML must be unique…

music21/musicxml/m21ToXml.py

@@ -2548,6 +2548,10 @@ def __init__(self,
         # has changed
         self.lastDivisions = None
 
+        # maxVoiceNumber is keyed by measureNumberWithSuffix, value is the maximum
+        # exported voice number used for that measure (in this Part).
+        self.maxVoiceNumbers: t.Dict[str, int] = {}
+
         self.spannerBundle = partObj.spannerBundle
 
     def parse(self):
@@ -2613,6 +2617,7 @@ def parse(self):
             measureExporter.spannerBundle = self.spannerBundle
             try:
                 mxMeasure = measureExporter.parse()
+                self.maxVoiceNumbers[m.measureNumberWithSuffix()] = measureExporter.maxVoiceNumber
             except MusicXMLExportException as e:
                 e.measureNumber = str(m.number)
                 if isinstance(self.stream, stream.Part):
@@ -3020,7 +3025,9 @@ def __init__(self,
         self.measureOffsetStart = 0.0
         self.offsetInMeasure = 0.0
         self.currentVoiceId: t.Optional[int] = None
-        self.nextFreeVoiceNumber = 1
+        self.nextFreeVoiceNumber: int = 1
+        self.maxVoiceNumber: int = 0
+        self.voiceNumberOffset: t.Optional[int] = None
 
         self.rbSpanners: t.List[spanner.RepeatBracket] = []  # repeatBracket spanners
 
@@ -3076,6 +3083,77 @@ def mainElementsParse(self):
             # Assumes voices are flat...
             self.parseFlatElements(v, backupAfterwards=backupAfterwards)
 
+    def getVoiceNumberOffset(self) -> int:
+        '''
+            Returns the offset that should be added to any voice numbers in this measure,
+            in order to avoid voice numbers in other simultaneous measures within a staff
+            group.  If this measure is not in a staff group, the offset is 0.
+            This is computed once, and cached, for performance.
+        '''
+        if self.voiceNumberOffset is not None:
+            # return the cached computed offset
+            return self.voiceNumberOffset
+
+        def getMaxVoiceNumber(
+                scoreExp: ScoreExporter,
+                partStaff: stream.Part,
+                measure: stream.Measure) -> int:
+            # find the partExporter for partStaff
+            partStaffExp: t.Optional[PartExporter] = None
+            for partx in scoreExp.partExporterList:
+                if partx.stream is partStaff:
+                    partStaffExp = partx
+                    break
+            if partStaffExp is None:
+                return 0
+
+            maxVoiceNumber: int = partStaffExp.maxVoiceNumbers.get(
+                measure.measureNumberWithSuffix(),
+                0
+            )
+            return maxVoiceNumber
+
+        voiceNumberOffset: int = 0
+
+        partExp: t.Optional[PartExporter] = self.parent
+        if partExp is None:
+            return voiceNumberOffset
+
+        scoreExp: t.Optional[ScoreExporter] = partExp.parent
+        if scoreExp is None:
+            return voiceNumberOffset
+
+        # scoreExp.groupsToJoin is a list of StaffGroups
+        if not scoreExp.groupsToJoin:
+            return voiceNumberOffset
+
+
+        assert(isinstance(self.stream, stream.Measure))
+        myMeasure: stream.Measure = self.stream
+
+        for group in scoreExp.groupsToJoin:
+            if partExp.stream not in group:
+                continue
+
+            for partStaff in group:
+                if partStaff is partExp.stream:
+                    # skip our own PartStaff
+                    continue
+
+                otherMeasure: stream.Measure = partStaff.measure(
+                    myMeasure.measureNumberWithSuffix()
+                )
+                if otherMeasure is None:
+                    # if measures don't have numbers, we can't do this
+                    continue
+
+                maxVoiceNumber: int = getMaxVoiceNumber(scoreExp, partStaff, otherMeasure)
+                voiceNumberOffset = max(voiceNumberOffset, maxVoiceNumber)
+
+        # cache and return the new computed offset
+        self.voiceNumberOffset = voiceNumberOffset
+        return voiceNumberOffset
+
     def parseFlatElements(self, m, *, backupAfterwards=False):
         '''
         Deals with parsing all the elements in .elements, assuming that .elements is flat.
@@ -3094,10 +3172,20 @@ def parseFlatElements(self, m, *, backupAfterwards=False):
         if isinstance(m, stream.Voice):
             m: stream.Voice
             if isinstance(m.id, int) and m.id < defaults.minIdNumberToConsiderMemoryLocation:
+                # this voice id is a low number, assigned by someone for a reason.  We should
+                # not change it, but we will avoid it if we need to change some other voice id.
                 voiceId = m.id
+                self.maxVoiceNumber = max(self.maxVoiceNumber, voiceId)
             elif isinstance(m.id, int):
+                # this voice id is actually a memory location, so we need to change it
+                # to a low number so it can be used in MusicXML.  Note that we need to
+                # carefully make it unique across the entire part (i.e. if the part is
+                # actually a group of PartStaffs, it must be unique across all those
+                # PartStaffs, not just this one).
                 voiceId = self.nextFreeVoiceNumber
                 self.nextFreeVoiceNumber += 1
+                voiceId += self.getVoiceNumberOffset()
+                self.maxVoiceNumber = max(self.maxVoiceNumber, voiceId)
             else:
                 voiceId = m.id
         else:

music21/musicxml/partStaffExporter.py

@@ -889,6 +889,12 @@ def testJoinPartStaffsB(self):
         s.insert(0, layout.StaffGroup([ps1, ps2]))
         root = self.getET(s)
         notes = root.findall('.//note')
+
+        # since there are no voices in either PartStaff, the voice number of each note
+        # should be the same as the staff number.
+        for mxNote in notes:
+            self.assertEqual(mxNote.find('voice').text, mxNote.find('staff').text)
+
         forward = root.find('.//forward')
         backup = root.find('.//backup')
         amountToBackup = (
@@ -945,6 +951,62 @@ def testJoinPartStaffsD(self):
         self.assertEqual(len(measures), 2)
         self.assertEqual(len(notes), 12)
 
+    def testJoinPartStaffsD2(self):
+        '''
+        Add measures and voices and check for unique voice numbers across the StaffGroup.
+        '''
+        from music21 import layout
+        from music21 import note
+        s = stream.Score()
+        ps1 = stream.PartStaff()
+        m1 = stream.Measure(1)
+        ps1.insert(0, m1)
+        v1 = stream.Voice()
+        v2 = stream.Voice()
+        m1.insert(0, v1)
+        m1.insert(0, v2)
+        v1.repeatAppend(note.Note('C4'), 4)
+        v2.repeatAppend(note.Note('E4'), 4)
+        ps1.makeNotation(inPlace=True)  # makeNotation to freeze notation
+
+        ps2 = stream.PartStaff()
+        m2 = stream.Measure(1)
+        ps2.insert(0, m2)
+        v3 = stream.Voice()
+        v4 = stream.Voice()
+        m2.insert(0, v3)
+        m2.insert(0, v4)
+        v3.repeatAppend(note.Note('C3'), 4)
+        v4.repeatAppend(note.Note('G3'), 4)
+        ps2.makeNotation(inPlace=True)  # makeNotation to freeze notation
+
+        s.insert(0, ps2)
+        s.insert(0, ps1)
+        s.insert(0, layout.StaffGroup([ps1, ps2]))
+        root = self.getET(s)
+        measures = root.findall('.//measure')
+        notes = root.findall('.//note')
+        # from music21.musicxml.helpers import dump
+        # dump(root)
+        self.assertEqual(len(measures), 1)
+        self.assertEqual(len(notes), 16)
+
+        # check those voice and staff numbers
+        for mxNote in notes:
+            mxPitch = mxNote.find('pitch')
+            if mxPitch.find('step').text == 'C' and mxPitch.find('octave').text == '4':
+                self.assertEqual(mxNote.find('voice').text, '3')
+                self.assertEqual(mxNote.find('staff').text, '1')
+            elif mxPitch.find('step').text == 'E' and mxPitch.find('octave').text == '4':
+                self.assertEqual(mxNote.find('voice').text, '4')
+                self.assertEqual(mxNote.find('staff').text, '1')
+            elif mxPitch.find('step').text == 'C' and mxPitch.find('octave').text == '3':
+                self.assertEqual(mxNote.find('voice').text, '1')
+                self.assertEqual(mxNote.find('staff').text, '2')
+            elif mxPitch.find('step').text == 'G' and mxPitch.find('octave').text == '3':
+                self.assertEqual(mxNote.find('voice').text, '2')
+                self.assertEqual(mxNote.find('staff').text, '2')
+
     def testJoinPartStaffsE(self):
         '''
         Measure numbers existing only in certain PartStaffs: don't collapse together