Control Surface Integration (CSI) Technical Feature Discussion

[Geoff Waddington]

Some may recall the drag and drop code for the ReMap window was fairly tricky.

In fact, there is separate code for Mac and Windows, PITA.

With the new approach, do we even need drag and drop anymore ?

[jacksoonbrowne]

Originally Posted by Geoff Waddington

Cool, going to do it a bit differently.

Seems strange to have the standalone function int X32GetColorValue(const rgba_color &color) that replaces some of the repeated code, but then in each of the overrides there is this repeated code:

       lastColor_ = color;

        int surfaceColor = X32GetColorValue(color);

        string oscAddress = "/ch/";
        if (widget_->GetChannelNumber() < 10)   oscAddress += '0';
        oscAddress += int_to_string(widget_->GetChannelNumber()) + "/config/color";
        surface_->SendOSCMessage(this, oscAddress.c_str(), surfaceColor);

No perfect answer here, but I prefer to see all of the code repeated and encapsulated in X32 classes, rather than having it spread out amongst class methods and standalone functions.

void OSC_X32FeedbackProcessor::SetColorValue(const rgba_color &color)
{
    if (lastColor_ != color)
    {
        lastColor_ = color;

        
        int surfaceColor = 0;
        int r = color.r;
        int g = color.g;
        int b = color.b;
        
        if (r == 64 && g == 64 && b == 64)                               surfaceColor = 8;    // BLACK
        else if (r > g && r > b)                                         surfaceColor = 1;    // RED
        else if (g > r && g > b)                                         surfaceColor = 2;    // GREEN
        else if (abs(r - g) < 30 && r > b && g > b)                      surfaceColor = 3;    // YELLOW
        else if (b > r && b > g)                                         surfaceColor = 4;    // BLUE
        else if (abs(r - b) < 30 && r > g && b > g)                      surfaceColor = 5;    // MAGENTA
        else if (abs(g - b) < 30 && g > r && b > r)                      surfaceColor = 6;    // CYAN
        else if (abs(r - g) < 30 && abs(r - b) < 30 && abs(g - b) < 30)  surfaceColor = 7;    // WHITE
        
        string oscAddress = "/ch/";
        if (widget_->GetChannelNumber() < 10)   oscAddress += '0';
        oscAddress += int_to_string(widget_->GetChannelNumber()) + "/config/color";
        surface_->SendOSCMessage(this, oscAddress.c_str(), surfaceColor);
    }
}

void OSC_X32IntFeedbackProcessor::SetColorValue(const rgba_color &color)
{
    if (lastColor_ != color)
    {
        lastColor_ = color;

        
        int surfaceColor = 0;
        int r = color.r;
        int g = color.g;
        int b = color.b;
        
        if (r == 64 && g == 64 && b == 64)                               surfaceColor = 8;    // BLACK
        else if (r > g && r > b)                                         surfaceColor = 1;    // RED
        else if (g > r && g > b)                                         surfaceColor = 2;    // GREEN
        else if (abs(r - g) < 30 && r > b && g > b)                      surfaceColor = 3;    // YELLOW
        else if (b > r && b > g)                                         surfaceColor = 4;    // BLUE
        else if (abs(r - b) < 30 && r > g && b > g)                      surfaceColor = 5;    // MAGENTA
        else if (abs(g - b) < 30 && g > r && b > r)                      surfaceColor = 6;    // CYAN
        else if (abs(r - g) < 30 && abs(r - b) < 30 && abs(g - b) < 30)  surfaceColor = 7;    // WHITE
        
        string oscAddress = "/ch/";
        if (widget_->GetChannelNumber() < 10)   oscAddress += '0';
        oscAddress += int_to_string(widget_->GetChannelNumber()) + "/config/color";
        surface_->SendOSCMessage(this, oscAddress.c_str(), surfaceColor);
    }
}

The other alternative would be to put it all in a standalone function, dunno, what do you think ?

What ever you would prefer.

[Geoff Waddington]

Originally Posted by jacksoonbrowne

