JuliaMusic / MusicXML.jl

MusicXML in Julia
MIT License
13 stars 7 forks source link

MIDI.jl package compatibility #3

Open aminya opened 5 years ago

aminya commented 5 years ago

Links: http://usermanuals.musicxml.com/MusicXML/MusicXML.htm#TutMusicXML4-1.htm%3FTocPath%3DMusicXML%25203.0%2520Tutorial%7C_____5

Some packages (from other languages) that convert musicxml to midi or vice-versa to learn lessons from: https://github.com/magenta/note-seq/blob/master/note_seq/musicxml_parser.py https://github.com/oov/mxl2mid/blob/master/mxl/convert.go https://github.com/slpopejoy/fadno/blob/master/src/Fadno/Note.hs https://github.com/Perlence/mxml2midi/blob/master/mxml2midi.py https://github.com/raine0524/XmlMidiParser/tree/master/oc_source

Transferred issue from MIDI.jl :

I developed a powerful MusicXML package in Julia, which allows both writing and parsing musicxml files! https://github.com/aminya/MusicXML.jl

We need to be able to convert the information from MIDI.jl types to MusicXML.jl types and vice versa.

This can also solve other issues, such as https://github.com/JuliaMusic/MIDI.jl/issues/116. Generally, this can help make a more powerful Julia Music ecosystem,

Datseris commented 5 years ago

Awesome! :) Would you like to join JuliaMusic with it? I can give you owner level access immediately after the transfer.

aminya commented 5 years ago

@Datseris I transferred the repository! :)

Datseris commented 5 years ago

Cool, you can now branch and do PRs directly in MIDI or MusicXML.

Now the question is only where should the conversion functionality be: in MIDI.jl or MusicXML.jl ?

I would vote in MusicXML.jl, which could have MIDI as a dependency. So far we have been successful in keeping MIDI.jl entirely pure (only dependency is the Julia lang) and there is a benefit in keeping it this way.

aminya commented 5 years ago

Cool, you can now branch and do PRs directly in MIDI or MusicXML.

Now the question is only where should the conversion functionality be: in MIDI.jl or MusicXML.jl ?

I would vote in MusicXML.jl, which could have MIDI as a dependency. So far we have been successful in keeping MIDI.jl entirely pure (only dependency is the Julia lang) and there is a benefit in keeping it this way.

Thank you!

I agree with having it inside musicxml.

I have written my code such way that extracting data is very easy, and I already have written some code that extracts information from a musicxml file https://github.com/JuliaMusic/MusicXML.jl/blob/b1f01d6fd3343ae3a7125d376701e15ee71ebb9b/src/MusicXML.jl#L979 But because I don't have any variables to store the information, the function is not directly that useful.

I need some help regarding MIDI.jl types. I did not fully understand the concept of events and how it is related to measures or notes.

The important part of MusicXML.jl types are like this:

Datseris commented 5 years ago

See https://juliamusic.github.io/JuliaMusic_documentation.jl/dev/#midi-the-least-you-need-to-know .

At the moment you should care about the getnotes function. This returns all played notes in a MIDI track. The time signature seems important here, and one has to solve https://github.com/JuliaMusic/MIDI.jl/issues/110 first to obtain it.

There is no Rest type in MIDI.jl at the moment, but I am not so sure it deserves a definition. Better would be to define a function that takes the stream of notes and a time signature and then devide the stream into bars. Then another function takes in a bar and makes it music XML. then the individual bars are concatenated.

aminya commented 5 years ago

I believe we should define it. Not only for the sake of the MusicXML, but also because if a human wants to write a simple phrase they think of the rests between notes as some objects.

Datseris commented 5 years ago

What you say seems reasonable, but it is simply not how the MIDI format works. In the MIDI format everything (literally, everything) is defined in time with a temporal interval dt from the previous thing.

There are no rests, you just have increased dt. Therefore it does not make sense to define a Rest type in MIDI, as it will not help and in the opposite confuse people that want to write or read MIDI, simply because it is a concept that does not exist in MIDI.

also because if a human wants to write a simple phrase they think of the rests between notes as some objects

This is also NOT how MIDI.jl, the Julia implementation works. To define a note you simply give the starting position and the ending position, see here: https://juliamusic.github.io/JuliaMusic_documentation.jl/dev/midi/notes/#MIDI.Note

To define a quarter note, a quarter note rest and then a quarter note, I only have to define the second quarter note to start at time 2 * 960/4. I do not have to in addition define a rest existing before hand. And of course having to do this would make things more complicated, not less.

