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

music21/musicxml/m21ToXml.py

@@ -1490,6 +1490,7 @@ def parse(self):
             # before attempting to identify and count instruments
             self._populatePartExporterList()
             self.groupsToJoin = self.joinableGroups()
+            self.prePartProcess()
             self.parsePartlikeScore()
         else:
             self.parseFlatScore()
@@ -1735,6 +1736,66 @@ def parseFlatScore(self):
         pp.parse()
         self.partExporterList.append(pp)
 
+    def prePartProcess(self):
+        '''
+        Scans the parts/measures/voices in each joinable group to compute
+        the voiceNumberOffset needed in each part.  Parts that aren't in a
+        joinable group get offset 0.
+
+        Called automatically by .parse()
+        '''
+        partExporterForPart: t.Dict[stream.Part, PartExporter] = {}
+
+        # Compute partExp.staffGroup and partExp.maxNumVoices
+        for partExp in self.partExporterList:
+            partExporterForPart[partExp.stream] = partExp
+
+            joinableGroup = None
+            for sg in self.groupsToJoin:
+                if partExp.stream in sg:
+                    joinableGroup = sg
+                    break
+
+            partExp.staffGroup = joinableGroup
+            if partExp.staffGroup is None:
+                partExp.maxVoiceNum = 0
+                continue
+
+            # part is in a staffGroup, so we need to find the maximum number
+            # of voices used in any measure in this part.  We also find the
+            # maximum "low enough to not be considered a memory location"
+            # voice id.  The maxVoiceNum for a part is the higher of
+            # maxNumVoices-1 and maxNonMemLocationVoiceId.
+            maxNumVoices = 0
+            maxNonMemLocationVoiceId = 0
+            for measure in partExp.stream[stream.Measure]:
+                voices = tuple(measure[stream.Voice])
+                maxNumVoices = max(maxNumVoices, len(voices))
+                for voice in voices:
+                    if (isinstance(voice.id, int)
+                            and voice.id < defaults.minIdNumberToConsiderMemoryLocation):
+                        maxNonMemLocationVoiceId = max(maxNonMemLocationVoiceId, voice.id)
+            partExp.maxVoiceNum = max(maxNumVoices - 1, maxNonMemLocationVoiceId)
+
+        # Compute partExp.voiceNumberOffset
+        for partExp in self.partExporterList:
+            if partExp.staffGroup is None:
+                partExp.voiceNumberOffset = 0
+                continue
+
+            staffGroup = partExp.staffGroup
+
+            staffGroupStreamList = staffGroup.getSpannedElements()
+            partIndexInGroup = staffGroupStreamList.index(partExp.stream)
+            voiceNumberOffset = 0
+            # add up the offsetForPart of each part before us in the group
+            for i in range(0, partIndexInGroup):
+                pex = partExporterForPart[staffGroup[i]]
+                offsetForPart = pex.maxVoiceNum + 1
+                voiceNumberOffset += offsetForPart
+
+            partExp.voiceNumberOffset = voiceNumberOffset
+
     def postPartProcess(self):
         '''
         calls .joinPartStaffs() from the
@@ -2548,6 +2609,18 @@ def __init__(self,
         # has changed
         self.lastDivisions = None
 
+        # voiceNumberOffset is the offset that must be applied to any voice numbers automatically
+        # generated in this part, in order to avoid voice numbers (either explicitly set, or
+        # automatically generated) in other parts in this staffGroup.  This is computed in a
+        # pre-pass over all the voices in all the measures in all the parts (in ScoreExporter).
+        self.voiceNumberOffset: int = 0
+
+        # The maximum voiceNum found in the voices in all the measures in this part.
+        self.maxVoiceNum: int = 0
+
+        # The staffGroup to which this part belongs (if it belongs to one)
+        self.staffGroup: t.Optional[layout.StaffGroup] = None
+
         self.spannerBundle = partObj.spannerBundle
 
     def parse(self):
@@ -3020,7 +3093,7 @@ 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.rbSpanners: t.List[spanner.RepeatBracket] = []  # repeatBracket spanners
 
@@ -3076,6 +3149,18 @@ 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.  This voice number offset is pre-computed for each part before exporting
+        any measures. So we just get it from the parent partExporter here.
+        '''
+        partExp: t.Optional[PartExporter] = self.parent
+        if partExp is None:
+            return 0
+        return partExp.voiceNumberOffset
+
     def parseFlatElements(self, m, *, backupAfterwards=False):
         '''
         Deals with parsing all the elements in .elements, assuming that .elements is flat.
@@ -3096,8 +3181,12 @@ def parseFlatElements(self, m, *, backupAfterwards=False):
             if isinstance(m.id, int) and m.id < defaults.minIdNumberToConsiderMemoryLocation:
                 voiceId = m.id
             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
+                # add an offset to make it unique across the entire staffGroup.
                 voiceId = self.nextFreeVoiceNumber
                 self.nextFreeVoiceNumber += 1
+                voiceId += self.getVoiceNumberOffset()
             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()
+        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()
+        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, '1')
+                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, '2')
+                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, '3')
+                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, '4')
+                self.assertEqual(mxNote.find('staff').text, '2')
+
     def testJoinPartStaffsE(self):
         '''
         Measure numbers existing only in certain PartStaffs: don't collapse together