ncassetta / NiCMidi

A MIDI C++ library with objects for reading, writing, playing, editing and recording midi files
GNU Lesser General Public License v3.0
2 stars 1 forks source link

Compilation error when using test_recorder example #2

Closed goofy2k closed 2 years ago

goofy2k commented 2 years ago

../include/recorder.h:32:10: fatal error: process.h: No such file or directory #include "process.h"

Line 32 in recorder.h should be: #include "processor.h" ?

goofy2k commented 2 years ago

At least, using the proposed change removes the compilation error

ncassetta commented 2 years ago

Yes, the class MIDIRecorderState inherits from MIDIProcessor which is defined in "processor.h". Thank you, I will correct

goofy2k commented 2 years ago

Hi Nic, I think that I'm going to give in. Despite various attempts to get things running I did not manage it. I want to use a NimBLEDevice bluetooth connection. But my attempts to cast that into RtMidi did not succeed. I also tried to adapt driver.h / .cpp to use NimBLEDevice directly, bu t this is too complicated for me.

Information about NimBLEDevice is here: https://github.com/h2zero/esp-nimble-cpp

I can run that and send raw messages to my MIDI synthesiser board, but incorporation in your beautiful NiCMidi is too complicated for me.

goofy2k commented 2 years ago

This is a link to my repo..... (it is messy :-) )

https://github.com/goofy2k/MIDI-sequencer

ncassetta commented 2 years ago

I saw your driver.h file in the version 13 and I think that is the right way. You should implement a class with the same methods as the RtMidi ports and assign it the port attribute in the MIDIOutDriver class. NiCMidi uses only some methods of the RtMidi ports. The implementation should be like this:

class MidiOutNimBLE {
    public:
                                    MidiOutNimBLE();
                                    ~MidiOutNimBLE();

        void                         openPort(unsigned int portNumber=0);
        void                         closePort();
        virtual bool                 isPortOpen();
        unsigned int                 getPortCount()      { return 1; }
        std::string                  getPortName(unsigned int portNumber=0);
        void                         sendMessage(const std::vector<unsigned char> *message);
}

(see also the documentation of RtMidi) Moreover you should modify the method MIDIManager::Init() (in the manager.h file) which attempts to open RtMidi ports, making it open your ports. For the MIDIInDriver things are more complex as it relies on a callback called by RtMidi

I cannot help you on NimBLE because I don't know it at all.

goofy2k commented 2 years ago

I'll give that a try! Many thanks.

Meanwhile I went back to v12. So with RtMidi. I try to make a class, that should be globally accessible where I can store the pointer to the nimBLE driver.

Giving in is not easy ;-) The rewards are high when I succeed ;-).

Thanks again Fred

Op vr 3 dec. 2021 15:00 schreef Nicola Cassetta @.***>:

I saw your driver.h file in the version 13 and I think that is the right way. You should implement a class with the same methods as the RtMidi ports and assign it the port attribute in the MIDIOutDriver class. NiCMidi uses only some methods of the RtMidi ports. The implementation should be like this:

class MidiOutNimBLE { public: MidiOutNimBLE(); ~MidiOutNimBLE();

    void                         openPort(unsigned int portNumber=0);
    void                         closePort();
    virtual bool                 isPortOpen();
    unsigned int                 getPortCount()      { return 1; }
    std::string                  getPortName(unsigned int portNumber=0);
    void                         sendMessage(const std::vector<unsigned char> *message);

}

(see also the documentation of RtMidi) Moreover you should modify the method MIDIManager::Init() (in the manager.h file) which attempts to open RtMidi ports, making it open your ports. For the MIDIInDriver things are more complex as it relies on a callback called by RtMidi

I cannot help you on NimBLE because I don't know it at all.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/ncassetta/NiCMidi/issues/2#issuecomment-985543360, or unsubscribe https://github.com/notifications/unsubscribe-auth/AGII3TVPE3C2LYXRDMONGXTUPDEQPANCNFSM5IYMILWA .

goofy2k commented 2 years ago

So, I want to start the nimBLE server before doing any work with NicMidi.

This is because I have demonstrated proper opreation of the driver like that.

