#53 Bug in port discovery/connection logic

[Giacomo De Lazzari]

The investigation of this issue started here https://gitlab.freedesktop.org/pipewire/pipewire/-/issues/527 and revealed a bug in qjackctl.

What I was facing, as a user trying to connect JACK ports with qjackctl, is that I couldn't make arbitrary connections in the graph. By debugging and looking at qjackctl's code, I found that when a device is present which only has monitor ports (that are not marked as physical and terminal), the port discovery logic in qjackctlJackGraph::findClientPort messes up and creates, as node graphs, a mode=Input item and a mode=Duplex item (instead of an Input/Output nodes pair). When I then try to connect from any port to an output port of such device, the logic in qjackctlGraphConnectCommand::execute breaks since, in the second part (starting at line 90 of qjackctlGraphCommand.cpp - as of b0a32471968cbbb08f7aa7819d50e3829f029f10 in master) the node with mode=Duplex is selected as the node to lookup the port in, but no port is found since the wanted port is in the other node flagged as mode=Input.

While this was discovered in the context of using qjackctl with PipeWire, this bug is reproducible with JACK only. I can reproduce on a system with only JACK by launching jackd as jackd -d alsa -m --device hw:1 -P, at which point I get

$ jack_lsp -Acpt
system:playback_1
   alsa_pcm:hw:1:in1
    properties: input,physical,terminal,
    32 bit float mono audio
system:monitor_1
    properties: output,
    32 bit float mono audio
system:playback_2
   alsa_pcm:hw:1:in2
    properties: input,physical,terminal,
    32 bit float mono audio
system:monitor_2
    properties: output,
    32 bit float mono audio

and I can't connect anything with qjackctl to the playback ports of the system card.

This can be fixed in a very quick way by such a patch I have locally

diff --git a/src/qjackctlGraphCommand.cpp b/src/qjackctlGraphCommand.cpp
index 0f9e8db..ed655f7 100644
--- a/src/qjackctlGraphCommand.cpp
+++ b/src/qjackctlGraphCommand.cpp
@@ -69,12 +69,12 @@ bool qjackctlGraphConnectCommand::execute ( bool is_undo )
        qjackctlGraphNode *node1
                = canvas->findNode(
                        m_item.addr1.node_name,
-                       qjackctlGraphItem::Duplex,
+                       qjackctlGraphItem::Output,
                        m_item.addr1.node_type);
        if (node1 == nullptr)
                node1 = canvas->findNode(
                        m_item.addr1.node_name,
-                       qjackctlGraphItem::Output,
+                       qjackctlGraphItem::Duplex,
                        m_item.addr1.node_type);
        if (node1 == nullptr)
                return false;
@@ -90,12 +90,12 @@ bool qjackctlGraphConnectCommand::execute ( bool is_undo )
        qjackctlGraphNode *node2
                = canvas->findNode(
                        m_item.addr2.node_name,
-                       qjackctlGraphItem::Duplex,
+                       qjackctlGraphItem::Input,
                        m_item.addr2.node_type);
        if (node2 == nullptr)
                node2 = canvas->findNode(
                        m_item.addr2.node_name,
-                       qjackctlGraphItem::Input,
+                       qjackctlGraphItem::Duplex,
                        m_item.addr2.node_type);
        if (node2 == nullptr)
                return false;

i.e. by inverting the search order so that the correct node is found and the wanted port is looked up correctly. However, I guess it might be that qjackctl must not end up with a Input/Duplex nodes pair in the first place, as I don't think monitor ports should trigger the creation of a Duplex node. But this is unknown territory for me, so I have no idea of the correct way to fix this.

I hope I have explained the issue well enough. If that's not the case, I'm more than willing to clarify my findings, just let me know. Thanks for looking at this!

[Giacomo De Lazzari]

I meant to write

"When I then try to connect from any port to an input port of such device, the logic in qjackctlGraphConnectCommand::execute breaks ..."

(not output port)

Sorry for that.

[Rui Nuno Capela]

you could make it a merge request here or a pull request there and let your contribution get some credit in the git ...

ps. applied to develop branch [bfbc9f]

[Giacomo De Lazzari]

I would have sent a pull request but didn't know if my solution was the right one (and not just a "dirty fix"), since I do not have a global view of the source code.

Thanks for looking at it and applying the patch. Also I didn't know that GitHub is used to develop the software, so I didn't think I could make a PR. I thought it was just a mirror (but it seems like it's the opposite and SF is mirroring GitHub?). Sorry for that.