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,139 @@ 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:
+                continue
+
+            # part is in a staffGroup, so we need to find the maximum voice
+            # number used in each measure in this part.  The maxVoiceNum
+            # for a measure is the higher of the number of voices and the
+            # maximum "low enough to not be considered a memory location"
+            # voice id.
+            for measure in partExp.stream[stream.Measure]:
+                voices = measure[stream.Voice]
+                maxNonMemLocationVoiceId: int = 0
+                for voice in voices:
+                    if (isinstance(voice.id, int)
+                            and voice.id < defaults.minIdNumberToConsiderMemoryLocation):
+                        maxNonMemLocationVoiceId = max(maxNonMemLocationVoiceId, voice.id)
+
+                # maxVoiceNum is 1-based (voice numbers are 1-based in MusicXML)
+                partExp.maxVoiceNumForMeasure[measure] = max(len(voices), maxNonMemLocationVoiceId)
+
+        # Compute partExp.voiceNumberOffsetForMeasure from the maxVoiceNumForMeasure
+        # of each measure "above" us in the staff group.
+        for partExp in self.partExporterList:
+            if partExp.staffGroup is None:
+                continue
+
+            staffGroupStreamList = partExp.staffGroup.getSpannedElements()
+            partIndexInGroup = staffGroupStreamList.index(partExp.stream)
+
+            for simultaneousMeasures in self.zipMeasures(staffGroupStreamList):
+                # add up the voice number offsets contributed by each measure before
+                # (above) us in the staff group
+                measure = simultaneousMeasures[partIndexInGroup]
+                if measure is None:
+                    continue
+                voiceNumberOffset = 0
+                for i in range(0, partIndexInGroup):
+                    m = simultaneousMeasures[i]
+                    if m is None:
+                        continue
+                    pex = partExporterForPart[staffGroupStreamList[i]]
+                    voiceNumberOffset += pex.maxVoiceNumForMeasure[m]
+                partExp.voiceNumberOffsetForMeasure[measure] = voiceNumberOffset
+
+    @staticmethod
+    def zipMeasures(
+        streams: t.Sequence[stream.Stream]
+    ) -> t.Iterator[t.List[t.Optional[stream.Measure]]]:
+        '''
+            A generator that takes a group of streams, and yields a list of one simultaneous
+            measure from each stream at a time (with None in the list when a stream doesn't
+            have a matching measure), where simultaneous means "has the same measure number".
+            If there are unnumbered measures, they are treated when encountered as if they
+            have measure number '0' (they actually do), so they are simply yielded in order
+            until they are exhausted. Then we continue with the next numbered measures as
+            before (if there are any).
+        '''
+        numStreams = len(streams)
+        if numStreams == 0:
+            return
+
+        def getSimultaneousMeasureIndices(
+                measures: t.Sequence[t.Optional[stream.Measure]]) -> t.List[int]:
+            output: t.List[int] = []
+
+            # Note: m.measureNumberWithSuffix() returns '0' for unnumbered measures
+            minMeasureNumber: t.Optional[str] = None
+            for m in measures:
+                if m is None:
+                    continue
+                if minMeasureNumber is None:
+                    minMeasureNumber = m.measureNumberWithSuffix()
+                    continue
+                if helpers.measureNumberComesBefore(m.measureNumberWithSuffix(), minMeasureNumber):
+                    minMeasureNumber = m.measureNumberWithSuffix()
+
+            for i, m in enumerate(measures):
+                if m is None:
+                    continue
+
+                if m.measureNumberWithSuffix() == minMeasureNumber:
+                    output.append(i)
+
+            return output
+
+        streamIterators = [s[stream.Measure] for s in streams]
+
+        # load up nextForZip with the first measure in each streamIterator
+        nextForZip: t.List[t.Optional[stream.Measure]] = [None] * numStreams
+        for snum, sIter in enumerate(streamIterators):
+            try:
+                nextForZip[snum] = next(sIter)
+            except StopIteration:
+                nextForZip[snum] = None
+
+        # generate (and yield) zippedMeasures from nextForZip (and reload those slots
+        # in nextForZip), until we run out of measures
+        while True:
+            zippedMeasures: t.List[t.Optional[stream.Measure]] = [None] * numStreams
+            nextIndices: t.List[int] = getSimultaneousMeasureIndices(nextForZip)
+            for idx in nextIndices:
+                zippedMeasures[idx] = nextForZip[idx]
+                try:
+                    nextForZip[idx] = next(streamIterators[idx])
+                except StopIteration:
+                    nextForZip[idx] = None
+
+            if all(m is None for m in zippedMeasures):
+                # we have run out of measures
+                return
+
+            yield zippedMeasures
+
     def postPartProcess(self):
         '''
         calls .joinPartStaffs() from the
@@ -2555,6 +2689,19 @@ def __init__(self,
         # has changed
         self.lastDivisions = None
 
+        # voiceNumberOffsetForMeasure is the offset that must be applied to any voice
+        # numbers automatically generated for the key measure 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.voiceNumberOffsetForMeasure: t.Dict[stream.Measure, int] = {}
+
+        # The maximum voiceNum found in the measure.
+        self.maxVoiceNumForMeasure: t.Dict[stream.Measure, int] = {}
+
+        # 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):
@@ -3027,7 +3174,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
 
@@ -3083,6 +3230,20 @@ 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
+        if t.TYPE_CHECKING:
+            assert isinstance(self.stream, stream.Measure)
+        return partExp.voiceNumberOffsetForMeasure.get(self.stream, 0)
+
     def parseFlatElements(self, m, *, backupAfterwards=False):
         '''
         Deals with parsing all the elements in .elements, assuming that .elements is flat.