But then I have to pass info (a pointer, or even an object with more connection info about the server into NicMidi (v12) or RtMidi (v13).

Note: v12 is not up to date in the repo

Fred

goofy2k commented 2 years ago

Updated it ...

ncassetta commented 2 years ago

If you have to initialize something before using NiCMidi you can do it in the MIDIManager::Init() method: this is called the first time NicMidi uses a MIDIManager method and creates all the in/out drives (I initially made it static, but I got the so called "static initialization fiasco"). It would be a good place to set the NimBLE initialization.

goofy2k commented 2 years ago

OK. I'll keep that in mind.

Is that a better location than in driver?

I will try to start a hardware interface contoller well before calling your manager. I have demonstrated that it can work in that way. But I will give it the same interface as used by your code. So, compatible with the RtMidi API.

What do you think about direct communication with manager/driver or casting it in RtMidi?

Fred

Today I managed to share data between a "global" nimBLEdriver object and other code. This is in v12 (RtMidi) , but I can use the same trick in v13 (no RtMidi).

Op vr 3 dec. 2021 21:09 schreef Nicola Cassetta @.***>:

If you have to initialize something before using NiCMidi you can do it in the MIDIManager::Init() method: this is called the first time NicMidi uses a MIDIManager method and creates all the in/out drives (I initially made it static, but I got the so called "static initialization fiasco"). It would be a good place to set the NimBLE initialization.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/ncassetta/NiCMidi/issues/2#issuecomment-985796347, or unsubscribe https://github.com/notifications/unsubscribe-auth/AGII3TU6LOPTFXDV52VVVH3UPEPW7ANCNFSM5IYMILWA .

ncassetta commented 2 years ago

In the MIDIManager::Init() method (line 277 of manager.h) the manager asks RtMidi for the number of MIDI out ports in the system, and then (lines 278 279) creates a MIDIOutDriver for each of them and pushes it into its internal vector. Similarly in lines 282-284 for the MIDI in. Therefore you should make RtMidi recognize your NimBLE drivers as MIDI ports, or, without RtMidi, you should create by yourself the MIDIInDriver and MIDIOutDriver that communicate with your hardware ports.

goofy2k commented 2 years ago

Thanks, work in progress. At first WITH the RtMidi glue.

Op za 4 dec. 2021 16:56 schreef Nicola Cassetta @.***>:

In the MIDIManager::Init() method (line 277 of manager.h) the manager asks RtMidi for the number of MIDI out ports in the system, and then (lines 278 279) creates a MIDIOutDriver for each of them and pushes it into its internal vector. Similarly in lines 282-284 for the MIDI in. Therefore you should make RtMidi recognize your NimBLE drivers as MIDI ports, or, without RtMidi, you should create by yourself the MIDIInDriver and MIDIOutDriver that communicate with your hardware ports.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/ncassetta/NiCMidi/issues/2#issuecomment-986049285, or unsubscribe https://github.com/notifications/unsubscribe-auth/AGII3TQUFOXJPIZOB7EMXX3UPI23DANCNFSM5IYMILWA .

goofy2k commented 2 years ago

Have a MidiOutDriver in place.(v15). It reponds OK when a peer device connects or disconnects. Initialisation of the driver is not in MIDIManager. I'll probably do that later. I'll now adapt MIDIManager such that it detects the port (v16). I use your test_component example for testing as it is relatively simple.

goofy2k commented 2 years ago

Currently the app crashes as MIDIManager detects no ports.

goofy2k commented 2 years ago

Succes in v16! The MidiOutNimBLE class has an interface that is similar to the interface offered by RtMidi. It does NOT use RtMidi. I demonstrated it's functionality with an example similar to your component example. Three byte note on- note off messages arrive OK at the synth board.

I had to make some slight adaptations in your code to get things running. These have to do with the fact that I don't use RtMidi.

The code is still messy and there are some bugs relating to the messaging. I think that I move on with exploring more complicated examples of NiCMidi.

ncassetta commented 2 years ago

Glad you succeeded! As I told you in a previous message, NiCMidi does not use all the methods of RtMidi but only the ones I have listed above, so you can simplify the implementation of your class and replace all the calls to RtMidi with your own methods (they are only in the files "driver.h" and "manager.h "). If you want to test the AdvancedSequencer component you can copy and paste the two examples in the "Overview - Loading and playing midi files" section of the documentation, which play a midi file (you have to copy also the "twinkle.mid" file).

goofy2k commented 2 years ago

Thanks for the suggestions! My ESP32 board (a Heltec Lora32 V2) does not have a direct way to read files. It does not have an SD card. I can reach the board (for any I/O) via MQTT served by Nodered. For testing I can send e.g. single note on/off messages in a Nodered GUI. So, for the time being I want to keep it with playing single notes....

Before I go on with the input side, I want to test running your advancedsequencer and recording capability. Also want to discover GUInotifications in that go.

Op wo 8 dec. 2021 11:55 schreef Nicola Cassetta @.***>:

Glad you succeeded! As I told you in a previous message, NiCMidi does not use all the methods of RtMidi but only the ones I have indicated above, so you can simplify the implementation and replace all the calls to RtMidi with your own methods (they are only in the files "driver.h" and "manager.h "). If you want to test the AdvancedSequencer component you can copy and paste the two examples in the "Overview - Loading and playing midi files" section of the documentation (you have to copy also the "twinkle.mid" file)

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/ncassetta/NiCMidi/issues/2#issuecomment-988706803, or unsubscribe https://github.com/notifications/unsubscribe-auth/AGII3TSFSRNV57VPRQ2VUF3UP42Q7ANCNFSM5IYMILWA .

goofy2k commented 2 years ago

The instantiations for the test_recorder example immediately went well! After that, I can call DumpAllTracksAttr and the log screen spits out the empty track :-)

