Allow the nqptp shared memory interface name to be set per instance - #2273
Merged
Merged
Conversation
Shairport Sync already sends its shared memory interface name at the front of every control message to nqptp, and config.nqptp_shared_memory_interface_name has always existed to hold it -- but it was hardcoded to NQPTP_INTERFACE_NAME and could not be changed. That is fine for a single instance. With several AirPlay 2 instances on one host it means they all share one nqptp clock, and nqptp keys its clock table and master clock on that name. When two of those instances are members of the same AirPlay 2 group -- which Control Center will happily do -- and one room leaves the group, that instance sends a "T" (no active timer) and nqptp clears the clock the other instances are still playing to. They stop writing audio and never recover, with nothing logged. This adds general.nqptp_shared_memory_interface_name so each instance can be given its own, e.g. "/nqptp-kitchen". The name is validated as a POSIX shared memory name (leading "/", no other "/", 2-63 characters, since nqptp stores it in a 64-byte field) and the default is unchanged, so a single instance and every existing configuration behave exactly as before. Requires an nqptp that honours the name it is sent. Note there is already dead code just above this computing a unique per-instance name from the app name and the AirPlay device id. Making the name explicit and configurable was preferred over quietly switching to that: it keeps existing setups on the name they have, and it puts each instance's interface name in the log where it can be checked.
Every AirPlay 2 instance on a host needs its own nqptp shared memory interface name, but the setting added in the previous commit is reachable only from the configuration file. That forces a separate configuration file per instance, whose only differences are the handful of settings that must differ. Add a matching --nqptp-shared-memory-interface-name so the name can be given on the command line, where it takes precedence over the configuration file, as the command line does for the other settings. Instances can then share one configuration file and differ only in their command lines. The validation is unchanged and is now shared by both paths, so an invalid name is rejected the same way whichever way it arrives, and the error message names the source it came from. The default is unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Owner
|
Many thanks! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Shairport Sync already sends its shared memory interface name at the front
of every control message to nqptp, and config.nqptp_shared_memory_interface_name
has always existed to hold it -- but it was hardcoded to NQPTP_INTERFACE_NAME
and could not be changed.
That is fine for a single instance. With several AirPlay 2 instances on one
host it means they all share one nqptp clock, and nqptp keys its clock table
and master clock on that name. When two of those instances are members of the
same AirPlay 2 group -- which Control Center will happily do -- and one room
leaves the group, that instance sends a "T" (no active timer) and nqptp clears
the clock the other instances are still playing to. They stop writing audio
and never recover, with nothing logged.
This adds general.nqptp_shared_memory_interface_name so each instance can be
given its own, e.g. "/nqptp-kitchen". The name is validated as a POSIX shared
memory name (leading "/", no other "/", 2-63 characters, since nqptp stores it
in a 64-byte field) and the default is unchanged, so a single instance and
every existing configuration behave exactly as before.
Requires an nqptp that honours the name it is sent.
Note there is already dead code just above this computing a unique per-instance
name from the app name and the AirPlay device id. Making the name explicit and
configurable was preferred over quietly switching to that: it keeps existing
setups on the name they have, and it puts each instance's interface name in the
log where it can be checked.
Requires mikebrady/nqptp#50, which makes nqptp honour the name it is sent.
Neither change does anything without the other.