@@ -3103,8 +3264,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

@@ -894,6 +894,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 = (
@@ -950,6 +956,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

music21/musicxml/test_m21ToXml.py

@@ -145,6 +145,82 @@ def testLowVoiceNumbers(self):
         xmlOut = self.getXml(m)
         self.assertIn('<voice>hello</voice>', xmlOut)
 
+    def testVoiceNumberOffsetsThreeStaffsInGroup(self):
+        n1_1 = note.Note()
+        v1_1 = stream.Voice([n1_1])
+        m1_1 = stream.Measure([v1_1])
+        n1_2 = note.Note()
+        v1_2 = stream.Voice([n1_2])
+        m1_2 = stream.Measure([v1_2])
+        ps1 = stream.PartStaff([m1_1, m1_2])
+
+        n2_1 = note.Note()
+        v2_1 = stream.Voice([n2_1])
+        m2_1 = stream.Measure([v2_1])
+        n2_2 = note.Note()
+        v2_2 = stream.Voice([n2_2])
+        m2_2 = stream.Measure([v2_2])
+        ps2 = stream.PartStaff([m2_1, m2_2])
+
+        n3_1 = note.Note()
+        v3_1 = stream.Voice([n3_1])
+        m3_1 = stream.Measure([v3_1])
+        n3_2 = note.Note()
+        v3_2 = stream.Voice([n3_2])
+        m3_2 = stream.Measure([v3_2])
+        ps3 = stream.PartStaff([m3_1, m3_2])
+
+        s = stream.Score([ps1, ps2, ps3])
+        staffGroup = layout.StaffGroup([ps1, ps2, ps3])
+        s.insert(0, staffGroup)
+
+        tree = self.getET(s)
+        # helpers.dump(tree)
+        mxNotes = tree.findall('part/measure/note')
+        for mxNote in mxNotes:
+            voice = mxNote.find('voice')
+            staff = mxNote.find('staff')
+            # Because there is one voice per staff/measure, voicenum == staffnum
+            self.assertEqual(voice.text, staff.text)
+
+    def testZipMeasures(self):
+        p0 = stream.Part()
+        p1 = stream.Part()
+        m1 = stream.Measure(number='1')
+        m1a = stream.Measure(number='1a')
+        m1b = stream.Measure(number='1b')
+        mm1 = stream.Measure(number='1')
+        mm1a = stream.Measure(number='1a')
+        mm1b = stream.Measure(number='1b')
+        m2 = stream.Measure(number='2')
+        m2a = stream.Measure(number='2a')
+        m2b = stream.Measure(number='2b')
+        mm2 = stream.Measure(number='2')
+        mm2a = stream.Measure(number='2a')
+        mm2b = stream.Measure(number='2b')
+        p0.append([m1, m1a, m1b, m2, m2a, m2b])
+        p1.append([mm1, mm1a, mm1b, mm2, mm2a, mm2b])
+        expectedNums = ['1', '1a', '1b', '2', '2a', '2b']
+        for mnum, measures in enumerate(ScoreExporter.zipMeasures([p0, p1])):
+            self.assertEqual(len(measures), 2)
+            for m in measures:
+                self.assertEqual(m.measureNumberWithSuffix(), expectedNums[mnum])
+
+        p1 = stream.Part()
+        p1.append([mm1, mm2])
+        expectedNumsPart0 = ['1', '1a', '1b', '2', '2a', '2b']
+        expectedNumsPart1 = ['1', None, None, '2', None, None]
+        for mnum, measures in enumerate(ScoreExporter.zipMeasures([p0, p1])):
+            self.assertEqual(len(measures), 2)
+            for partIdx, m in enumerate(measures):
+                if partIdx == 0:
+                    self.assertEqual(m.measureNumberWithSuffix(), expectedNumsPart0[mnum])
+                else:  # partIdx == 1
+                    if expectedNumsPart1[mnum] is None:
+                        self.assertIsNone(m)
+                    else:
+                        self.assertEqual(m.measureNumberWithSuffix(), expectedNumsPart1[mnum])
+
     def testCompositeLyrics(self):
         xmlDir = common.getSourceFilePath() / 'musicxml' / 'lilypondTestSuite'
         fp = xmlDir / '61l-Lyrics-Elisions-Syllables.xml'