What ever you would prefer.

Yeah, it's a close call, but the standalone function would have to be passed the surface, widget, feedbackProcessor, and colour, so I think the duplicated encapsulated code is slightly better, as it will likely never change, so maintenance isn't really an issue.

[Funkybot]

Originally Posted by Geoff Waddington

Yup, and the option quagmire is sorted too

If you have EnableFocusedFXMapping on (the default), and there is no Zone definition, the ReMap window appears in Learn mode, ready to go.

Buttons at the bottom for AutoMap, EraseLastTouched, RemoveZone, Save, and Cancel, done.

I'm thinking about how this will play out and I'm not sure I love the idea of another window popping up on screen with focused FX mapping on. The windows popping up can get annoying in use. We already had a similar pop up window and nixed it.

I think it's better to have the surface in learn mode or AutoMap mode myself. That will be less intrusive and put the users in control. If AutoMap mode, the FX just maps. No action necessary. If Learn Mode you can Learn or do nothing. But on screen windows popping up can be peevish. I would say keep it as a "remap" for remapping things after the fact.

Also, how would this system work with GoFxSlot+NoMap? Same deal? Again, I would rather the surface do something and AutoMap or jump into Learn mode than an on screen pop-up window.

Just my two cents. I'm open to being wrong on this but I feel like we tried this already and nixed it for similar reasons.

[Geoff Waddington]

Originally Posted by Funkybot

I'm thinking about how this will play out and I'm not sure I love the idea of another window popping up on screen with focused FX mapping on. The windows popping up can get annoying in use. We already had a similar pop up window and nixed it.

I think it's better to have the surface in learn mode or AutoMap mode myself. That will be less intrusive and put the users in control. If AutoMap mode, the FX just maps. No action necessary. If Learn Mode you can Learn or do nothing. But on screen windows popping up can be peevish. I would say keep it as a "remap" for remapping things after the fact.

Also, how would this system work with GoFxSlot+NoMap? Same deal? Again, I would rather the surface do something and AutoMap or jump into Learn mode than an on screen pop-up window.

Just my two cents. I'm open to being wrong on this but I feel like we tried this already and nixed it for similar reasons.

Good points all !

The thinking is that if you have Focused FX mapping on and there is no map, you would like to map it, so the window only opens if there is a no map.

Same for GoFXSlot.

Another benefit is for surfaces with low, or even nonexistent button counts, like an extender.

The jury is definitely still out on all of this...

[edit] One thing important to note -- in this new approach the Remap window is Learn mode.

[jacksoonbrowne]

Found a problem with Shutdown() when OSC messages per slice is active (message queueing active).
This is not X32 specific.

void Shutdown()
{
    // We want to stop polling
    shouldRun_ = false;
        
    // Zero out all Widgets before shutting down
    if (pages_.Get(currentPageIndex_))
        pages_.Get(currentPageIndex_)->ForceClear();
}

Because OSC message queueing is on, the ForceClear() will cause immediate queueing of all shutdown messages.

And then Shutdown() returns and Reaper exits before all queued messages have been sent.

What we need is a way to ensure all queued message have been flushed out before Shutdown() returns.

Something like this

void Shutdown()
{
    // We want to stop polling
    shouldRun_ = false;
        
    // Zero out all Widgets before shutting down
    if (pages_.Get(currentPageIndex_))
        pages_.Get(currentPageIndex_)->ForceClear();

    // PSUEDO CODE HERE
    FOR each OSC surface
    {
        IF queue NOT empty
            SLEEP(100)
    }
}

[Geoff Waddington]

Originally Posted by jacksoonbrowne

Found a problem with Shutdown() when OSC messages per slice is active (message queueing active).
This is not X32 specific.

void Shutdown()
{
    // We want to stop polling
    shouldRun_ = false;
        
    // Zero out all Widgets before shutting down
    if (pages_.Get(currentPageIndex_))
        pages_.Get(currentPageIndex_)->ForceClear();
}

Because OSC message queueing is on, the ForceClear() will cause immediate queueing of all shutdown messages.

