AAX with sidechain: how to know when "no key input" is selected with new Multibus API


#1

As stated in the title, I have an AAX plug-in with a sidechain bus implemented with the new Multibus API promoted to master in JUCE 4.3.0.

When I load it in Pro Tools, I would expect the number of channels in the sidechain bus to be 0 when “no key input” is selected, but what I get is always 1, regardless of what I select there.

The number of channels in the sidechain bus can easily be displayed by adding the following to your processBlock() implementation:

DBG (String (getBusesLayout().getNumChannels (true, 1)));

I think this was fixed in the pre-multibus implementation by taking inspiration from this pull request of mine (https://github.com/julianstorer/JUCE/pull/67) but unfortunately that does not apply to the new multibus code.

Also relevant to this subject is the following thread, which unfortunately also refers to the pre-multibus code: AAX SC Input Set? - Solved

So, any chance to see some of those solution ported to the new API?


#2

Working further on this issue, I think I am getting closer to a solution, but I need some more help to actually resolve this.

Following @frankfilipanits advice, given in one of the linked posts, I have checked that:

  • NotificationReceived() gets called on the main thread for the events I’m interested to. (AAX_eNotificationEvent_SideChainBeingConnected and AAX_eNotificationEvent_SideChainBeingDisconnected)

  • I also have found that it is correctly called with the “connected” notification upon load of a session that was saved with the connected sidechain.

The two considerations above lead me to believe that NotificationReceived() would be a perfect place where to put an invocation to preparePlugin(), as shown below:

    AAX_Result NotificationReceived (AAX_CTypeID type, const void* data, uint32_t size) override
    {
        if (type == AAX_eNotificationEvent_EnteringOfflineMode)  pluginInstance->setNonRealtime (true);
        if (type == AAX_eNotificationEvent_ExitingOfflineMode)   pluginInstance->setNonRealtime (false);

        // added by me
        if (type == AAX_eNotificationEvent_SideChainBeingConnected)     preparePlugin ();
        if (type == AAX_eNotificationEvent_SideChainBeingDisconnected)  preparePlugin ();

        return AAX_CEffectParameters::NotificationReceived (type, data, size);
    }

The nice thing about preparePlugin() is that it takes care of calling prepareToPlay() when it detects that the buses layout has changed.

The not-so-nice think about preparePlugin() is that its current implementation does not check whether the sidechain channel is actually connected.

Instead, if a sidechain bus has been declared to be present upon construction of the AudioProcessor, then the corresponding channel set is always mono() and never disabled().

Luckily, the possibility to check whether the sidechain is actually connected exists, and traces of it can be found in algorithmCallback():

    static void algorithmCallback (JUCEAlgorithmContext* const instancesBegin[], const void* const instancesEnd)
    {
        for (JUCEAlgorithmContext* const* iter = instancesBegin; iter < instancesEnd; ++iter)
        {
            const JUCEAlgorithmContext& i = **iter;

            int sideChainBufferIdx = i.pluginInstance->parameters.hasSidechain && i.sideChainBuffers != nullptr
                                         ? static_cast<int> (*i.sideChainBuffers) : -1;

            // sidechain index of zero is an invalid index
            if (sideChainBufferIdx <= 0)
                sideChainBufferIdx = -1;
            // yfede: I'm talking about this IF above

            i.pluginInstance->parameters.process (i.inputChannels, i.outputChannels, sideChainBufferIdx,
                                                  *(i.bufferSize), *(i.bypass) != 0,
                                                  getMidiNodeIn(i), getMidiNodeOut(i));
        }
    }

I have checked and can confirm that in the code above, sideChainBufferIdx is -1 when sidechain is disconnected, and it assumes a > 0 value when sidechain is connected.

That information would be perfect for our goal, but unfortunately it lives in the algorithm context and I found no easy way to access it from within preparePlugin().

As a second option, we can pass that information to preparePlugin() depending on the notification that is received. For that, I added a bool sidechainConnected parameter to preparePlugin(), and there I pass the value that corresponds to the notification, as shown below:

    AAX_Result NotificationReceived (AAX_CTypeID type, const void* data, uint32_t size) override
    {
        if (type == AAX_eNotificationEvent_EnteringOfflineMode)  pluginInstance->setNonRealtime (true);
        if (type == AAX_eNotificationEvent_ExitingOfflineMode)   pluginInstance->setNonRealtime (false);

        // added by me
        if (type == AAX_eNotificationEvent_SideChainBeingConnected)     preparePlugin (true);
        if (type == AAX_eNotificationEvent_SideChainBeingDisconnected)  preparePlugin (false);

        return AAX_CEffectParameters::NotificationReceived (type, data, size);
    }

That seems to work well, but now I need to actually “do something” inside preparePlugin() to enable/disable the sidechain input. What I attempted is this:

        ... snippet from preparePlugin() ....

        AudioProcessor::BusesLayout newLayout;
        if (! fullBusesLayoutFromMainLayout (audioProcessor, inputSet, outputSet, newLayout))
        {
            if (isPrepared)
            {
                isPrepared = false;
                audioProcessor.releaseResources();
            }

            return AAX_ERROR_UNIMPLEMENTED;
        }

        /* yfede: I added this IF in an attempt to disable the otherwise always mono sidechain
           when no sidechain is actually connected */
        if (newLayout.inputBuses.size () > 1 && sidechainConnected == false)
            newLayout.inputBuses.getReference (1) = AudioChannelSet::disabled ();

        const bool layoutChanged = (oldLayout != newLayout);

The problem with the above code is that I hit an assertion at line 1048. My understanding is that this needs to be supported in more places throughout the AAX wrapper, but I’m not expert enough of the implementation of the multibus API to really know where to make changes.

What I noticed, however, is that all throughout the AAX code there are hasSidechain flags which sometimes are used with meaning “the AAX supports connection of a sidechain channel”, while some other times they mean “the sidechain bus is not disabled”.

Albeit similar, the two concepts are not the same: the plug-in can either support sidechain or not (isSidechainSupported would be a better name for this concept), and if it supports sidechain, then an actual sidechain input can be connected to it or not (isSidechainConnected would be a better name for this other concept).

With that straightened out, then it is easy to see that these equivalences should hold:

isSidechainSupported == (layout.inputBuses.size() > 1);

and

isSidechainConnected == (layout.getNumChannels (true, 1) > 0);

Perhaps if the AAX code is clarified in those terms, then the correct way to implement sidechain connection detection will pop out naturally.

@fabian, we know you are the expert here, any chance you can look into this?

EDIT: it goes without saying that, if the plug-in supports sidechain, but a channel is not currently connected to it, then layout.inputBuses.size() should be 2 but layout.inputBuses.getReference (1) should equal to AudioChannelSet::disabled() (which is the same to say that the number of channels in that bus is 0).


#3

Bump… I really need someone from JUCE to comment on this issue.


#4

Sorry @yfede, for only replying now. You are right that this was a bit of an oversight when developing the multi-bus API. The way the API is written is that the plug-in needs to be in suspended state to disable a bus - this is because the disabled state is represented as an AudioChannelSet with zero speakers in JUCE and therefore is treated like a layout change - and plug-ins are normally always suspended for layout changes. However, ProTools allows you to disconnect the side-chain without suspending the plug-in.

So there are two options here:

  1. I change the AAX wrapper in a similar way to what you suggested. When ProTools de-activates the sidechain, we lock the callback lock, suspend the plug-in and call prepareToPlay again with the new layout (i.e. with the sidechain disabled). We then resume the plug-in callbacks again after releasing the lock. I’d prefer this solution as it doesn’t require API changes but I’m not sure if some plug-in vendors may consider an inevitable audible “click” unacceptable when changing the sidechain.

  2. We modify the multi-bus API to add a new indication when a certain bus is temporarily disabled. This would be quite confusing as there would be two ways to disable a bus. But it wouldn’t require suspending the plug-in.

What do you think?


#5

From a users POV this is reasonable. Routing usually have some mute going for a second. Unlike bypassing, sidechain I/O changes aren’t automatable. so that sounds reasonable.


#6

I’d say that solution #1 is fine, for the same reason pointed out by @ttg


#7

I disagree with the acceptability of #1; the root cause is an incorrect assumption in the initial design and solution #1 does nothing to resolve this. The bad assumption is still there and is prone to cause problems again at some other time. I used to be fond of these “clever shortcuts” myself, like using a null speaker set to represent a disabled bus. But over many years of writing plugs I’ve seen they have a nasty way of coming back to bite you (us) in the rear over and over again.

Please address the actual problem by revising the API to add a distinct indication of a disabled bus instead of introducing an unnecessary constraint to work around a flawed design choice.


#8

Whatever way you decide to proceed, @fabian, are there news about this?


#9

Don’t worry: looking into this. Cleaning up some code along the way…


#10

This is on develop now.


#11

Cool! thanks, very eager to test it.

Do you think it will work if I cherry-pick on master (in my local repo of course) those two commits made today, or there is some previous work done on develop upon which they depend?


#12

I think that should work. But can’t say 100%.


#13

FYI unfortunately that can’t be applied on master as is, because the implementation of meter parameters in between has interfered :’(

It would be cool if new features like meter parameters were developed independently from bugfixes.

Perhaps it would be more appropriate to fix bugfixes that apply to production code on a branch that starts from master, and merge it in develop?


ComboBox with sub menus in the JUCE code