Fixed bug #3293339: Midi file export SMF format 0 broken. Please see ChangeLog.

4 files changed, 364 insertions, 271 deletions

diff --git a/muse2/ChangeLog b/muse2/ChangeLog
index bf5172f9..5d0ae536 100644
--- a/muse2/ChangeLog
+++ b/muse2/ChangeLog
@@ -1,3 +1,11 @@
+24.10.2010:
+        - Fixed bug #3293339: Midi file export SMF format 0 broken. (Tim)
+          Removed erroneous extra 2 bytes. Allow metas in SMF0 in MidiFile::writeEvent().
+          Fixed very weird timing issue MidiFile::write(). See comments there (hidden problems lurking still?). 
+          Use single MidiFileTrack for SMF0 in MusE::exportMidi(). Added info to export dialog indicating that 
+           SMF 0 grabs the track name and comment from first midi track in arranger. Iterate full tracks list 
+           instead of midis list in MusE::exportMidi(), so that user can rearrange tracks on the fly before exporting. 
+          Some not fully understood useage of meta 0x0F/0x01 text events in MusE::exportMidi(). Hopefully correct...
 22.10.2010:
         - Catch return in spin boxes and move focus away (rj)
 20.10.2011:
diff --git a/muse2/muse/exportmidi.cpp b/muse2/muse/exportmidi.cpp
index b892e808..97029d9d 100644
--- a/muse2/muse/exportmidi.cpp
+++ b/muse2/muse/exportmidi.cpp
@@ -149,15 +149,28 @@ void MusE::exportMidi()
             return;
       MusECore::MidiFile mf(fp);
 
-      MusECore::MidiTrackList* tl = MusEGlobal::song->midis();
-      int ntracks = tl->size();
+      //MusECore::MidiTrackList* tl = MusEGlobal::song->midis();
+      MusECore::TrackList* tl = MusEGlobal::song->tracks();       // Changed to full track list so user can rearrange tracks.
+      //int ntracks = tl->size();
       MusECore::MidiFileTrackList* mtl = new MusECore::MidiFileTrackList;
 
       int i = 0;
