Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
b205222
Initial Commit
IanCaio Feb 3, 2021
cfb533a
Update Pattern.cpp to account for the Note::Type
IanCaio Feb 3, 2021
a5cf05d
Update PatternView::paintEvent to draw step notes
IanCaio Feb 3, 2021
df88140
Implements StepNotes setting a NPH with 0 frames
IanCaio Feb 3, 2021
7a5ad82
Improves PatternView::paintEvent conditional
IanCaio Feb 3, 2021
725169a
Adds upgrade method for backwards compatibility
IanCaio Feb 3, 2021
faa5209
Addresses Veratil's review
IanCaio Feb 11, 2021
9decf11
Uses ternary expression on statement
IanCaio Feb 11, 2021
3576300
Merge branch 'master' into feature/BBNotes
IanCaio Mar 5, 2021
f5cc889
Merge branch 'master' into feature/BBNotes
IanCaio Apr 18, 2021
1df46a5
Merge branch 'master' into feature/BBNotes
IanCaio Jul 7, 2023
db0d0b7
Addresses PR review (sakertooth)
IanCaio Jul 9, 2023
6b3c75e
Finished changes from review (sakertooth)
IanCaio Jul 9, 2023
4b69615
Uses std::find_if to save codelines
IanCaio Jul 10, 2023
89a62f6
Addresses review from sakertooth
IanCaio Jul 17, 2023
b3511c2
Addresses DomClark's review
IanCaio Jul 23, 2023
dbefa9c
Updates MidiExport to use Note Types
IanCaio Aug 21, 2023
463050a
Merge branch 'master' into feature/BBNotes
IanCaio Aug 28, 2023
0054ece
Fixes ambiguity on enum usage
IanCaio Aug 28, 2023
fe42e97
Merge branch 'master' into feature/BBNotes
IanCaio Oct 17, 2023
38dd8ec
Addresses new code reviews
IanCaio Oct 17, 2023
31a2bd3
Fixes note drawing on Song Editor
IanCaio Oct 17, 2023
43018d7
Adds cassert header to TimePos.cpp
IanCaio Oct 18, 2023
389a245
Apply suggestions from code review
IanCaio Oct 26, 2023
dcb41af
Reverts some changes on MidiExport
IanCaio Nov 3, 2023
2ecadb0
Merge remote-tracking branch 'upstream/master' into feature/BBNotes
IanCaio Nov 18, 2023
67b0b24
Fix the order of included files
IanCaio Nov 18, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Updates MidiExport to use Note Types
	- Now MidiExport is updated to use note types instead of relying
on negative length notes.
	- For that change it was necessary to find a way of letting
MidiExport know how long step notes should be. The solution found was to
add an attribute to the Instrument XML called "beatlen", which would
hold the number of frames of the instrument's beat. That would be
converted to ticks, so we could calculate how long the MIDI notes would
have to be to play the whole step note. If the attribute was not found,
the default value of 16 ticks would be used as a length of step notes,
as a fallback.
  • Loading branch information
IanCaio committed Aug 21, 2023
commit dbefa9c0f3e731985fc98e2a82ff35efeb34dad3
33 changes: 28 additions & 5 deletions plugins/MidiExport/MidiExport.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@

#include "MidiExport.h"

