Skip to content

Commit

Permalink
Reverts some changes on MidiExport
Browse files Browse the repository at this point in the history
	- Some changes were reverted on MidiExport and InstrumentTrack.
We were storing the beat length on the XML of Instrument Tracks, but in
reality the beat length is a per note attribute, and some instruments
could run into a segmentation fault when calling beat length without a
NotePlayHandle (i.e.: AFP). Because of that I reverted this change, so
the beat length is not stored on the XML anymore, and instead we have a
magic number on the MidiExport class that holds a default beat length
which is actually an upper limit for the MIDI notes of step notes. In
the future we can improve this by finding a way to store the beat length
on the note class to use it instead. The MidiExport logic is not
worsened at all because previously the beat length wasn't even
considered during export (it was actually improved making the exported
notes extend until the next one instead of cutting shorter).
  • Loading branch information
IanCaio committed Nov 3, 2023
1 parent 389a245 commit dcb41af
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 29 deletions.
31 changes: 4 additions & 27 deletions plugins/MidiExport/MidiExport.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,6 @@ bool MidiExport::tryExport(const TrackContainer::TrackList &tracks,
int base_pitch = 0;
double base_volume = 1.0;
int base_time = 0;
int base_beatLen = 16;

MidiNoteVector midiClip;

Expand All @@ -130,18 +129,6 @@ bool MidiExport::tryExport(const TrackContainer::TrackList &tracks,
base_pitch += masterPitch;
}
base_volume = LocaleHelper::toDouble(it.attribute("volume", "100"))/100.0;
// From the PR 5902 forward (Note Types), the instrument XML includes the beat length
// in frames. We try to fetch it and convert to the length in ticks. If there's no beat
// length in XML, the default beat length of 16 ticks will be used.
QDomNode iNode = it.elementsByTagName("instrument").at(0);
if (!iNode.isNull())
{
QDomElement i = iNode.toElement();
if (i.hasAttribute("beatlen"))
{
base_beatLen = i.attribute("beatlen", "0").toInt() / Engine::framesPerTick();
}
}
}

if (n.nodeName() == "midiclip")
Expand All @@ -151,7 +138,7 @@ bool MidiExport::tryExport(const TrackContainer::TrackList &tracks,
}

}
processPatternNotes(midiClip, base_beatLen, INT_MAX);
processPatternNotes(midiClip, INT_MAX);
writeMidiClipToTrack(mtrack, midiClip);
size = mtrack.writeToBuffer(buffer.data());
midiout.writeRawData((char *)buffer.data(), size);
Expand Down Expand Up @@ -202,7 +189,6 @@ bool MidiExport::tryExport(const TrackContainer::TrackList &tracks,

int base_pitch = 0;
double base_volume = 1.0;
int base_beatLen = 16;

// for each pattern in the pattern editor
for (QDomNode n = element.firstChild(); !n.isNull(); n = n.nextSibling())
Expand All @@ -216,15 +202,6 @@ bool MidiExport::tryExport(const TrackContainer::TrackList &tracks,
base_pitch += masterPitch;
}
base_volume = LocaleHelper::toDouble(it.attribute("volume", "100")) / 100.0;
QDomNode iNode = it.elementsByTagName("instrument").at(0);
if (!iNode.isNull())
{
QDomElement i = iNode.toElement();
if (i.hasAttribute("beatlen"))
{
base_beatLen = i.attribute("beatlen", "0").toInt()/Engine::framesPerTick();
}
}
}

if (n.nodeName() == "midiclip")
Expand Down Expand Up @@ -271,7 +248,7 @@ bool MidiExport::tryExport(const TrackContainer::TrackList &tracks,
st.pop_back();
}

processPatternNotes(nv, base_beatLen, pos);
processPatternNotes(nv, pos);
writeMidiClipToTrack(mtrack, nv);

// next pattern track
Expand Down Expand Up @@ -344,7 +321,7 @@ void MidiExport::writePatternClip(MidiNoteVector& src, MidiNoteVector& dst,



void MidiExport::processPatternNotes(MidiNoteVector& nv, int beatLen, int cutPos)
void MidiExport::processPatternNotes(MidiNoteVector& nv, int cutPos)
{
std::sort(nv.begin(), nv.end());
int cur = INT_MAX, next = INT_MAX;
Expand All @@ -357,7 +334,7 @@ void MidiExport::processPatternNotes(MidiNoteVector& nv, int beatLen, int cutPos
}
if (it->type == Note::Type::Step)
{
it->duration = qMin(qMin(beatLen, next - cur), cutPos - it->time);
it->duration = qMin(qMin(DefaultBeatLength, next - cur), cutPos - it->time);
}
}
}
Expand Down
12 changes: 11 additions & 1 deletion plugins/MidiExport/MidiExport.h
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,16 @@ class MidiExport: public ExportFilter
MidiExport();
~MidiExport() override = default;

// Default Beat Length in ticks for step notes
// TODO: The beat length actually varies per note, however the method that
// calculates it (InstrumentTrack::beatLen) requires a NotePlayHandle to do
// so. While we don't figure out a way to hold the beat length of each note
// on its member variables, we will use a default value as a beat length that
// will be used as an upper limit of the midi note length. This doesn't worsen
// the current logic used for MidiExport because right now the beat length is
// not even considered during the generation of the MIDI.
static constexpr int DefaultBeatLength = 1500;

gui::PluginView* instantiateView(QWidget *) override
{
return nullptr;
Expand All @@ -80,7 +90,7 @@ class MidiExport: public ExportFilter
void writeMidiClipToTrack(MTrack &mtrack, MidiNoteVector &nv);
void writePatternClip(MidiNoteVector &src, MidiNoteVector &dst,
int len, int base, int start, int end);
void processPatternNotes(MidiNoteVector &nv, int beatLen, int cutPos);
void processPatternNotes(MidiNoteVector &nv, int cutPos);

void error();

Expand Down
1 change: 0 additions & 1 deletion src/tracks/InstrumentTrack.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -848,7 +848,6 @@ void InstrumentTrack::saveTrackSpecificSettings( QDomDocument& doc, QDomElement
{
QDomElement i = doc.createElement( "instrument" );
i.setAttribute( "name", m_instrument->descriptor()->name );
i.setAttribute("beatlen", static_cast<int>(beatLen(nullptr)));
QDomElement ins = m_instrument->saveState( doc, i );
if(m_instrument->key().isValid()) {
ins.appendChild( m_instrument->key().saveXML( doc ) );
Expand Down

0 comments on commit dcb41af

Please sign in to comment.