Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Track tempo ticks in MIDI to avoid duplicate tempo events #3789

Merged
merged 2 commits into from
Sep 19, 2024

Conversation

brdvd
Copy link
Contributor

@brdvd brdvd commented Sep 18, 2024

This MR adds the tracking of tempo ticks in the GenerateMIDIFunctor to avoid having multiple tempo events at the same time. Closes #3776 .

@lpugin lpugin merged commit 5ff1c0b into rism-digital:develop Sep 19, 2024
5 checks passed
@craigsapp
Copy link
Contributor

Running test from PR #3777

All test produce expected results (test 6 is not optimal, but OK).

Test 1: (should not be related to this PR, but being careful):

Test 1: Implicit q=120 for file
<?xml version="1.0" encoding="UTF-8"?>
<?xml-model href="https://music-encoding.org/schema/5.0/mei-all.rng" type="application/xml" schematypens="http://relaxng.org/ns/structure/1.0"?>
<?xml-model href="https://music-encoding.org/schema/5.0/mei-all.rng" type="application/xml" schematypens="http://purl.oclc.org/dsdl/schematron"?>
<mei xmlns="http://www.music-encoding.org/ns/mei" meiversion="5.0">
 <meiHead>
  <fileDesc>
   <titleStmt>
    <title />
   </titleStmt>
   <pubStmt />
  </fileDesc>
  <encodingDesc>
  </encodingDesc>
 </meiHead>
 <music>
  <body>
   <mdiv xml:id="ma1fc0h">
    <score xml:id="su1whfa">
     <scoreDef xml:id="szz9sil">
      <staffGrp xml:id="sddnjaw">
       <staffDef xml:id="staffdef-L1F1" n="1" lines="5">
        <clef xml:id="c17xffyw" shape="G" line="2" />
       </staffDef>
      </staffGrp>
     </scoreDef>
     <section xml:id="section-L1F1">
      <measure xml:id="measure-L1">
       <staff xml:id="staff-L1F1" n="1">
        <layer xml:id="layer-L1F1N1" n="1">
         <note xml:id="note-L2F1" dur="1" oct="4" pname="c" />
        </layer>
       </staff>
      </measure>
     </section>
    </score>
   </mdiv>
  </body>
 </music>
</mei>

Resulting MIDI file with implicit q=120 preserved:

"MThd"			; MIDI header chunk marker
4'6			; bytes to follow in header chunk
2'1			; file format: Type-1 (multitrack)
2'2			; number of tracks
2'120			; ticks per quarter note

;;; TRACK 0 ----------------------------------
"MTrk"			; MIDI track chunk marker
4'4			; bytes to follow in track chunk
v0	ff 2f v0	; end-of-track

;;; TRACK 1 ----------------------------------
"MTrk"			; MIDI track chunk marker
4'13			; bytes to follow in track chunk
v0	90 '60 '90	; note-on C4
v480	90 '60 '0	; note-off C4
v0	ff 2f v0	; end-of-track

Test 2: explicit default tempo of q=120. This also should not be affected by this PR

Test 2: Explicit default tempo of q=120 for file
<?xml version="1.0" encoding="UTF-8"?>
<?xml-model href="https://music-encoding.org/schema/5.0/mei-all.rng" type="application/xml" schematypens="http://relaxng.org/ns/structure/1.0"?>
<?xml-model href="https://music-encoding.org/schema/5.0/mei-all.rng" type="application/xml" schematypens="http://purl.oclc.org/dsdl/schematron"?>
<mei xmlns="http://www.music-encoding.org/ns/mei" meiversion="5.0">
 <meiHead>
  <fileDesc>
   <titleStmt>
    <title />
   </titleStmt>
   <pubStmt />
  </fileDesc>
  <encodingDesc>
  </encodingDesc>
 </meiHead>
 <music>
  <body>
   <mdiv xml:id="ma1fc0h">
    <score xml:id="su1whfa">
     <scoreDef xml:id="szz9sil" midi.bpm="120.000" >
      <staffGrp xml:id="sddnjaw">
       <staffDef xml:id="staffdef-L1F1" n="1" lines="5">
        <clef xml:id="c17xffyw" shape="G" line="2" />
       </staffDef>
      </staffGrp>
     </scoreDef>
     <section xml:id="section-L1F1">
      <measure xml:id="measure-L1">
       <staff xml:id="staff-L1F1" n="1">
        <layer xml:id="layer-L1F1N1" n="1">
         <note xml:id="note-L2F1" dur="1" oct="4" pname="c" />
        </layer>
       </staff>
      </measure>
     </section>
    </score>
   </mdiv>
  </body>
 </music>
