cansik / artnet4j

Art-Net DMX over IP library for Java and Processing
GNU General Public License v3.0
95 stars 21 forks source link

Unnecessary Port Binding for ArtNet Sender #26

Open tylerstraub opened 6 months ago

tylerstraub commented 6 months ago

When using the artnet4j library in a Processing 4 application, the library creates a UDP port binding even when acting as an ArtNet Sender. This behavior leads to binding conflicts with other applications that may need to communicate locally, such as DigiShow LINK and QLC+, which do not bind to a specific port when sending ArtNet data.

Expected Behavior

ArtNet Sender applications should use an ephemeral port for sending data and should not bind to a specific port (e.g. 6454). Only ArtNet Receiver applications require a static bind to listen for incoming ArtNet packets.

Observed Behavior

The artnet4j library binds a port regardless of whether it is used as a Sender or Receiver. This causes conflicts when other applications attempt to bind to the same port.

Steps to Reproduce

  1. Condition 1: Sender = Processing 4, Receiver = DigiShow LINK

    • Launch DigiShow LINK before Processing 4: No port conflict.
    • Launch Processing 4 before DigiShow LINK: Port conflict occurs.
  2. Condition 2: Receiver = Processing 4, Sender = DigiShow LINK

    • Launch DigiShow LINK before Processing 4: No port conflict.
    • Launch Processing 4 before DigiShow LINK: No port conflict.
  3. Extra Tests:

    • DigiShow LINK Sender launched in isolation: No binding on port 6454.
    • Processing 4 Sender launched in isolation: Binding occurs on port 6454.

Diagnostic Results

Using lsof on macOS reveals that DigiShow LINK does not create a binding when sending ArtNet, but the Processing 4 application using artnet4j does:

Validation of Ephemeral Port Usage in DigiShow LINK

After examining the DigiShow LINK source code, it was confirmed that DigiShow LINK uses ephemeral ports for sending ArtNet data. This approach prevents port conflicts and allows the application to coexist with other applications that need to bind as a Reciever.

Key Points from dgs_artnet_interface.cpp:

  1. UDP Port Definition:

    #define ARTNET_UDP_PORT 6454
  2. UDP Socket Initialization:

    m_udp = new QUdpSocket();
  3. Binding the UDP Socket for Input:

    if (m_interfaceInfo.mode == INTERFACE_ARTNET_INPUT) {
       // for artnet receiver
       m_dataAll.clear();
       m_udp->bind(m_udpPort);
       connect(m_udp, SIGNAL(readyRead()), this, SLOT(onUdpDataReceived()));
    }
  4. Setting Up the UDP Socket for Output:

    if (m_interfaceInfo.mode == INTERFACE_ARTNET_OUTPUT) {
       // for artnet sender
       m_sequence = 0;
       const unsigned char bytes[] = {
           0x41, 0x72, 0x74, 0x2d, 0x4e, 0x65, 0x74, 0x00,  // Art-Net
           0x00, 0x20, // opcode ARTNET_POLL
           0x00, 0x0e, // version
           0x00, 0x00  // take-to-me
       };
       m_udp->writeDatagram((const char*)bytes, sizeof(bytes),
                            m_udpHost,
                            (quint16)m_udpPort);
       m_timer = new QTimer();
       connect(m_timer, SIGNAL(timeout()), this, SLOT(onTimerFired()));
       m_timer->setTimerType(Qt::PreciseTimer);
       m_timer->setSingleShot(false);
       m_timer->setInterval(1000 / frequency);
       m_timer->start();
    }

Suggested Fix

Modify the artnet4j library to avoid static binding its port when acting solely as a Sender. Instead, it should use an ephemeral port for sending data.

Proposed Code Changes

Here is an example modification to the ArtNetServer class:

public void start(InetAddress networkAddress, boolean isReceiver) throws SocketException, ArtNetException {
    if (broadCastAddress == null) {
        setBroadcastAddress(DEFAULT_BROADCAST_IP);
    }
    if (socket == null) {
        socket = new DatagramSocket(null);
        socket.setReuseAddress(true);
        socket.setBroadcast(true);

        if (isReceiver) {
            if (networkAddress == null)
                networkAddress = socket.getLocalAddress();

            socket.bind(new InetSocketAddress(networkAddress, port));
            logger.info("Art-Net server started at: " + networkAddress.getHostAddress() + ":" + port);
        } else {
            logger.info("Art-Net server started as sender using ephemeral port.");
        }

        for (ArtNetServerListener l : listeners) {
            l.artNetServerStarted(this);
        }
        isRunning = true;
        serverThread = new Thread(this);
        serverThread.start();
    } else {
        throw new ArtNetException(
                "Couldn't create server socket, server already running?");
    }
}

In the application code, specify whether the instance is acting as a Sender or Receiver:

ArtNetServer server = new ArtNetServer();
server.start(null, false); // For sender
// or
server.start(null, true);  // For receiver

Conclusion

By implementing these changes, the artnet4j library will avoid unnecessary port bindings when acting as a Sender, preventing conflicts with other applications and allowing for more flexible execution order.

tylerstraub commented 6 months ago

PR sent: https://github.com/cansik/artnet4j/pull/27

This branch implements a boolean option to start() which launches the port binding in "ephemeral mode".

I'm not sure if this fix is necessarily ideal for main branch but it exists now for anyone out there struggling with this same problem.