In your examples you use command line entries. As I can not use that (*), I now have to create a basic API over MQTT to have some flexible way to enter commands....

I think the real fun part is coming soon!

goofy2k commented 2 years ago

Getting midi input running is pretty complex for me. This is probably due to the complexity of RtMidi, which is a starting point for me. In my current version I also applied patches in the NiCMidi code. This makes it hard to detect if new bugs are caused by shortcomings in my low level midi input code due to poor translation of the RtMidi template OR due to the changes in your code.

I think that I open 2 new projects. A first one that uses NiCMidi and RtMidi. I will try to implement MQTT input and nimBLE output by only adding new hardware dependent, low level routines in RtMidi. A second one uses NicMidi and newly created low level MQTTin and nimBLEout routines.

In both projects I will try to leave the NiCMidi code untouched, except from the include RtMidi lines in the second project.

Do you think that both of these options are realistic paths to success?

For project 1. I will start with a simple app, based on the examples in the RtMidi docs and some examples from the NiCMidi lib to be used as demonstrators for Midi in and Midi out.

goofy2k commented 2 years ago

I created a code with some of the simplest demonstrators that can be used for testing of Midi in and Midi out drivers. After removing some bugs (commented in main.cpp) there is one single compilation error left over. It is related to selection of the hardware driver. Maybe this has to do with the way I use the code. Perhaps you can have a look and give your thoughts. I created a new repo for this: https://github.com/goofy2k/NiCMidi-RtMidi

ncassetta commented 2 years ago

I can give you some suggestions for implementing a MIDIInnimBLE port similar to the out one. This should be the interface, with only the methods used by NicMidi:

typedef void(* RtMidiCallback )(double timeStamp, std::vector< unsigned char > *message, void *userData);

class MidiInNimBLE {
   public:

                                MidiInNimBLE();
                                ~MidiInNimBLE();           
    void                         openPort(unsigned int portNumber=0);
    void                         closePort();
    virtual bool                 isPortOpen();
    unsigned int                 getPortCount()      { return 1; }
    std::string                  getPortName(unsigned int portNumber=0);
    void                          setCallback(RtMidiCallback callback, void * userData = 0);
   // you can implement this if you want to ignore certain types of incoming messages, otherwise do nothing
    void                        ignoreTypes( bool midiSysex = true, bool midiTime = true, bool midiSense = true) {}
}

(and eventually an error handling class similar to RtMidiError)

The difficult thing is to implement a callback mechanism which calls an user defined function when MIDI data income (in NiCMidi I set the MIDIInDriver::HardwareMsgIn() as callback)

In NiCMidi I done something similar to this in "timer.cpp": the MIDITimer class starts a background thread which calls the ThreadProc() every 10 msecs. Perhaps you may take inspiration by this, eventually using your board timers instead of std::chrono as I done. Your thread procedure should check if MIDI data are arrived, and eventually call the callback set with setCallback() (i.e. HardwareMsgIn())

bool MIDITimer::Start () {
    if (tick_proc == 0)
        return false;                           // Callback not set

    num_open++;
    if (num_open == 1) {                         // Must create thread
        current = std::chrono::steady_clock::now();
        bg_thread = std::thread(ThreadProc);
        std::cout << "Timer open with " << resolution << " msecs resolution" << std::endl;
    }
    return true;
}

void MIDITimer::Stop() {
    if (num_open > 0) {
        num_open--;
        if (num_open == 0) {
            bg_thread.join();

            std:: cout << "Timer stopped by MIDITimer::Stop()" << std::endl;
        }
    }
}

.  .  .

void MIDITimer::ThreadProc() {
    duration tick(resolution);

    while(MIDITimer::num_open) {
        // execute the supplied function
        MIDITimer::tick_proc(MIDITimer::GetSysTimeMs(), MIDITimer::tick_param);
        // find the next timepoint and sleep until it
        current += tick;
        std::this_thread::sleep_until(current);
    }
}

If you succed, replacing calls to RtMidi with calls to your class in "manager.h" and "driver.h" (as you did for the out ports) should be sufficent.

goofy2k commented 2 years ago

Thanks! Need to study this;-). Did you see my reports in the new repo?

Q: do I need to set a compiler directive in RtMidi to give NicMidi access to the proper hardware driver? I now hit on the dummy driver.

goofy2k commented 2 years ago

I understand from your very last remark that you suggest to bypass RtMidi . Correct? My current approach is to add my driver(s) to RtMidi. But I can try both options. Still need to read your suggestions. Watching F1 ;-)

ncassetta commented 2 years ago

@goofy2k Yes, my solution bypasses RtMidi. If you want to use RtMidi you need to understand how to get your hardware ports recognized. I think this is done by interrogating the OS, but I don't know the details: I used Rtmidi precisely because it recognizes and enumerates MIDI ports without worrying about calls to the OS. If you get the dummy driver it means that RtMidi does not recognize any hardware ports. Perhaps you could force the recognition by modifying RtMidi, or perhaps you can add your driver to the system list with some specific system call, I think.

ncassetta commented 2 years ago

Yes, I saw the Win MM Api in RtMidi (file "RtMidi.cpp" from line 2554) and it asks Windows for tthe number of MIDI In devices when initializing MidiInWinMM (at line 2676). So you should implement a MINIInNimBle Api which recognizes and uses your ports using OS calls.

goofy2k commented 2 years ago

I am now working on a version 19 in https://github.com/goofy2k/MIDI-sequencer. Note: at first I want to create an MQTT input (not yet a nimBLE one). My MQTT code is event-based. When a message with topic /fckx_seq/midi/single arrives (main.ccp:553) the content is decoded and displayed for debugging purposes and it is stored in a ring buffer inQ (main.cpp:515). This is (still) a jdksmidi object :-) . I must probably find a way to:

  1. let my MQTT event handler in some way get access to your MIDIRawMessageQueue, which is protected.
  2. use a NiCMidi Get/Read Message in my MQTT handler....

Does this make sense?

goofy2k commented 2 years ago

1) What if I make the HardwareMsgIn function just a public member function, such that I can put a call to it inside my MQTT event handler for an incoming message?

2) I don't see instantiation of the message queue. You pass a queue size to the MidiIndriver constructor, but I don't see where that is used to create the queue.

goofy2k commented 2 years ago

With 2. I mean: I see in_queue in the constructor, but don't recognize a reference to the MidiRawMessageQueue class. So I'm not sure if it exists.

Iffff it exists I could carry data into it when calling HardwareMsg in after it has been made public. Correct?

ncassetta commented 2 years ago

The constructor of MidiRawMessageQueue is in driver.h:91, it is called by the constructor of MIDIInDriver in driver.cpp:242.

The HardwareMsgIn is protected (and static) because RtMidi wants it to be passed as a function pointer, in setCallback() (driver.cpp:245), then calls it by the pointer. Of course you could make it public (and member function) and call it directly, (this, however, would lead to changes in the code of the MIDIInDriver class).

I think you have no need to access the MIDIRawMessageQueue in your code: when a message arrives, put its bytes into a std::vector (from the beginning), and then call HardwareMsgIn giving it as parameters the timestamp, the vector pointer and (if you keep it static) the MIDIInDriver class instance pointer. Then HardwareMsgIn does all the job of assembling the bytes into a MIDIRawMessage and putting it into the MIDIInDriver queue. I think this is the easier solution, if you want to manipulate the queue from within your MQTT driver you should also modify HardwareMsgIn.

