melanchall / drywetmidi

.NET library to read, write, process MIDI files and to work with MIDI devices
https://melanchall.github.io/drywetmidi
MIT License
545 stars 75 forks source link

Fractional Beats? #32

Closed ronnieoverby closed 5 years ago

ronnieoverby commented 5 years ago

First… amazing library! Thank you, very much.

I want to use drywetmidi to playback midi events. It seems that my source event times and lengths are not quite compatible with yours.

For example, my program is generating midi events with the following structure:

struct Event<T>
{
    public int RelativeBar { get; set; }
    public double Position { get; set; }
    public double Length { get; set; }
    public T Data { get; set; }
}

RelativeBar: Simply the bar that an event starts. Position: The beat that an event starts. 1 would mean first beat. 2.5 would mean halfway before between 2nd and 3rd beat. Length: The duration of the event in beats. '1.33' would mean roughly 1 + 1/3 beats.

You can see that I'm representing the number of beats as a double where the value represents {Beats}.{Cents}. I borrowed this system directly from my favorite DAW, Reaper:

image

It doesn't seem that BarBeatTimeSpan offers parity with this representation. It has Bars/Beats/Ticks. Beats is an whole number and Ticks seems to store an additional MidiTimeSpan value that doesn't really have anything to do with bars/beats.

I'm sure that I can do the math on my own to convert my fractional beats to your ITimeSpan but I wanted to go ahead and ask you if I was missing something in the library that handles this for me.

Thanks for your help!

ronnieoverby commented 5 years ago

This logic seems to work for me.

static readonly BarBeatTimeSpan OneBeat = new BarBeatTimeSpan(0, 1);
long Convert(int bar, double beats, TempoMap tempoMap)
{
    var wholeBeatTimeSpan = new BarBeatTimeSpan(bar, (long)beats);
    var timeOfWholeBeat = TimeConverter.ConvertFrom(wholeBeatTimeSpan, tempoMap);

    var wholeBeats = (long)beats;
    if (beats == wholeBeats)
        // not fractional
        return timeOfWholeBeat;

    var beatLength = LengthConverter.ConvertFrom(
        OneBeat, timeOfWholeBeat, tempoMap);

    var partialBeats = beats - wholeBeats;
    var partialTicks = partialBeats * beatLength;

    return timeOfWholeBeat + (long)partialTicks;
}

If there's a more straightforward way, please let me know. Thanks, again!

melanchall commented 5 years ago

Hi,

Thanks for using the library!

I took Bar.Beat.Ticks notation from Cubase (another DAW like Reaper). Also it allows to set time and length as accurately as possible. double can lead to rounding errors on conversion to ticks (which is required since all timestamps stored as integer numbers inside a MIDI file).

Your code is good. I already have plans to expose utilities to get bar/beat length in MIDI ticks so users can easily perform such conversions. It seems I should increase priority of this task :) I'll implement this utilities within this issue so please don't close it.

melanchall commented 5 years ago

I've added BarBeatTimeSpanUtilities with GetBarLength and GetBeatLength methods which allow to rewrite your method:

long Convert(int bar, double beats, TempoMap tempoMap)
{
    var barPart = TimeConverter.ConvertFrom(new BarBeatTimeSpan(bar), tempoMap);
    var beatsPart = BarBeatTimeSpanUtilities.GetBeatLength(bar, tempoMap) * beats;
    return barPart + (long)Math.Round(beatsPart);
}

New API is in develop branch so if you want to try it right now you can build the library from sources.

Also I'm thinking on providing customizable beat division so it can be ticks or cents.

ronnieoverby commented 5 years ago

Awesome!

For my purposes I needed to expand my original Convert method to calculate both time and length of an event:

static readonly BarBeatTimeSpan WdOneBeat = new BarBeatTimeSpan(0, 1);
(long time, long len) ConvertWetDry((long bar, double beat, double len) e, TempoMap tempoMap)
{
        // my bars/beats are 1-based
    var bar = e.bar - 1;
    var beat = e.beat - 1;

    var wdTime = TimeConverter.ConvertFrom(
        new BarBeatTimeSpan(bar, (long)beat), tempoMap);

    var lzOneBeatTicks = new Lazy<long>(Get1BeatTicks);
    var fraction = e.beat - Math.Truncate(e.beat);
    if (fraction != 0)
    {
        // there is a fraction of a beat to account for
        // how manyTicks is OneBeat at this time?
        var extraTicks = fraction * lzOneBeatTicks.Value;
        wdTime += (long)extraTicks;
    }

    // now the length
    var wdLen = LengthConverter.ConvertFrom(
        new BarBeatTimeSpan(bar, (long)e.len), wdTime, tempoMap);

    fraction = e.len - Math.Truncate(e.len);
    if (fraction != 0)
    {
        var extraTicks = fraction * lzOneBeatTicks.Value;
        wdLen += (long)extraTicks;
    }

    return (wdTime, wdLen);

    long Get1BeatTicks() =>
        LengthConverter.ConvertFrom(WdOneBeat, wdTime, tempoMap);
}