#include "Engine.h"
#include "TrackContainer.h"
#include "DataFile.h"
#include "InstrumentTrack.h"
Expand Down Expand Up @@ -113,6 +114,7 @@ bool MidiExport::tryExport(const TrackContainer::TrackList &tracks,
int base_pitch = 0;
double base_volume = 1.0;
int base_time = 0;
int base_beatLen = 16;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like there are a few places where the default step note length of 16 is hard-coded in. You should probably create a constexpr constant for it and use that instead.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just followed the example of the other variables, that value is more like a default in case there's no stored value on the XML. Should I change it to be constexpr int base_beatLen = 16; here and in the other method?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, I meant if you want the default step note length to be a 1/16th note, you shouldn't have "16" hard-coded in 3 or 4 different places. You should probably use a static constexpr member in the Note class called DefaultStepNoteLength or something like that:

// Inside Note class in Note.h
static constexpr int DefaultStepNoteLength = 16; // 16th note

Then replace every "16" which you're using to represent the default length of step notes with Note::DefaultStepNoteLength.

The point of this is to avoid magic numbers and have the default step note length defined in a single place so it's easy to change later if we ever decide to do that.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, got it! I wrote the changes here, I'm just double checking because I think base_beatLen is actually not related to the default step note length of 1/16th (the one that shows on the piano roll):

Updates MidiExport to use Note Types

  • Now MidiExport is updated to use note types instead of relying
    on negative length notes.
  • For that change it was necessary to find a way of letting
    MidiExport know how long step notes should be. The solution found was to
    add an attribute to the Instrument XML called "beatlen", which would
    hold the number of frames of the instrument's beat. That would be
    converted to ticks, so we could calculate how long the MIDI notes would
    have to be to play the whole step note. If the attribute was not found,
    the default value of 16 ticks would be used as a length of step notes,
    as a fallback.

I think I'd need another constexpr for the beatLen fallback value. Should I have it on "InstrumentTrack.h"?


MidiNoteVector midiClip;

Expand All @@ -128,6 +130,17 @@ 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"))
Comment thread
IanCaio marked this conversation as resolved.
Outdated
{
base_beatLen = i.attribute("beatlen", "0").toInt() / Engine::framesPerTick();
}
}
}

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

}
processPatternNotes(midiClip, INT_MAX);
processPatternNotes(midiClip, base_beatLen, INT_MAX);
writeMidiClipToTrack(mtrack, midiClip);
size = mtrack.writeToBuffer(buffer.data());
midiout.writeRawData((char *)buffer.data(), size);
Expand Down Expand Up @@ -188,6 +201,7 @@ 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 @@ -201,6 +215,13 @@ 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")){
Comment thread
IanCaio marked this conversation as resolved.
Outdated
base_beatLen = i.attribute("beatlen", "0").toInt()/Engine::framesPerTick();
}
}
}

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

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

// next pattern track
Expand Down Expand Up @@ -279,6 +300,7 @@ void MidiExport::writeMidiClip(MidiNoteVector &midiClip, const QDomNode& n,
mnote.volume = qMin(qRound(base_volume * LocaleHelper::toDouble(note.attribute("vol", "100")) * (127.0 / 200.0)), 127);
mnote.time = base_time + note.attribute("pos", "0").toInt();
mnote.duration = note.attribute("len", "0").toInt();
mnote.type = static_cast<Note::Type>(note.attribute("type", "0").toInt());
midiClip.push_back(mnote);
}
}
Expand Down Expand Up @@ -311,14 +333,15 @@ void MidiExport::writePatternClip(MidiNoteVector& src, MidiNoteVector& dst,
note.pitch = srcNote.pitch;
note.time = base + time;
note.volume = srcNote.volume;
note.type = srcNote.type;
dst.push_back(note);
}
}
}



void MidiExport::processPatternNotes(MidiNoteVector& nv, int cutPos)
void MidiExport::processPatternNotes(MidiNoteVector& nv, int beatLen, int cutPos)
{
std::sort(nv.begin(), nv.end());
int cur = INT_MAX, next = INT_MAX;
Expand All @@ -329,9 +352,9 @@ void MidiExport::processPatternNotes(MidiNoteVector& nv, int cutPos)
next = cur;
cur = it->time;
}
if (it->duration < 0)
if (it->type == Note::Type::Step)
{
it->duration = qMin(qMin(-it->duration, next - cur), cutPos - it->time);
it->duration = qMin(qMin(beatLen, next - cur), cutPos - it->time);
}
}
}
Expand Down
4 changes: 3 additions & 1 deletion plugins/MidiExport/MidiExport.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@

#include "ExportFilter.h"
#include "MidiFile.hpp"
#include "Note.h"

class QDomNode;

Expand All @@ -46,6 +47,7 @@ struct MidiNote
uint8_t pitch;
int duration;
uint8_t volume;
Note::Type type;

inline bool operator<(const MidiNote &b) const
{
Expand Down Expand Up @@ -78,7 +80,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 cutPos);
void processPatternNotes(MidiNoteVector &nv, int beatLen, int cutPos);

void error();

Expand Down
1 change: 1 addition & 0 deletions src/tracks/InstrumentTrack.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -831,6 +831,7 @@ 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