-      for (MusECore::iMidiTrack im = tl->begin(); im != tl->end(); ++im, ++i) {
-            MusECore::MidiTrack* track = *im;
-            MusECore::MidiFileTrack* mft = new MusECore::MidiFileTrack;
-            mtl->push_back(mft);
+      MusECore::MidiFileTrack* mft = 0;
+      //for (MusECore::iMidiTrack im = tl->begin(); im != tl->end(); ++im, ++i) {
+      for (MusECore::ciTrack im = tl->begin(); im != tl->end(); ++im) {
+        
+            if(!(*im)->isMidiTrack())
+              continue;
+              
+            MusECore::MidiTrack* track = (MusECore::MidiTrack*)(*im);
+
+            //MusECore::MidiFileTrack* mft = new MusECore::MidiFileTrack;
+            if (i == 0 || (i != 0 && MusEGlobal::config.smfFormat != 0))    // Changed to single track. Tim
+            {  
+              mft = new MusECore::MidiFileTrack;
+              mtl->push_back(mft);
+            }
+            
             MusECore::MPEventList* l   = &(mft->events);
             int port         = track->outPort();
             int channel      = track->outChannel();
@@ -200,14 +213,17 @@ void MusE::exportMidi()
                   //---------------------------------------------------
                   //    Write Coment
                   //
-                  QString comment = track->comment();
-                  if (!comment.isEmpty()) {
-                        int len = comment.length();
-                        MusECore::MidiPlayEvent ev(0, port, MusECore::ME_META, (const unsigned char*)(comment.toLatin1().constData()), len);
-                        ev.setA(0x1);
-                        l->add(ev);
-                        }
-
+                  if (MusEGlobal::config.smfFormat == 0)  // Only for smf 0 added by Tim. FIXME: Is this correct? See below.
+                  {
+                    QString comment = track->comment();
+                    if (!comment.isEmpty()) {
+                          int len = comment.length();
+                          MusECore::MidiPlayEvent ev(0, port, MusECore::ME_META, (const unsigned char*)(comment.toLatin1().constData()), len);
+                          ev.setA(0x1);
+                          l->add(ev);
+                          }
+                  }
+                  
                   //---------------------------------------------------
                   //    Write Songtype SYSEX: GM/GS/XG
                   //
@@ -287,27 +303,38 @@ void MusE::exportMidi()
             //   track name
             //-----------------------------------
 
-            if (!track->name().isEmpty()) {
-                  QByteArray ba = track->name().toLatin1();
-                  const char* name = ba.constData();
-                  int len = strlen(name);
-                  MusECore::MidiPlayEvent ev(0, port, MusECore::ME_META, (unsigned char*)name, len+1);
-                  ev.setA(0x3);    // Meta Sequence/Track Name
-                  l->add(ev);
-                  }
-
+            if (i == 0 || (i != 0 && MusEGlobal::config.smfFormat != 0))
+            {
+              if (!track->name().isEmpty()) {
+                    QByteArray ba = track->name().toLatin1();
+                    const char* name = ba.constData();
+                    int len = strlen(name);
+                    MusECore::MidiPlayEvent ev(0, port, MusECore::ME_META, (unsigned char*)name, len+1);
+                    ev.setA(0x3);    // Meta Sequence/Track Name
+                    l->add(ev);
+                    }
+            }
+            
             //-----------------------------------
             //   track comment
             //-----------------------------------
 
-            if (!track->comment().isEmpty()) {
-                  QByteArray ba = track->comment().toLatin1();
-                  const char* comment = ba.constData();
-                  int len = strlen(comment);
-                  MusECore::MidiPlayEvent ev(0, port, MusECore::ME_META, (unsigned char*)comment, len+1);
-                  ev.setA(0xf);    // Meta Text
-                  l->add(ev);
-                  }
+            // FIXME: What are these 0x0F? All I found was that they are unspecified part of the sixteen text meta events.
+            //        So why not use 0x01? And do we include it for all tracks, or only above 1?   Tim.
+            //if (i == 0 || (i != 0 && MusEGlobal::config.smfFormat != 0))
+            //if (i != 0 && MusEGlobal::config.smfFormat != 0)
+            if (MusEGlobal::config.smfFormat != 0)
+            {
+              if (!track->comment().isEmpty()) {
+                    QByteArray ba = track->comment().toLatin1();
+                    const char* comment = ba.constData();
+                    int len = strlen(comment);
+                    MusECore::MidiPlayEvent ev(0, port, MusECore::ME_META, (unsigned char*)comment, len+1);
+                    ev.setA(0xf);    // Meta Text
+                    l->add(ev);
+                    }
+            }
+            
             MusECore::PartList* parts = track->parts();
             for (MusECore::iPart p = parts->begin(); p != parts->end(); ++p) {
                   MusECore::MidiPart* part    = (MusECore::MidiPart*) (p->second);
@@ -315,7 +342,6 @@ void MusE::exportMidi()
                   for (MusECore::iEvent i = evlist->begin(); i != evlist->end(); ++i) {
                         MusECore::Event ev = i->second;
                         int tick = ev.tick() + part->tick();
-
                         switch (ev.type()) {
                               case MusECore::Note:
                                     {
@@ -400,11 +426,19 @@ void MusE::exportMidi()
                               }
                         }
                   }
+              ++i;  
+            
             }
       mf.setDivision(MusEGlobal::config.midiDivision);
       mf.setMType(MusEGlobal::song->mtype());
-      mf.setTrackList(mtl, ntracks);
+      //mf.setTrackList(mtl, ntracks);
+      mf.setTrackList(mtl, i);
       mf.write();
+      
+      // TESTING: Cleanup. I did not valgrind this feature in last memleak fixes, but I suspect it leaked. 
+      //for(MusECore::iMidiFileTrack imft = mtl->begin(); imft != mtl->end(); ++imft)
+      //  delete *imft;
+      //delete mtl;
       }
 
 } // namespace MusEGui
diff --git a/muse2/muse/midifile.cpp b/muse2/muse/midifile.cpp
index a94644ff..2db2855d 100644
--- a/muse2/muse/midifile.cpp
+++ b/muse2/muse/midifile.cpp
@@ -568,9 +568,9 @@ void MidiFile::writeEvent(const MidiPlayEvent* event)
       int nstat = event->type();
 
       // we dont save meta data into smf type 0 files:
-
-      if (MusEGlobal::config.smfFormat == 0 && nstat == ME_META)
-            return;
+      // Oct 16, 2011: Apparently it is legal to do that. Part of fix for bug tracker 3293339.
+      //if (MusEGlobal::config.smfFormat == 0 && nstat == ME_META)
+      //      return;
 
       nstat |= c;
       //
@@ -621,24 +621,45 @@ bool MidiFile::write()
       writeLong(6);                 // header len
       writeShort(MusEGlobal::config.smfFormat);
       if (MusEGlobal::config.smfFormat == 0) {
-            writeShort(1);
+            /*
+            //writeShort(1);    // Removed. Bug tracker 3293339
             MidiFileTrack dst;
             for (iMidiFileTrack i = _tracks->begin(); i != _tracks->end(); ++i) {
                   MPEventList* sl = &((*i)->events);
                   for (iMPEvent ie = sl->begin(); ie != sl->end(); ++ie)
+                  {  
+                        // ALERT: Observed a problem here, apparently some of the events are being added too fast. 
+                        //        The dump below tells me some of the events (sysex/meta) are missing from the list! 
+                        //        Apparently it's a timing problem. Very puzzling. 
+                        //        Attempting wild-guess fix now to eliminate multiple MidiFileTracks in MusE::exportMidi()...
+                        //        Nope. Didn't help. Now that it's a single MidiFileTrack, try skipping this section altogether...
+                        //        Yes that appears to have fixed it. Weird. What's the difference - the local 'dst' variable ?
+                        //        Or are there still lurking problems, or something more fundamentally wrong with Event or MPEvent?
+                        printf("MidiFile::write adding event to dst:\n"); // REMOVE Tim.
+                        ie->dump();  // REMOVE Tim.
                         dst.events.add(*ie);
+                  }      
                   }
             writeShort(1);
             writeShort(_division);
             writeTrack(&dst);
+            */
+            
+            writeShort(1);
+            //writeShort(_division);
+            //if(!_tracks->empty())
+            //  writeTrack(*(_tracks->begin()));
+            
             }
       else {
+        
+        
             writeShort(ntracks);
-
+      }
             writeShort(_division);
             for (ciMidiFileTrack i = _tracks->begin(); i != _tracks->end(); ++i)
                   writeTrack(*i);
-            }
+///      }
       return (ferror(fp) != 0);
       }
 
diff --git a/muse2/muse/widgets/configmidifilebase.ui b/muse2/muse/widgets/configmidifilebase.ui
index 920596ec..ca64f2d8 100644
--- a/muse2/muse/widgets/configmidifilebase.ui
+++ b/muse2/muse/widgets/configmidifilebase.ui
@@ -1,238 +1,268 @@
 <?xml version="1.0" encoding="UTF-8"?>
-<ui version="4.0" stdsetdef="1">
-  <author></author>
-  <comment></comment>
-  <exportmacro></exportmacro>
-  <class>ConfigMidiFileBase</class>
-  <widget class="QDialog" name="ConfigMidiFileBase">
-    <property name="geometry">
-      <rect>
-        <x>0</x>
-        <y>0</y>
-        <width>548</width>
-        <height>353</height>
-      </rect>
-    </property>
-    <property name="windowTitle">
-      <string>MusE: Config Midi File Import/Export</string>
-    </property>
-    <property name="sizeGripEnabled">
-      <bool>true</bool>
-    </property>
-    <layout class="QVBoxLayout">
-      <item>
-        <widget class="QGroupBox" name="midiImportGroupBox">
-          <property name="sizePolicy">
-            <sizepolicy>
-              <hsizetype>5</hsizetype>
-              <vsizetype>1</vsizetype>
-              <horstretch>0</horstretch>
-              <verstretch>0</verstretch>
-            </sizepolicy>
-          </property>
-          <property name="title">
-            <string>Import:</string>
-          </property>
-          <layout class="QGridLayout">
-            <item row="0" column="0">
-              <widget class="QCheckBox" name="splitPartsCheckBox">
-                <property name="text">
-                  <string>Split tracks into &amp;parts</string>
-                </property>
-                <property name="shortcut">
-                  <string>Alt+P</string>
-                </property>
-                <property name="toolTip" stdset="0">
-                  <string>Split tracks into parts, or one single part</string>
-                </property>
-              </widget>
-            </item>
-          </layout>
-        </widget>
+<ui version="4.0">
+ <class>ConfigMidiFileBase</class>
+ <widget class="QDialog" name="ConfigMidiFileBase">
+  <property name="geometry">
+   <rect>
+    <x>0</x>
+    <y>0</y>
+    <width>548</width>
+    <height>353</height>
+   </rect>
+  </property>
+  <property name="windowTitle">
+   <string>MusE: Config Midi File Import/Export</string>
+  </property>
+  <property name="sizeGripEnabled">
+   <bool>true</bool>
+  </property>
+  <layout class="QVBoxLayout">
+   <item>
+    <widget class="QGroupBox" name="midiImportGroupBox">
+     <property name="sizePolicy">
+      <sizepolicy hsizetype="Preferred" vsizetype="Minimum">
+       <horstretch>0</horstretch>
+       <verstretch>0</verstretch>
+      </sizepolicy>
+     </property>
+     <property name="title">
+      <string>Import:</string>
+     </property>
+     <layout class="QGridLayout">
+      <item row="0" column="0">
+       <widget class="QCheckBox" name="splitPartsCheckBox">
+        <property name="toolTip">
+         <string>Split tracks into parts, or one single part</string>
+        </property>
+        <property name="text">
+         <string>Split tracks into &amp;parts</string>
+        </property>
+        <property name="shortcut">
+         <string>Alt+P</string>
+        </property>
+       </widget>
       </item>