Please try to familiarize yourself a bit more with the MIDI framework. I really believe that you will find it intuitive after you have seen it in use. At the moment it might sound unreasonable but I can vouch that it is actually very smart. Looking at the documentation here: https://juliamusic.github.io/JuliaMusic_documentation.jl/dev/blog/garibaldi_dragadiddle/ will help a lot as I write real music without using any rests.


On the other side, if you think it is useful in MusicXML, then there it makes perfect sense to define it.

aminya commented 5 years ago

@Datseris Thank you for the response! I guess we can make it work without the rest.

On MusicXML side, Rest is defined because it appears as < rest /> in the code, and we need to take care of it, however, for MIDI conversion, I guess we can just use the Pitches and Unpitcheds.

Actually, the example code I wrote, discards the pitches that are nothing (rest):

using MusicXML

# Reads musicxml file and then extracts the data, builds all the types and stores them in proper format.
doc = readmusicxml(joinpath("examples", "musescore.musicxml"))

# Example1:
# Prints Each instrument name and then the pitches

# Extracting each instrument information
scprts = doc.scorepartwise.partlist.scoreparts

# Extracting parts
prts = doc.scorepartwise.parts

# Extracting each part
for prt in prts

    ind = findfirst(x -> prt.ID == x.ID, scprts) # returns the index of scorepart that matches the ID of part

    # printing the instrument name
    println(scprts[ind].name)

    # Extracting each measure of the part
    for msr in prt.measures

        # Extracting notes of each measure
        for nt in msr.notes

            if !isnothing(nt.pitch)
            # print pitch of the note
                println(nt.pitch)
            end

        end

    end
end

MusicXML also has a special object for unpitched notes. So for conversion, both Pitch and Unpitched should be considered, and the rest can be discarded.

Datseris commented 5 years ago

In the code example that you post, how does one obtain the position of each note in a bar? like where they are?

aminya commented 5 years ago

@Datseris Oh my bad. MusicXML doesn't have a time location. Instead, it has the duration of each Note. Each note can be a Pitch, Rest, or Unpitched. So we need to define some function that calculates the time location by summing up the durations

Note: https://juliamusic.github.io/MusicXML.jl/dev/#MusicXML.Note Pitch: https://juliamusic.github.io/MusicXML.jl/dev/#MusicXML.Pitch Rest: https://juliamusic.github.io/MusicXML.jl/dev/#MusicXML.Rest Unpitched: https://juliamusic.github.io/MusicXML.jl/dev/#MusicXML.Unpitched

Datseris commented 5 years ago

But then it seems that it is extremely easy to translate any series of MIDI.Note to a series of MusicXML.Note.

Here I sketch how, without too much thinking about details (that always reveal bugs :D )

notes = getnotes(midi) # this is a series of notes
notesxml = []
prevnote = notes[1]
for note in notes
  if note.pos == prevnote.pos # notes are at the same place
    # make a MusicXML.note with note.pitch and and note.duration
    # and push it to musicxmlnotes ensuring that it starts with the previous note
  elseif note.pos == prevnote.pos + prevnote.duration # the new note starts immediatelly after the previous
    # make a MusicXML.note with note.pitch and and note.duration
    # and push it to musicxmlnotes
  else
    # make a MusicXML.note with rest. The rest duration is equal to:
    rest = note.pos - prevnote.pos - prevnote.duration
    push!(musicxml, rest)
    # then make a note that is has the pitch and duration of the current note and push it to musicxml
  end
  prevnote = note
end
Datseris commented 5 years ago

p.s.: translating the note durations and positions to percentages of the quarter note is straightforward with the notes.tpq number. Each tpq represents one quarter note, typically 960. Therefore an 8th note is 960/2 and a sixteenth note is 960/4, etc.

Datseris commented 5 years ago

Also, I really don't understand why the MusicXML.Note struct has ALL three of these fields:

pitch::Pitch
rest::Rest
unpitched::Unpitched

A note can only be one of these three, not all at the same time. What does this even mean?

aminya commented 5 years ago

I updated the example to represent how MusicXML works: https://github.com/JuliaMusic/MusicXML.jl#usage-example

I will play with your code and commit it! Thanks for the help.

aminya commented 5 years ago

Also, I really don't understand why the MusicXML.Note struct has ALL three of these fields:

pitch::Pitch
rest::Rest
unpitched::Unpitched

A note can only be one of these three, not all at the same time. What does this even mean?

These fields are all under Note because they represent an object in xml. But it doesn't mean that they are always present, and if they aren't passed to the type, they are considered to be nothing.

Originally, I had a logic for checking that only one is provided, but I didn't see this checking necessary. Someone can make a wrong MusicXML file, but they shouldn't. In @aml definition, the function constructor is defined automatically. I haven't added an option for custom error checking to AML yet

Datseris commented 5 years ago

I am not sure whether this Note is good design. That is because it introduces unecessary complexity in the struct and unecessary performance loss due to the unstable types.... Wouldn't it be better to change this

@aml mutable struct Note "note"
    pitch::UN{Pitch} = nothing, "pitch"
    rest::UN{Rest} = nothing, "rest"
    unpitched::UN{Unpitched} = nothing, "unpitched"
    duration::UInt, "duration"
    # voice
    type::UN{String} = nothing, "type"
    accidental::UN{String} = nothing, "accidental"
    tie::UN{String} = nothing, "tie" # start, stop, nothing TODO
end

to this

@aml mutable struct Note{X} "note"
    identity::X = nothing # or something compatible with @aml macro
    duration::UInt, "duration"
    # voice
    type::UN{String} = nothing, "type"
    accidental::UN{String} = nothing, "accidental"
    tie::UN{String} = nothing, "tie" # start, stop, nothing TODO
end

and leverage parametric Types? Here X will be either Pitch, Unpitched, Rest, depending on what you want to create.

Datseris commented 5 years ago

P.S.: I would also recommend to rename Note to NoteXML, otherwise you will have conflicts with MIDI.jl, since it also exports Note.

aminya commented 5 years ago

I am not sure whether this Note is good design. That is because it introduces unecessary complexity in the struct and unecessary performance loss due to the unstable types.... Wouldn't it be better to change this

@aml mutable struct Note "note"
    pitch::UN{Pitch} = nothing, "pitch"
    rest::UN{Rest} = nothing, "rest"
    unpitched::UN{Unpitched} = nothing, "unpitched"
    duration::UInt, "duration"
    # voice
    type::UN{String} = nothing, "type"
    accidental::UN{String} = nothing, "accidental"
    tie::UN{String} = nothing, "tie" # start, stop, nothing TODO
end

to this

@aml mutable struct Note{X} "note"
    identity::X = nothing # or something compatible with @aml macro
    duration::UInt, "duration"
    # voice
    type::UN{String} = nothing, "type"
    accidental::UN{String} = nothing, "accidental"
    tie::UN{String} = nothing, "tie" # start, stop, nothing TODO
end

and leverage parametric Types? Here X will be either Pitch, Unpitched, Rest, depending on what you want to create.

Yes I can look into this. This or something similar may be straightforward to implement.

P.S.: I would also recommend to rename Note to NoteXML, otherwise you will have conflicts with MIDI.jl, since it also exports Note.

I think I can keep the name, and instead not export the types.

Datseris commented 5 years ago

I think I can keep the name, and instead not export the types.

Hm, I urge you to reconsider this point.

First, you would have to type MIDI.Note every time you use it in the module. It will also mean that every user that wants to use both MIDI and MusicXML (which seems a very reasonable thing to do) would also have to type MusicXML.Note all the time.

But most importantly, I also think it is a bit confusing for the users. I have so far not seen two different types have exactly the same name in all the Julia packages I use. I would assume that others will find this confusing as well.

aminya commented 5 years ago

I think I can keep the name, and instead not export the types.

Hm, I urge you to reconsider this point.

First, you would have to type MIDI.Note every time you use it in the module. It will also mean that every user that wants to use both MIDI and MusicXML (which seems a very reasonable thing to do) would also have to type MusicXML.Note all the time.

But most importantly, I also think it is a bit confusing for the users. I have so far not seen two different types have exactly the same name in all the Julia packages I use. I would assume that others will find this confusing as well.

Good point. NoteX is probably a better name.

to this

@aml mutable struct Note{X} "note"
    identity::X = nothing, "identity" # or something compatible with @aml macro
end

and leverage parametric Types? Here X will be either Pitch, Unpitched, Rest, depending on what you want to create.

AML doesn't support curly braces now. I had this in plan though, and I have a sketch for it

But if we want to have the logic checking for now, we can use AML utility functions and write something like:

# UN{T} means Union{T, Nothing}

mutable struct Identity
    pitch::UN{Pitch}
    rest::UN{Rest}
    unpitched::UN{Unpitched}
    aml::Node
end

function Identity(;pitch = nothing, rest = nothing, unpitched = nothing)

    if pitch != nothing
        addelementOne!(aml, "pitch", pitch)
    elseif rest != nothing
        addelementOne!(aml, "rest", rest)
    elseif unpitched != nothing
        addelementOne!(aml, "unpitched", unpitched)
    else
        error("one of the pitch, rest or unpitched should be given")
    end

    return Identity(pitch, rest, unpitched, aml)
end

function Identity(;aml)

        pitch = findfirstcontent(Pitch, "pitch", aml, 0)
        rest = findfirstcontent(Rest, "rest", aml, 0)
        unpitched = findfirstcontent(Unpitched, "unpitched", aml, 0)

        return Identity(pitch, rest, unpitched, aml)