And then Shutdown() returns and Reaper exits before all queued messages have been sent.

What we need is a way to ensure all queued message have been flushed out before Shutdown() returns.

Something like this

void Shutdown()
{
    // We want to stop polling
    shouldRun_ = false;
        
    // Zero out all Widgets before shutting down
    if (pages_.Get(currentPageIndex_))
        pages_.Get(currentPageIndex_)->ForceClear();

    // PSUEDO CODE HERE
    FOR each OSC surface
    {
        IF queue NOT empty
            SLEEP(100)
    }
}

Nice catch !!

Will look into it.

[Geoff Waddington]

Does this flush the queue properly ?

    void Shutdown()
    {
        // GAW -- IMPORTANT
        
        // We want to stop polling
        shouldRun_ = false;
        
        // Zero out all Widgets before shutting down
        if (pages_.Get(currentPageIndex_))
        {
            pages_.Get(currentPageIndex_)->ForceClear();
            pages_.Get(currentPageIndex_)->Run(); // To flush queues
        }
    }

[Geoff Waddington]

Hmmm...

Wondering what it would be like if we reversed the Learn workflow.

Focus an FX for which there is no map
Learn Window (Remap) appears
Move a surface control
That control is selected in the Learn Window
Move a gui FX control with the mouse
They are linked

What say you ?

[Funkybot]

Originally Posted by Geoff Waddington

Hmmm...

Wondering what it would be like if we reversed the Learn workflow.

Focus an FX for which there is no map
Learn Window (Remap) appears
Move a control
That control is selected in the Learn Window
Move a gui FX control with the mouse
They are linked

What say you ?

What does that look like on screen? You wouldn't want users having to scroll through a long list of plugin parameters to find what Fader1 is mapped to. Some plugins have hundreds of parameters. Workflow wise it sounds good with my prior concerns about pop up screens remaining.

[jacksoonbrowne]

Originally Posted by Geoff Waddington

Does this flush the queue properly ?

    void Shutdown()
    {
        // GAW -- IMPORTANT
        
        // We want to stop polling
        shouldRun_ = false;
        
        // Zero out all Widgets before shutting down
        if (pages_.Get(currentPageIndex_))
        {
            pages_.Get(currentPageIndex_)->ForceClear();
            pages_.Get(currentPageIndex_)->Run(); // To flush queues
        }
    }

Nope.

I have my Messages Per run set to 16 and when there are more than 16 messages queued, for example 54,
"pages_.Get(currentPageIndex_)->Run();" will only flush 16 messages

Seems we need a way to do multiple Run()'s if the queue is not empty.

Maye something like this:

    void Shutdown()
    {
        // GAW -- IMPORTANT
        
        // We want to stop polling
        shouldRun_ = false;
        
        // Zero out all Widgets before shutting down
        if (pages_.Get(currentPageIndex_))
        {
            pages_.Get(currentPageIndex_)->ForceClear();
            
            // PSUEDO CODE
            WHILE queue IS NOT EMPTY
                pages_.Get(currentPageIndex_)->Run(); // To flush queues
        }
    }

[Geoff Waddington]

Originally Posted by jacksoonbrowne

Nope.

I have my Messages Per run set to 16 and when there are more than 16 messages queued, for example 54,
"pages_.Get(currentPageIndex_)->Run();" will only flush 16 messages

Seems we need a way to do multiple Run()'s if the queue is not empty.

Maye something like this:

    void Shutdown()
    {
        // GAW -- IMPORTANT
        
        // We want to stop polling
        shouldRun_ = false;
        
        // Zero out all Widgets before shutting down
        if (pages_.Get(currentPageIndex_))
        {
            pages_.Get(currentPageIndex_)->ForceClear();
            
            // PSUEDO CODE
            WHILE queue IS NOT EMPTY
                pages_.Get(currentPageIndex_)->Run(); // To flush queues
        }
    }

Thanks, got it, will continue thinking.

It's tempting to just call Run over and over until the queue is empty, but that likely leads to packet loss when called back to back like that.

