Xlinka / Project-Obsidian

Project Obsidian: A Resonite plugin adding extended Protoflux nodes and features for enhanced functionality and user experience
GNU General Public License v3.0
15 stars 6 forks source link

MIDI: Fine adjust CC controls are not interpreted correctly #51

Closed Nytra closed 1 month ago

Nytra commented 1 month ago

Some CC controls need to be interpreted as a 14 bit value instead of a 7 bit value

These are sent as two consecutive MIDI messages where the third byte of the first message represents bits 7 to 13 (Msb/coarse) and the third byte of the second message represents bits 0 to 6 (Lsb/fine) and these bytes need to be combined to get the final value

http://midi.teragonaudio.com/tech/midispec/ctllist.htm

Xlinka commented 1 month ago

thrown together some proposed changes.

private void OnMessageReceived(object sender, MidiReceivedEventArgs args)
{
    if (DEBUG) UniLog.Log($"Received {args.Length} bytes");
    if (DEBUG) UniLog.Log($"Timestamp: {args.Timestamp}");
    if (DEBUG) UniLog.Log($"Start: {args.Start}");
    var events = MidiEvent.Convert(args.Data, args.Start, args.Length);
    foreach (var e in events)
    {
        if (DEBUG) UniLog.Log(e.ToString());
        RunSynchronously(() =>
        {
            _lastEvent.Value = e.ToString();
        });

        switch (e.EventType)
        {
            case MidiEvent.NoteOn:
                NoteOn?.Invoke(this, new MIDI_NoteEventData(e.Channel, e.Msb, e.Lsb));
                break;
            case MidiEvent.NoteOff:
                NoteOff?.Invoke(this, new MIDI_NoteEventData(e.Channel, e.Msb, e.Lsb));
                break;
            case MidiEvent.CAf:
                ChannelPressure?.Invoke(this, new MIDI_ChannelPressureEventData(e.Channel, e.Msb));
                break;
            case MidiEvent.CC:
                HandleControlChange(e);
                break;
            case MidiEvent.Pitch:
                PitchWheel?.Invoke(this, new MIDI_PitchWheelEventData(e.Channel, CombineBytes(e.Msb, e.Lsb)));
                break;
            case MidiEvent.PAf:
                Aftertouch?.Invoke(this, new MIDI_AftertouchEventData(e.Channel, e.Msb, e.Lsb));
                break;

            // Unhandled events:

            //SysEx events are probably not worth handling
            case MidiEvent.SysEx1:
                //if (DEBUG) UniLog.Log("UnhandledEvent: SysEx1");
                break;
            case MidiEvent.SysEx2:
                // Same as EndSysEx
                //if (DEBUG) UniLog.Log("UnhandledEvent: SysEx2");
                break;

            case MidiEvent.Program:
                if (DEBUG) UniLog.Log("UnhandledEvent: Program");
                break;
            case MidiEvent.MtcQuarterFrame:
                if (DEBUG) UniLog.Log("UnhandledEvent: MtcQuarterFrame");
                break;
            case MidiEvent.SongPositionPointer:
                if (DEBUG) UniLog.Log("UnhandledEvent: SongPositionPointer");
                break;
            case MidiEvent.SongSelect:
                if (DEBUG) UniLog.Log("UnhandledEvent: SongSelect");
                break;
            case MidiEvent.TuneRequest:
                if (DEBUG) UniLog.Log("UnhandledEvent: TuneRequest");
                break;
            case MidiEvent.MidiClock:
                if (DEBUG) UniLog.Log("UnhandledEvent: Clock");
                break;
            case MidiEvent.MidiTick:
                if (DEBUG) UniLog.Log("UnhandledEvent: MidiTick");
                break;
            case MidiEvent.MidiStart:
                if (DEBUG) UniLog.Log("UnhandledEvent: MidiStart");
                break;
            case MidiEvent.MidiStop:
                if (DEBUG) UniLog.Log("UnhandledEvent: MidiStart");
                break;
            case MidiEvent.MidiContinue:
                if (DEBUG) UniLog.Log("UnhandledEvent: MidiContinue");
                break;
            case MidiEvent.ActiveSense:
                if (DEBUG) UniLog.Log("UnhandledEvent: ActiveSense");
                break;
            case MidiEvent.Reset:
                // Same as Meta
                if (DEBUG) UniLog.Log("UnhandledEvent: Reset");
                break;
            default:
                break;
        }
    }
}
//no clue if this works but this a slapped together job 
private void HandleControlChange(MidiEvent e)
{
    int controller = e.Msb; // eeeeeeeeeeee
    int value = e.Lsb;

    // Check if the controller number is a coarse adjust (MSB)
    if (controller >= 0 && controller <= 31)
    {
        // Store the MSB value
        _ccMsbValues[controller] = value; //god hopes this doesnt cause mem issues
    }
    // Check if the controller number is a fine adjust (LSB)
    else if (controller >= 32 && controller <= 63)
    {
        int coarseController = controller - 32;
        if (_ccMsbValues.TryGetValue(coarseController, out int msbValue))
        {
            // Combine MSB and LSB values to form the final 14-bit value :)
            int finalValue = (msbValue << 7) | value;
            Control?.Invoke(this, new MIDI_CC_EventData(e.Channel, coarseController, finalValue));
            _ccMsbValues.Remove(coarseController);  // Clear the stored MSB value
        }
    }
    else
    {
        // Handle as a 7-bit CC value 

        //who the hell uses 7 bit ?
        Control?.Invoke(this, new MIDI_CC_EventData(e.Channel, controller, value));
    }
}
Nytra commented 1 month ago

you can just check if the length of events is 2 and if both event types are CC then do it before the for loop

Nytra commented 1 month ago

also there is a CombineBytes method already to combine the bytes

Nytra commented 1 month ago

i don't think you need the _ccMsbValues really

Xlinka commented 1 month ago

so something like this then ?

private void OnMessageReceived(object sender, MidiReceivedEventArgs args)
{
    if (DEBUG) UniLog.Log($"Received {args.Length} bytes");
    if (DEBUG) UniLog.Log($"Timestamp: {args.Timestamp}");
    if (DEBUG) UniLog.Log($"Start: {args.Start}");
    var events = MidiEvent.Convert(args.Data, args.Start, args.Length);

    if (events.Length == 2 && events[0].EventType == MidiEvent.CC && events[1].EventType == MidiEvent.CC)
    {
        var e1 = events[0];
        var e2 = events[1];
        if (e1.Channel == e2.Channel && (e1.Msb < 32 && e2.Msb == e1.Msb + 32))
        {
            int finalValue = CombineBytes(e1.Lsb, e2.Lsb);
            Control?.Invoke(this, new MIDI_CC_EventData(e1.Channel, e1.Msb, finalValue));
            return;
        }
    }

    foreach (var e in events)
    {
        if (DEBUG) UniLog.Log(e.ToString());
        RunSynchronously(() =>
        {
            _lastEvent.Value = e.ToString();
        });

        switch (e.EventType)
        .... //and so on 
Nytra commented 1 month ago

yea but you don't need this check (e1.Msb < 32 && e2.Msb == e1.Msb + 32)

and finalValue should be CombineBytes(e2.Lsb, e1.Lsb) I think

Xlinka commented 1 month ago
private void OnMessageReceived(object sender, MidiReceivedEventArgs args)
{
    if (DEBUG) UniLog.Log($"Received {args.Length} bytes");
    if (DEBUG) UniLog.Log($"Timestamp: {args.Timestamp}");
    if (DEBUG) UniLog.Log($"Start: {args.Start}");
    var events = MidiEvent.Convert(args.Data, args.Start, args.Length);

    if (events.Length == 2 && events[0].EventType == MidiEvent.CC && events[1].EventType == MidiEvent.CC)
    {
        var e1 = events[0];
        var e2 = events[1];
        int finalValue = CombineBytes(e2.Lsb, e1.Lsb);
        Control?.Invoke(this, new MIDI_CC_EventData(e1.Channel, e1.Msb, finalValue));
        return;
    }

    foreach (var e in events)
    {
        if (DEBUG) UniLog.Log(e.ToString());
        RunSynchronously(() =>
        {
            _lastEvent.Value = e.ToString();
        });

reeeeee ??

Nytra commented 1 month ago

yeah that looks good ty

Nytra commented 1 month ago

probably Control?.Invoke(this, new MIDI_CC_EventData(e2.Channel, e2.Msb, finalValue)); tho