</mei>

Resulting MIDI file with explicit q=120 as expected:

"MThd"			; MIDI header chunk marker
4'6			; bytes to follow in header chunk
2'1			; file format: Type-1 (multitrack)
2'2			; number of tracks
2'120			; ticks per quarter note

;;; TRACK 0 ----------------------------------
"MTrk"			; MIDI track chunk marker
4'11			; bytes to follow in track chunk
v0	ff 51 v3 t120	; tempo
v0	ff 2f v0	; end-of-track

;;; TRACK 1 ----------------------------------
"MTrk"			; MIDI track chunk marker
4'13			; bytes to follow in track chunk
v0	90 '60 '90	; note-on C4
v480	90 '60 '0	; note-off C4
v0	ff 2f v0	; end-of-track

Test 3: Changing the global tempo to non-default value of q=76. This also should not be affected by this PR.

Click to view MEI data with global q=76 tempo in ``
<?xml version="1.0" encoding="UTF-8"?>
<?xml-model href="https://music-encoding.org/schema/5.0/mei-all.rng" type="application/xml" schematypens="http://relaxng.org/ns/structure/1.0"?>
<?xml-model href="https://music-encoding.org/schema/5.0/mei-all.rng" type="application/xml" schematypens="http://purl.oclc.org/dsdl/schematron"?>
<mei xmlns="http://www.music-encoding.org/ns/mei" meiversion="5.0">
 <meiHead>
  <fileDesc>
   <titleStmt>
    <title />
   </titleStmt>
   <pubStmt/>
  </fileDesc>
  <encodingDesc/>
 </meiHead>
 <music>
  <body>
   <mdiv xml:id="m1awq1za">
    <score xml:id="sx8r9me">
     <scoreDef xml:id="s1a637ng" midi.bpm="76.000000" >
      <staffGrp xml:id="soihhji">
       <staffDef xml:id="staffdef-L1F1" n="1" lines="5">
        <clef xml:id="c1v5wh60" shape="G" line="2" />
        <meterSig xml:id="metersig-L2F1" count="4" unit="4" />
       </staffDef>
      </staffGrp>
     </scoreDef>
     <section xml:id="section-L1F1">
      <measure xml:id="measure-L1">
       <staff xml:id="staff-L1F1" n="1">
        <layer xml:id="layer-L1F1N1" n="1">
         <note xml:id="note-L5F1" dur="1" oct="4" pname="c" accid.ges="n" />
        </layer>
       </staff>
      </measure>
     </section>
    </score>
   </mdiv>
  </body>
 </music>
</mei>

Exported MIDI file is as expected with one tempo change at q=76

"MThd"			; MIDI header chunk marker
4'6			; bytes to follow in header chunk
2'1			; file format: Type-1 (multitrack)
2'2			; number of tracks
2'120			; ticks per quarter note

;;; TRACK 0 ----------------------------------
"MTrk"			; MIDI track chunk marker
4'11			; bytes to follow in track chunk
v0	ff 51 v3 t76	; tempo
v0	ff 2f v0	; end-of-track

;;; TRACK 1 ----------------------------------
"MTrk"			; MIDI track chunk marker
4'21			; bytes to follow in track chunk
v0	ff 58 v4 '4 '2 '24 '8	; time signature
v0	90 '60 '90	; note-on C4
v480	90 '60 '0	; note-off C4
v0	ff 2f v0	; end-of-track

Test 4: Tempo change in measure instead of scoreDef (working as expected).

tempo change stored in `measure/tempo` (non-global, but effectively global)
<?xml version="1.0" encoding="UTF-8"?>
<?xml-model href="https://music-encoding.org/schema/5.0/mei-all.rng" type="application/xml" schematypens="http://relaxng.org/ns/structure/1.0"?>
<?xml-model href="https://music-encoding.org/schema/5.0/mei-all.rng" type="application/xml" schematypens="http://purl.oclc.org/dsdl/schematron"?>
<mei xmlns="http://www.music-encoding.org/ns/mei" meiversion="5.0">
 <meiHead xml:id="m14wnguh">
  <fileDesc xml:id="f1d45wkl">
   <titleStmt xml:id="t4y2knx">
    <title />
    <respStmt />
   </titleStmt>
   <pubStmt xml:id="p1rjaoka" />
  </fileDesc>
  <encodingDesc xml:id="e18c52q2">
   <appInfo xml:id="arg2kqw">
    <application xml:id="a1x0j92c" isodate="2024-09-14T08:49:20" version="4.3.0-dev-7cf6fff">
     <name xml:id="n15sve10">Verovio</name>
     <p xml:id="p185xpxi">Transcoded from MusicXML</p>
    </application>
   </appInfo>
  </encodingDesc>
 </meiHead>
 <music>
  <body>
   <mdiv xml:id="m1v95fep">
    <score xml:id="spo5sy8">
     <scoreDef xml:id="s187x7ks">
      <staffGrp xml:id="s1320hq6">
       <staffDef xml:id="P1" n="1" lines="5" ppq="1">
        <clef xml:id="c196dx1r" shape="G" line="2" />
        <keySig xml:id="k1qhemas" sig="0" />
        <meterSig xml:id="m1512ndw" count="4" unit="4" />
       </staffDef>
      </staffGrp>
     </scoreDef>
     <section xml:id="st20c6k">
      <measure xml:id="m1xxvpxi" n="1">
       <staff xml:id="s1eixgng" n="1">
        <layer xml:id="l1ufrjl9" n="1">
         <note xml:id="n1uf1n6s" dur.ppq="4" dur="1" oct="4" pname="e" />
        </layer>
       </staff>
       <tempo xml:id="t1n2y34t" place="above" staff="1" tstamp="1.000000" xml:lang="it" midi.bpm="35.000000">
        <rend xml:id="r10vrbjb" fontweight="bold">Grave</rend>
       </tempo>
      </measure>
     </section>
    </score>
   </mdiv>
  </body>
 </music>
</mei>

MIDI output

4'6			; bytes to follow in header chunk
2'1			; file format: Type-1 (multitrack)
2'2			; number of tracks
2'120			; ticks per quarter note

;;; TRACK 0 ----------------------------------
"MTrk"			; MIDI track chunk marker
4'11			; bytes to follow in track chunk
v0	ff 51 v3 t35	; tempo
v0	ff 2f v0	; end-of-track

;;; TRACK 1 ----------------------------------
"MTrk"			; MIDI track chunk marker
4'27			; bytes to follow in track chunk
v0	ff 59 v2 '0 '0	; key signature
v0	ff 58 v4 '4 '2 '24 '8	; time signature
v0	90 '64 '90	; note-on E4
v480	90 '64 '0	; note-off E4
v0	ff 2f v0	; end-of-track

Test 5: Multiple staves with single measure tempo change (working as expected)

Multiple staves

Exported MIDI file is as expected (one tempo q=35 in track 0):

"MThd"			; MIDI header chunk marker
4'6			; bytes to follow in header chunk
2'1			; file format: Type-1 (multitrack)
2'11			; number of tracks
2'120			; ticks per quarter note

;;; TRACK 0 ----------------------------------
"MTrk"			; MIDI track chunk marker
4'11			; bytes to follow in track chunk
v0	ff 51 v3 t35	; tempo
v0	ff 2f v0	; end-of-track

;;; TRACK 1 ----------------------------------
"MTrk"			; MIDI track chunk marker
4'27			; bytes to follow in track chunk
v0	ff 59 v2 '0 '0	; key signature
v0	ff 58 v4 '4 '2 '24 '8	; time signature
v0	90 '64 '90	; note-on E4
v480	90 '64 '0	; note-off E4
v0	ff 2f v0	; end-of-track

;;; TRACK 2 ----------------------------------
"MTrk"			; MIDI track chunk marker
4'18			; bytes to follow in track chunk
v0	ff 59 v2 '0 '0	; key signature
v0	ff 58 v4 '4 '2 '24 '8	; time signature
v0	ff 2f v0	; end-of-track

;;; TRACK 3 ----------------------------------
"MTrk"			; MIDI track chunk marker
4'18			; bytes to follow in track chunk
v0	ff 59 v2 '0 '0	; key signature
v0	ff 58 v4 '4 '2 '24 '8	; time signature
v0	ff 2f v0	; end-of-track

;;; TRACK 4 ----------------------------------
"MTrk"			; MIDI track chunk marker
4'18			; bytes to follow in track chunk
v0	ff 59 v2 '0 '0	; key signature
v0	ff 58 v4 '4 '2 '24 '8	; time signature
v0	ff 2f v0	; end-of-track

;;; TRACK 5 ----------------------------------
"MTrk"			; MIDI track chunk marker
4'18			; bytes to follow in track chunk
v0	ff 59 v2 '0 '0	; key signature
v0	ff 58 v4 '4 '2 '24 '8	; time signature
v0	ff 2f v0	; end-of-track

;;; TRACK 6 ----------------------------------
"MTrk"			; MIDI track chunk marker
4'18			; bytes to follow in track chunk
v0	ff 59 v2 '0 '0	; key signature
v0	ff 58 v4 '4 '2 '24 '8	; time signature
v0	ff 2f v0	; end-of-track

;;; TRACK 7 ----------------------------------
"MTrk"			; MIDI track chunk marker
4'18			; bytes to follow in track chunk
v0	ff 59 v2 '0 '0	; key signature
v0	ff 58 v4 '4 '2 '24 '8	; time signature
v0	ff 2f v0	; end-of-track

;;; TRACK 8 ----------------------------------
"MTrk"			; MIDI track chunk marker
4'18			; bytes to follow in track chunk
v0	ff 59 v2 '0 '0	; key signature
v0	ff 58 v4 '4 '2 '24 '8	; time signature
v0	ff 2f v0	; end-of-track

;;; TRACK 9 ----------------------------------
"MTrk"			; MIDI track chunk marker
4'18			; bytes to follow in track chunk
v0	ff 59 v2 '0 '0	; key signature
v0	ff 58 v4 '4 '2 '24 '8	; time signature
v0	ff 2f v0	; end-of-track

;;; TRACK 10 ----------------------------------
"MTrk"			; MIDI track chunk marker
4'18			; bytes to follow in track chunk
v0	ff 59 v2 '0 '0	; key signature
v0	ff 58 v4 '4 '2 '24 '8	; time signature
v0	ff 2f v0	; end-of-track