Methinks we'll have to keep responding to the Run calls Reaper issues (every 30 msec or so) until the queues are all flushed.

[Geoff Waddington]

Originally Posted by Funkybot

What does that look like on screen? You wouldn't want users having to scroll through a long list of plugin parameters to find what Fader1 is mapped to. Some plugins have hundreds of parameters. Workflow wise it sounds good with my prior concerns about pop up screens remaining.

Say you have an FXLayout that supports Shift+Alt.

Engage Shift and Alt
Move Fader
Learn Window scrolls the appropriate line into view
Learn Window highlights the Fader on that line
Move gui FX control with mouse
Done

This isn't really a pop up Window, it is the Learn Window, something that you always use whilst mapping, it is non modal and the centerpiece of the whole operation.

[Funkybot]

That sounds good. I was concerned the param list would be shown then the fader assigned somewhere in the long list. But sounds like that won't be the case. Thanks for explaining.

[jacksoonbrowne]

Originally Posted by Geoff Waddington

Thanks, got it, will continue thinking.

It's tempting to just call Run over and over until the queue is empty, but that likely leads to packet loss when called back to back like that.

Methinks we'll have to keep responding to the Run calls Reaper issues (every 30 msec or so) until the queues are all flushed.

Methinks the same

[MixMonkey]

Originally Posted by Geoff Waddington

Hmmm...

Wondering what it would be like if we reversed the Learn workflow.

Focus an FX for which there is no map
Learn Window (Remap) appears
Move a surface control
That control is selected in the Learn Window
Move a gui FX control with the mouse
They are linked

What say you ?

This sounds good to me, providing the ReMap window is going to auto-scroll to the selected control. Then you're setup to do a quick re-name and move on. I'm liking this "Learn from the ReMap window" more and more

[Geoff Waddington]

Originally Posted by MixMonkey

This sounds good to me, providing the ReMap window is going to auto-scroll to the selected control. Then you're setup to do a quick re-name and move on. I'm liking this "Learn from the ReMap window" more and more

Yup it will auto scroll, that's why I'm asking if we can lose drag and drop, the above will be much easier to accomplish if we don't have the added complexity of drag and drop.

[MixMonkey]

Originally Posted by Geoff Waddington

Yup it will auto scroll, that's why I'm asking if we can lose drag and drop, the above will be much easier to accomplish if we don't have the added complexity of drag and drop.

I'm happy to lose drag'n'drop in favor of auto-scroll. Drag'n'drop was never a very efficient way of organising parameters on a surface, because, unless you were very disciplined about it, any change made to the list affected every assignment after it.

To me, auto-scroll on touching a control makes much more sense. A couple of things spring to mind though. If we lose drag'n'drop, then Learn will become the only way to re-organise an AutoMapped plugin. I'm fine with this, providing we have an easy way of deleting a parameter from a control to leave it unassigned (ie NoAction) and that assigning a parameter to an already mapped control overwrites the existing mapping, even if that means you have two controls mapped to the same parameter (unless you want to get into swapping parameters between controls when this condition occurs).

[Geoff Waddington]

Originally Posted by MixMonkey

I'm happy to lose drag'n'drop in favor of auto-scroll. Drag'n'drop was never a very efficient way of organising parameters on a surface, because, unless you were very disciplined about it, any change made to the list affected every assignment after it.

To me, auto-scroll on touching a control makes much more sense. A couple of things spring to mind though. If we lose drag'n'drop, then Learn will become the only way to re-organise an AutoMapped plugin. I'm fine with this, providing we have an easy way of deleting a parameter from a control to leave it unassigned (ie NoAction) and that assigning a parameter to an already mapped control overwrites the existing mapping, even if that means you have two controls mapped to the same parameter (unless you want to get into swapping parameters between controls when this condition occurs).

Cool, I'll put a vote in the yes column for losing drag 'n' drop.

I'm even beginning to question AutoMap itself.

It makes no sense for humongous plugins.

For plugins with a small number of params, it's likely just as quick to Learn as it is to AutoMap and rearrange.