-      <item>
-        <widget class="QGroupBox" name="midiExportGroupBox">
-          <property name="sizePolicy">
-            <sizepolicy>
-              <hsizetype>5</hsizetype>
-              <vsizetype>7</vsizetype>
-              <horstretch>0</horstretch>
-              <verstretch>0</verstretch>
-            </sizepolicy>
-          </property>
-          <property name="title">
-            <string>Export:</string>
-          </property>
-          <layout class="QGridLayout">
-            <item row="2" column="1">
-              <widget class="QLineEdit" name="copyrightEdit"/>
-            </item>
-            <item row="1" column="1">
-              <widget class="QComboBox" name="divisionCombo">
-                <item>
-                  <property name="text">
-                    <string>96</string>
-                  </property>
-                </item>
-                <item>
-                  <property name="text">
-                    <string>192</string>
-                  </property>
-                </item>
-                <item>
-                  <property name="text">
-                    <string>384</string>
-                  </property>
-                </item>
-              </widget>
-            </item>
-            <item row="3" column="0" rowspan="1" colspan="2">
-              <widget class="QCheckBox" name="extendedFormat">
-                <property name="enabled">
-                  <bool>true</bool>
-                </property>
-                <property name="text">
-                  <string>Enable extended smf format (currently not implemented)</string>
-                </property>
-              </widget>
-            </item>
-            <item row="4" column="0" rowspan="1" colspan="2">
-              <widget class="QCheckBox" name="twoByteTimeSigs">
-                <property name="text">
-                  <string>Use &amp;2-byte time signatures instead of standard 4</string>
-                </property>
-                <property name="shortcut">
-                  <string>Alt+2</string>
-                </property>
-              </widget>
-            </item>
-            <item row="2" column="0">
-              <widget class="QLabel" name="textLabel2">
-                <property name="text">
-                  <string>Copyright:</string>
-                </property>
-                <property name="wordWrap">
-                  <bool>false</bool>
-                </property>
-              </widget>
-            </item>
-            <item row="0" column="0">
-              <widget class="QLabel" name="textLabel3">
-                <property name="text">
-                  <string>Format:</string>
-                </property>
-                <property name="wordWrap">
-                  <bool>false</bool>
-                </property>
-              </widget>
-            </item>
-            <item row="1" column="0">
-              <widget class="QLabel" name="textLabel1">
-                <property name="text">
-                  <string>Division:</string>
-                </property>
-                <property name="wordWrap">
-                  <bool>false</bool>
-                </property>
-              </widget>
-            </item>
-            <item row="5" column="0" rowspan="1" colspan="2">
-              <widget class="QCheckBox" name="optNoteOffs">
-                <property name="text">
-                  <string>Save space by replacing note-offs with &amp;zero velocity note-ons</string>
-                </property>
-                <property name="shortcut">
-                  <string>Alt+Z</string>
-                </property>
-              </widget>
-            </item>
-            <item row="0" column="1">
-              <widget class="QComboBox" name="formatCombo">
-                <item>
-                  <property name="text">
-                    <string>0 (single track)</string>
-                  </property>
-                </item>
-                <item>
-                  <property name="text">
-                    <string>1 (multiple tracks)</string>
-                  </property>
-                </item>
-              </widget>
-            </item>
-          </layout>
-        </widget>
+     </layout>
+    </widget>
+   </item>
+   <item>
+    <widget class="QGroupBox" name="midiExportGroupBox">
+     <property name="sizePolicy">
+      <sizepolicy hsizetype="Preferred" vsizetype="Expanding">
+       <horstretch>0</horstretch>
+       <verstretch>0</verstretch>
+      </sizepolicy>
+     </property>
+     <property name="title">
+      <string>Export:</string>
+     </property>
+     <layout class="QGridLayout">
+      <item row="3" column="1">
+       <widget class="QLineEdit" name="copyrightEdit"/>
       </item>
