From 3cdb8b790c6af59a7e39c2ab29e54efb4f0684c6 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Sat, 10 Dec 2022 11:57:19 +0000 Subject: [PATCH 1/9] Send B/E/P signals to NQPTP to tell it when SPS is playing. The clock should not be sleeping when SPS is playing or paused. --- player.c | 3 +++ rtsp.c | 5 +++++ 2 files changed, 8 insertions(+) diff --git a/player.c b/player.c index 61b4e5ff..b920971c 100644 --- a/player.c +++ b/player.c @@ -3565,6 +3565,9 @@ int player_stop(rtsp_conn_info *conn) { // debuglev = 3; debug(3, "player_stop"); if (conn->player_thread) { +#ifdef CONFIG_AIRPLAY_2 + ptp_send_control_message_string("E"); // signify play is "E"nding +#endif debug(3, "player_thread cancel..."); pthread_cancel(*conn->player_thread); debug(3, "player_thread join..."); diff --git a/rtsp.c b/rtsp.c index c68feb43..abebb645 100644 --- a/rtsp.c +++ b/rtsp.c @@ -1276,6 +1276,7 @@ void set_client_as_ptp_clock(rtsp_conn_info *conn) { strncat(timing_list_message, (const char *)&conn->client_ip_string, sizeof(timing_list_message) - 1 - strlen(timing_list_message)); ptp_send_control_message_string(timing_list_message); + // the clock will be active for a few seconds after this. } void clear_ptp_clock() { ptp_send_control_message_string("T"); } @@ -2003,6 +2004,7 @@ void handle_setrateanchori(rtsp_conn_info *conn, rtsp_message *req, rtsp_message pthread_cleanup_push(mutex_unlock, &conn->flush_mutex); conn->ap2_rate = rate; if ((rate & 1) != 0) { + ptp_send_control_message_string("B"); // signify play is "B"eginning or resuming debug(2, "Connection %d: Start playing, with anchor clock %" PRIx64 ".", conn->connection_number, conn->networkTimeTimelineID); activity_monitor_signify_activity(1); @@ -2013,6 +2015,7 @@ void handle_setrateanchori(rtsp_conn_info *conn, rtsp_message *req, rtsp_message conn->ap2_play_enabled = 1; } else { debug(2, "Connection %d: Stop playing.", conn->connection_number); + ptp_send_control_message_string("P"); // signify play is "P"ausing conn->ap2_play_enabled = 0; activity_monitor_signify_activity(0); reset_anchor_info(conn); @@ -3161,6 +3164,8 @@ void handle_setup_2(rtsp_conn_info *conn, rtsp_message *req, rtsp_message *resp) debug(2, "Connection %d: SETUP on %s. A \"streams\" array has been found", conn->connection_number, get_category_string(conn->airplay_stream_category)); if (conn->airplay_stream_category == ptp_stream) { + ptp_send_control_message_string("B"); // signify play is "B"eginning + // get stream[0] plist_t stream0 = plist_array_get_item(streams, 0); From 12586cd82ee81e3d0cd26b55ead436335f4cf397 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Tue, 13 Dec 2022 09:29:58 +0000 Subject: [PATCH 2/9] Bump the SHM Interface number to note new commands. --- nqptp-shm-structures.h | 42 ++++++++++++++++++++++++++---------------- 1 file changed, 26 insertions(+), 16 deletions(-) diff --git a/nqptp-shm-structures.h b/nqptp-shm-structures.h index 3dbe0c90..251d06c1 100644 --- a/nqptp-shm-structures.h +++ b/nqptp-shm-structures.h @@ -22,26 +22,36 @@ #define NQPTP_INTERFACE_NAME "/nqptp" -#define NQPTP_SHM_STRUCTURES_VERSION 8 +#define NQPTP_SHM_STRUCTURES_VERSION 9 #define NQPTP_CONTROL_PORT 9000 -// The control port expects a UDP packet with the first space-delimited string -// being the name of the shared memory interface (SMI) to be used. -// This allows client applications to have a dedicated named SMI interface with -// a timing peer list independent of other clients. -// The name given must be a valid SMI name and must contain no spaces. -// If the named SMI interface doesn't exist it will be created by NQPTP. -// The SMI name should be delimited by a space and followed by a command letter. -// At present, the only command is "T", which must followed by nothing or by -// a space and a space-delimited list of IPv4 or IPv6 numbers, -// the whole not to exceed 4096 characters in total. +// The control port expects a UDP packet with the first character being a command letter +// and the rest being any arguments, the whole not to exceed 4096 characters. +// The "T" command, must followed by nothing or by +// a space and a space-delimited list of IPv4 or IPv6 numbers. // The IPs, if provided, will become the new list of timing peers, replacing any -// previous list. If the master clock of the new list is the same as that of the old list, -// the master clock is retained without resynchronisation; this means that non-master -// devices can be added and removed without disturbing the SMI's existing master clock. +// previous list. The first IP number is the clock that NQPTP will listen to. +// The remaining IP address are the addresses of all the timing peers. The timing peers +// are not used in this version of NQPTP. // If no timing list is provided, the existing timing list is deleted. -// (In future version of NQPTP the SMI interface may also be deleted at this point.) -// SMI interfaces are not currently deleted or garbage collected. +// The "B" command is a message that the client -- which generates the clock -- +// is about to start playing. +// NQPTP uses it to determine that the clock is active and will not sleep. +// The "E" command signifies that the client has stopped playing and that +// the clock may shortly sleep. +// The "P" command signifies that SPS has paused play (buffered audio only). +// The clock seems to stay running in this state. + +// When the clock is active, it is assumed that any decreases in the offset +// between the local and remote clocks are due to delays in the network. +// NQPTP smooths the offset by clamping any decreases to a small value. +// In this way, it can follow clock drift but ignore network delays. + +// When the clock is inactive, it can stop running. This causes the offset to decrease. +// NQPTP clock smoothing would treat this as a network delay, causing true sync to be lost. +// To avoid this, when the clock goes from inactive to active, +// NQPTP resets clock smoothing to the new offset. + #include #include From db155abb64e49ea7b01ed98c9f47da26447af864 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Wed, 14 Dec 2022 10:12:01 +0000 Subject: [PATCH 3/9] Danger note. --- README.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/README.md b/README.md index 9a65b3ec..8b181e91 100644 --- a/README.md +++ b/README.md @@ -7,6 +7,11 @@ Metadata such as artist information and cover art can be requested and provided Shairport Sync does not support AirPlay video or photo streaming. +# Danger +This branch of the Shairport Sync repository is very likely to be buggy and can change very rapidly and without warning. The `development` branch is more stable, although it too may contains bugs. The `master` branch is the most stable. + +The `danger` branch version of Shairport Sync may only be compatible with the `danger` branch of NQPTP. + # Quick Start * A building guide is available [here](BUILD.md). * A Docker image is available on the [Docker Hub](https://hub.docker.com/r/mikebrady/shairport-sync). From 51b9c5989c2850118d4ec29e4403161a372c7fe2 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Wed, 14 Dec 2022 10:17:24 +0000 Subject: [PATCH 4/9] Update BUILD.md --- BUILD.md | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/BUILD.md b/BUILD.md index f6dc3ad3..6594a482 100644 --- a/BUILD.md +++ b/BUILD.md @@ -7,6 +7,11 @@ Overall, you'll be building and installing two programs – Shairport Sync itsel In the commands below, note the convention that a `#` prompt means you are in superuser mode and a `$` prompt means you are in a regular unprivileged user mode. You can use `sudo` *("SUperuser DO")* to temporarily promote yourself from user to superuser, if permitted. For example, if you want to execute `apt-get update` in superuser mode and you are in user mode, enter `sudo apt-get update`. +# Danger +This branch of the Shairport Sync repository is very likely to be buggy and can change very rapidly and without warning. The `development` branch is more stable, although it too may contains bugs. The `master` branch is the most stable. + +The `danger` branch version of Shairport Sync may only be compatible with the `danger` branch of NQPTP. + ## 1. Prepare #### Remove Old Copies of Shairport Sync Before you begin building Shairport Sync, it's best to remove any existing copies of the application, called `shairport-sync`. Use the command `$ which shairport-sync` to find them. For example, if `shairport-sync` has been installed previously, this might happen: @@ -118,11 +123,11 @@ If you are building classic Shairport Sync, the list of packages is shorter: ### NQPTP Skip this section if you are building classic Shairport Sync – NQPTP is not needed for classic Shairport Sync. -Download, install, enable and start NQPTP from [here](https://github.com/mikebrady/nqptp). +Download, install, enable and start the `danger` branch of NQPTP from [here](https://github.com/mikebrady/nqptp/blob/danger/README.md). ### Shairport Sync #### Build and Install -Download Shairport Sync, check out the `development` branch and configure, compile and install it. Before executing the commands, please note the following: +Download Shairport Sync, check out the `danger` branch and configure, compile and install it. Before executing the commands, please note the following: * If building for FreeBSD, replace `--with-systemd` with `--with-os=freebsd --with-freebsd-service`. * Omit the `--with-airplay-2` from the `./configure` options if you are building classic Shairport Sync. @@ -131,7 +136,7 @@ Download Shairport Sync, check out the `development` branch and configure, compi ``` $ git clone https://github.com/mikebrady/shairport-sync.git $ cd shairport-sync -$ git checkout development +$ git checkout danger $ autoreconf -fi $ ./configure --sysconfdir=/etc --with-alsa \ --with-soxr --with-avahi --with-ssl=openssl --with-systemd --with-airplay-2 From 14b3fc4de86451445a535954b0994f1a232105c4 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Wed, 14 Dec 2022 10:30:22 +0000 Subject: [PATCH 5/9] Fix endless error messages. --- audio_stdout.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/audio_stdout.c b/audio_stdout.c index 61d91bfa..2a54f072 100644 --- a/audio_stdout.c +++ b/audio_stdout.c @@ -37,21 +37,23 @@ #include static int fd = -1; +static int warned = 0; + static void start(__attribute__((unused)) int sample_rate, __attribute__((unused)) int sample_format) { fd = STDOUT_FILENO; + warned = 0; } static int play(void *buf, int samples, __attribute__((unused)) int sample_type, __attribute__((unused)) uint32_t timestamp, __attribute__((unused)) uint64_t playtime) { char errorstring[1024]; - int warned = 0; int rc = write(fd, buf, samples * 4); if ((rc < 0) && (warned == 0)) { strerror_r(errno, (char *)errorstring, 1024); - warn("Error %d writing to stdout: \"%s\".", errno, errorstring); + warn("Error %d writing to stdout (fd: %d): \"%s\".", errno, fd, errorstring); warned = 1; } return rc; @@ -69,6 +71,7 @@ static int init(__attribute__((unused)) int argc, __attribute__((unused)) char * // get settings from settings file // do the "general" audio options. Note, these options are in the "general" stanza! parse_general_audio_options(); + return 0; } From 74c093351a85711820974570becee7611cf6a821 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Wed, 14 Dec 2022 10:33:28 +0000 Subject: [PATCH 6/9] Update check_ap2_systemd_full.yml Run on the `danger` branch too. --- .github/workflows/check_ap2_systemd_full.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/check_ap2_systemd_full.yml b/.github/workflows/check_ap2_systemd_full.yml index 780dd3a4..858b2af5 100644 --- a/.github/workflows/check_ap2_systemd_full.yml +++ b/.github/workflows/check_ap2_systemd_full.yml @@ -2,7 +2,7 @@ name: Full configuration (but without apple-alac) for systemd. on: push: - branches: [ "development" ] + branches: [ "development", "danger" ] jobs: build: From 527edf6aa5f55928c34523aff095c5a3d9872c6f Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Wed, 14 Dec 2022 10:33:57 +0000 Subject: [PATCH 7/9] Update check_classic_systemd_full.yml Run on the `danger` branch too. --- .github/workflows/check_classic_systemd_full.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/check_classic_systemd_full.yml b/.github/workflows/check_classic_systemd_full.yml index bbc7c02e..06744a03 100644 --- a/.github/workflows/check_classic_systemd_full.yml +++ b/.github/workflows/check_classic_systemd_full.yml @@ -2,7 +2,7 @@ name: Full classic configuration (but without apple-alac) for systemd, using a b on: push: - branches: [ "development" ] + branches: [ "development", "danger" ] jobs: build: From b72e2c649db924693a4b27b63b9e905172cecbf2 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Wed, 14 Dec 2022 10:35:37 +0000 Subject: [PATCH 8/9] Update check_classic_mac_basic.yml Run on the `danger` branch too. --- .github/workflows/check_classic_mac_basic.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/check_classic_mac_basic.yml b/.github/workflows/check_classic_mac_basic.yml index 2dc4e715..d41d0fc8 100644 --- a/.github/workflows/check_classic_mac_basic.yml +++ b/.github/workflows/check_classic_mac_basic.yml @@ -2,7 +2,7 @@ name: Basic libao configuration for macOS with BREW -- classic only, because mac on: push: - branches: [ "development" ] + branches: [ "development", "danger" ] jobs: build: From e518891b298e3aa7e3ea36c025333aa31a17009a Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Mon, 19 Dec 2022 09:12:23 +0000 Subject: [PATCH 9/9] Shorten the keepalive timeout time to 15 seconds. --- rtsp.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/rtsp.c b/rtsp.c index abebb645..02c6c975 100644 --- a/rtsp.c +++ b/rtsp.c @@ -5503,12 +5503,12 @@ void *rtsp_listen_loop(__attribute((unused)) void *arg) { // Thanks to https://holmeshe.me/network-essentials-setsockopt-SO_KEEPALIVE/ for this. - // turn on keepalive stuff -- wait for keepidle + (keepcnt * keepinttvl time) seconds before giving up - // an ETIMEOUT error is returned if the keepalive check fails + // Turn on keepalive stuff -- wait for keepidle + (keepcnt * keepinttvl time) seconds before giving up + // An ETIMEOUT error is returned if the keepalive check fails - int keepAliveIdleTime = 35; // wait this many seconds before checking for a dropped client + int keepAliveIdleTime = 10; // wait this many seconds before checking for a dropped client int keepAliveCount = 5; // check this many times - int keepAliveInterval = 5; // wait this many seconds between checks + int keepAliveInterval = 1; // wait this many seconds between checks #if defined COMPILE_FOR_BSD || defined COMPILE_FOR_OSX