goofy2k commented 2 years ago

My point is: how to call HardwareMessageIn? Probably my lack of experience with callbacks. I see that it is set as the callback. But how to make sure that it is called once a message arrives over my MQTT channel?

ncassetta commented 2 years ago

You said your MQTT code is event driven. So, I suppose, you can get your storeMIDI_Input function (in main.cpp:486) called every time a message arrives. If it is so call HardwareMsgIn from within storeMIDI_Input. At first you can make HardwareMsgIn public and member and call it normally, seeing if you can get it working. This is however a bit messy, if you succeed you can study better function pointers and, I think, embed the storeMIDI_Input in a class and parametrize the called function with a setCallback() method. I did something of similar in classes MIDITimer and MIDIManager.

goofy2k commented 2 years ago

OK. Thanks again for your patience ;-). I will also have a look at the clean NicMidi / Rmidi lib and see where things happen.

As I see it now a callback like HardwareMessageIn should appear in at least three places:

1) the actual implementation of it's code 2) where the pointer at it is set (setCallback) 3) where the callback is actually called, said differently, where the data (msg) is put into the chain.

goofy2k commented 2 years ago

OK, found the actual calls (location 3) in RtMidi.cpp! The pointer to the callback is available in the struct inputData_ . It's name is userCallback. RtMidi contains multiple examples in the different OS's on how to use it..... I'll give it a go!

goofy2k commented 2 years ago

V20: I made HardwareMsgIn public and call it in my MQTT event handler. I see "MQTT_In callback executed Got message, queue size: " . On each received message the queue size increases. I do this after recorder.Start() in a setup equal to the test_recorder example. I see beat messages: "GUI EVENT: Transport MEAS 3117 BEAT 3" and receive notes on my synth board.

I don't receive the MQTT notes on my synth board, but probably I at least need to set MIDI thru.

BUT: the fact that the queue size increases probably means that the mechanism to process incoming data and send notes to a track is not in place yet. I have to study your code where this should take place. I expected that this would be in place after a call of HardwareMsgIn.

My handling of the MidiInData also may be too dirty/ minimal still. But the increasing queue size gives me some confidence.

I expect that I don't have to set track nrs, channels etc in the recorder example and that this is covered by defaults.

EDIT1: I forgot to add the following two lines to the example:

MIDIManager::AddMIDITick(&recorder);
text_n.SetSequencer(&sequencer);

I hoped that especially the first one would be reponsible for continuously picking messages from the input queue. But I seeno changes in the behaviour.

EDIT 2: I tested getting messages from the input queue with MIDIManager::GetInDriver(0)->InputMessage(mymsg)) That works in the sense that the length of the queue shrinks.

goofy2k commented 2 years ago

I try to use your thru example to test MQTT input and encounter a problem that may have to do with double initialisation of the input port.

There is a line 47: MIDIThru* thru; and a line 78: thru = new MIDIThru;

First of all I am confused as 47 defines thru as a pointer on type MIDIThru whereas 78 defines it as a MIDIThru object. Isn't that conflicting?

I think that both lines trigger initialisation of the input port. So I have to understand better what is the difference between both methods. I will consult Google and Stackoverflow for that.

Can you comment on the * ?

goofy2k commented 2 years ago

OK. Found that the first statement creates a pointer. The actual object is destroyed "when the scope ends". I have to make sure that my destructor correctly gets rid of the port instantiation OR let every instantiation detect if the port already exists and skip initialisation of the port ifff it exists.

goofy2k commented 2 years ago

I am tracing into the recorder code to discover what happens with the messages that are stored in the queue by HardwareMsgIn.....

goofy2k commented 2 years ago

OK, now we are getting somewhere! By carefully tracing the code in the recorder TickProc I learned that before starting the recorder I had to use:

recorder.EnableTrack(0); recorder.SetTrackRecChannel(0,0);

The incoming notes over MQTT are now playing over the nimBLE output ! (still with HardwareMsgIn called directly in my MQTT input handler).

Now the point is that the messages are not removed from the input queue and they are played at every new tick. It is clear to me that messages are not removed by the call of the MidiInDriver port->ReadMessage(rmsg, j) in recorder.cpp:439.

Do you intend to remove the messages from the input queue once they are stored in a track? Should they be removed after all tick components in the component queue have done their work??

ncassetta commented 2 years ago

MIDIThru* thru only declares (and creates) a pointer. The keyword "new" creates an object and returns a pointer to it, so line 78 correctly assigns to thru a pointer (otherwise the compiler would give an error). The ports are initialized by the MIDIManager the first time a method involving them is called (see the code of MIDIManager methods), and never are destroyed (only at the program exit), so shouldn't be any initialization trouble. The MIDIRecorder and MIDIThru use (see MIDIThru.cpp:180) MIDIInDriver::ReadMessage() which doesn't pop the message from the queue (so it remains available for a subsequent MIDITickComponent: for examplr you could have a thru and a recorder together). However, at every timer tick the queue is finally flushed by the MIDIManager::TickProc (manager,cpp:255), so I can't figure out why your queue size increases (you shouldn't need to remove them by yourself). I'll try to see your code tomorrow

goofy2k commented 2 years ago

Thanks! Now I know at least how it is intended to work. In the adapted recorder.cpp you will see a (now commented) FlushQueue that I added after the Unlock. This helped, but obviously should not be done there.

I will probably proceed with studying the contents of the track(s) after playing some notes. I also have to check if the actually stored and played data corresponds with the input data. [EDIT1: it is NOT, repair in progress] [EDIT2: solved in v20a, v20 was using dummy data for testing. Now use data from MQTT in]

I'll leave transferring HardwareMsgIn to "protected" and "callback" status for later. Unless the direct call is the cause of the current unexpected behaviour. [EDIT3: possible cause is my commenting of the code in manager.cpp:255. Was probably done when MIDI_Ins were not yet available..... ;-) ]

ncassetta commented 2 years ago

Try to print something (as "Queue flushed") between lines 256 and 257 of "manager.h" to see if line 257 is executed at every tick. Perhaps the IsPortOpen() at 256 returns false and the if is not executed

goofy2k commented 2 years ago

Your message crossed my Edit in the previous message (only visible on Github, but not in email threads). The issue is solved! I commented the lines myself at the stage that I could not yet handle MidiIns.

goofy2k commented 2 years ago

So: problem with queue not flushing was solved.

Next hurdle: events are not added to a track (not recorded). I added some breadcrumbs and log statements in track.cpp, so I can follow the trace. I notice that the timestamp in the message is always 0. My input routines (HardwareMsgIn and neighbouring) are probably still too sloppy on this point.... going to dive into that!


[EDIT 1: timestamps properly added now. Insertion of every next event recognizes timestamp of it's predecessor. But a bool verbose = true; DumpAllTracksAttr(sequencer.GetMultiTrack(), verbose);

after handling each event always shows (for 16 tracks):

In port: MQTT_In Out port: fckx_seq Time shift: 0 Events in track: 1 End of track time: 0 Track 2 Name: Type: EMPTY In port: MQTT_In Out port: fckx_seq Time shift: 0 Events in track: 1 End of track time: 0 Track 3 Name: Type: EMPTY In port: MQTT_In Out port: fckx_seq Time shift: 0 Events in track: 1 End of track time: 0


goofy2k commented 2 years ago

SOLVED ! I dug into the recorder code until events.insert in track.cpp. Added GetNumEvents() before and after insertion. Everything OK. I changed the inspection of the tracks from: bool verbose = true; DumpAllTracksAttr(sequencer.GetMultiTrack(), verbose); to: DumpMIDITrackWithPauses(trk, 1);

and now I see the history of my events passing by!

Also want to test playing the recorded notes in a loop. Do you have suggestions how to do that in the case that I still don't have a UI/GUI to have live control.

I was thinking of defining a loop, that records and plays endlessly, while I can enter notes in via MQTT. Is the recorder example capable of doing this?

ncassetta commented 2 years ago

The MIDIRecorder class was the last one I implemented, with considerable difficulty. The class is connected to a MIDISequencer whose tracks are written (it holds the pointer seq_tracks to the sequencer MIDIMultiTrack). At first I tried to write the recorded messages directly into the sequencer tracks, but this led to a lot of trouble because it changed the tracks while they were playing. So I had to choose a rather complicated solution: 1) The MIDIRecorder has its internal MIDIMultiTrack (tracks). The MIDIRecorder::Start () method calls MIDIRecorder::PrepareTracks() which copies the sequencer tracks into the internal multitrack; 2) During the recording the messages are written in the MIDIRecorder multitrack; 3) At the end of the recording (recorder.cpp: 305) the tracks of the recorder are copied to the sequencer. In the MIDIRecorder :: TickProc (from recorder.cpp: 438 onwards) the recorder reads the messages from the queue (440), assigns them the time (441) and writes them to the recorder tracks (450). In 441 the timestamp of the messages is ignored, because in the end I realized that it was much easier to assign the current tempo of the sequencer.

goofy2k commented 2 years ago

OK. It is good to have that background information.

Did you already test playing a loop that is empty at the start and where notes / events are added by recording? I now tried the following code, but it leads to a crash:

MIDISequencerGUINotifierText text_n;        // the AdvancedSequencer GUI notifier
/*******************************************************************************************
* AdvancedSequencer also instantiates nimBLEDevice (via sequencer, manager, driver)
********************************************************************************************/
ESP_LOGE(TAG,"Entering AdvancedSequencer");
AdvancedSequencer sequencer(&text_n);       // an AdvancedSequencer (with GUI notifier)
MIDIRecorder recorder(&sequencer);          // a MIDIRecorder //FCKX

MIDIManager::AddMIDITick(&recorder); text_n.SetSequencer(&sequencer);

ESP_LOGW(TAG, "setEndRecTime 1"); MIDIClockTime t = sequencer.MeasToMIDI(5,0); //endMeasure, endBeat record the first 6 beats ESP_LOGW(TAG, "setEndRecTime 2"); recorder.SetEndRecTime(t);

ESP_LOGW(TAG, "setRepeatPlay 2");
sequencer.SetRepeatPlay(true, 0, 10); //beginMeasure endMeasure loop the first 11 beats

ESP_LOGW(TAG, "EnableTrack"); recorder.EnableTrack(1); //FCKX

if (recorder.SetTrackRecChannel(1,0)) { // recorder.Start(); } else { while(1) { ESP_LOGE(TAG,"SetTrackRecChannel FALSE");
}
}

There are several choices that I have made, not knowing the precisedesign of your code:

  1. set recording range (in measures) before loop size
  2. create a loop that is definitely larger than the recording range (recording range is "embedded" in loop)

I also changed the order (not the sizes) but that leads to a crash reporting the same line nrs in the code

ncassetta commented 2 years ago

No, loop recording is not implemented yet, the MIDIRecorder gave me a lot of problems and I was a bit tired ... At each repetition the already recorded messages should remain, so there is no possibility to record several takes. I will try to investigate the reasons for the crash

goofy2k commented 2 years ago

The last line reported is notifier = s.notifier in the definition of const MIDISequencerState, sequencer.cpp:91

But my system doesn't report a reason.

Op za 18 dec. 2021 22:14 schreef Nicola Cassetta @.***>:

No, loop recording is not implemented yet, the MIDIRecorder gave me a lot of problems and I was a bit tired ... At each repetition the already recorded messages should remain, so there is no possibility to record several takes. I will try to investigate the reasons for the crash

— Reply to this email directly, view it on GitHub https://github.com/ncassetta/NiCMidi/issues/2#issuecomment-997285636, or unsubscribe https://github.com/notifications/unsubscribe-auth/AGII3TT3G2EXVTKGHX3DJGDURT2UBANCNFSM5IYMILWA . You are receiving this because you were mentioned.Message ID: @.***>

goofy2k commented 2 years ago

What was the problem with your first approach on recording? (just want to suggest some "dumb" options. Easy for me as I don't have your history;-) )

Did the insertion of a new event take too much time, causing unacceptable interruption of the sequencing process? If so, the insertion task of a single event could be divided over smaller tasks. There is normally a lot of time available until the position in the track should be ready for a new cycle of the loop.

Or did it have to do with access to the track? With having access to it blocked during the insertion process?

In both cases, doing the insertion process in a TickProc could be of help?

Does insertion involve sorting to keep events in the order of it's timestamp?

goofy2k commented 2 years ago

OK, I see that your InsertEvent (track.cpp:210) is always at the end of the track. So, no sorting involved in this approach and the position of insertion is immediately clear. Correct?

I also see that the transfer of the recorded track is really after calling Stop() for recording. So, no "real time" insertion of recorded events?

Or do I overlook something?