-      <item>
-        <layout class="QHBoxLayout">
-          <property name="margin">
-            <number>0</number>
-          </property>
-          <property name="spacing">
-            <number>6</number>
-          </property>
-          <item>
-            <spacer name="Horizontal Spacing2">
-              <property name="sizeHint">
-                <size>
-                  <width>20</width>
-                  <height>20</height>
-                </size>
-              </property>
-              <property name="sizeType">
-                <enum>QSizePolicy::Expanding</enum>
-              </property>
-              <property name="orientation">
-                <enum>Qt::Horizontal</enum>
-              </property>
-            </spacer>
-          </item>
-          <item>
-            <widget class="QPushButton" name="buttonOk">
-              <property name="text">
-                <string>&amp;OK</string>
-              </property>
-              <property name="shortcut">
-                <string/>
-              </property>
-              <property name="autoDefault">
-                <bool>true</bool>
-              </property>
-              <property name="default">
-                <bool>true</bool>
-              </property>
-            </widget>
-          </item>
-          <item>
-            <widget class="QPushButton" name="buttonCancel">
-              <property name="text">
-                <string>&amp;Cancel</string>
-              </property>
-              <property name="shortcut">
-                <string/>
-              </property>
-              <property name="autoDefault">
-                <bool>true</bool>
-              </property>
-            </widget>
-          </item>
-        </layout>
+      <item row="2" column="1">
+       <widget class="QComboBox" name="divisionCombo">
+        <item>
+         <property name="text">
+          <string>96</string>
+         </property>
+        </item>
+        <item>
+         <property name="text">
+          <string>192</string>
+         </property>
+        </item>
+        <item>
+         <property name="text">
+          <string>384</string>
+         </property>
+        </item>
+       </widget>
       </item>
