OSC disabling fx causes segfault (MacOS)

[gitsmol]

Bug Description: Sending /param/fx/a/1/deactivate, 1.0 causes a segfault.

Extra info https://github.com/surge-synthesizer/surge/commit/74d4dde6c6af71cb0d69e64ccdd1331b1c976299 Standalone, 64-bit. MacOS Ventura 13.6.6

Additional Information: surge_crashlog.txt

[baconpaul]

was a1 set to an effect before hand? nothing looks obviously wrong in the code though

[gitsmol]

Good question. Yes, I tried it with an effect in that slot. However using disable on an empty slot also segfaults.

It's specifically this line that segfaults:

sspPtr->oscRingBuf.push(SurgeSynthProcessor::oscToAudio(selected_mask, onoff));

I've tried filling in random ints and it will reliable fail on setting 1 for any given fx slot.

[baconpaul]

is '1.0' being a float rather than an int and getting the wrong type the problem I wonder?

the overloads of oscToAudo look a touch scary now i look at them, and if you accidentally get the wrong one (easy to do in c++) it will blow up.

Wonder if it is better to make those constructors explicit.

(also just for future: super helpful to give line numbers if you can! but i found it. Again nothing obvious wrong)

[gitsmol]

It could be, I also looked at the overloads to figure out what oscToAudio even does... I also wondered if we were getting the wrong type.

But even when explicitly feeding int(1) and int(1) to it, it segfaults. I'm inclined to look into what happens when the UI disables an effect but I've not grappled with that code before.

[baconpaul]

Are you sending the message to the cli or to a session with the editor open?

[gitsmol]

I tried both, both respond the same way.

[pkstone]

It might be time to clean up that constructor mess! Anyway, I'm looking into this

[baconpaul]

The way I would do it is

1. Make the constructor set protected or private or best just remove them
2. Add a bunch of static public methods like

  static OpenSoundControl::Thingy createForFXDeactivate(int slot, bool what) {
      res = OpenSoundControl::Thingy();
      res.type = FX;
      res.num = slot;
      return res;
   }

then fix the code to use that static

[pkstone]

I will definitely reorganize that constructor mess, but in looking for this crash, I think I've found the line that is guaranteed to fail: SSP ~955:

            surge->queueForRefresh(om.param->id);

There is no parameter pointer passed with this 'oscToAudio' message (what parameter is referenced by this operation, anyway?) FX disabling/enabling worked before the commit where this line was inserted.

(It first appeared in commit: "Osc work: extended parameter options (#7569) a529545d" Unfortunately, there are no comments accompanying its appearance, so I'm not sure why I added it! ) Removing it from the latest commit makes FX Deact/act work again. EDIT: not quite, see below!

Any idea why it might have been added (aside from a mistake on my part)?

[pkstone]

OK, partially wrong...with that line commented out, the FX correctly deactivates and reactivates, but the GUI is not updated. So I think the added line was an attempt to update the GUI. But since FX-deactivate is not a 'true' parameter, what is 'param' in

surge->queueForRefresh(om.param->id);

supposed to point to?

[baconpaul]

Who is that question for @pkstone ?

Probably the thing to do is just rebuild the entire ui there rather than try a single refresh?

[pkstone]

It was for you, @baconpaul , sorry. How would I do the ui rebuild?

[baconpaul]

synth->refreshUI = true I think? or synth->queue_refresh = true or some such?

do that instead of pushing the param and see? maybe?

otherwise: if you can put a small python script here which causes the problem I'm sure I can figure it out.

[pkstone]

thanks, I'll try those possibilities (right after lunch)

[pkstone]

@baconpaul The python script below will toggle FX A 1, first off, then on, two seconds each, repeated 4 times. Make sure OSC in is enabled on Surge, and if you have a patch with an obvious FX A1, you'll hear it switch on and off. Visually, the FX box only switches off the first time, and never (re)activates (even though the effect does reactivate).

(this is true for my current code at SSP 931):

    case SurgeSynthProcessor::FX_DISABLE:
    {
        int selected_mask = om.ival;
        int curmask = surge->storage.getPatch().fx_disable.val.i;
        int msk = selected_mask;
        int newDisabledMask = 0;
        if (om.on == 0) // set selected bit to zero
        {
            msk = ~(msk & 0) ^ selected_mask; // all bits to 1 except selected bit
            newDisabledMask = curmask & msk;
        }
        else // set selected bit to one
        {
            newDisabledMask = curmask | msk;
        }

        surge->storage.getPatch().fx_disable.val.i = newDisabledMask;
        if (surge->fx_suspend_bitmask != newDisabledMask)
        {
            surge->fx_suspend_bitmask = newDisabledMask;
            surge->storage.getPatch().isDirty = true;
            surge->refresh_editor = true;
        }
    }

Python:

from osc4py3.as_eventloop import *
from osc4py3 import oscbuildparse
import time

ip = "127.0.0.1"
port = 53280    #Surge XT default OSC in port

def oscOut(addr, val):
    msg = oscbuildparse.OSCMessage("/param/fx/a/1/deactivate", ",f", [val])
    osc_send(msg, "oscout")
    osc_process()

osc_startup()
osc_udp_client(ip, port, "oscout")

for x in range(4):
    oscOut("/param/fx/a/1/deactivate", 1)
    time.sleep(2)
    oscOut("/param/fx/a/1/deactivate", 0)
    time.sleep(2)

osc_terminate()