I will rewrite this method using your new utilities on the develop branch the next chance I get.

Thanks!

melanchall commented 5 years ago

@ronnieoverby I've added new time span class – BarBeatCentsTimeSpan. Cents is a double number from 0 to 100. So you can throw away your conversion and work with this new class :)

BarBeatCentsTimeSpan is in develop branch so if you want to try it right now you can build the library from sources.

melanchall commented 5 years ago

@ronnieoverby Did you try new time span class?

ronnieoverby commented 5 years ago

Not yet, though I'm excited about it. I'll try it today and let you know. Thanks!

On Tue, Sep 10, 2019, 6:09 PM Maxim Dobroselsky notifications@github.com wrote:

@ronnieoverby https://github.com/ronnieoverby Did you try new time span class?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/melanchall/drywetmidi/issues/32?email_source=notifications&email_token=AAAYVJGFCXQLKAAGWAA2TVLQJALJPA5CNFSM4IQLYVFKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD6MUWXI#issuecomment-530139997, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAYVJE25AKNXU6T3QM2NG3QJALJPANCNFSM4IQLYVFA .

melanchall commented 5 years ago

@ronnieoverby Any news? :)

ronnieoverby commented 5 years ago

Seems to work, but I did run into an argument range exception with cents.

I would prefer if BarBeatCentsTimeSpan didn't have a Cents property, but instead had a double Beats property, that could represent both the beat and the cents with one value. That way, any value for the cents part would be valid.

In any case, it's nice to avoid having to do the math myself. Thanks for the improvement.

melanchall commented 5 years ago

OK, I'll think on combining beats and cents together. The problem is how to name the time span class :)

ronnieoverby commented 5 years ago

Why should the name change? It still has the same purpose and advantages, irrespective of its implementation.

On Tue, Sep 17, 2019 at 5:17 PM Maxim Dobroselsky notifications@github.com wrote:

OK, I'll think on combining beats and cents together. The problem is how to name the time span class :)

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/melanchall/drywetmidi/issues/32?email_source=notifications&email_token=AAAYVJCTOI4PVVTBCD3KZSDQKFCN5A5CNFSM4IQLYVFKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD6554MI#issuecomment-532405809, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAYVJAV72AHW7BKQK55UYLQKFCN5ANCNFSM4IQLYVFA .

melanchall commented 5 years ago

Current name implies there is a cents part of the time span. But for double beat there is no this part. It's just bar and fractional beat. Cent is a hundredth part (like percent). But fractional part of a beat is not.

There is a classical programming joke: There are two hard things in computer science: cache invalidation, naming things, and off-by-one errors. :)

Maybe BarBeatFractionTimeSpan is a good name?

ronnieoverby commented 5 years ago

You're right. Cents != fractional beat. I guess that name is as good as any.

On Wed, Sep 18, 2019 at 7:40 AM Maxim Dobroselsky notifications@github.com wrote:

Current name implies there is a cents part of the time span. But for double beat there is no this part. It's just bar and fractional beat. Cent is a hundredth part (like percent). But fractional part of a beat is not.

There is a classical programming joke: There are two hard things in computer science: cache invalidation, naming things, and off-by-one errors. :)

Maybe BarBeatFractionTimeSpan is a good name?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/melanchall/drywetmidi/issues/32?email_source=notifications&email_token=AAAYVJFBNCEVSMGSK4G3S4TQKIHUBA5CNFSM4IQLYVFKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD67YT5Q#issuecomment-532646390, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAYVJGJCWTZXIFZMXNEFCDQKIHUBANCNFSM4IQLYVFA .

melanchall commented 5 years ago

I've changed BarBeatCentsTimeSpan to BarBeatFractionTimeSpan with Bars and Beats properties where Beats are double.

@ronnieoverby Please check this new API and close the issue if you're happy with it.

ronnieoverby commented 5 years ago

That change suits my purposes perfectly. I'm very grateful that you were so considerate of what I was trying to accomplish.