end
Datseris commented 5 years ago
mutable struct Identity
    pitch::UN{Pitch}
    rest::UN{Rest}
    unpitched::UN{Unpitched}
    aml::Node
end

isn't this the same as the original version? What is the benefit of using this Identity...? you once again have these three fields, even though only one can exist at one instance...

aminya commented 5 years ago
mutable struct Identity
    pitch::UN{Pitch}
    rest::UN{Rest}
    unpitched::UN{Unpitched}
    aml::Node
end

isn't this the same as the original version? What is the benefit of using this Identity...? you once again have these three fields, even though only one can exist at one instance...

If you see the original musicxml definition of the <note> it includes all the fields together, but it is of type choice, which I have added an issue to support in AML (https://github.com/aminya/AML.jl/issues/23):

https://usermanuals.musicxml.com/MusicXML/Content/EL-MusicXML-note.htm: image

We need to write a code that generates/extracts the xml(aml) automatically, so the constructors for the type should be like this. But maybe we can write sound and identity like this if you want to not have fields for it:

@aml mutable struct NoteX "note"
    sound::Sound= nothing, "sound" 
    duration::UInt, "duration"
    # voice
    type::UN{String} = nothing, "type"
    accidental::UN{String} = nothing, "accidental"
    tie::UN{String} = nothing, "tie" # start, stop, nothing TODO
end

# UN{T} means Union{T, Nothing}

mutable struct Sound{X}
    identity::X
    aml::Node
end

function Sound(;identity = nothing)

    if isa(sound, Pitch)
        addelementOne!(aml, "pitch", pitch)
    elseif isa(sound, Rest)
        addelementOne!(aml, "rest", rest)
    elseif  isa(sound, Unpitched)
        addelementOne!(aml, "unpitched", unpitched)
    else
        error("one of the pitch, rest or unpitched should be given")
    end

    return Sound(identity, aml)
end

function Sound(;aml)

        pitch = findfirstcontent(Pitch, "pitch", aml, 0)
        if !isnothing(pitch)
        idenitty = pitch
        end
        rest = findfirstcontent(Rest, "rest", aml, 0)
        if !isnothing(rest )
        idenitty = rest 
        end
        unpitched = findfirstcontent(Unpitched, "unpitched", aml, 0)
        if !isnothing(unpitched )
        idenitty = unpitched 
        end

        return Sound(identity, aml)
end
aminya commented 5 years ago

I believe the current version is much more compact. We can use the 2nd version or 3rd version, but other than this logic checking they don't add anything to the code. We need to have a struct with aml field that builds the XML file. So, I prefer to stick to the 1st version.

P.S: I think because these are all under <note>, I need to find another function, and I think addelementOne! doesn't work here. Probably aml=ElementNode("name") should be used

Datseris commented 5 years ago

Okay then, yes the 1st version is definitely a much better alternative. In fact now that you show me the musicxml situation the original version of NoteX with all three fields rest, unpitched pitched is the most compact.

I think for now I'll leave you finish the XML logic. I don't know XML anyway. Once you have a convertor ready, using the preliminary sketch I provided, then I can check again.

aminya commented 5 years ago

Because in musicxml files, all the types are written in lower case, if I define the types in lower case, then we don't need to change Note to NoteX or something.

Edit: this causes more issues because of time, transpose, etc

aminya commented 5 years ago

Also, I added support for parametric type structs to AcuteML (the new name for AML). https://github.com/aminya/AcuteML.jl/issues/25

@aml struct note{X}
sound::X, "~"
end

note{Pitch}(sound= Pitch(step = "C", alter = 0, octave = 0) )

If we want, we can switch to this syntax

aminya commented 5 years ago

Edit: this doesn't work for note, because it adds a <sound> <sound/> around the added pitch, rest, or unpitched

Also, I added support for parametric type structs to AcuteML (the new name for AML). https://github.com/aminya/AcuteML.jl/issues/25

@aml struct note{X}
sound::X, "~"
end

note{Pitch}(sound= Pitch(step = "C", alter = 0, octave = 0) )

If we want, we can switch to this syntax

aminya commented 5 years ago

Some packages that convert musicxml to midi or vice-versa to learn lessons from: https://github.com/tensorflow/magenta/blob/master/magenta/music/musicxml_parser.py https://github.com/oov/mxl2mid/blob/master/mxl/convert.go https://github.com/slpopejoy/fadno/blob/master/src/Fadno/Note.hs https://github.com/Perlence/mxml2midi/blob/master/mxml2midi.py https://github.com/raine0524/XmlMidiParser/tree/master/oc_source