From dcb41af1eae5e5795cd88d82ad1a59d700caabf5 Mon Sep 17 00:00:00 2001 From: Ian Caio Date: Fri, 3 Nov 2023 18:22:09 -0300 Subject: [PATCH] Reverts some changes on MidiExport - 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). --- plugins/MidiExport/MidiExport.cpp | 31 ++++--------------------------- plugins/MidiExport/MidiExport.h | 12 +++++++++++- src/tracks/InstrumentTrack.cpp | 1 - 3 files changed, 15 insertions(+), 29 deletions(-) diff --git a/plugins/MidiExport/MidiExport.cpp b/plugins/MidiExport/MidiExport.cpp index 193d943efe5..2600a40f2f9 100644 --- a/plugins/MidiExport/MidiExport.cpp +++ b/plugins/MidiExport/MidiExport.cpp @@ -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; @@ -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") @@ -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); @@ -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()) @@ -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") @@ -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 @@ -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; @@ -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); } } } diff --git a/plugins/MidiExport/MidiExport.h b/plugins/MidiExport/MidiExport.h index e2c0833373b..7c77c7af252 100644 --- a/plugins/MidiExport/MidiExport.h +++ b/plugins/MidiExport/MidiExport.h @@ -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; @@ -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(); diff --git a/src/tracks/InstrumentTrack.cpp b/src/tracks/InstrumentTrack.cpp index 0c83be51601..7f88bb51a83 100644 --- a/src/tracks/InstrumentTrack.cpp +++ b/src/tracks/InstrumentTrack.cpp @@ -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(beatLen(nullptr))); QDomElement ins = m_instrument->saveState( doc, i ); if(m_instrument->key().isValid()) { ins.appendChild( m_instrument->key().saveXML( doc ) );