And yes, there will be workflow methods for deleting, editing etc.

We can probably improve editing substantially if there is no drag 'n' drop.

Basically we are switching from surface based mapping Actions, which eat up buttons like crazy, to a purely (mostly ) gui solution.

What do you think ?

[MixMonkey]

Originally Posted by Geoff Waddington

I'm even beginning to question AutoMap itself.

It's a function that's a lot less useful than it initially appears to be. That said, if you get rid of it, you'll be fielding endless requests for its re-instatement, from people who haven't yet realised that it's a lot less useful than it appears to be

Basically we are switching from surface based mapping Actions, which eat up buttons like crazy, to a purely (mostly ) gui solution.

What do you think ?

For the purposes of mapping plugins, I think it's fine The buttons are better employed for actually using Reaper, rather than setting it up

[MT4U]

Originally Posted by Geoff Waddington

Say you have an FXLayout that supports Shift+Alt.

Engage Shift and Alt
Move Fader
Learn Window scrolls the appropriate line into view
Learn Window highlights the Fader on that line
Move gui FX control with mouse
Done

This isn't really a pop up Window, it is the Learn Window, something that you always use whilst mapping, it is non modal and the centerpiece of the whole operation.

I believe it's the best approach we've had so far.

[Funkybot]

The best reasons in favor of keeping Automap is that: for small plugins on displays with surfaces, it's good enough. But additionally: people don't read manuals or wikis. It sets the bar for entry quite low.

I wouldn't use it myself without intending to but it could be a nice gateway into FX mapping for newbies. Sort of a baby step into full blown mapping.

[Geoff Waddington]

Originally Posted by Funkybot

The best reasons in favor of keeping Automap is that: for small plugins on displays with surfaces, it's good enough. But additionally: people don't read manuals or wikis. It sets the bar for entry quite low.

I wouldn't use it myself without intending to but it could be a nice gateway into FX mapping for newbies. Sort of a baby step into full blown mapping.

Cool, are you in favour of keeping AutoMap even if we remove drag 'n' drop ?

[Funkybot]

The Automap code is there, it works. It can even work great on the right plugin. It would be a shame to waste perfectly good code. I say keep it as an options on the Remap window (we need a new name for that methinks).

[jacksoonbrowne]

Hi Geoff,

Need the change in blue, it was sending a float message before, with the change it sends an int message

void OSC_IntFeedbackProcessor::ForceValue(const PropertyList &properties, double value)
{
    lastDoubleValue_ = value;
    
    surface_->SendOSCMessage(this, oscAddress_.c_str(), (int)value);
}

[jacksoonbrowne]

Hi Geoff,

I merged with the latest source commits as of today and the OSC flush messages on exit almost worked.

Found the issue and fixed it.

In the code below I derived the sleep time from Reapers default Run() refresh interval of ~30 per second (as observed using WireShark), since the CSI OSC implementation does not change the interval.
I did this since my empirical tuning for maxMessages was done using the default refresh interval .

This is what I have for a working change:

