Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for SequencerSpecific Meta Event. #172

Closed
NeroBlackstone opened this issue Jul 16, 2024 · 6 comments
Closed

Add support for SequencerSpecific Meta Event. #172

NeroBlackstone opened this issue Jul 16, 2024 · 6 comments

Comments

@NeroBlackstone
Copy link
Member

NeroBlackstone commented Jul 16, 2024

Is your feature request related to a problem? Please describe.
If MIDI file is recorded by a physical device, it will include Sequencer Specific Event.

Sequencer specific
FF 7F len data

The Sequencer specific meta-event is used to store vendor-proprietary data in a MIDI file. The Length can be any value between 0 and 228-1, specifying the number of Data bytes (values between 0 and 255) which follow. Sequencer specific records may be very long.

Currently, MIDI.jl has not defined this event in MIDI_EVENTS_DEFS, so any MIDI file opening that contains a Sequencer Specific Event will throw an exception.

Describe the solution you'd like
Since the data of this event is variable length, to be honest, I don't know how to define this meta event.

"""    SequencerSpecificEvent <: MetaEvent
The `SequencerSpecificEvent` is used to store vendor-proprietary data in a MIDI file.

## Fields:
* `dT::Int` : Delta time in ticks.
* `metatype::UInt8` : Meta type byte of the event.
* `ssdata::Vector{UInt8}` : Vendor-proprietary data.
"""
SequencerSpecificEvent

const MIDI_EVENTS_DEFS = Dict(
0x7f => (
        type = :SequencerSpecificEvent,
 # this should receive multiple variable since data length is variable
        fields = ["ssdata::Vector{UInt8}"],
        decode = :(data),
        encode = :(event.ssdata)
    ),
....
)

# When construct SequencerSpecificEvent:
# MethodError: no method matching MIDI.SequencerSpecificEvent(::Int64, ::UInt8, ::UInt8, ::UInt8, ::UInt8, ::UInt8, ::UInt8, ::UInt8)
@Datseris
Copy link
Member

Since the data of this event is variable length, to be honest, I don't know how to define this meta event

What do you mean? MIDI.jl already has the infrastructure to read variable length events, we already do it in the source code for some events. What stops you from using the same code for this new event?

@NeroBlackstone
Copy link
Member Author

NeroBlackstone commented Jul 16, 2024

It's a bit complicated, but I'll try to explain the problem clearly.

Since we don't have to decode the data in Sequencer Specific Event raw data (vendor-proprietary data), we just want to move the data intact to the ssdata field of SequencerSpecificEvent type.

So the fields of SequencerSpecificEvent is:

Fields:

  • dT::Int : Delta time in ticks.
  • metatype::UInt8 : Meta type byte of the event.
  • ssdata::Vector{UInt8} : Vendor-proprietary raw data. (# here is the problem)

And if we define (store raw data so no encode and decode, just same):

0x7f => (
        type = :SequencerSpecificEvent,
        fields = ["ssdata::Vector{UInt8}"],
        decode = :(data), # raw data here
        encode = :(event.ssdata)
    ), 

We wiil use these codes to generate the Types and Constrctors:

MIDI.jl/src/events.jl

Lines 120 to 142 in 3868e9e

function define_type(type, fields, decode, encode_, supertype, typebyte)
@eval begin
mutable struct $type <: $supertype
dT::Int
$(fields...)
end
""" $($type)(dT::Int, typebyte::UInt8, data::Vector{UInt8})
Returns a `$($type)` event from it's byte representation.
The parameter `typebyte::UInt8` is its's $(($type <: MetaEvent) ? "metatype" : "status") byte.
"""
function $type(dT::Int, typebyte::UInt8, data::Vector{UInt8})
$type(dT, typebyte, $decode...)
end
if $type <: MetaEvent
@doc """ $($type)(dT::Int, args...)
Construct a `$($type)` event without specifying the metatype byte.
"""
function $type(dT::Int, args...)
length(args) != length(fieldnames($type)) - 2 && throw(MethodError($type, (dT, args...)))
$type(dT, $typebyte, args...)
end

Okay, so when we construct SequencerSpecificEvent (type(dT, metatype, data)), we invoke these code to decode the Bytes first (but do nothing here except ... spread, since we want raw data):

MIDI.jl/src/events.jl

Lines 131 to 133 in 3868e9e

function $type(dT::Int, typebyte::UInt8, data::Vector{UInt8})
$type(dT, typebyte, $decode...)
end

then the invoke:

MIDI.jl/src/events.jl

Lines 139 to 142 in 3868e9e

function $type(dT::Int, args...)
length(args) != length(fieldnames($type)) - 2 && throw(MethodError($type, (dT, args...)))
$type(dT, $typebyte, args...)
end

Since it's variable length events so length(args) never be same, and a MethodError will throw, this is problem.

@NeroBlackstone
Copy link
Member Author

NeroBlackstone commented Jul 16, 2024

Yes Text is also variable length but it doesn't have this problem, because after decode it will turn to Vector{String}, after spread it will become normal String.

MIDI.jl/src/events.jl

Lines 110 to 114 in 3868e9e

text_defs = (
fields = ["metatype::UInt8", "text::String"],
decode = :([join(Char.(data))]),
encode = :(UInt8.(collect(event.text)))
)

What if we decode SequencerSpecificEvent as above Text?:

    0x7f => (
        type = :SequencerSpecificEvent,
        fields = ["ssdata::Vector{UInt8}"],
        decode = :([data]), # decode to Vector{Vector{UInt8}}
        encode = :(event.ssdata)
    ),

It's a infinity loop and cause StackOverflowError, because it repeatly spread and collect once and once again.

MIDI.jl/src/events.jl

Lines 131 to 133 in 3868e9e

function $type(dT::Int, typebyte::UInt8, data::Vector{UInt8})
$type(dT, typebyte, $decode...)
end

@NeroBlackstone
Copy link
Member Author

I have a ugly but work solution:

    0x7f => (
        type = :SequencerSpecificEvent,
        fields = ["ssdata::Vector{Vector{UInt8}}"],
        decode = :([[data]]),
        encode = :(event.ssdata)
    ),

And length(ssdata) is 1, ssdata[1] is raw MIDI event data

@NeroBlackstone
Copy link
Member Author

What do you think? I can make a PR to temporarily support SequencerSpecificEvent until we can come up with a better solution. (At least opening MIDI files won't throw an exception)

@NeroBlackstone
Copy link
Member Author

Okay I have a better solution and I will post tomorrow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants