bitfocus / companion

Bitfocus Companion enables the reasonably priced Elgato Streamdeck and other controllers to be a professional shotbox surface for an increasing amount of different presentation switchers, video playback software and broadcast equipment.
http://bitfocus.io/companion
Other
1.47k stars 489 forks source link

Listening port management #1547

Open Julusian opened 3 years ago

Julusian commented 3 years ago

Describe the feature When modules need to listen on a local port, commonly when using osc, this is often done badly leading to a high chance of issues of a port clash with other software or being unable to use multiple instances of a module.
eg https://github.com/bitfocus/companion-module-audiostrom-liveprofessor/issues/1

The problem is that choosing a good port is hard. There are a couple of scenarios: 1) You don't really care what your local port is (eg, port is not defined in external software/hardware) 2) You need to choose and remember a local port

Option 1 is something that we should simply look out for and ensure that in these cases no localPort is passed to the osc library (like done in behringer-x32)

Option 2 is where the complexity lies. We need to choose a port that isnt already in use, and reserve it from use by other modules. Some modules uses a package called get-port to help them choose a free port, but that will only work reliably if every other instance is already running.
If you were to disable an instance of a module, and add a new instance, that new instance would likely get the ports the disabled one was just using.

I propose we add some new methods for instances to implement. This flow is similar to how the config is done currently, so will be familiar to module authors.
A function getPortRequests(): object describing the ports it wants. The chosen ports will get passed into the init function, and another function updatePorts(ports: object): void that instances can implement to be told that its port assignments changed whilst it is enabled

Companion can then keep track of which instance has what port, and should persist this in the db. These reservations and the accompanying ports should be constant until the instance is deleted.
The reservations and ports should be shown in a table in the ui for the user. (We could let the user manually edit these too if we want)

Q) how should we handle a reserved port being already in use? (eg a program was started before companion that uses 8000, but we previously assigned that to an instance)
Should we change it and log an error, or provide a button for the user to trigger getting new ports?

Also, a closer eye should be kept on how the local ports are chosen to ensure modules dont have issues like these.

Usecases https://github.com/bitfocus/companion-module-audiostrom-liveprofessor/blob/863f264987520bb23a21c734ae618e31f818f66b/index.js#L434 Is currently using a hardcoded port, causing a second instance to fail. Even if the commented out config s restored, it requires the user to manage ports themselves.

https://github.com/bitfocus/companion-module-behringer-xair/blob/109fd54fbd909b2d6f0215e603238bd3e5d024f9/index.js#L1499 is avoiding duplicate issues by applying an offset. If the x32 module was doing the same then this would likely have collision issues as they would work on the same port ranges. In this case the localPort isnt needed, and could be replaced with 0 (osc will randomly choose a port)
As the number of modules grow, the chance of them choosing the same pool of ports increases

istnv commented 3 years ago

I bumped into this almost immediately when adding feedback to QLab. The xair offset is generated the same way: A simple sum of the module id. Unless x32 and xair somehow ended up with the same module id, collisions should be extremely rare.

Julusian commented 3 years ago

@istnv yes, it is unlikely to collide (for a list of reasons) which is why I havent raised this as an issue when I first noticed it. but it is unnecessary complexity and it could happen. the x32 module simply sets localPort to 0, so I know that works and will get a fresh port every time it reinitialises osc (there was a bug where old ports werent being closed, causing companion to eventually crash..)

This task is mostly about the other case of really simple and naive port selection, and the xair was just an example of other cases to consider