OSC_ControlSurfaceIO::~OSC_ControlSurfaceIO()
{
    int   refreshRateInMs = (1.0/30.0) * 1000;

    while (packetQueue_.GetSize()>=sizeof(int))
    {
        if (maxPacketsPerRun_ > 0)
            Sleep(refreshRateInMs); // Sleep() must be before BeginRun()
        BeginRun();
    }

[Geoff Waddington]

Originally Posted by jacksoonbrowne

Hi Geoff,

I merged with the latest source commits as of today and the OSC flush messages on exit almost worked.

Found the issue and fixed it.

In the code below I derived the sleep time from Reapers default Run() refresh interval of ~30 per second (as observed using WireShark), since the CSI OSC implementation does not change the interval.
I did this since my empirical tuning for maxMessages was done using the default refresh interval .

This is what I have for a working change:

OSC_ControlSurfaceIO::~OSC_ControlSurfaceIO()
{
    int   refreshRateInMs = (1.0/30.0) * 1000;

    while (packetQueue_.GetSize()>=sizeof(int))
    {
        if (maxPacketsPerRun_ > 0)
            Sleep(refreshRateInMs); // Sleep() must be before BeginRun()
        BeginRun();
    }

I recall you saying you only needed 1-2 msec between sends, does this work:

OSC_ControlSurfaceIO::~OSC_ControlSurfaceIO()
{
    Sleep(10);
    
    int cnt = 0;
    while (packetQueue_.GetSize()>=sizeof(int) && cnt < 100)
    {
        BeginRun();
        if (cnt++) Sleep(10);
    }
...

[Geoff Waddington]

Originally Posted by jacksoonbrowne

Hi Geoff,

Need the change in blue, it was sending a float message before, with the change it sends an int message

void OSC_IntFeedbackProcessor::ForceValue(const PropertyList &properties, double value)
{
    lastDoubleValue_ = value;
    
    surface_->SendOSCMessage(this, oscAddress_.c_str(), (int)value);
}

Don't see that here, it already looks like your correct fix, I must be missing something, what line of code ?

[Geoff Waddington]

VERY EXPERIMENTAL PROOF OF CONCEPT
DO NOT USE FOR ANYTHING REMOTELY RESEMBLING PRODUCTION

MAC ONLY NOW -- WINDOWS TO COME

New Exp build is up.

https://stash.reaper.fm/v/42044/CSI%20Exp.zip

Super excited to see what you folks think of this bare bones proof of concept.

Steps:

Focus a mapped FX
Press button mapped to ReMap
Notice the colour coded layout
Move a surface control
Oh yeah babay !!

Absolutely love the fundamental workflow ideas here.

If this was working you would then move a control on the focused FX gui -- voila !!

Redesigning this is quite a large task, so want to be sure everyone agrees that we're heading in the right direction as we go along this new journey

[fourdogslong]

Originally Posted by Geoff Waddington

VERY EXPERIMENTAL PROOF OF CONCEPT
DO NOT USE FOR ANYTHING REMOTELY RESEMBLING PRODUCTION

MAC ONLY NOW -- WINDOWS TO COME

New Exp build is up.

https://stash.reaper.fm/v/42044/CSI%20Exp.zip

Super excited to see what you folks think of this bare bones proof of concept.

Steps:

Focus a mapped FX
Press button mapped to ReMap
Notice the colour coded layout
Move a surface control
Oh yeah babay !!

Absolutely love the fundamental workflow ideas here.

If this was working you would then move a control on the focused FX gui -- voila !!

Redesigning this is quite a large task, so want to be sure everyone agrees that we're heading in the right direction as we go along this new journey

I'll keep an eye open for the windows version! Seems interesting.
Thanks

[jacksoonbrowne]

Originally Posted by Geoff Waddington

Don't see that here, it already looks like your correct fix, I must be missing something, what line of code ?

Ooops,

I pasted the wrong code snippet.
So here it is again.

Line 3467 change

void OSC_IntFeedbackProcessor::ForceClear()
{
    lastDoubleValue_ = 0.0;
    surface_->SendOSCMessage(this, oscAddress_.c_str(), 0.0);
}

Should be

svoid OSC_IntFeedbackProcessor::ForceClear()
{
    lastDoubleValue_ = 0.0;
    surface_->SendOSCMessage(this, oscAddress_.c_str(), (int)0);
}

[Geoff Waddington]

Originally Posted by jacksoonbrowne

Ooops,

I pasted the wrong code snippet.
So here it is again.

Line 3467 change

void OSC_IntFeedbackProcessor::ForceClear()
{
    lastDoubleValue_ = 0.0;
    surface_->SendOSCMessage(this, oscAddress_.c_str(), 0.0);
}

Should be

svoid OSC_IntFeedbackProcessor::ForceClear()
{
    lastDoubleValue_ = 0.0;
    surface_->SendOSCMessage(this, oscAddress_.c_str(), (int)0);
}

Thanks, got it, fixed, will commit with the next batch of changes.