From 2e7d5233beaa8cd773add917945a811954afc65d Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Wed, 29 May 2024 12:28:37 +0100 Subject: [PATCH 01/29] Update CAR INSTALL.md --- CAR INSTALL.md | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/CAR INSTALL.md b/CAR INSTALL.md index a7e09d19..1aacaae1 100644 --- a/CAR INSTALL.md +++ b/CAR INSTALL.md @@ -200,7 +200,7 @@ fi exit 0 # normal exit here ``` -#### Disable Unused Services +#### Disable Unused Services (Optional) These optional steps have been tested on a Raspberry Pi only -- they have not been tested on other systems. Some services are not necessary for this setup and can be disabled as follows: ``` @@ -208,11 +208,9 @@ Some services are not necessary for this setup and can be disabled as follows: # systemctl disable triggerhappy # systemctl disable dphys-swapfile ``` -#### Optional: Read-only mode – Raspberry Pi Specific -This optional step is applicable to a Raspberry Pi only. Run `sudo raspi-config` and then choose `Performance Options` > `Overlay Filesystem` and choose to enable the overlay filesystem, and to set the boot partition to be write-protected. (The idea here is that this offers more protection against files being corrupted by the sudden removal of power.) -### Final Steps +#### Disable Unused Services (Mandatory) You now need to disable some services; that is, you need to stop them starting automatically on power-up. This is because they either interfere with the system's operation in WiFi Access Point mode, or because they won't work when the system isn't connected to the Internet. Only one of the `NetworkManager` and the `dhcpcd` service will be present in your system, but it's no harm to try to disable both. ``` # systemctl disable dhcpcd @@ -220,8 +218,12 @@ You now need to disable some services; that is, you need to stop them starting a # systemctl disable wpa_supplicant # systemctl disable systemd-timesyncd ``` -Lastly, note that the WiFi credentials you used initially to connect to your network (e.g. your home network) will have been stored in the system in plain text. This is convenient for when you want to reconnect to update (see later), but if you prefer to delete them, they will be in `/etc/wpa_supplicant/wpa_supplicant.conf` +Lastly, note that the WiFi credentials you used initially to connect to your network (e.g. your home network) will have been stored in the system in plain text. This is convenient for when you want to reconnect to update (see later), but if you prefer to delete them, they will be in `/etc/wpa_supplicant/wpa_supplicant.conf`. +#### Optional: Read-only mode – Raspberry Pi Specific +This optional step is applicable to a Raspberry Pi only. Run `sudo raspi-config` and then choose `Performance Options` > `Overlay Filesystem` and choose to enable the overlay filesystem, and to set the boot partition to be write-protected. (The idea here is that this offers more protection against files being corrupted by the sudden removal of power.) + +### Final Step When you are finished, carefully power down the machine before unplugging it from power: ``` # poweroff From c07247af852f64b0af549320c9b5fefb5d102ecc Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Wed, 29 May 2024 12:31:42 +0100 Subject: [PATCH 02/29] Update CAR INSTALL.md --- CAR INSTALL.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CAR INSTALL.md b/CAR INSTALL.md index 1aacaae1..29f6fd6f 100644 --- a/CAR INSTALL.md +++ b/CAR INSTALL.md @@ -228,6 +228,8 @@ When you are finished, carefully power down the machine before unplugging it fro ``` # poweroff ``` +Note: doing a `reboot` here doesn't seem to work properly -- it really does seem necessary to power off. + ### Ready Install the Raspberry Pi in your car. It should be powered from a source that is switched off when you leave the car, otherwise the slight current drain will eventually flatten the car's battery. From c945f3a9ea6e7bbd9d0c2dca263c98b0a56be48e Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Wed, 29 May 2024 15:29:22 +0100 Subject: [PATCH 03/29] 4.3.4-dev start --- configure.ac | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index 15636e96..e6295413 100644 --- a/configure.ac +++ b/configure.ac @@ -2,7 +2,7 @@ # Process this file with autoconf to produce a configure script. AC_PREREQ([2.50]) -AC_INIT([shairport-sync], [4.3.3-dev], [4265913+mikebrady@users.noreply.github.com]) +AC_INIT([shairport-sync], [4.3.4-dev], [4265913+mikebrady@users.noreply.github.com]) AM_INIT_AUTOMAKE([subdir-objects]) AC_CONFIG_SRCDIR([shairport.c]) AC_CONFIG_HEADERS([config.h]) From 19556bf8c858388f92399b539c72b79948cf408e Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Wed, 29 May 2024 16:37:09 +0100 Subject: [PATCH 04/29] Update RELEASENOTES-DEVELOPMENT.md --- RELEASENOTES-DEVELOPMENT.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/RELEASENOTES-DEVELOPMENT.md b/RELEASENOTES-DEVELOPMENT.md index ee0e8c5e..1b55442e 100644 --- a/RELEASENOTES-DEVELOPMENT.md +++ b/RELEASENOTES-DEVELOPMENT.md @@ -1,3 +1,7 @@ +Version 4.3.4-dev-1-gc945f3a9 +== +This is effectively release version 4.3.3 -- the starting point for 4.3.4. + Version 4.3.3-dev-81-ge0b13829 == **Bug Fix** From 238e4bbfa382e968bae10b0632e9613f157b604b Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Sat, 1 Jun 2024 22:08:41 +0100 Subject: [PATCH 05/29] Modify the alsa help to use the hdmi: prefix for the HDMI devices it finds. --- audio_alsa.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 59 insertions(+), 5 deletions(-) diff --git a/audio_alsa.c b/audio_alsa.c index bcd2d6a4..177ce8dc 100644 --- a/audio_alsa.c +++ b/audio_alsa.c @@ -279,16 +279,70 @@ uint64_t frames_sent_for_playing; // frames_sent_for_playing (which Shairport Sync might hold) would be invalid. int frames_sent_break_occurred; +// if a device name ends in ",DEV=0", drop it. Then if it also begins with "CARD=", drop that too. +static void simplify_and_printf_mutable_device_name(char *device_name) { + if (strstr(device_name, ",DEV=0") == device_name + strlen(device_name) - strlen(",DEV=0")) { + char *shortened_device_name = str_replace(device_name, ",DEV=0", ""); + char *simplified_device_name = str_replace(shortened_device_name, "CARD=", ""); + printf(" \"%s\"\n", simplified_device_name); + free(simplified_device_name); + free(shortened_device_name); + } else { + printf(" \"%s\"\n", device_name); + } +} + static void help(void) { + printf(" -d output-device set the output device, default is \"default\".\n" " -c mixer-control set the mixer control name, default is to use no mixer.\n" " -m mixer-device set the mixer device, default is the output device.\n" " -i mixer-index set the mixer index, default is 0.\n"); - int r = system("if [ -d /proc/asound ] ; then echo \" hardware output devices:\" ; ls -al " - "/proc/asound/ 2>/dev/null | grep '\\->' | tr -s ' ' | cut -d ' ' -f 9 | while " - "read line; do echo \" \\\"hw:$line\\\"\" ; done ; fi"); - if (r != 0) - debug(2, "error %d executing a script to list alsa hardware device names", r); + // look for devices with a name prefix of hw: or hdmi: + int card_number = -1; + snd_card_next(&card_number); + + if (card_number < 0) { + printf(" no hardware output devices found.\n"); + } + + int at_least_one_device_found = 0; + while (card_number >= 0) { + void **hints; + char *hdmi_str = NULL; + char *hw_str = NULL; + if (snd_device_name_hint(card_number, "pcm", &hints) == 0) { + void **device_on_card_hints = hints; + while (*device_on_card_hints != NULL) { + char *device_on_card_name = snd_device_name_get_hint(*device_on_card_hints, "NAME"); + if ((strstr(device_on_card_name, "hw:") == device_on_card_name) && (hw_str == NULL)) + hw_str = strdup(device_on_card_name); + if ((strstr(device_on_card_name, "hdmi:") == device_on_card_name) && (hdmi_str == NULL)) + hdmi_str = strdup(device_on_card_name); + free(device_on_card_name); + device_on_card_hints++; + } + snd_device_name_free_hint(hints); + if ((hdmi_str != NULL) || (hw_str != NULL)) { + if (at_least_one_device_found == 0) { + printf(" hardware output devices:\n"); + at_least_one_device_found = 1; + } + } + if (hdmi_str != NULL) { + simplify_and_printf_mutable_device_name(hdmi_str); + } else if (hw_str != NULL) { + simplify_and_printf_mutable_device_name(hw_str); + } + if (hdmi_str != NULL) + free(hdmi_str); + if (hw_str != NULL) + free(hw_str); + } + snd_card_next(&card_number); + } + if (at_least_one_device_found == 0) + printf(" no hardware output devices found.\n"); } void set_alsa_out_dev(char *dev) { alsa_out_dev = dev; } // ugh -- not static! From 7e16d6f91ee1acc8d78869ccb256b0d8f9bf029d Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Sat, 1 Jun 2024 22:30:50 +0100 Subject: [PATCH 06/29] Update RELEASENOTES-DEVELOPMENT.md --- RELEASENOTES-DEVELOPMENT.md | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/RELEASENOTES-DEVELOPMENT.md b/RELEASENOTES-DEVELOPMENT.md index 1b55442e..b3bd6fc8 100644 --- a/RELEASENOTES-DEVELOPMENT.md +++ b/RELEASENOTES-DEVELOPMENT.md @@ -1,3 +1,36 @@ +Version 4.3.4-dev-3-g238e4bbf +== +**Enhancement -- HDMI Ports** +* If your device has HDMI ports, the Shairport Sync help command has previously listed them with a `hw:` prefix, e.g. `hw:vc4hdmi0`. Unfortunately, if you use a name like that as an `output_device` name for a HDMI device in Shairport Sync, the device may not be usable. + + But if you use the `hdmi:` prefix, e.g. `hdmi:vc4hdmi0`, it _may_ work: something capable of receiving audio must be connected to the HDMI port and it must be powered on _when your device boots up_. + + The enhancement is to update the help text for the ALSA backend to denote HDMI devices using the `hdmi:` prefix rather than `hw:`. Now, for example, on a Raspberry Pi 4, the output is: + ``` + Settings and options for the audio backend "alsa": + -d output-device set the output device, default is "default". + -c mixer-control set the mixer control name, default is to use no mixer. + -m mixer-device set the mixer device, default is the output device. + -i mixer-index set the mixer index, default is 0. + hardware output devices: + "hdmi:vc4hdmi0" + "hdmi:vc4hdmi1" + ``` + Previously it was: + ``` + Settings and options for the audio backend "alsa": + -d output-device set the output device, default is "default". + -c mixer-control set the mixer control name, default is to use no mixer. + -m mixer-device set the mixer device, default is the output device. + -i mixer-index set the mixer index, default is 0. + hardware output devices: + "hw:vc4hdmi0" + "hw:vc4hdmi1" + ``` + ...which is technically correct but unfortunately not very useful. + + Get more information about the hardware output devices using [sps-alsa-explore](https://github.com/mikebrady/sps-alsa-explore) (also available as a Docker image). + Version 4.3.4-dev-1-gc945f3a9 == This is effectively release version 4.3.3 -- the starting point for 4.3.4. From eaf923614a52d416d494a9745a1ec1bbb199cd9e Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Mon, 3 Jun 2024 09:36:37 +0100 Subject: [PATCH 07/29] Give access to mixers in a HDMI device by attaching the mixer to the device's name but replacing the header hdmi: with hw: --- audio_alsa.c | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/audio_alsa.c b/audio_alsa.c index bcd2d6a4..1833150c 100644 --- a/audio_alsa.c +++ b/audio_alsa.c @@ -1,7 +1,7 @@ /* * libalsa output driver. This file is part of Shairport. * Copyright (c) Muffinman, Skaman 2013 - * Copyright (c) Mike Brady 2014 -- 2022 + * Copyright (c) Mike Brady 2014 -- 2024 * All rights reserved. * * Permission is hereby granted, free of charge, to any person @@ -146,6 +146,7 @@ long alsa_mix_minv, alsa_mix_maxv; long alsa_mix_mindb, alsa_mix_maxdb; char *alsa_out_dev = "default"; +char *hw_alsa_out_dev = NULL; char *alsa_mix_dev = NULL; char *alsa_mix_ctrl = NULL; int alsa_mix_index = 0; @@ -291,7 +292,12 @@ static void help(void) { debug(2, "error %d executing a script to list alsa hardware device names", r); } -void set_alsa_out_dev(char *dev) { alsa_out_dev = dev; } // ugh -- not static! +void set_alsa_out_dev(char *dev) { + alsa_out_dev = dev; + if (hw_alsa_out_dev != NULL) + free(hw_alsa_out_dev); + hw_alsa_out_dev = str_replace(alsa_out_dev, "hdmi:", "hw:"); +} // ugh -- not static! // assuming pthread cancellation is disabled // returns zero of all is okay, a Unx error code if there's a problem @@ -902,7 +908,7 @@ static int prepare_mixer() { pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &oldState); // make this un-cancellable if (alsa_mix_dev == NULL) - alsa_mix_dev = alsa_out_dev; + alsa_mix_dev = hw_alsa_out_dev; // Now, start trying to initialise the alsa device with the settings // obtained @@ -1355,11 +1361,19 @@ static int init(int argc, char **argv) { } debug(1, "alsa: output device name is \"%s\".", alsa_out_dev); - + + + // now, we need a version of the alsa_out_dev that substitutes "hw:" for "hdmi" if it's + // there. It seems hw:1 would be a valid devcie name where hdmi:1 would not + + if (alsa_out_dev != NULL) + hw_alsa_out_dev = str_replace(alsa_out_dev, "hdmi:", "hw:"); + // so, now, if the option to keep the DAC running has been selected, start a // thread to monitor the // length of the queue // if the queue gets too short, stuff it with silence + pthread_create(&alsa_buffer_monitor_thread, NULL, &alsa_buffer_monitor_thread_code, NULL); @@ -1376,6 +1390,8 @@ static void deinit(void) { debug(3, "Join buffer monitor thread."); pthread_join(alsa_buffer_monitor_thread, NULL); pthread_setcancelstate(oldState, NULL); + if (hw_alsa_out_dev != NULL) + free(hw_alsa_out_dev); } static int set_mute_state() { From 2456dcc5916d67bbe4f3a0801c345b138a2de90c Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Mon, 3 Jun 2024 09:44:40 +0100 Subject: [PATCH 08/29] Update RELEASENOTES-DEVELOPMENT.md --- RELEASENOTES-DEVELOPMENT.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/RELEASENOTES-DEVELOPMENT.md b/RELEASENOTES-DEVELOPMENT.md index b3bd6fc8..dfbd8dbb 100644 --- a/RELEASENOTES-DEVELOPMENT.md +++ b/RELEASENOTES-DEVELOPMENT.md @@ -1,3 +1,8 @@ +Version 4.3.4-dev-6-g5f64722a +== +**Enhancement -- HDMI Ports** +* As a further enhancement for HDMI, enable access to mixers on a HDMI output device. The issue appears to be that a mixer can not be "attached" to a device whose name begins with `hdmi:`, so it must be attached to the same device, but with the `hdmi:` prefix replaced by `hw:`. + Version 4.3.4-dev-3-g238e4bbf == **Enhancement -- HDMI Ports** From 111e247b657923a04ce2374dba2fd95841072329 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Sat, 8 Jun 2024 14:05:26 +0100 Subject: [PATCH 09/29] Limit maximum open file handles in Docker images. Increase the level of optimisation while building the Shairport Sync image. --- docker/Dockerfile | 2 +- docker/classic/Dockerfile | 2 +- docker/classic/etc/s6-overlay/s6-rc.d/01-startup/script.sh | 3 +++ docker/etc/s6-overlay/s6-rc.d/01-startup/script.sh | 3 +++ 4 files changed, 8 insertions(+), 2 deletions(-) diff --git a/docker/Dockerfile b/docker/Dockerfile index c6da2efd..e47dc5d7 100644 --- a/docker/Dockerfile +++ b/docker/Dockerfile @@ -55,7 +55,7 @@ COPY . . RUN git checkout "$SHAIRPORT_SYNC_BRANCH" WORKDIR /shairport-sync/build RUN autoreconf -i ../ -RUN ../configure --sysconfdir=/etc --with-alsa --with-pa --with-soxr --with-avahi --with-ssl=openssl \ +RUN CFLAGS="-O3" CXXFLAGS="-O3" ../configure --sysconfdir=/etc --with-alsa --with-pa --with-soxr --with-avahi --with-ssl=openssl \ --with-airplay-2 --with-metadata --with-dummy --with-pipe --with-dbus-interface \ --with-stdout --with-mpris-interface --with-mqtt-client \ --with-apple-alac --with-convolution diff --git a/docker/classic/Dockerfile b/docker/classic/Dockerfile index d5978410..d41cac4f 100644 --- a/docker/classic/Dockerfile +++ b/docker/classic/Dockerfile @@ -40,7 +40,7 @@ COPY . . RUN git checkout "$SHAIRPORT_SYNC_BRANCH" WORKDIR /shairport-sync/build RUN autoreconf -i ../ -RUN ../configure --sysconfdir=/etc --with-alsa --with-pa --with-soxr --with-avahi --with-ssl=mbedtls \ +RUN CFLAGS="-O3" CXXFLAGS="-O3" ../configure --sysconfdir=/etc --with-alsa --with-pa --with-soxr --with-avahi --with-ssl=mbedtls \ --with-metadata --with-dummy --with-pipe --with-dbus-interface \ --with-stdout --with-mpris-interface --with-mqtt-client \ --with-apple-alac --with-convolution diff --git a/docker/classic/etc/s6-overlay/s6-rc.d/01-startup/script.sh b/docker/classic/etc/s6-overlay/s6-rc.d/01-startup/script.sh index 3037e98a..253c0afa 100644 --- a/docker/classic/etc/s6-overlay/s6-rc.d/01-startup/script.sh +++ b/docker/classic/etc/s6-overlay/s6-rc.d/01-startup/script.sh @@ -1,2 +1,5 @@ #!/bin/sh echo "STARTING - $(date)" + +# Set the limit to the same value Docker has been using in earlier version. +ulimit -n 1048576 diff --git a/docker/etc/s6-overlay/s6-rc.d/01-startup/script.sh b/docker/etc/s6-overlay/s6-rc.d/01-startup/script.sh index 3037e98a..253c0afa 100644 --- a/docker/etc/s6-overlay/s6-rc.d/01-startup/script.sh +++ b/docker/etc/s6-overlay/s6-rc.d/01-startup/script.sh @@ -1,2 +1,5 @@ #!/bin/sh echo "STARTING - $(date)" + +# Set the limit to the same value Docker has been using in earlier version. +ulimit -n 1048576 From 0d96857ff7ae0e03092958ef138a36ecdd236426 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Sat, 8 Jun 2024 14:35:10 +0100 Subject: [PATCH 10/29] Update RELEASENOTES-DEVELOPMENT.md --- RELEASENOTES-DEVELOPMENT.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/RELEASENOTES-DEVELOPMENT.md b/RELEASENOTES-DEVELOPMENT.md index dfbd8dbb..fccd087d 100644 --- a/RELEASENOTES-DEVELOPMENT.md +++ b/RELEASENOTES-DEVELOPMENT.md @@ -1,3 +1,9 @@ +Version 4.3.4-dev-8-g111e247b +== +**Docker Enhancement** +* Limit maximum open file handles in Docker images. Thanks to [Florian Wallner](https://github.com/powerflo) for [Issue #1756](https://github.com/mikebrady/shairport-sync/issues/1756), [iVolt1](https://github.com/iVolt1) for pointing to a fix and [rp86](https://github.com/rp86) for the reminder! +* Increase the level of optimisation while building the Shairport Sync application itself. + Version 4.3.4-dev-6-g5f64722a == **Enhancement -- HDMI Ports** From aade1b39b380499d88ca26a22ec4e36aac1f48a8 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Sun, 9 Jun 2024 18:41:02 +0100 Subject: [PATCH 11/29] Move the ulimit -n call from 01-startup/script.sh to the 02-dbus/run --- docker/classic/etc/s6-overlay/s6-rc.d/01-startup/script.sh | 3 --- docker/classic/etc/s6-overlay/s6-rc.d/02-dbus/run | 4 ++++ docker/etc/s6-overlay/s6-rc.d/01-startup/script.sh | 3 --- docker/etc/s6-overlay/s6-rc.d/02-dbus/run | 4 ++++ 4 files changed, 8 insertions(+), 6 deletions(-) diff --git a/docker/classic/etc/s6-overlay/s6-rc.d/01-startup/script.sh b/docker/classic/etc/s6-overlay/s6-rc.d/01-startup/script.sh index 253c0afa..3037e98a 100644 --- a/docker/classic/etc/s6-overlay/s6-rc.d/01-startup/script.sh +++ b/docker/classic/etc/s6-overlay/s6-rc.d/01-startup/script.sh @@ -1,5 +1,2 @@ #!/bin/sh echo "STARTING - $(date)" - -# Set the limit to the same value Docker has been using in earlier version. -ulimit -n 1048576 diff --git a/docker/classic/etc/s6-overlay/s6-rc.d/02-dbus/run b/docker/classic/etc/s6-overlay/s6-rc.d/02-dbus/run index 15018b7e..f0e4aa50 100644 --- a/docker/classic/etc/s6-overlay/s6-rc.d/02-dbus/run +++ b/docker/classic/etc/s6-overlay/s6-rc.d/02-dbus/run @@ -1,3 +1,7 @@ #!/command/with-contenv sh + +# Set the limit to the same value Docker has been using in earlier version. +ulimit -n 1048576 + echo "Starting dbus" exec s6-notifyoncheck dbus-daemon --system --nofork --nopidfile diff --git a/docker/etc/s6-overlay/s6-rc.d/01-startup/script.sh b/docker/etc/s6-overlay/s6-rc.d/01-startup/script.sh index 253c0afa..3037e98a 100644 --- a/docker/etc/s6-overlay/s6-rc.d/01-startup/script.sh +++ b/docker/etc/s6-overlay/s6-rc.d/01-startup/script.sh @@ -1,5 +1,2 @@ #!/bin/sh echo "STARTING - $(date)" - -# Set the limit to the same value Docker has been using in earlier version. -ulimit -n 1048576 diff --git a/docker/etc/s6-overlay/s6-rc.d/02-dbus/run b/docker/etc/s6-overlay/s6-rc.d/02-dbus/run index 15018b7e..f0e4aa50 100644 --- a/docker/etc/s6-overlay/s6-rc.d/02-dbus/run +++ b/docker/etc/s6-overlay/s6-rc.d/02-dbus/run @@ -1,3 +1,7 @@ #!/command/with-contenv sh + +# Set the limit to the same value Docker has been using in earlier version. +ulimit -n 1048576 + echo "Starting dbus" exec s6-notifyoncheck dbus-daemon --system --nofork --nopidfile From 976d93e96b1c78bae1069f6bedfefc1a61b72ebc Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Mon, 10 Jun 2024 09:13:23 +0100 Subject: [PATCH 12/29] Update RELEASENOTES-DEVELOPMENT.md --- RELEASENOTES-DEVELOPMENT.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/RELEASENOTES-DEVELOPMENT.md b/RELEASENOTES-DEVELOPMENT.md index fccd087d..278f8863 100644 --- a/RELEASENOTES-DEVELOPMENT.md +++ b/RELEASENOTES-DEVELOPMENT.md @@ -1,3 +1,7 @@ +Version 4.3.4-dev-10-gaade1b39 +== +**Docker Enhancement -- Continued** +* Move the `ulimit -n ...` command to set the maximum file limit from the `01-startup/script.sh` (where it didn't seem to have any effect) to the `02-dbus/run`. Thanks to [rp86](https://github.com/rp86) for the feedback. Version 4.3.4-dev-8-g111e247b == **Docker Enhancement** From 179c83189554b9246070ae4e7a5c3504950f2b5c Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Mon, 10 Jun 2024 09:13:46 +0100 Subject: [PATCH 13/29] Update RELEASENOTES-DEVELOPMENT.md --- RELEASENOTES-DEVELOPMENT.md | 1 + 1 file changed, 1 insertion(+) diff --git a/RELEASENOTES-DEVELOPMENT.md b/RELEASENOTES-DEVELOPMENT.md index 278f8863..7af7490c 100644 --- a/RELEASENOTES-DEVELOPMENT.md +++ b/RELEASENOTES-DEVELOPMENT.md @@ -2,6 +2,7 @@ Version 4.3.4-dev-10-gaade1b39 == **Docker Enhancement -- Continued** * Move the `ulimit -n ...` command to set the maximum file limit from the `01-startup/script.sh` (where it didn't seem to have any effect) to the `02-dbus/run`. Thanks to [rp86](https://github.com/rp86) for the feedback. + Version 4.3.4-dev-8-g111e247b == **Docker Enhancement** From c064b8bae5f1d2816857646382de5ca3a1ab5e15 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Sat, 15 Jun 2024 14:31:14 +0100 Subject: [PATCH 14/29] Change order of some include file to facilitiate old Mac OS X builds. Check existence of TCP_KEEPINTVL before trying to use it. Fix a race condition with initial messages to the metadata queues. The race was that the queues and mutexes were definied in a thread, so could be delayed to after when the main process was using them. --- rtsp.c | 78 +++++++++++++++++++++++++++++++++-------------------- shairport.c | 17 +++++------- 2 files changed, 56 insertions(+), 39 deletions(-) diff --git a/rtsp.c b/rtsp.c index c1c38792..f997824e 100644 --- a/rtsp.c +++ b/rtsp.c @@ -430,7 +430,8 @@ int pc_queue_add_item(pc_queue *the_queue, const void *the_stuff, int block) { } else rc = pthread_mutex_lock(&the_queue->pc_queue_lock); if (rc) - debug(1, "Error locking for pc_queue_add_item"); + debug(1, "Error %d (\"%s\") locking for pc_queue_add_item. Block is %d.", rc, strerror(rc), + block); pthread_cleanup_push(pc_queue_cleanup_handler, (void *)the_queue); // leave this out if you want this to return if the queue is already full // irrespective of the block flag. @@ -1687,7 +1688,7 @@ void handle_get_info(__attribute((unused)) rtsp_conn_info *conn, rtsp_message *r hdr); } } - + // In Stage 1, look for the DACP and Active-Remote char *ar = msg_get_header(req, "Active-Remote"); if (ar) { @@ -2101,8 +2102,8 @@ void handle_post(rtsp_conn_info *conn, rtsp_message *req, rtsp_message *resp) { if (strcmp(req->path, "/feedback") == 0) { resp->respcode = 501; } else { - debug(1, "Connection %d: Airplay 1. Unhandled POST %s Content-Length %d", conn->connection_number, - req->path, req->contentlength); + debug(1, "Connection %d: Airplay 1. Unhandled POST %s Content-Length %d", + conn->connection_number, req->path, req->contentlength); debug_log_rtsp_message(2, "POST request", req); } } @@ -3085,7 +3086,7 @@ void handle_setup_2(rtsp_conn_info *conn, rtsp_message *req, rtsp_message *resp) // iap->ifa_name); if ((iap->ifa_addr) && (iap->ifa_netmask) && (iap->ifa_flags & IFF_UP) && ((iap->ifa_flags & IFF_LOOPBACK) == 0) && - (config.interface == NULL || (strcmp(config.interface, iap->ifa_name) == 0))) { + (config.interface == NULL || (strcmp(config.interface, iap->ifa_name) == 0))) { char buf[INET6_ADDRSTRLEN + 1]; // +1 for a NUL memset(buf, 0, sizeof(buf)); if (iap->ifa_addr->sa_family == AF_INET6) { @@ -4081,13 +4082,9 @@ void metadata_pack_cleanup_function(void *arg) { void metadata_thread_cleanup_function(__attribute__((unused)) void *arg) { // debug(2, "metadata_thread_cleanup_function called"); metadata_close(); - pc_queue_delete(&metadata_queue); } void *metadata_thread_function(__attribute__((unused)) void *ignore) { - // create a pc_queue for passing information to a threaded metadata handler - pc_queue_init(&metadata_queue, (char *)&metadata_queue_items, sizeof(metadata_package), - metadata_queue_size, "pipe"); metadata_create_multicast_socket(); metadata_package pack; pthread_cleanup_push(metadata_thread_cleanup_function, NULL); @@ -4113,13 +4110,9 @@ void *metadata_thread_function(__attribute__((unused)) void *ignore) { void metadata_multicast_thread_cleanup_function(__attribute__((unused)) void *arg) { // debug(2, "metadata_multicast_thread_cleanup_function called"); metadata_delete_multicast_socket(); - pc_queue_delete(&metadata_multicast_queue); } void *metadata_multicast_thread_function(__attribute__((unused)) void *ignore) { - // create a pc_queue for passing information to a threaded metadata handler - pc_queue_init(&metadata_multicast_queue, (char *)&metadata_multicast_queue_items, - sizeof(metadata_package), metadata_multicast_queue_size, "multicast"); metadata_create_multicast_socket(); metadata_package pack; pthread_cleanup_push(metadata_multicast_thread_cleanup_function, NULL); @@ -4154,13 +4147,9 @@ void metadata_hub_close(void) {} void metadata_hub_thread_cleanup_function(__attribute__((unused)) void *arg) { // debug(2, "metadata_hub_thread_cleanup_function called"); metadata_hub_close(); - pc_queue_delete(&metadata_hub_queue); } void *metadata_hub_thread_function(__attribute__((unused)) void *ignore) { - // create a pc_queue for passing information to a threaded metadata handler - pc_queue_init(&metadata_hub_queue, (char *)&metadata_hub_queue_items, sizeof(metadata_package), - metadata_hub_queue_size, "hub"); metadata_package pack; pthread_cleanup_push(metadata_hub_thread_cleanup_function, NULL); while (1) { @@ -4188,14 +4177,10 @@ void metadata_mqtt_close(void) {} void metadata_mqtt_thread_cleanup_function(__attribute__((unused)) void *arg) { // debug(2, "metadata_mqtt_thread_cleanup_function called"); metadata_mqtt_close(); - pc_queue_delete(&metadata_mqtt_queue); // debug(2, "metadata_mqtt_thread_cleanup_function done"); } void *metadata_mqtt_thread_function(__attribute__((unused)) void *ignore) { - // create a pc_queue for passing information to a threaded metadata handler - pc_queue_init(&metadata_mqtt_queue, (char *)&metadata_mqtt_queue_items, sizeof(metadata_package), - metadata_mqtt_queue_size, "mqtt"); metadata_package pack; pthread_cleanup_push(metadata_mqtt_thread_cleanup_function, NULL); while (1) { @@ -4242,24 +4227,39 @@ void metadata_init(void) { if ((fd == -1) && (errno != ENXIO)) { char errorstring[1024]; strerror_r(errno, (char *)errorstring, sizeof(errorstring)); - debug(1, "metadata_hub_thread_function -- error %d (\"%s\") opening pipe: \"%s\".", errno, + debug(1, "metadata_init -- error %d (\"%s\") opening pipe: \"%s\".", errno, (char *)errorstring, path); warn("can not open metadata pipe -- error %d (\"%s\") opening pipe: \"%s\".", errno, (char *)errorstring, path); } free(path); + + // initialise the metadata queues first, otherwise the might be a race condition + // create a pc_queue for the metadata pipe + pc_queue_init(&metadata_queue, (char *)&metadata_queue_items, sizeof(metadata_package), + metadata_queue_size, "pipe"); + if (pthread_create(&metadata_thread, NULL, metadata_thread_function, NULL) != 0) debug(1, "Failed to create metadata thread!"); + // create a pc_queue for the metadata_multicast_queue + pc_queue_init(&metadata_multicast_queue, (char *)&metadata_multicast_queue_items, + sizeof(metadata_package), metadata_multicast_queue_size, "multicast"); if (pthread_create(&metadata_multicast_thread, NULL, metadata_multicast_thread_function, NULL) != 0) debug(1, "Failed to create metadata multicast thread!"); } #ifdef CONFIG_METADATA_HUB + // create a pc_queue for the metadata hub + pc_queue_init(&metadata_hub_queue, (char *)&metadata_hub_queue_items, sizeof(metadata_package), + metadata_hub_queue_size, "hub"); if (pthread_create(&metadata_hub_thread, NULL, metadata_hub_thread_function, NULL) != 0) debug(1, "Failed to create metadata hub thread!"); #endif #ifdef CONFIG_MQTT + // create a pc_queue for the MQTT handler + pc_queue_init(&metadata_mqtt_queue, (char *)&metadata_mqtt_queue_items, sizeof(metadata_package), + metadata_mqtt_queue_size, "mqtt"); if (pthread_create(&metadata_mqtt_thread, NULL, metadata_mqtt_thread_function, NULL) != 0) debug(1, "Failed to create metadata mqtt thread!"); #endif @@ -4273,6 +4273,7 @@ void metadata_stop(void) { // debug(2, "metadata stop mqtt thread."); pthread_cancel(metadata_mqtt_thread); pthread_join(metadata_mqtt_thread, NULL); + pc_queue_delete(&metadata_mqtt_queue); // debug(2, "metadata stop mqtt done."); #endif #ifdef CONFIG_METADATA_HUB @@ -4280,6 +4281,7 @@ void metadata_stop(void) { pthread_cancel(metadata_hub_thread); pthread_join(metadata_hub_thread, NULL); // debug(2, "metadata stop hub done."); + pc_queue_delete(&metadata_hub_queue); #endif if (config.metadata_enabled) { // debug(2, "metadata stop multicast thread."); @@ -4287,12 +4289,14 @@ void metadata_stop(void) { pthread_cancel(metadata_multicast_thread); pthread_join(metadata_multicast_thread, NULL); // debug(2, "metadata stop multicast done."); + pc_queue_delete(&metadata_multicast_queue); } if (metadata_thread) { // debug(2, "metadata stop metadata_thread thread."); pthread_cancel(metadata_thread); pthread_join(metadata_thread, NULL); // debug(2, "metadata_stop finished successfully."); + pc_queue_delete(&metadata_queue); } } } @@ -4343,6 +4347,8 @@ int send_metadata_to_queue(pc_queue *queue, uint32_t type, uint32_t code, char * if (data) pack.data = memdup(data, length); // only if it's not a null } + + // debug(1, "send_metadata_to_queue %x/%x", type, code); int rc = pc_queue_add_item(queue, &pack, block); if (rc != 0) { if (pack.carrier) { @@ -5642,22 +5648,33 @@ void *rtsp_listen_loop(__attribute((unused)) void *arg) { } else { size_of_reply = sizeof(SOCKADDR); if (getsockname(conn->fd, (struct sockaddr *)&conn->local, &size_of_reply) == 0) { + +// skip this stuff in OpenBSD #ifndef COMPILE_FOR_OPENBSD // 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 + // if TCP_KEEPINTVL is defined, check a few times before declaring the line dead + // otherwise just wait a little while longer + +#ifdef TCP_KEEPINTVL int keepAliveIdleTime = 35; // 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 +#else + int keepAliveIdleTime = 60; // wait this many seconds before dropping a client +#endif +// --- the following is a bit too complicated +// decide to use IPPROTO_TCP or SOL_TCP #if defined COMPILE_FOR_BSD || defined COMPILE_FOR_OSX #define SOL_OPTION IPPROTO_TCP #else #define SOL_OPTION SOL_TCP #endif - +// decide to use TCP_KEEPALIVE or TCP_KEEPIDLE #ifdef COMPILE_FOR_OSX #define KEEP_ALIVE_OR_IDLE_OPTION TCP_KEEPALIVE #else @@ -5666,17 +5683,20 @@ void *rtsp_listen_loop(__attribute((unused)) void *arg) { if (setsockopt(conn->fd, SOL_OPTION, KEEP_ALIVE_OR_IDLE_OPTION, (void *)&keepAliveIdleTime, sizeof(keepAliveIdleTime))) { - debug(1, "can't set the keepidle wait time"); + debug(1, "can't set the keepAliveIdleTime wait time"); } - +// --- +// if TCP_KEEPINTVL is defined... +#ifdef TCP_KEEPINTVL if (setsockopt(conn->fd, SOL_OPTION, TCP_KEEPCNT, (void *)&keepAliveCount, sizeof(keepAliveCount))) { - debug(1, "can't set the keepidle missing count"); + debug(1, "can't set the keepAliveCount count"); } if (setsockopt(conn->fd, SOL_OPTION, TCP_KEEPINTVL, (void *)&keepAliveInterval, sizeof(keepAliveInterval))) { - debug(1, "can't set the keepidle missing count interval"); + debug(1, "can't set the keepAliveCount count interval"); }; +#endif #endif // initialise the connection info @@ -5736,8 +5756,8 @@ void *rtsp_listen_loop(__attribute((unused)) void *arg) { pthread_cleanup_pop(1); // should never happen } else { die("could not establish a service on port %d -- program terminating. Is another instance of " - "Shairport Sync running on this device?", - config.port); + "Shairport Sync running on this device?", + config.port); } debug(1, "Oops -- fell out of the RTSP select loop"); pthread_exit(NULL); diff --git a/shairport.c b/shairport.c index cb1e6cb3..79d5e78f 100644 --- a/shairport.c +++ b/shairport.c @@ -1207,8 +1207,8 @@ int parse_options(int argc, char **argv) { } else { if (config_error_type(&config_file_stuff) == CONFIG_ERR_FILE_IO) - die("Error reading configuration file \"%s\": \"%s\".", - config_file_real_path, config_error_text(&config_file_stuff)); + die("Error reading configuration file \"%s\": \"%s\".", config_file_real_path, + config_error_text(&config_file_stuff)); else { die("Line %d of the configuration file \"%s\":\n%s", config_error_line(&config_file_stuff), config_error_file(&config_file_stuff), config_error_text(&config_file_stuff)); @@ -2110,12 +2110,9 @@ int main(int argc, char **argv) { #ifdef COMPILE_FOR_OPENBSD /* Any command to be executed at runtime? */ - int run_cmds = - config.cmd_active_start != NULL || - config.cmd_active_stop != NULL || - config.cmd_set_volume != NULL || - config.cmd_start != NULL || - config.cmd_stop != NULL; + int run_cmds = config.cmd_active_start != NULL || config.cmd_active_stop != NULL || + config.cmd_set_volume != NULL || config.cmd_start != NULL || + config.cmd_stop != NULL; #endif // mDNS supports maximum of 63-character names (we append 13). @@ -2381,11 +2378,11 @@ int main(int argc, char **argv) { #ifdef COMPILE_FOR_OPENBSD /* Past first and last sio_open(3), sndio(7) only needs "audio". */ -# ifdef CONFIG_METADATA +#ifdef CONFIG_METADATA /* Only coverart cache is created. * Only metadata pipe is special. */ if (!config.metadata_enabled) -# endif +#endif { /* Drop "cpath dpath". */ if (run_cmds) { From b77092f2a4acf4b26cd5f74726340d471951a291 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Sat, 15 Jun 2024 14:50:01 +0100 Subject: [PATCH 15/29] Update RELEASENOTES-DEVELOPMENT.md --- RELEASENOTES-DEVELOPMENT.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/RELEASENOTES-DEVELOPMENT.md b/RELEASENOTES-DEVELOPMENT.md index 7af7490c..1e749914 100644 --- a/RELEASENOTES-DEVELOPMENT.md +++ b/RELEASENOTES-DEVELOPMENT.md @@ -1,3 +1,10 @@ +Version 4.3.4-dev-13-gc064b8ba +== +**Bug Fixes** +* Reorder some of the files to be included in `shairport.c` to fix compilation errors in old versions of Mac OS X. +* Avoid using `TCP_KEEPINTVL` and `TCP_KEEPCNT` if they are not defined (they are not defined in older versions of Mac OS X). Thanks to [Sergey Federov](https://github.com/barracuda156) for raising these [issues](https://github.com/mikebrady/shairport-sync/issues/1860) and pointing at a potential solution. +* Fix a race condition with the metadata queues. The problem was that the queues were being initialised by threads launched by the main thread which, having started the threads, proceeded to use the queues. But if the threads were late in starting, the queues might not be initialised by the time the main thread tried to use them. Fixed by initialising the queues in the main thread. + Version 4.3.4-dev-10-gaade1b39 == **Docker Enhancement -- Continued** From 0412a84006773098c558ed3f8d4b57b8540244ce Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Sat, 15 Jun 2024 16:18:29 +0100 Subject: [PATCH 16/29] BB -- change the order of includes as mentioned in version 4.3.4-dev-13-gc064b8ba --- shairport.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/shairport.c b/shairport.c index 79d5e78f..7a184ce6 100644 --- a/shairport.c +++ b/shairport.c @@ -25,6 +25,8 @@ * OTHER DEALINGS IN THE SOFTWARE. */ +#include +#include #include #include #include @@ -35,9 +37,7 @@ #include #include #include -#include #include -#include #include #include From 8e23ad3f899ec2f12fa22f2d1ca0c4fd73edb788 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Sat, 29 Jun 2024 19:59:28 +0100 Subject: [PATCH 17/29] prevent cancellation while performing ao-related operations. --- audio_ao.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/audio_ao.c b/audio_ao.c index 61746487..b53eedc7 100644 --- a/audio_ao.c +++ b/audio_ao.c @@ -69,6 +69,8 @@ static void help(void) { } static int init(int argc, char **argv) { + int oldState; + pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &oldState); // make this un-cancellable ao_initialize(); driver = ao_default_driver_id(); if (driver == -1) { @@ -133,14 +135,18 @@ static int init(int argc, char **argv) { fmt.byte_format = AO_FMT_NATIVE; fmt.matrix = strdup("L,R"); } + pthread_setcancelstate(oldState, NULL); return 0; } static void deinit(void) { + int oldState; + pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &oldState); // make this un-cancellable if (dev != NULL) ao_close(dev); dev = NULL; ao_shutdown(); + pthread_setcancelstate(oldState, NULL); } static void start(__attribute__((unused)) int sample_rate, @@ -166,10 +172,13 @@ static int play(void *buf, int samples, __attribute__((unused)) int sample_type, static void stop(void) { // debug(1,"libao stop"); + int oldState; + pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &oldState); // make this un-cancellable if (dev != NULL) { ao_close(dev); dev = NULL; } + pthread_setcancelstate(oldState, NULL); } audio_output audio_ao = {.name = "ao", From 6be34b7f2b77e14fe36014a45cd32c4d05d2caec Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Sat, 29 Jun 2024 20:01:39 +0100 Subject: [PATCH 18/29] set the session timeout to one minute by default. --- scripts/shairport-sync.conf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/shairport-sync.conf b/scripts/shairport-sync.conf index 78a79c7e..8a2edb43 100644 --- a/scripts/shairport-sync.conf +++ b/scripts/shairport-sync.conf @@ -110,7 +110,7 @@ sessioncontrol = // wait_for_completion = "no"; // set to "yes" to get Shairport Sync to wait until the "run_this..." applications have terminated before continuing // allow_session_interruption = "no"; // set to "yes" to allow another device to interrupt Shairport Sync while it's playing from an existing audio source -// session_timeout = 120; // wait for this number of seconds after a source disappears before terminating the session and becoming available again. +// session_timeout = 60; // wait for this number of seconds after a source disappears before terminating the session and becoming available again. }; // Back End Settings From fe8c6dd2437e7849db45a1a52ebe4a736dfca131 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Sat, 29 Jun 2024 20:02:09 +0100 Subject: [PATCH 19/29] set the session timeout to one minute by default. --- shairport.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/shairport.c b/shairport.c index 7a184ce6..29f7f468 100644 --- a/shairport.c +++ b/shairport.c @@ -2039,8 +2039,8 @@ int main(int argc, char **argv) { config.debugger_show_file_and_line = 1; // by default, log the file and line of the originating message config.debugger_show_relative_time = - 1; // by default, log the time back to the previous debug message - config.timeout = 120; // this number of seconds to wait for [more] audio before switching to idle. + 1; // by default, log the time back to the previous debug message + config.timeout = 60; // wait this number of seconds to wait for a dropped RTSP connection to come back before declaring it lost. config.buffer_start_fill = 220; config.resync_threshold = 0.050; // default From 419ff394f5ace0f28752b1fc3219aafe098c6aab Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Sat, 29 Jun 2024 20:04:29 +0100 Subject: [PATCH 20/29] Remove watchdog stuff. USe the session timeout to set the TCP keep intervals and stuff. Change response to ETIMEDOUT to rtsp_read_request_response_immediate_shutdown_requested. --- rtsp.c | 128 ++++++++++++++++----------------------------------------- 1 file changed, 36 insertions(+), 92 deletions(-) diff --git a/rtsp.c b/rtsp.c index f997824e..1712e480 100644 --- a/rtsp.c +++ b/rtsp.c @@ -3,7 +3,7 @@ * Copyright (c) James Laird 2013 * Modifications associated with audio synchronization, multithreading and - * metadata handling copyright (c) Mike Brady 2014-2023 + * metadata handling copyright (c) Mike Brady 2014-2024 * All rights reserved. * * Permission is hereby granted, free of charge, to any person @@ -612,57 +612,6 @@ int get_play_lock(rtsp_conn_info *conn, int allow_session_interruption) { return response; } -void player_watchdog_thread_cleanup_handler(void *arg) { - rtsp_conn_info *conn = (rtsp_conn_info *)arg; - debug(3, "Connection %d: Watchdog Exit.", conn->connection_number); -} - -void *player_watchdog_thread_code(void *arg) { - pthread_cleanup_push(player_watchdog_thread_cleanup_handler, arg); - rtsp_conn_info *conn = (rtsp_conn_info *)arg; - do { - usleep(2000000); // check every two seconds - // debug(3, "Connection %d: Check the thread is doing something...", conn->connection_number); -#ifdef CONFIG_AIRPLAY_2 - if ((config.dont_check_timeout == 0) && (config.timeout != 0) && (conn->airplay_type == ap_1)) { -#else - if ((config.dont_check_timeout == 0) && (config.timeout != 0)) { -#endif - debug_mutex_lock(&conn->watchdog_mutex, 1000, 0); - uint64_t last_watchdog_bark_time = conn->watchdog_bark_time; - debug_mutex_unlock(&conn->watchdog_mutex, 0); - if (last_watchdog_bark_time != 0) { - uint64_t time_since_last_bark = - (get_absolute_time_in_ns() - last_watchdog_bark_time) / 1000000000; - uint64_t ct = config.timeout; // go from int to 64-bit int - - if (time_since_last_bark >= ct) { - conn->watchdog_barks++; - if (conn->watchdog_barks == 1) { - // debuglev = 3; // tell us everything. - debug(1, - "Connection %d: As Yeats almost said, \"Too long a silence / can make a stone " - "of the heart\".", - conn->connection_number); - conn->stop = 1; - pthread_cancel(conn->thread); - } else if (conn->watchdog_barks == 3) { - if ((config.cmd_unfixable) && (config.unfixable_error_reported == 0)) { - config.unfixable_error_reported = 1; - command_execute(config.cmd_unfixable, "unable_to_cancel_play_session", 1); - } else { - die("an unrecoverable error, \"unable_to_cancel_play_session\", has been detected.", - conn->connection_number); - } - } - } - } - } - } while (1); - pthread_cleanup_pop(0); // should never happen - pthread_exit(NULL); -} - static void track_thread(rtsp_conn_info *conn) { debug_mutex_lock(&conns_lock, 1000000, 3); // look for an empty slot first @@ -1349,8 +1298,11 @@ enum rtsp_read_request_response rtsp_read_request(rtsp_conn_info *conn, rtsp_mes continue; } if (errno == ETIMEDOUT) { - debug(1, "Connection %d: ETIMEDOUT -- keepalive timeout.", conn->connection_number); - reply = rtsp_read_request_response_channel_closed; + debug(1, + "Connection %d: As Yeats almost said, \"Too long a silence / can make a stone " + "of the heart\".", + conn->connection_number); + reply = rtsp_read_request_response_immediate_shutdown_requested; // Note: the socket will be closed when the thread exits goto shutdown; } @@ -2116,7 +2068,7 @@ struct pairings { uint8_t public_key[32]; struct pairings *next; -} * pairings; +} *pairings; static struct pairings *pairing_find(const char *device_id) { for (struct pairings *pairing = pairings; pairing; pairing = pairing->next) { @@ -5113,7 +5065,7 @@ void rtsp_conversation_thread_cleanup_function(void *arg) { if (conn != NULL) { int oldState; pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &oldState); - debug(2, "Connection %d: %s rtsp_conversation_thread_func_cleanup_function called.", + debug(3, "Connection %d: %s rtsp_conversation_thread_func_cleanup_function called.", conn->connection_number, get_category_string(conn->airplay_stream_category)); #ifdef CONFIG_AIRPLAY_2 // AP2 @@ -5205,14 +5157,7 @@ void rtsp_conversation_thread_cleanup_function(void *arg) { if (rc) debug(1, "Connection %d: error %d destroying flush_mutex.", conn->connection_number, rc); - debug(3, "Cancel watchdog thread."); - pthread_cancel(conn->player_watchdog_thread); - debug(3, "Join watchdog thread."); - pthread_join(conn->player_watchdog_thread, NULL); - debug(3, "Delete watchdog mutex."); - pthread_mutex_destroy(&conn->watchdog_mutex); - - debug(2, "Connection %d: Closed.", conn->connection_number); + debug(3, "Connection %d: Closed.", conn->connection_number); conn->running = 0; // for the garbage collector pthread_setcancelstate(oldState, NULL); } @@ -5226,11 +5171,6 @@ void msg_cleanup_function(void *arg) { static void *rtsp_conversation_thread_func(void *pconn) { rtsp_conn_info *conn = pconn; - // create the watchdog mutex, initialise the watchdog time and start the watchdog thread; - conn->watchdog_bark_time = get_absolute_time_in_ns(); - pthread_mutex_init(&conn->watchdog_mutex, NULL); - pthread_create(&conn->player_watchdog_thread, NULL, &player_watchdog_thread_code, (void *)conn); - int rc = pthread_mutex_init(&conn->flush_mutex, NULL); if (rc) die("Connection %d: error %d initialising flush_mutex.", conn->connection_number, rc); @@ -5649,22 +5589,26 @@ void *rtsp_listen_loop(__attribute((unused)) void *arg) { size_of_reply = sizeof(SOCKADDR); if (getsockname(conn->fd, (struct sockaddr *)&conn->local, &size_of_reply) == 0) { + if ((config.dont_check_timeout == 0) && (config.timeout != 0)) { // skip this stuff in OpenBSD #ifndef COMPILE_FOR_OPENBSD - // Thanks to https://holmeshe.me/network-essentials-setsockopt-SO_KEEPALIVE/ for this. + // 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 - // if TCP_KEEPINTVL is defined, check a few times before declaring the line dead - // otherwise just wait a little while longer + // if TCP_KEEPINTVL is defined, check a few times before declaring the line dead + // otherwise just wait a little while longer #ifdef TCP_KEEPINTVL - int keepAliveIdleTime = 35; // 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 keepAliveIdleTime = + config.timeout - + 5 * 5; // 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 #else - int keepAliveIdleTime = 60; // wait this many seconds before dropping a client + int keepAliveIdleTime = + config.timeout; // wait this many seconds before dropping a client #endif // --- the following is a bit too complicated @@ -5681,24 +5625,24 @@ void *rtsp_listen_loop(__attribute((unused)) void *arg) { #define KEEP_ALIVE_OR_IDLE_OPTION TCP_KEEPIDLE #endif - if (setsockopt(conn->fd, SOL_OPTION, KEEP_ALIVE_OR_IDLE_OPTION, - (void *)&keepAliveIdleTime, sizeof(keepAliveIdleTime))) { - debug(1, "can't set the keepAliveIdleTime wait time"); - } + if (setsockopt(conn->fd, SOL_OPTION, KEEP_ALIVE_OR_IDLE_OPTION, + (void *)&keepAliveIdleTime, sizeof(keepAliveIdleTime))) { + debug(1, "can't set the keepAliveIdleTime wait time"); + } // --- // if TCP_KEEPINTVL is defined... #ifdef TCP_KEEPINTVL - if (setsockopt(conn->fd, SOL_OPTION, TCP_KEEPCNT, (void *)&keepAliveCount, - sizeof(keepAliveCount))) { - debug(1, "can't set the keepAliveCount count"); + if (setsockopt(conn->fd, SOL_OPTION, TCP_KEEPCNT, (void *)&keepAliveCount, + sizeof(keepAliveCount))) { + debug(1, "can't set the keepAliveCount count"); + } + if (setsockopt(conn->fd, SOL_OPTION, TCP_KEEPINTVL, (void *)&keepAliveInterval, + sizeof(keepAliveInterval))) { + debug(1, "can't set the keepAliveCount count interval"); + }; +#endif +#endif } - if (setsockopt(conn->fd, SOL_OPTION, TCP_KEEPINTVL, (void *)&keepAliveInterval, - sizeof(keepAliveInterval))) { - debug(1, "can't set the keepAliveCount count interval"); - }; -#endif -#endif - // initialise the connection info void *client_addr = NULL, *self_addr = NULL; conn->connection_ip_family = conn->local.SAFAMILY; From bccf0775d37c3c5e3081986fb9f7d19d8a0bf7ad Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Sat, 29 Jun 2024 20:05:15 +0100 Subject: [PATCH 21/29] remove watchdog stuff --- player.h | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/player.h b/player.h index b35eb765..aaa5b1f4 100644 --- a/player.h +++ b/player.h @@ -167,14 +167,11 @@ typedef struct { SOCKADDR remote, local; volatile int stop; volatile int running; - volatile uint64_t watchdog_bark_time; - volatile int watchdog_barks; // number of times the watchdog has timed out and done something uint64_t playstart; uint64_t connection_start_time; // the time the device is selected, which could be a long time // before a play - pthread_t thread, timer_requester, rtp_audio_thread, rtp_control_thread, rtp_timing_thread, - player_watchdog_thread; + pthread_t thread, timer_requester, rtp_audio_thread, rtp_control_thread, rtp_timing_thread; // buffers to delete on exit int32_t *tbuf; @@ -380,7 +377,6 @@ typedef struct { // request in flight at the same time pthread_mutex_t reference_time_mutex; - pthread_mutex_t watchdog_mutex; double local_to_remote_time_gradient; // if no drift, this would be exactly 1.0; likely it's // slightly above or below. From 105299a71113652cb19d593b583f88bb232ef8ac Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Sat, 29 Jun 2024 20:06:27 +0100 Subject: [PATCH 22/29] Remove watchdog stuff. Move the pthreadtestcancel into the loop inside buffergetframe() -- somethime the player can stay inside that loop indefinitely. --- player.c | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/player.c b/player.c index e3122e57..46ef100f 100644 --- a/player.c +++ b/player.c @@ -921,7 +921,7 @@ static abuf_t *buffer_get_frame(rtsp_conn_info *conn) { pthread_cleanup_push(buffer_get_frame_cleanup_handler, (void *)conn); // undo what's been done so far do { - + pthread_testcancel(); // even if no packets are coming in... // get the time local_time_now = get_absolute_time_in_ns(); // type okay // debug(3, "buffer_get_frame is iterating"); @@ -2269,9 +2269,7 @@ void *player_thread_func(void *arg) { if (conn->input_bytes_per_frame == 0) debug(1, "conn->input_bytes_per_frame is zero!"); - pthread_testcancel(); // allow a pthread_cancel request to take effect. - abuf_t *inframe = buffer_get_frame(conn); // this has cancellation point(s), but it's not - // guaranteed that they'll always be executed + abuf_t *inframe = buffer_get_frame(conn); // this has a (needed!) deliberate cancellation point in it. uint64_t local_time_now = get_absolute_time_in_ns(); // types okay config.last_access_to_volume_info_time = local_time_now; // ensure volume info remains seen as valid @@ -3233,14 +3231,6 @@ void *player_thread_func(void *arg) { frames_seen_in_this_logging_interval = 0; } - // update the watchdog - if ((config.dont_check_timeout == 0) && (config.timeout != 0)) { - uint64_t time_now = get_absolute_time_in_ns(); - debug_mutex_lock(&conn->watchdog_mutex, 1000, 0); - conn->watchdog_bark_time = time_now; - debug_mutex_unlock(&conn->watchdog_mutex, 0); - } - // debug(1,"Sync error %lld frames. Amount to stuff %d." ,sync_error,amount_to_stuff); // new stats calculation. We want a running average of sync error, drift, adjustment, From 6c64b52faabae47cf3bf9d4efcca8353526444ff Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Sat, 29 Jun 2024 20:24:35 +0100 Subject: [PATCH 23/29] Update RELEASENOTES-DEVELOPMENT.md --- RELEASENOTES-DEVELOPMENT.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/RELEASENOTES-DEVELOPMENT.md b/RELEASENOTES-DEVELOPMENT.md index 1e749914..c882476b 100644 --- a/RELEASENOTES-DEVELOPMENT.md +++ b/RELEASENOTES-DEVELOPMENT.md @@ -1,3 +1,15 @@ +Version 4.3.4-dev-21-g105299a7 +== +**Bug Fix** +* Fixed a bug that prevented Shairport Sync from recovering cleanly if a player disconnected without warning. The problem was that the player thread would not respond to cancel request. Fixed by moving a `pthreadtestcancel()` call to the innermost loop. + + Associated with this, some obsolete watchdog code has been removed. + + Also, the `session_timeout` default has been changed from 120 seconds to 60 seconds. + + Thanks to [Tobias Huttinger](https://github.com/tobiashuttinger) for the bug report -- Issue [#1870](https://github.com/mikebrady/shairport-sync/issues/1870). + + Version 4.3.4-dev-13-gc064b8ba == **Bug Fixes** From e09b9c6330c2d173c7e73ccb7211f104451a5237 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Sat, 29 Jun 2024 20:38:43 +0100 Subject: [PATCH 24/29] Don't automatically disable the session timeout on AirPlay 2. --- shairport.c | 1 - 1 file changed, 1 deletion(-) diff --git a/shairport.c b/shairport.c index 29f7f468..e36d2cc3 100644 --- a/shairport.c +++ b/shairport.c @@ -2048,7 +2048,6 @@ int main(int argc, char **argv) { config.tolerance = 0.002; #ifdef CONFIG_AIRPLAY_2 - config.timeout = 0; // disable watchdog config.port = 7000; #else config.port = 5000; From 0099831cb7cdd48745fd71a5531e8136ca5a4ef7 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Sun, 30 Jun 2024 09:53:41 +0100 Subject: [PATCH 25/29] Revert the session timeout from one munite back to two minutes (120 seconds) --- scripts/shairport-sync.conf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/shairport-sync.conf b/scripts/shairport-sync.conf index 8a2edb43..78a79c7e 100644 --- a/scripts/shairport-sync.conf +++ b/scripts/shairport-sync.conf @@ -110,7 +110,7 @@ sessioncontrol = // wait_for_completion = "no"; // set to "yes" to get Shairport Sync to wait until the "run_this..." applications have terminated before continuing // allow_session_interruption = "no"; // set to "yes" to allow another device to interrupt Shairport Sync while it's playing from an existing audio source -// session_timeout = 60; // wait for this number of seconds after a source disappears before terminating the session and becoming available again. +// session_timeout = 120; // wait for this number of seconds after a source disappears before terminating the session and becoming available again. }; // Back End Settings From 15a7090a5fa3683d254d324d9dd823b74949cb0c Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Sun, 30 Jun 2024 09:56:29 +0100 Subject: [PATCH 26/29] Fix a bug that blocked a fatal error exit if the player thread couldn't be terminated. Revert the session timeout from one minute back to two minutes (120 seconds). --- shairport.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/shairport.c b/shairport.c index e36d2cc3..1060e0b7 100644 --- a/shairport.c +++ b/shairport.c @@ -1570,12 +1570,16 @@ const char *pid_file_proc(void) { #endif void exit_rtsp_listener() { - pthread_cancel(rtsp_listener_thread); - pthread_join(rtsp_listener_thread, NULL); // not sure you need this + debug(3, "exit_rtsp_listener begins"); + if (type_of_exit_cleanup != TOE_emergency) { + pthread_cancel(rtsp_listener_thread); + pthread_join(rtsp_listener_thread, NULL); // not sure you need this + } + debug(3, "exit_rtsp_listener ends"); } void exit_function() { - + debug(3, "exit_function begins"); if (type_of_exit_cleanup != TOE_emergency) { // the following is to ensure that if libdaemon has been included // that most of this code will be skipped when the parent process is exiting @@ -2040,7 +2044,7 @@ int main(int argc, char **argv) { 1; // by default, log the file and line of the originating message config.debugger_show_relative_time = 1; // by default, log the time back to the previous debug message - config.timeout = 60; // wait this number of seconds to wait for a dropped RTSP connection to come back before declaring it lost. + config.timeout = 120; // wait this number of seconds to wait for a dropped RTSP connection to come back before declaring it lost. config.buffer_start_fill = 220; config.resync_threshold = 0.050; // default From 55de53321bf22d62035613f391439620727092bb Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Sun, 30 Jun 2024 10:08:01 +0100 Subject: [PATCH 27/29] Update RELEASENOTES-DEVELOPMENT.md --- RELEASENOTES-DEVELOPMENT.md | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/RELEASENOTES-DEVELOPMENT.md b/RELEASENOTES-DEVELOPMENT.md index c882476b..607abf9f 100644 --- a/RELEASENOTES-DEVELOPMENT.md +++ b/RELEASENOTES-DEVELOPMENT.md @@ -1,3 +1,12 @@ +Version 4.3.4-dev-25-g15a7090a +== +**Change** +* Revert the `session_timeout` to two minutes (120 seconds). In other words, if a player becomes disconnected for more than two minutes while playing, the session is terminated. + +**Bug Fixes** +* Enable the disconnected session timeout for AirPlay 2 operation -- it had been disabled by default. +* Fix a bug that prevented a fatal error that occured while directly accessing the alsa output device from terminating the application cleanly. + Version 4.3.4-dev-21-g105299a7 == **Bug Fix** From e6da1a1db3663ab3cd47a49bf1d2cbf328334ce8 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Wed, 3 Jul 2024 20:13:30 +0100 Subject: [PATCH 28/29] Update CAR INSTALL.md --- CAR INSTALL.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CAR INSTALL.md b/CAR INSTALL.md index 29f6fd6f..cfbc1558 100644 --- a/CAR INSTALL.md +++ b/CAR INSTALL.md @@ -192,8 +192,9 @@ if test $MODE = RUN ; then else # If script execution gets in here, it starts services needed for normal operation. - /bin/systemctl start systemd-timesyncd || : /bin/systemctl start dhcpcd || /bin/systemctl start NetworkManager || : + /bin/sleep 2 # may be necessary while the network becomes available + /bin/systemctl start systemd-timesyncd || : fi From 8941caec783688c2f1970723c53360b44b749f87 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Wed, 3 Jul 2024 22:35:07 +0100 Subject: [PATCH 29/29] Update CAR INSTALL.md --- CAR INSTALL.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/CAR INSTALL.md b/CAR INSTALL.md index cfbc1558..12366e0c 100644 --- a/CAR INSTALL.md +++ b/CAR INSTALL.md @@ -201,7 +201,7 @@ fi exit 0 # normal exit here ``` -#### Disable Unused Services (Optional) +#### Disable Unused Services - Optional These optional steps have been tested on a Raspberry Pi only -- they have not been tested on other systems. Some services are not necessary for this setup and can be disabled as follows: ``` @@ -211,7 +211,7 @@ Some services are not necessary for this setup and can be disabled as follows: ``` -#### Disable Unused Services (Mandatory) +#### Disable Unused Services - Mandatory You now need to disable some services; that is, you need to stop them starting automatically on power-up. This is because they either interfere with the system's operation in WiFi Access Point mode, or because they won't work when the system isn't connected to the Internet. Only one of the `NetworkManager` and the `dhcpcd` service will be present in your system, but it's no harm to try to disable both. ``` # systemctl disable dhcpcd @@ -247,7 +247,7 @@ However, if you're *upgrading* the operating system to e.g. from Bullseye to Boo If it's a Raspberry Pi and you have optionally enabled the read-only mode, you must take the device out of Read-only mode: Run `sudo raspi-config` and then choose `Performance Options` > `Overlay Filesystem` and choose to disable the overlay filesystem and to set the boot partition not to be write-protected. This is so that changes can be written to the file system; you can make the filesystem read-only again later. Save the changes and reboot the system. #### Undo Optimisations -If you have disabled any of the services listed in the [Disable Unused Services](#disable-unused-services) section, you should re-enable them. (But *do not* re-eneable `NetworkManager`, `dhcpcd`, `wpa_supplicant` or `systemd-timesyncd` -- they are handled specially by the startup script.) +If you have disabled any of the services listed in the [Disable Unused Services - Optional](#disable-unused-services---optional) section, you should re-enable them. (But *do not* re-eneable `NetworkManager`, `dhcpcd`, `wpa_supplicant` or `systemd-timesyncd` -- they are handled specially by the startup script.) #### Perform Legacy Updates Over time, the arrangements by which the system is prepared for operation has changed to make it easier to revert to normal operation when necessary for maintenance, updates, etc. A small number of the old settings need to be changed to bring them up to date with the present arrangements. Once the required changes have been made, your system will be ready for the update process detailed below. Here are those legacy changes you need to make, just once: