muse-sequencer / muse

MusE is a digital audio workstation with support for both Audio and MIDI
https://muse-sequencer.github.io/
Other
645 stars 69 forks source link

Feature Request: Implement FF, Rew, and Record for MMC #652

Open alextone opened 5 years ago

alextone commented 5 years ago

This is a FR asking for implementation of FF, REW and Record for MMC, to enable use with an external keyboard that has transport controls. (AKAI MPK88)

terminator356 commented 5 years ago

According to our source we don't support FF, REW, or Record MMC commands, nor several others. The supported commands include Stop, Play, Deferred Play, and Locate.

I will look into adding support for them. Can't promise anything soon but... you never know...

alextone commented 5 years ago

Ok, thanks Tim.

github-actions[bot] commented 3 years ago

This issue is stale because it has been inactive for two years. Remove Stale label or write a comment, otherwise it will be closed in 30 days.

kybos commented 3 years ago

I guess this is still relevant...

spamatica commented 3 years ago

Support for these commands are added to git master now.

kybos commented 3 years ago

Cool, so it can be closed?

spamatica commented 3 years ago

I always have conflicting feelings about closing it immediately, but yeah. If it isn't right it can be reopened.

terminator356 commented 3 years ago

FYI: It is no longer necessary to create FIFOs specifically for every kind of data type. We used to have several, the FIFO code in all of them being identical except that the data type was different. This led to a lot of wasted same code in all the different FIFO implementations.

So a few years ago I created several FIFO classes to deal with the problem. See lock_free_buffer.h and lock_free_data_buffer.h The most generally useful of them, the one you'll want to look at, is: LockFreeMPSCRingBuffer in lock_free_buffer.h. It is a Multi Producer Single Consumer FIFO. It is template-ized ! You tell it the data type and it works using templates. Thus, this same FIFO class can be used over and over again, for any data type. Use 'em liberally. I do. They are the great 'thread de-couplers'. Sort of like capacitors in electronic circuits. Check out my various other offerings in those headers. Useful stuff. For example there's a general data FIFO (for pure byte data, not typed items).

@spamatica Is there a compelling reason why the MMC_FastForward and MMC_Rewind commands put into the FIFO, to be read by the song thread? It is legal to call AudioDevice::seekTransport() at any time from from any thread (see Jack docs on locate). Thus you can see I call it from the MMC_Goto command directly in the audio thread, just below your new MMC_FastForward and MMC_Rewind commands. It should be possible to grab the current audio tick position from there (not the current song tick position).

I want to tweak your code and the existing code to use LockFreeMPSCRingBuffer, but I wanted to ask about that first.

spamatica commented 3 years ago

Ah thanks Tim, I was pondering to ask about you about ipc mechanism as the current looked a bit rickety, but I pushed on while I had the steam up :) I'd be happy to give it a go replacing the ipc mechanism. I can do the same for the midiCC remote control while I'm at it.

Regarding the seek also using the IPC, the truth is I didn't think too hard about it. I used the means I had at hand that did the trick. I can take a second look.

terminator356 commented 3 years ago

Yes I wanted to replaced both of those FIFOs there. It'd be a lot cleaner, getting rid of all those variables, less chance for mistakes, just drop in one of the FIFO classes.

OK So I won't touch it for now.

I must point out one correction that should be done, wherever you see FIFO processing: It is not advisable to do something like this pseudo code:

while(fifo->size() != 0)
{
process an item
next item
}

What can happen there is that the loop could become stuck forever if the producer continuously pushes more items while you are inside the loop. fifo->size() might never reach zero in that case.

Instead, take a snapshot of the FIFO's current size and then loop over that size snapshot, something like this:

int sz = fifo->size();   // Snapshot of size.
for(int i = 0; i < sz; ++i)
{
process an item
}

If any new items are pushed by the producer while inside the loop, they'll be handled next time around.

Note that my FIFO class conveniently provides a cached size snapshot, see the header file. But you don't have to use it. The second code block above would be fine. (Ironically, my cached size snapshot actually makes the first code block above workable, you just need to carefully use the snapshot mechanism. But it's not really necessary to use it, it's used in some special cases. So just go with the second code block example.)

terminator356 commented 3 years ago

Nice. Thanks!

I have the official MIDI specs. Been reading about all the other available MMC commands. Some of them would be tricky to implement, but maybe not impossible.

One thing that puzzles me about MMC Rewind and FF. Here is what it says about Fast Forward for example, which is also virtually identical to the Rewind command:

"Move forward at maximum possible speed." "Cancelled by the receipt of another (MCS or MCP) command."

What does it mean exactly? Are we to stay in FF mode until cancelled? And what if our "maximum possible speed" is infinite? Do we just zip right to the end and stop? Or if it is to be a 'one-shot movement', there isn't any indication of how far to move each time.

spamatica commented 3 years ago

Ok... hmmm, I guess it makes sense if you think in terms of a tape recorder...

I used qmidictl to test and thought FF and RWD wasn't working right, but it could actually be that it expects STOP to be pressed to cancel it. Does make sense. Might be a bit more complicated to get 'right' in our code though, I'll check it out.

spamatica commented 3 years ago

Regarding "maximum possible speed", I'm thinking tape recorder again, which probably had a pretty low upper limit. We'll just find a speed that seems sensible.

spamatica commented 3 years ago

Another addendum, I checked how REW and FF work in the transport for Qtractor and Ardour. Ardour has the same behaviour as we do, each click causes a step in that direction, while qtractor does exactly what we are discussing above. In other words, there are two schools and, with the wording you pasted, I'm pretty sure the MMC commands are supposed to work like qtractors transport, continuously move forward until stopped.

terminator356 commented 3 years ago

Upon further reading the official specs, we have these: shuttle_step_mmc

So yeah the rew ff thing makes sense. The specs are obviously geared toward tape devices. The text is peppered with tape terms.

terminator356 commented 3 years ago

And there seems to be an implied limit on the speed, not sure if 'infinite' is a possibility: standard_speed_mmc

github-actions[bot] commented 1 year ago

This issue is stale because it has been inactive for two years. Remove Stale label or write a comment, otherwise it will be closed in 30 days.