Test 6: staffDef tempo change and initial measure/tempo tempo change (only one should be exported to MIDI file. In this case conflicting tempos: scoreDef tempo is q=99 and measure/tempo of q=35 to see which one has priority in the output MIDI data.

MEI input data for test 6
<?xml version="1.0" encoding="UTF-8"?>
<?xml-model href="https://music-encoding.org/schema/5.0/mei-all.rng" type="application/xml" schematypens="http://relaxng.org/ns/structure/1.0"?>
<?xml-model href="https://music-encoding.org/schema/5.0/mei-all.rng" type="application/xml" schematypens="http://purl.oclc.org/dsdl/schematron"?>
<mei xmlns="http://www.music-encoding.org/ns/mei" meiversion="5.0">
 <meiHead xml:id="m14xna6y">
  <fileDesc xml:id="fsx0srh">
   <titleStmt xml:id="t1goud5g">
    <title />
    <respStmt />
   </titleStmt>
   <pubStmt xml:id="p1i4mjcd" />
  </fileDesc>
  <encodingDesc xml:id="ed41gqn">
   <appInfo xml:id="a19oajyq">
    <application xml:id="a5unz87" isodate="2024-09-14T08:50:19" version="4.3.0-dev-7cf6fff">
     <name xml:id="n1s73k1x">Verovio</name>
     <p xml:id="p27c9lp">Transcoded from MusicXML</p>
    </application>
   </appInfo>
  </encodingDesc>
 </meiHead>
 <music>
  <body>
   <mdiv xml:id="mwo2k4n">
    <score xml:id="s1u9phh6">
     <scoreDef xml:id="s1jlphng" midi.bpm="99" >
      <staffGrp xml:id="s1g3np5v">
       <staffGrp xml:id="P1" bar.thru="true">
        <grpSym xml:id="g14hj57n" symbol="brace" />
        <staffDef xml:id="sgdy4hf" n="1" lines="5" ppq="1">
         <clef xml:id="c1o94jat" shape="G" line="2" />
         <keySig xml:id="kqq74q9" sig="0" />
         <meterSig xml:id="m19p67pi" count="4" unit="4" />
        </staffDef>
        <staffDef xml:id="s1r5gozb" n="2" lines="5" ppq="1">
         <keySig xml:id="k3h9wl6" sig="0" />
         <meterSig xml:id="m1pl1uea" count="4" unit="4" />
        </staffDef>
        <staffDef xml:id="s187lbbq" n="3" lines="5" ppq="1">
         <keySig xml:id="k1dl3bre" sig="0" />
         <meterSig xml:id="mf2qkya" count="4" unit="4" />
        </staffDef>
        <staffDef xml:id="s12cxybf" n="4" lines="5" ppq="1">
         <keySig xml:id="khn9lbn" sig="0" />
         <meterSig xml:id="m1wo832w" count="4" unit="4" />
        </staffDef>
        <staffDef xml:id="sie8bqa" n="5" lines="5" ppq="1">
         <keySig xml:id="kjuzqst" sig="0" />
         <meterSig xml:id="m1k2h8ua" count="4" unit="4" />
        </staffDef>
        <staffDef xml:id="ss7u8ck" n="6" lines="5" ppq="1">
         <keySig xml:id="kckksgr" sig="0" />
         <meterSig xml:id="mmnn245" count="4" unit="4" />
        </staffDef>
        <staffDef xml:id="s1ols9fd" n="7" lines="5" ppq="1">
         <keySig xml:id="kxvh852" sig="0" />
         <meterSig xml:id="m112w31c" count="4" unit="4" />
        </staffDef>
        <staffDef xml:id="s1lj08n5" n="8" lines="5" ppq="1">
         <keySig xml:id="kkd1qz1" sig="0" />
         <meterSig xml:id="m1ipi6hk" count="4" unit="4" />
        </staffDef>
        <staffDef xml:id="se9fj2d" n="9" lines="5" ppq="1">
         <keySig xml:id="k1j0p3xi" sig="0" />
         <meterSig xml:id="mna3wai" count="4" unit="4" />
        </staffDef>
        <staffDef xml:id="s1j2hc82" n="10" lines="5" ppq="1">
         <keySig xml:id="ksk8s0l" sig="0" />
         <meterSig xml:id="mvffc2z" count="4" unit="4" />
        </staffDef>
       </staffGrp>
      </staffGrp>
     </scoreDef>
     <section xml:id="s2v34rf">
      <measure xml:id="mcr2cx5" n="1">
       <staff xml:id="sb0xbp9" n="1">
        <layer xml:id="l1g2ftpr" n="1">
         <note xml:id="n136ak32" dur.ppq="4" dur="1" oct="4" pname="e" />
        </layer>
       </staff>
       <staff xml:id="s1anmds3" n="2">
        <layer xml:id="l1d25f47" n="1">
         <mSpace xml:id="m1wqv62" />
        </layer>
       </staff>
       <staff xml:id="sq1x2fb" n="3">
        <layer xml:id="lcpdm9x" n="1">
         <mSpace xml:id="m1pdocdz" />
        </layer>
       </staff>
       <staff xml:id="s1afpb97" n="4">
        <layer xml:id="lcbfak2" n="1">
         <mSpace xml:id="m1tz87sz" />
        </layer>
       </staff>
       <staff xml:id="s1y5c1iu" n="5">
        <layer xml:id="l1ddwt5k" n="1">
         <mSpace xml:id="m124yvoe" />
        </layer>
       </staff>
       <staff xml:id="sb25363" n="6">
        <layer xml:id="l65kxli" n="1">
         <mSpace xml:id="mfkgv5q" />
        </layer>
       </staff>
       <staff xml:id="szy0vyz" n="7">
        <layer xml:id="l1ampylj" n="1">
         <mSpace xml:id="m1g3ni4x" />
        </layer>
       </staff>
       <staff xml:id="s106cfjv" n="8">
        <layer xml:id="l4y43vv" n="1">
         <mSpace xml:id="m1lgmtvs" />
        </layer>
       </staff>
       <staff xml:id="sld1cut" n="9">
        <layer xml:id="lf5206y" n="1">
         <mSpace xml:id="m1w10pht" />
        </layer>
       </staff>
       <staff xml:id="sv1qw60" n="10">
        <layer xml:id="l1p6aqhr" n="1">
         <mSpace xml:id="m8wnyfb" />
        </layer>
       </staff>
       <tempo xml:id="tjxp434" place="above" staff="1" tstamp="1.000000" xml:lang="it" midi.bpm="35.000000">
        <rend xml:id="r1kxs2bg" fontweight="bold">Grave</rend>
       </tempo>
      </measure>
     </section>
    </score>
   </mdiv>
  </body>
 </music>
</mei>

MIDI output with one tempo change in output (q=99). This is acceptable, although it might be better to choose the later tempo change (measure/tempo); however, I would view this as a data error, and the user should expect undesired tempo choice in such cases if they are not consistent themselves.

"MThd"			; MIDI header chunk marker
4'6			; bytes to follow in header chunk
2'1			; file format: Type-1 (multitrack)
2'11			; number of tracks
2'120			; ticks per quarter note

;;; TRACK 0 ----------------------------------
"MTrk"			; MIDI track chunk marker
4'11			; bytes to follow in track chunk
v0	ff 51 v3 t98.9999	; tempo
v0	ff 2f v0	; end-of-track

;;; TRACK 1 ----------------------------------
"MTrk"			; MIDI track chunk marker
4'27			; bytes to follow in track chunk
v0	ff 59 v2 '0 '0	; key signature
v0	ff 58 v4 '4 '2 '24 '8	; time signature
v0	90 '64 '90	; note-on E4
v480	90 '64 '0	; note-off E4
v0	ff 2f v0	; end-of-track

;;; TRACK 2 ----------------------------------
"MTrk"			; MIDI track chunk marker
4'18			; bytes to follow in track chunk
v0	ff 59 v2 '0 '0	; key signature
v0	ff 58 v4 '4 '2 '24 '8	; time signature
v0	ff 2f v0	; end-of-track

;;; TRACK 3 ----------------------------------
"MTrk"			; MIDI track chunk marker
4'18			; bytes to follow in track chunk
v0	ff 59 v2 '0 '0	; key signature
v0	ff 58 v4 '4 '2 '24 '8	; time signature
v0	ff 2f v0	; end-of-track

;;; TRACK 4 ----------------------------------
"MTrk"			; MIDI track chunk marker
4'18			; bytes to follow in track chunk
v0	ff 59 v2 '0 '0	; key signature
v0	ff 58 v4 '4 '2 '24 '8	; time signature
v0	ff 2f v0	; end-of-track

;;; TRACK 5 ----------------------------------
"MTrk"			; MIDI track chunk marker
4'18			; bytes to follow in track chunk
v0	ff 59 v2 '0 '0	; key signature
v0	ff 58 v4 '4 '2 '24 '8	; time signature
v0	ff 2f v0	; end-of-track

;;; TRACK 6 ----------------------------------
"MTrk"			; MIDI track chunk marker
4'18			; bytes to follow in track chunk
v0	ff 59 v2 '0 '0	; key signature
v0	ff 58 v4 '4 '2 '24 '8	; time signature
v0	ff 2f v0	; end-of-track

;;; TRACK 7 ----------------------------------
"MTrk"			; MIDI track chunk marker
4'18			; bytes to follow in track chunk
v0	ff 59 v2 '0 '0	; key signature
v0	ff 58 v4 '4 '2 '24 '8	; time signature
v0	ff 2f v0	; end-of-track

;;; TRACK 8 ----------------------------------
"MTrk"			; MIDI track chunk marker
4'18			; bytes to follow in track chunk
v0	ff 59 v2 '0 '0	; key signature
v0	ff 58 v4 '4 '2 '24 '8	; time signature
v0	ff 2f v0	; end-of-track

;;; TRACK 9 ----------------------------------
"MTrk"			; MIDI track chunk marker
4'18			; bytes to follow in track chunk
v0	ff 59 v2 '0 '0	; key signature
v0	ff 58 v4 '4 '2 '24 '8	; time signature
v0	ff 2f v0	; end-of-track

;;; TRACK 10 ----------------------------------
"MTrk"			; MIDI track chunk marker
4'18			; bytes to follow in track chunk
v0	ff 59 v2 '0 '0	; key signature
v0	ff 58 v4 '4 '2 '24 '8	; time signature
v0	ff 2f v0	; end-of-track

@craigsapp
Copy link
Contributor

A future enhancement to think about:

Tempo changes should always be added to track 0 (as is now being done) since only one tempo can be active, and always affects all other tracks.

The enhancement to think about is: if the key signature or time signature is given in scoreDef, then it is best to add these only to track 0, and not to duplicate in the other tracks (this is the most common convention in MIDI files). If the key signature/ time signature are given in staffDef, then they should be handled as is done currently (inserting into the track that the key/time signature occurs). This will match the MIDI file structure more closely to the MEI data. Possible sub-enhancement: if all staves have the same key/time signature, then remove duplicate metamessages from staff-level tracks (tracks 1 and higher) and place one metamessage for all tracks in track 0 (system-level track).

@craigsapp
Copy link
Contributor

I notice in src/toolkit.cpp that there are two separate calls in different function sto m_doc::ExportMIDI():

     smf::MidiFile outputfile;
     outputfile.absoluteTicks();
     m_doc.ExportMIDI(&outputfile);
     outputfile.sortTracks();

I will move the lines before and after m_doc::ExportMIDI() inside of ExportMIDI since they are part of the MIDI generation process. (I will add to next PR for develop-humdrum.

@craigsapp
Copy link
Contributor

Thanks @brdvd for adding enhancement to the functor system (better you than me :-)

I was going to implement the following system, which is not necessary anymore (but can be used for reference if there are corner cases to deal with in the future):

    // Do final preparation of the MidiFile: sort tracks into time order and
    // then check for and remove any cases of multiple tempo changes ocurring
    // at the same time (preserving only the last tempo change).

    smf::MidiFile& outFile = *midiFile;

    // Preserve insertion sequence of tempo-change messages (only in track 0):
    for (int i=0; i<outFile[0].getEventCount(); ++i) {
        outFile[0][i].seq = i;
    }

    // Sort MIDI messages by absolute time ticks:
    outFile.sortTracks();

    // Remove unnecesary tempo-change messages, giving a warning
    // for cases where two different tempos occur at the same tick.
    int lastTempoTick = -1;
    double lastTempo = -1.0;
    for (int i=outFile[0].getEventCount() - 1; i>=0; --i) {
        if (outFile[0][i].isTempo()) {
            double tempo = outFile[0][i].getTempoBPM();
            if (outFile[0][i].tick == lastTempoTick) {
                if ((lastTempoTick > 0.0) && (tempo != lastTempo)) {
                   LogWarning("Ignoring tempo %f at tick $%d", tempo, outFile[0][i].tick);
                }
                outFile[0][i].clear();
            } else {
               lastTempo = tempo;
            }
            lastTempoTick = outFile[0][i].tick;
        }
    }

This would go at the very end of the doc::ExportMIDI() function. It examines the tempo changes in the exported MIDI file and removes duplicates. If there are different tempo changes occurring at the same time but with different values, this code will print a warning (current implementation quietly suppresses different tempo changes and probably chooses the first tempo change added to the MIDI file rather than the last in this code).

craigsapp added a commit that referenced this pull request Sep 27, 2024
@brdvd
Copy link
Contributor Author

brdvd commented Oct 3, 2024

@craigsapp Thanks for the additional testing 👍

The main purpose of the PR was to avoid multiple tempo events, if we have several staves and layers (Test 5). Of course, the cleanup could also happen after the MIDI file was generated. However, at this point it is impossible to decide which tempo event should be preferred if we have conflicting tempo events at the same time (Test 6). In principle, this can only be resolved by Verovio when generating the MIDI file and having full access to the encoding. So I think the preferred strategy should always be to not create a messy MIDI file in the first place, instead of cleaning up afterwards.

@brdvd brdvd deleted the fix/duplicate-tempo branch October 3, 2024 13:05
@craigsapp
Copy link
Contributor

So I think the preferred strategy should always be to not create a messy MIDI file in the first place, instead of cleaning up afterwards.

Yes, that is the best. But it will primarily be up to the encoder of the score to not add conflicting tempos, and it is not possible to resolve such conflicts in software most likely.

Of course, the cleanup could also happen after the MIDI file was generated. However, at this point it is impossible to decide which tempo event should be preferred if we have conflicting tempo events at the same time

The cleanup process in the example code I give is based on how the MIDI file will be processed, not an arbitrary decision by me. The order of the tempo changes at the same time may be arbitrary, but it is only the last one that has an affect on the performance timings of notes in the MIDI file. The important thing is that there is a warning generated when two or more tempos occurring at the same time are different.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Duplicate tempo change export in MIDI output
3 participants