+      <item row="4" column="0" colspan="2">
+       <widget class="QCheckBox" name="extendedFormat">
+        <property name="enabled">
+         <bool>true</bool>
+        </property>
+        <property name="text">
+         <string>Enable extended smf format (currently not implemented)</string>
+        </property>
+       </widget>
+      </item>
+      <item row="5" column="0" colspan="2">
+       <widget class="QCheckBox" name="twoByteTimeSigs">
+        <property name="text">
+         <string>Use &amp;2-byte time signatures instead of standard 4</string>
+        </property>
+        <property name="shortcut">
+         <string>Alt+2</string>
+        </property>
+       </widget>
+      </item>
+      <item row="3" column="0">
+       <widget class="QLabel" name="textLabel2">
+        <property name="text">
+         <string>Copyright:</string>
+        </property>
+        <property name="wordWrap">
+         <bool>false</bool>
+        </property>
+       </widget>
+      </item>
+      <item row="0" column="0">
+       <widget class="QLabel" name="textLabel3">
+        <property name="text">
+         <string>Format:</string>
+        </property>
+        <property name="wordWrap">
+         <bool>false</bool>
+        </property>
+       </widget>
+      </item>
+      <item row="1" column="0" colspan="2">
+       <widget class="QLabel" name="textLabel4">
+        <property name="sizePolicy">
+         <sizepolicy hsizetype="Preferred" vsizetype="Maximum">
+          <horstretch>0</horstretch>
+          <verstretch>0</verstretch>
+         </sizepolicy>
+        </property>
+        <property name="text">
+         <string>Note: Format 0 uses the FIRST midi track's name/comment in the arranger</string>
+        </property>
+        <property name="wordWrap">
+         <bool>false</bool>
+        </property>
+       </widget>
+      </item>
+      <item row="2" column="0">
+       <widget class="QLabel" name="textLabel1">
+        <property name="text">
+         <string>Division:</string>
+        </property>
+        <property name="wordWrap">
+         <bool>false</bool>
+        </property>
+       </widget>
+      </item>
+      <item row="6" column="0" colspan="2">
+       <widget class="QCheckBox" name="optNoteOffs">
+        <property name="text">
+         <string>Save space by replacing note-offs with &amp;zero velocity note-ons</string>
+        </property>
+        <property name="shortcut">
+         <string>Alt+Z</string>
+        </property>
+       </widget>
+      </item>
+      <item row="0" column="1">
+       <widget class="QComboBox" name="formatCombo">
+        <item>
+         <property name="text">
+          <string>0 (single track)</string>
+         </property>
+        </item>
+        <item>
+         <property name="text">
+          <string>1 (multiple tracks)</string>
+         </property>
+        </item>
+       </widget>
+      </item>
+     </layout>
+    </widget>
+   </item>
+   <item>
+    <layout class="QHBoxLayout">
+     <property name="spacing">
+      <number>6</number>
+     </property>
+     <property name="margin">
+      <number>0</number>
+     </property>
+     <item>
+      <spacer name="Horizontal Spacing2">
+       <property name="orientation">
+        <enum>Qt::Horizontal</enum>
+       </property>
+       <property name="sizeType">
+        <enum>QSizePolicy::Expanding</enum>
+       </property>
+       <property name="sizeHint" stdset="0">
+        <size>
+         <width>20</width>
+         <height>20</height>
+        </size>
+       </property>
+      </spacer>
+     </item>
+     <item>
+      <widget class="QPushButton" name="buttonOk">
+       <property name="text">
+        <string>&amp;OK</string>
+       </property>
+       <property name="shortcut">
+        <string/>
+       </property>
+       <property name="autoDefault">
+        <bool>true</bool>
+       </property>
+       <property name="default">
+        <bool>true</bool>
+       </property>
+      </widget>
+     </item>
+     <item>
+      <widget class="QPushButton" name="buttonCancel">
+       <property name="text">
+        <string>&amp;Cancel</string>
+       </property>
+       <property name="shortcut">
+        <string/>
+       </property>
+       <property name="autoDefault">
+        <bool>true</bool>
+       </property>
+      </widget>
+     </item>
     </layout>
-  </widget>
-  <layoutdefault spacing="6" margin="11"/>
-  <connections>
-    <connection>
-      <sender>buttonOk</sender>
-      <signal>clicked()</signal>
-      <receiver>ConfigMidiFileBase</receiver>
-      <slot>accept()</slot>
-    </connection>
-    <connection>
-      <sender>buttonCancel</sender>
-      <signal>clicked()</signal>
-      <receiver>ConfigMidiFileBase</receiver>
-      <slot>reject()</slot>
-    </connection>
-  </connections>
+   </item>
+  </layout>
+ </widget>
+ <layoutdefault spacing="6" margin="11"/>
+ <resources/>
+ <connections>
+  <connection>
+   <sender>buttonOk</sender>
+   <signal>clicked()</signal>
+   <receiver>ConfigMidiFileBase</receiver>
+   <slot>accept()</slot>
+   <hints>
+    <hint type="sourcelabel">
+     <x>20</x>
+     <y>20</y>
+    </hint>
+    <hint type="destinationlabel">
+     <x>20</x>
+     <y>20</y>
+    </hint>
+   </hints>
+  </connection>
+  <connection>
+   <sender>buttonCancel</sender>
+   <signal>clicked()</signal>
+   <receiver>ConfigMidiFileBase</receiver>
+   <slot>reject()</slot>
+   <hints>
+    <hint type="sourcelabel">
+     <x>20</x>
+     <y>20</y>
+    </hint>
+    <hint type="destinationlabel">
+     <x>20</x>
+     <y>20</y>
+    </hint>
+   </hints>
+  </connection>
+ </connections>
 </ui>