Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

audio.jack.autoconnect erroneously connects to MIDI ports #1149

Closed
ReinholdH opened this issue Sep 8, 2022 · 4 comments · Fixed by #1153
Closed

audio.jack.autoconnect erroneously connects to MIDI ports #1149

ReinholdH opened this issue Sep 8, 2022 · 4 comments · Fixed by #1153
Labels

Comments

@ReinholdH
Copy link

When using audio.jack.multi=1 not all ports and fx are connected, and as a consequence - after the bug happens - ports are connected alternating to the wrong output side. The bug is in fluid_jack.c line 637, 658

If jack_connect fails with err because jack_ports[o] won't exist "i" needs to be decremented. See "i--" added in order to connect port "i" to jack_ports[0] again. Otherwise port "i" does not get connected, and the following port gets connected to the wrong output side.

Here is the snapshot of fluid_jack.c starting with line 613
` if(autoconnect)
{
jack_ports = jack_get_ports(client, NULL, NULL, JackPortIsInput | JackPortIsPhysical);

    if(jack_ports && jack_ports[0])
    {
        int err, o = 0;
        int connected = 0;

        for(i = 0; i < 2 * dev->num_output_ports; ++i)
        {
            err = jack_connect(client, jack_port_name(dev->output_ports[i]), jack_ports[o++]);

            if(err)
            {
                FLUID_LOG(FLUID_ERR, "Error connecting jack port");
            }
            else
            {
                connected++;
            }

           if (!jack_ports[o])
           {
                o = 0;
                i--;    //  i needs to be decremented
            }
        }

        o = 0;
        for(i = 0; i < 2 * dev->num_fx_ports; ++i)
        {
            err = jack_connect(client, jack_port_name(dev->fx_ports[i]), jack_ports[o++]);

            if(err)
            {
                FLUID_LOG(FLUID_ERR, "Error connecting jack port");
            }
            else
            {
                connected++;
            }

            if (!jack_ports[o])
            {
                o = 0;
                i--; //  i needs to be decremented
            }
        }

        jack_free(jack_ports);   /* free jack ports array (not the port values!) */
    }
    else
    {
        FLUID_LOG(FLUID_WARN, "Could not connect to any physical jack ports; fluidsynth is unconnected");
    }
}`

I hope this is the final solution. Works fine in my setup. Can somebody double-check. Thx.

@ReinholdH ReinholdH added the bug label Sep 8, 2022
@ReinholdH
Copy link
Author

I want to add that the used settings are

audio.jack.multi=1
synth.audio-channels=16
synth.audio-groups=16

@derselbst
Copy link
Member

The purpose of that line

if(!jack_ports[o])

is to check whether we have reached the end of the NULL terminated array of Jack's physical ports. Note that o has been incremented during the jack_connect call, so this now points to the next port. This is not related to any potential connection error during jack_connect.

With your change, some of fluidsynth's ports are being connected twice (e.g. l05, l10, l15, r02, r07, r12):
grafik

You probably meant to decrement i in that if-clause:

if(err)
{
FLUID_LOG(FLUID_ERR, "Error connecting jack port");
}

However, doing so is potentially dangerous as it might cause the loop to never terminate.

I think it would be better to figure out why jack is failing to connect to these ports. Any details that caused the error?

@ReinholdH
Copy link
Author

Thanks for the immediate response. Yes, I agree. My proposal from above is wrong but the solution is much more simple.

"audio autoconnect" needs to be restricted to audio ports only :-).

In line 615 of fluid_jack.cpp
jack_ports = jack_get_ports(client, NULL, NULL, JackPortIsInput | JackPortIsPhysical);
all ports are taken into account including MIDI ports. If MIDI ports are present and activated in addition to audio ports the "audio autoconnect" fails. Line 615 needs to be
jack_ports = jack_get_ports(client, NULL, JACK_DEFAULT_AUDIO_TYPE, JackPortIsInput | JackPortIsPhysical);
This change works fine in my setup. Is this proposal OK?

@derselbst
Copy link
Member

Oh, ofc. Thanks Reinhold! Just opened the PR.

@derselbst derselbst changed the title audio.jack.autoconnect=1 does not connect all ports, and connects some to the wrong output side audio.jack.autoconnect erroneously connects to MIDI ports Sep 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants