From 53d922e4d4651815fcbd51fe530d2b61466989c5 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Sun, 17 Sep 2023 15:35:02 +0100 Subject: [PATCH 01/61] Update RELEASENOTES-DEVELOPMENT.md --- RELEASENOTES-DEVELOPMENT.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/RELEASENOTES-DEVELOPMENT.md b/RELEASENOTES-DEVELOPMENT.md index d4a3b205a..7422509df 100644 --- a/RELEASENOTES-DEVELOPMENT.md +++ b/RELEASENOTES-DEVELOPMENT.md @@ -1,3 +1,8 @@ +Version 4.3.1d-2-ge6a2a429 +=== +**Bug Fix** +* Fix a recently-introduced bug that prevented Shairport Sync being added to Home. + Version 4.2.1d0-67-gef14cace === **IMPORTANT Bug Fixes** From 465c5a3ee2d7734c25ceefa405a7f621f561265d Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Mon, 18 Sep 2023 19:51:36 +0100 Subject: [PATCH 02/61] Tidy up some code and add diagnostics to tackle issue #1723. --- common.c | 2 +- rtsp.c | 31 +++++++++++++++++++++++++------ 2 files changed, 26 insertions(+), 7 deletions(-) diff --git a/common.c b/common.c index 618293ae3..d16e3b1f1 100644 --- a/common.c +++ b/common.c @@ -1694,7 +1694,7 @@ int _debug_mutex_unlock(pthread_mutex_t *mutex, const char *mutexname, const cha } void malloc_cleanup(void *arg) { - // debug(1, "malloc cleanup called."); + // debug(1, "malloc cleanup freeing %" PRIxPTR ".", arg); free(arg); } diff --git a/rtsp.c b/rtsp.c index 732bb93e2..e131ac0b1 100644 --- a/rtsp.c +++ b/rtsp.c @@ -542,6 +542,7 @@ void cancel_all_RTSP_threads(airplay_stream_c stream_category, int except_this_o (stream_category == conns[i]->airplay_stream_category))) { pthread_join(conns[i]->thread, NULL); debug(1, "Connection %d: joined.", conns[i]->connection_number); + free(conns[i]); conns[i] = NULL; } @@ -603,7 +604,7 @@ int get_play_lock(rtsp_conn_info *conn, int allow_session_interruption) { response = -1; // can't get it... } if (principal_conn != NULL) - debug(3, "Connection %d has acquired principal_conn.", principal_conn->connection_number); + debug(3, "Connection %d has principal_conn.", principal_conn->connection_number); pthread_cleanup_pop(1); // release the principal_conn lock return response; } @@ -699,7 +700,7 @@ void cleanup_threads(void) { debug(2, "Found RTSP connection thread %d in a non-running state.", conns[i]->connection_number); pthread_join(conns[i]->thread, &retval); - debug(2, "Connection %d: deleted in cleanup.", conns[i]->connection_number); + debug(1, "Connection %d: deleted.", conns[i]->connection_number); free(conns[i]); conns[i] = NULL; } @@ -4482,7 +4483,7 @@ static void handle_announce(rtsp_conn_info *conn, rtsp_message *req, rtsp_messag #ifdef CONFIG_AIRPLAY_2 conn->airplay_type = ap_1; conn->timing_type = ts_ntp; - debug(2, "Connection %d: Classic AirPlay connection from %s:%u to self at %s:%u.", + debug(1, "Connection %d: Classic AirPlay connection from %s:%u to self at %s:%u.", conn->connection_number, conn->client_ip_string, conn->client_rtsp_port, conn->self_ip_string, conn->self_rtsp_port); #endif @@ -5007,7 +5008,6 @@ 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.", conn->connection_number, get_category_string(conn->airplay_stream_category)); #ifdef CONFIG_AIRPLAY_2 @@ -5162,6 +5162,19 @@ static void *rtsp_conversation_thread_func(void *pconn) { while (conn->stop == 0) { int debug_level = 2; // for printing the request and response + + +// check to see if a conn has been zeroed + + debug_mutex_lock(&conns_lock, 1000000, 3); + int i; + for (i = 0; i < nconns; i++) { + if ((conns[i] != NULL) && (conns[i]->connection_number == 0)) { + debug(1, "conns[%d] at %" PRIxPTR " has a Connection Number of 0.", conns[i]); + } + } + debug_mutex_unlock(&conns_lock, 3); + reply = rtsp_read_request(conn, &req); if (reply == rtsp_read_request_response_ok) { pthread_cleanup_push(msg_cleanup_function, (void *)&req); @@ -5508,11 +5521,16 @@ void *rtsp_listen_loop(__attribute((unused)) void *arg) { if (acceptfd < 0) // timeout continue; + int release_conn = 1; // on exit, deallocate the buffer unless everything was okay + + rtsp_conn_info *conn = malloc(sizeof(rtsp_conn_info)); if (conn == 0) die("Couldn't allocate memory for an rtsp_conn_info record."); + pthread_cleanup_push(malloc_cleanup, conn); memset(conn, 0, sizeof(rtsp_conn_info)); conn->connection_number = RTSP_connection_index++; + debug(1, "Connection %d is at: 0x%" PRIxPTR ".", conn->connection_number, conn); #ifdef CONFIG_AIRPLAY_2 conn->airplay_type = ap_2; // changed if an ANNOUNCE is received conn->timing_type = ts_ptp; // changed if an ANNOUNCE is received @@ -5524,8 +5542,7 @@ void *rtsp_listen_loop(__attribute((unused)) void *arg) { debug(1, "Connection %d: New connection on port %d not accepted:", conn->connection_number, config.port); perror("failed to accept connection"); - free(conn); - } else { + } else { size_of_reply = sizeof(SOCKADDR); if (getsockname(conn->fd, (struct sockaddr *)&conn->local, &size_of_reply) == 0) { @@ -5614,7 +5631,9 @@ void *rtsp_listen_loop(__attribute((unused)) void *arg) { debug(3, "Successfully created RTSP receiver thread %d.", conn->connection_number); conn->running = 1; // this must happen before the thread is tracked track_thread(conn); + release_conn = 0; // successfully initialised } + pthread_cleanup_pop(release_conn); // release the conn malloc if any kind of error } while (1); pthread_cleanup_pop(1); // should never happen } else { From 2fb81ca9f7823a39ca33e36318060b687efb0f91 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Tue, 19 Sep 2023 11:28:45 +0100 Subject: [PATCH 03/61] Set debug verbosity to 2 when the client closes the connection. --- rtsp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rtsp.c b/rtsp.c index e131ac0b1..56e06c934 100644 --- a/rtsp.c +++ b/rtsp.c @@ -1319,7 +1319,7 @@ enum rtsp_read_request_response rtsp_read_request(rtsp_conn_info *conn, rtsp_mes if (nread == 0) { // a blocking read that returns zero means eof -- implies connection closed by client - debug(3, "Connection %d: Connection closed by client.", conn->connection_number); + debug(2, "Connection %d: Connection closed by client.", conn->connection_number); reply = rtsp_read_request_response_channel_closed; // Note: the socket will be closed when the thread exits goto shutdown; From d602009dd2230d3c4ad8b152825bb808560a8370 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Tue, 19 Sep 2023 11:35:22 +0100 Subject: [PATCH 04/61] Update RELEASENOTES-DEVELOPMENT.md --- RELEASENOTES-DEVELOPMENT.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/RELEASENOTES-DEVELOPMENT.md b/RELEASENOTES-DEVELOPMENT.md index 7422509df..12f72493c 100644 --- a/RELEASENOTES-DEVELOPMENT.md +++ b/RELEASENOTES-DEVELOPMENT.md @@ -1,3 +1,8 @@ +Version 4.3.1-dev-3-g2fb81ca9 +=== +**Exploration** +* Add some slightly noisy diagnostics to try to locate a fault. + Version 4.3.1d-2-ge6a2a429 === **Bug Fix** From 3474941dd861caa85babafceccc3787598773eff Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Tue, 19 Sep 2023 11:40:09 +0100 Subject: [PATCH 05/61] Update check_ap2_systemd_full_build_folder.yml --- .github/workflows/check_ap2_systemd_full_build_folder.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/check_ap2_systemd_full_build_folder.yml b/.github/workflows/check_ap2_systemd_full_build_folder.yml index bdab8195f..8d8a97210 100644 --- a/.github/workflows/check_ap2_systemd_full_build_folder.yml +++ b/.github/workflows/check_ap2_systemd_full_build_folder.yml @@ -9,7 +9,7 @@ on: jobs: build: - runs-on: ubuntu-22.04 + runs-on: ubuntu-20.04 steps: - uses: actions/checkout@v3.5.2 From 2edc6a04c3c7bb62da385e7ac0f8f3b19a76f2ea Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Tue, 19 Sep 2023 12:00:43 +0100 Subject: [PATCH 06/61] bump version number --- configure.ac | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index e5da15e1a..cc047293b 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.1-dev], [4265913+mikebrady@users.noreply.github.com]) +AC_INIT([shairport-sync], [4.3.2-dev], [4265913+mikebrady@users.noreply.github.com]) AM_INIT_AUTOMAKE([subdir-objects]) AC_CONFIG_SRCDIR([shairport.c]) AC_CONFIG_HEADERS([config.h]) From c8d186db3e0039c1ed2445be0748c0186d0bafe4 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Tue, 19 Sep 2023 12:06:19 +0100 Subject: [PATCH 07/61] Update RELEASENOTES-DEVELOPMENT.md --- RELEASENOTES-DEVELOPMENT.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/RELEASENOTES-DEVELOPMENT.md b/RELEASENOTES-DEVELOPMENT.md index 12f72493c..59e939ed6 100644 --- a/RELEASENOTES-DEVELOPMENT.md +++ b/RELEASENOTES-DEVELOPMENT.md @@ -1,3 +1,7 @@ +Version 4.3.2-dev-4-g20a45def +=== +* Bump version information. + Version 4.3.1-dev-3-g2fb81ca9 === **Exploration** From 64ac1be613af78f38746b3b3ac43eb24dca4f5cb Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Tue, 19 Sep 2023 12:22:55 +0100 Subject: [PATCH 08/61] Update check_ap2_systemd_full.yml --- .github/workflows/check_ap2_systemd_full.yml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/workflows/check_ap2_systemd_full.yml b/.github/workflows/check_ap2_systemd_full.yml index e99e0a232..57ca5dfab 100644 --- a/.github/workflows/check_ap2_systemd_full.yml +++ b/.github/workflows/check_ap2_systemd_full.yml @@ -1,6 +1,7 @@ name: Full configuration (but without apple-alac) for systemd. on: + workflow_dispatch: push: branches: [ "development", "danger" ] pull_request: @@ -14,11 +15,11 @@ jobs: steps: - uses: actions/checkout@v3.5.2 - name: Install Dependencies - run: sudo apt-get -y --no-install-recommends install build-essential git xmltoman autoconf automake libtool libpopt-dev libconfig-dev libasound2-dev libao-dev libjack-dev libpulse-dev libsoundio-dev libsndio-dev libglib2.0-dev libmosquitto-dev avahi-daemon libavahi-client-dev libssl-dev libsoxr-dev libplist-dev libsodium-dev libavutil-dev libavcodec-dev libavformat-dev uuid-dev libgcrypt-dev xxd + run: sudo apt-get -y --no-install-recommends install build-essential git xmltoman autoconf automake libtool libpopt-dev libconfig-dev libasound2-dev libao-dev libjack-dev libpulse-dev libglib2.0-dev libmosquitto-dev avahi-daemon libavahi-client-dev libssl-dev libsoxr-dev libplist-dev libsodium-dev libavutil-dev libavcodec-dev libavformat-dev uuid-dev libgcrypt-dev xxd - name: Configure run: | autoreconf -i - ./configure --sysconfdir=/etc --with-alsa --with-ao --with-dummy --with-jack --with-pa --with-pipe --with-sndio --with-soundio --with-stdout --with-soxr --with-avahi --with-ssl=openssl --with-systemd --with-dbus-interface --with-mpris-interface --with-mqtt-client --with-airplay-2 + ./configure --sysconfdir=/etc --with-alsa --with-ao --with-dummy --with-jack --with-pa --with-pipe --with-stdout --with-soxr --with-avahi --with-ssl=openssl --with-systemd --with-dbus-interface --with-mpris-interface --with-mqtt-client --with-airplay-2 - name: Make run: | make -j From 76c8a958c5dcf95664ea631a0d36ca2f8cc332b0 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Tue, 19 Sep 2023 12:47:23 +0100 Subject: [PATCH 09/61] Update check_classic_systemd_full.yml --- .github/workflows/check_classic_systemd_full.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/check_classic_systemd_full.yml b/.github/workflows/check_classic_systemd_full.yml index 75229469b..a6de54c85 100644 --- a/.github/workflows/check_classic_systemd_full.yml +++ b/.github/workflows/check_classic_systemd_full.yml @@ -1,6 +1,7 @@ name: Full classic configuration (but without apple-alac) for systemd, using a build folder. on: + workflow_dispatch: push: branches: [ "development", "danger" ] pull_request: From afe93a64d0e4506c96755fb06b18398c8254e2a6 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Tue, 19 Sep 2023 13:34:56 +0100 Subject: [PATCH 10/61] Update workflows. --- .github/workflows/check_ap2_systemd_basic.yml | 1 + .github/workflows/check_ap2_systemd_full.yml | 6 +++--- .../workflows/check_ap2_systemd_full_build_folder.yml | 9 +++++---- .github/workflows/check_ap2_systemv_full.yml | 7 ++++--- .github/workflows/check_classic_mac_basic.yml | 1 + .github/workflows/check_classic_systemd_basic.yml | 1 + .github/workflows/check_classic_systemd_full.yml | 6 +++--- .../workflows/docker-build-on-push_and_pull_request.yaml | 1 + .github/workflows/docker-build-on-tag.yaml | 1 + 9 files changed, 20 insertions(+), 13 deletions(-) diff --git a/.github/workflows/check_ap2_systemd_basic.yml b/.github/workflows/check_ap2_systemd_basic.yml index 23e155bee..367495646 100644 --- a/.github/workflows/check_ap2_systemd_basic.yml +++ b/.github/workflows/check_ap2_systemd_basic.yml @@ -1,6 +1,7 @@ name: Basic ALSA configuration for systemd, using a build folder. on: + workflow_dispatch: push: branches: [ "development" ] pull_request: diff --git a/.github/workflows/check_ap2_systemd_full.yml b/.github/workflows/check_ap2_systemd_full.yml index 57ca5dfab..bc2d912d4 100644 --- a/.github/workflows/check_ap2_systemd_full.yml +++ b/.github/workflows/check_ap2_systemd_full.yml @@ -1,4 +1,4 @@ -name: Full configuration (but without apple-alac) for systemd. +name: Configuration (without pa, soundio or apple-alac) for systemd. on: workflow_dispatch: @@ -15,11 +15,11 @@ jobs: steps: - uses: actions/checkout@v3.5.2 - name: Install Dependencies - run: sudo apt-get -y --no-install-recommends install build-essential git xmltoman autoconf automake libtool libpopt-dev libconfig-dev libasound2-dev libao-dev libjack-dev libpulse-dev libglib2.0-dev libmosquitto-dev avahi-daemon libavahi-client-dev libssl-dev libsoxr-dev libplist-dev libsodium-dev libavutil-dev libavcodec-dev libavformat-dev uuid-dev libgcrypt-dev xxd + run: sudo apt-get -y --no-install-recommends install build-essential git xmltoman autoconf automake libtool libpopt-dev libconfig-dev libasound2-dev libao-dev libjack-dev libsndio-dev libglib2.0-dev libmosquitto-dev avahi-daemon libavahi-client-dev libssl-dev libsoxr-dev libplist-dev libsodium-dev libavutil-dev libavcodec-dev libavformat-dev uuid-dev libgcrypt-dev xxd - name: Configure run: | autoreconf -i - ./configure --sysconfdir=/etc --with-alsa --with-ao --with-dummy --with-jack --with-pa --with-pipe --with-stdout --with-soxr --with-avahi --with-ssl=openssl --with-systemd --with-dbus-interface --with-mpris-interface --with-mqtt-client --with-airplay-2 + ./configure --sysconfdir=/etc --with-alsa --with-ao --with-dummy --with-jack --with-pipe --with-sndio --with-stdout --with-soxr --with-avahi --with-ssl=openssl --with-systemd --with-dbus-interface --with-mpris-interface --with-mqtt-client --with-airplay-2 - name: Make run: | make -j diff --git a/.github/workflows/check_ap2_systemd_full_build_folder.yml b/.github/workflows/check_ap2_systemd_full_build_folder.yml index 8d8a97210..98f964cb4 100644 --- a/.github/workflows/check_ap2_systemd_full_build_folder.yml +++ b/.github/workflows/check_ap2_systemd_full_build_folder.yml @@ -1,6 +1,7 @@ -name: Full configuration (but without apple-alac) for systemd, using a build folder. +name: Configuration (without pa, soundio or apple-alac) for systemd, using a build folder. on: + workflow_dispatch: push: branches: [ "development" ] pull_request: @@ -9,18 +10,18 @@ on: jobs: build: - runs-on: ubuntu-20.04 + runs-on: ubuntu-22.04 steps: - uses: actions/checkout@v3.5.2 - name: Install Dependencies - run: sudo apt-get -y --no-install-recommends install build-essential git xmltoman autoconf automake libtool libpopt-dev libconfig-dev libasound2-dev libao-dev libjack-dev libpulse-dev libsoundio-dev libsndio-dev libglib2.0-dev libmosquitto-dev avahi-daemon libavahi-client-dev libssl-dev libsoxr-dev libplist-dev libsodium-dev libavutil-dev libavcodec-dev libavformat-dev uuid-dev libgcrypt-dev xxd + run: sudo apt-get -y --no-install-recommends install build-essential git xmltoman autoconf automake libtool libpopt-dev libconfig-dev libasound2-dev libao-dev libjack-dev libsndio-dev libglib2.0-dev libmosquitto-dev avahi-daemon libavahi-client-dev libssl-dev libsoxr-dev libplist-dev libsodium-dev libavutil-dev libavcodec-dev libavformat-dev uuid-dev libgcrypt-dev xxd - name: Configure run: | mkdir build cd build autoreconf -i .. - ../configure --sysconfdir=/etc --with-alsa --with-ao --with-dummy --with-jack --with-pa --with-pipe --with-sndio --with-soundio --with-stdout --with-soxr --with-avahi --with-ssl=openssl --with-systemd --with-dbus-interface --with-mpris-interface --with-mqtt-client --with-airplay-2 + ../configure --sysconfdir=/etc --with-alsa --with-ao --with-dummy --with-jack --with-pipe --with-sndio --with-stdout --with-soxr --with-avahi --with-ssl=openssl --with-systemd --with-dbus-interface --with-mpris-interface --with-mqtt-client --with-airplay-2 - name: Make run: | cd build diff --git a/.github/workflows/check_ap2_systemv_full.yml b/.github/workflows/check_ap2_systemv_full.yml index 1f8dab073..3f46c0d62 100644 --- a/.github/workflows/check_ap2_systemv_full.yml +++ b/.github/workflows/check_ap2_systemv_full.yml @@ -1,6 +1,7 @@ -name: Full configuration (but without apple-alac) for a System V system. +name: Configuration (but without pa, soundio, apple-alac) for a System V system. on: + workflow_dispatch: push: branches: [ "development" ] pull_request: @@ -14,11 +15,11 @@ jobs: steps: - uses: actions/checkout@v3.5.2 - name: Install Dependencies - run: sudo apt-get -y --no-install-recommends install build-essential git xmltoman autoconf automake libtool libpopt-dev libdaemon-dev libconfig-dev libasound2-dev libao-dev libjack-dev libpulse-dev libsoundio-dev libsndio-dev libglib2.0-dev libmosquitto-dev avahi-daemon libavahi-client-dev libssl-dev libsoxr-dev libplist-dev libsodium-dev libavutil-dev libavcodec-dev libavformat-dev uuid-dev libgcrypt-dev xxd + run: sudo apt-get -y --no-install-recommends install build-essential git xmltoman autoconf automake libtool libpopt-dev libdaemon-dev libconfig-dev libasound2-dev libao-dev libjack-dev libsndio-dev libglib2.0-dev libmosquitto-dev avahi-daemon libavahi-client-dev libssl-dev libsoxr-dev libplist-dev libsodium-dev libavutil-dev libavcodec-dev libavformat-dev uuid-dev libgcrypt-dev xxd - name: Configure run: | autoreconf -i - ./configure --sysconfdir=/etc --with-alsa --with-ao --with-dummy --with-libdaemon --with-jack --with-pa --with-pipe --with-sndio --with-soundio --with-stdout --with-soxr --with-avahi --with-ssl=openssl --with-systemv --with-dbus-interface --with-mpris-interface --with-mqtt-client --with-airplay-2 + ./configure --sysconfdir=/etc --with-alsa --with-ao --with-dummy --with-libdaemon --with-jack --with-pipe --with-sndio --with-stdout --with-soxr --with-avahi --with-ssl=openssl --with-systemv --with-dbus-interface --with-mpris-interface --with-mqtt-client --with-airplay-2 - name: Make run: | make -j diff --git a/.github/workflows/check_classic_mac_basic.yml b/.github/workflows/check_classic_mac_basic.yml index b06e046a2..40d0cbea8 100644 --- a/.github/workflows/check_classic_mac_basic.yml +++ b/.github/workflows/check_classic_mac_basic.yml @@ -1,6 +1,7 @@ name: Basic libao configuration for macOS with BREW -- classic only, because macOS can't host NQPTP. on: + workflow_dispatch: push: branches: [ "development", "danger" ] pull_request: diff --git a/.github/workflows/check_classic_systemd_basic.yml b/.github/workflows/check_classic_systemd_basic.yml index 95f9d9049..ead6de279 100644 --- a/.github/workflows/check_classic_systemd_basic.yml +++ b/.github/workflows/check_classic_systemd_basic.yml @@ -1,6 +1,7 @@ name: Basic ALSA classic configuration for systemd, using a build folder. on: + workflow_dispatch: push: branches: [ "development" ] pull_request: diff --git a/.github/workflows/check_classic_systemd_full.yml b/.github/workflows/check_classic_systemd_full.yml index a6de54c85..4d289c532 100644 --- a/.github/workflows/check_classic_systemd_full.yml +++ b/.github/workflows/check_classic_systemd_full.yml @@ -1,4 +1,4 @@ -name: Full classic configuration (but without apple-alac) for systemd, using a build folder. +name: Classic (without pa, soundio, apple-alac) for systemd, using a build folder. on: workflow_dispatch: @@ -15,13 +15,13 @@ jobs: steps: - uses: actions/checkout@v3.5.2 - name: Install Dependencies - run: sudo apt-get -y --no-install-recommends install build-essential git xmltoman autoconf automake libtool libpopt-dev libconfig-dev libasound2-dev libao-dev libjack-dev libpulse-dev libsoundio-dev libsndio-dev libglib2.0-dev libmosquitto-dev avahi-daemon libavahi-client-dev libssl-dev libsoxr-dev + run: sudo apt-get -y --no-install-recommends install build-essential git xmltoman autoconf automake libtool libpopt-dev libconfig-dev libasound2-dev libao-dev libjack-dev libsndio-dev libglib2.0-dev libmosquitto-dev avahi-daemon libavahi-client-dev libssl-dev libsoxr-dev - name: Configure run: | mkdir build cd build autoreconf -i .. - ../configure --sysconfdir=/etc --with-alsa --with-ao --with-dummy --with-jack --with-pa --with-pipe --with-sndio --with-soundio --with-stdout --with-soxr --with-avahi --with-ssl=openssl --with-systemd --with-dbus-interface --with-mpris-interface --with-mqtt-client + ../configure --sysconfdir=/etc --with-alsa --with-ao --with-dummy --with-pipe --with-stdout --with-soxr --with-avahi --with-ssl=openssl --with-systemd --with-dbus-interface --with-mpris-interface --with-mqtt-client - name: Make run: | cd build diff --git a/.github/workflows/docker-build-on-push_and_pull_request.yaml b/.github/workflows/docker-build-on-push_and_pull_request.yaml index fdbeb8106..01df710de 100644 --- a/.github/workflows/docker-build-on-push_and_pull_request.yaml +++ b/.github/workflows/docker-build-on-push_and_pull_request.yaml @@ -7,6 +7,7 @@ name: Build and push docker (push/pull request) on: + workflow_dispatch: push: branches: - master diff --git a/.github/workflows/docker-build-on-tag.yaml b/.github/workflows/docker-build-on-tag.yaml index a83882bc4..5c9187771 100644 --- a/.github/workflows/docker-build-on-tag.yaml +++ b/.github/workflows/docker-build-on-tag.yaml @@ -8,6 +8,7 @@ name: Build and push docker (tag) on: + workflow_dispatch: push: tags: - '[0-9]+' # X From b4a2e5da36d0b118e9bc69f117792a4e5c022170 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Wed, 20 Sep 2023 11:32:26 +0100 Subject: [PATCH 11/61] Fix warning picked up on FreeBSD15 --- shairport.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shairport.c b/shairport.c index 66d9028ad..4ced23928 100644 --- a/shairport.c +++ b/shairport.c @@ -1596,7 +1596,7 @@ void exit_function() { */ debug(2, "Stopping the activity monitor."); - activity_monitor_stop(0); + activity_monitor_stop(); debug(2, "Stopping the activity monitor done."); #ifdef CONFIG_DACP_CLIENT From f5475924dce9a23bea3ae1eccd1e2bd6dae3d0e6 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Wed, 20 Sep 2023 11:46:38 +0100 Subject: [PATCH 12/61] Fix a benign warning from Clang 16 on FreeBSD --- rtp.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/rtp.c b/rtp.c index c7c8efa3f..0f35f0341 100644 --- a/rtp.c +++ b/rtp.c @@ -199,13 +199,10 @@ void *rtp_audio_receiver(void *arg) { float stat_mean = 0.0; float stat_M2 = 0.0; - int frame_count = 0; ssize_t nread; while (1) { nread = recv(conn->audio_socket, packet, sizeof(packet), 0); - frame_count++; - uint64_t local_time_now_ns = get_absolute_time_in_ns(); if (time_of_previous_packet_ns) { float time_interval_us = (local_time_now_ns - time_of_previous_packet_ns) * 0.001; From 847ad7fb9c5a28a7b5014668850f7bf3787fbffe Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Thu, 21 Sep 2023 15:49:25 +0100 Subject: [PATCH 13/61] move the declaration of principal_conn_lock from player.h to rtsp.h where it should be. --- player.h | 1 - rtsp.h | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/player.h b/player.h index c5cfd002f..11435bf91 100644 --- a/player.h +++ b/player.h @@ -415,7 +415,6 @@ typedef struct { uint64_t dac_buffer_queue_minimum_length; } rtsp_conn_info; -extern pthread_mutex_t principal_conn_lock; extern int statistics_row; // will be reset to zero when debug level changes or statistics enabled void reset_buffer(rtsp_conn_info *conn); diff --git a/rtsp.h b/rtsp.h index 59b3ae3dd..d58a0bc52 100644 --- a/rtsp.h +++ b/rtsp.h @@ -3,6 +3,7 @@ #include "player.h" +extern pthread_rwlock_t principal_conn_lock; extern rtsp_conn_info *principal_conn; extern rtsp_conn_info **conns; From 60cee967dc56723ec67f04047a0d239e61721669 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Thu, 21 Sep 2023 15:50:43 +0100 Subject: [PATCH 14/61] Add a pthread_cleanup compatible function to release a pthread_rwlock. --- common.c | 2 ++ common.h | 1 + 2 files changed, 3 insertions(+) diff --git a/common.c b/common.c index d16e3b1f1..4dc75bc7f 100644 --- a/common.c +++ b/common.c @@ -1725,6 +1725,8 @@ void mutex_cleanup(void *arg) { void mutex_unlock(void *arg) { pthread_mutex_unlock((pthread_mutex_t *)arg); } +void rwlock_unlock(void *arg) { pthread_rwlock_unlock((pthread_rwlock_t *)arg); } + void thread_cleanup(void *arg) { debug(3, "thread_cleanup called."); pthread_t *thread = (pthread_t *)arg; diff --git a/common.h b/common.h index edd48fa8d..1b9e2f03d 100644 --- a/common.h +++ b/common.h @@ -495,6 +495,7 @@ uint16_t bind_UDP_port(int ip_family, const char *self_ip_address, uint32_t scop void socket_cleanup(void *arg); void mutex_unlock(void *arg); +void rwlock_unlock(void *arg); void mutex_cleanup(void *arg); void cv_cleanup(void *arg); void thread_cleanup(void *arg); From 152ddbb87df2bbccd0885454542f30a89b5b8f93 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Thu, 21 Sep 2023 16:04:23 +0100 Subject: [PATCH 15/61] Change principal_conn_lock from a regular mutex to a read-write mutex, so it can be used to check and hold the current principal_conn unless it's being altered in get_play_lock or a release_play_lock. Only allow access to the config.airplay_statusflags, build_bonjour_strings(NULL), mdns_update(NULL, secondary_txt_records) when read_lock is acquired on the principal_conn_lock. Only allow changes to config.airplay_statusflags and only allow access to mdns_update if you are the principal conn. The bonjour status flags and the info response plist calculations are still a dirty rotten hack. --- rtsp.c | 87 +++++++++++++++++++++++++++++++++++++++------------------- 1 file changed, 59 insertions(+), 28 deletions(-) diff --git a/rtsp.c b/rtsp.c index 56e06c934..5c750a2ab 100644 --- a/rtsp.c +++ b/rtsp.c @@ -141,12 +141,11 @@ rtsp_conn_info **conns; int metadata_running = 0; -// always lock this when trying to make a conn the principal conn, -// e.g. during an ANNOUNCE (Classic AirPlay) or SETUP (AirPlay 2) -pthread_mutex_t principal_conn_acquisition_lock = PTHREAD_MUTEX_INITIALIZER; // always lock this when accessing the principal conn value -pthread_mutex_t principal_conn_lock = PTHREAD_MUTEX_INITIALIZER; +// use a read lock when consulting and holding it +// use a write lock if you want to change it +pthread_rwlock_t principal_conn_lock = PTHREAD_RWLOCK_INITIALIZER; // always lock this when accessing the list of connection threads pthread_mutex_t conns_lock = PTHREAD_MUTEX_INITIALIZER; @@ -565,23 +564,24 @@ void cancel_all_RTSP_threads(airplay_stream_c stream_category, int except_this_o // other devices. void release_play_lock(rtsp_conn_info *conn) { - pthread_cleanup_debug_mutex_lock(&principal_conn_lock, 100000, - 1); // don't let the principal_conn be changed + // no need thread cancellation points in here + pthread_rwlock_wrlock(&principal_conn_lock); if (principal_conn == conn) { // if we have the player if (conn != NULL) debug(2, "Connection %d: principal_conn released.", conn->connection_number); principal_conn = NULL; // let it go } - pthread_cleanup_pop(1); // release the principal_conn lock + pthread_rwlock_unlock(&principal_conn_lock); } // stop the current principal_conn from playing if necessary and make conn the principal_conn. int get_play_lock(rtsp_conn_info *conn, int allow_session_interruption) { int response = 0; - pthread_cleanup_debug_mutex_lock(&principal_conn_lock, 100000, 1); + pthread_rwlock_wrlock(&principal_conn_lock); + pthread_cleanup_push(rwlock_unlock,(void *)&principal_conn_lock); if (principal_conn != NULL) - debug(2, "Connection %d: is requested to relinquish principal_conn.", + debug(1, "Connection %d: is requested to relinquish principal_conn.", principal_conn->connection_number); if (conn != NULL) debug(2, "Connection %d: request to acquire principal_conn.", conn->connection_number); @@ -594,10 +594,10 @@ int get_play_lock(rtsp_conn_info *conn, int allow_session_interruption) { warn("Connection %d: request to re-acquire principal_conn!", principal_conn->connection_number); } else if (allow_session_interruption != 0) { - player_stop(principal_conn); - debug(2, "Connection %d: termination requested.", principal_conn->connection_number); - pthread_cancel(principal_conn->thread); - usleep(2000000); // don't know why this delay is needed. + rtsp_conn_info *previous_principal_conn = principal_conn; + principal_conn = NULL; // no longer the principal conn + pthread_cancel(previous_principal_conn->thread); + // usleep(1000000); // don't know why this delay is needed. principal_conn = conn; // make the conn the new principal_conn response = 1; // interrupted an existing session } else { @@ -766,6 +766,11 @@ void msg_retain(rtsp_message *msg) { } rtsp_message *msg_init(void) { + // no thread cancellation points here + int rc = pthread_mutex_lock(&reference_counter_lock); + if (rc) + debug(1, "Error %d locking reference counter lock"); + rtsp_message *msg = malloc(sizeof(rtsp_message)); if (msg) { memset(msg, 0, sizeof(rtsp_message)); @@ -776,6 +781,11 @@ rtsp_message *msg_init(void) { die("msg_init -- can not allocate memory for rtsp_message %d.", msg_indexes); } // debug(1,"msg_init -- create item %d.", msg->index_number); + + rc = pthread_mutex_unlock(&reference_counter_lock); + if (rc) + debug(1, "Error %d unlocking reference counter lock"); + return msg; } @@ -1707,6 +1717,14 @@ void handle_get_info(__attribute((unused)) rtsp_conn_info *conn, rtsp_message *r } else { void *qualifier_response_data = NULL; size_t qualifier_response_data_length = 0; + + + pthread_rwlock_rdlock(&principal_conn_lock); // don't let the principal_conn be changed + pthread_cleanup_push(rwlock_unlock,(void *)&principal_conn_lock); + + + + if (add_pstring_to_malloc("acl=0", &qualifier_response_data, &qualifier_response_data_length) == 0) debug(1, "Problem"); @@ -1769,6 +1787,7 @@ void handle_get_info(__attribute((unused)) rtsp_conn_info *conn, rtsp_message *r free(vs); // pkString_make(pkString, sizeof(pkString), config.airplay_device_id); // plist_dict_set_item(response_plist, "pk", plist_new_string(pkString)); + pthread_cleanup_pop(1); // release the principal_conn lock plist_to_bin(response_plist, &resp->content, &resp->contentlength); if (resp->contentlength == 0) debug(1, "GET /info Stage 1: response bplist not created!"); @@ -2685,11 +2704,6 @@ void teardown_phase_two(rtsp_conn_info *conn) { #endif } conn->groupContainsGroupLeader = 0; - config.airplay_statusflags &= (0xffffffff - (1 << 11)); // DeviceSupportsRelay - build_bonjour_strings(NULL); - debug(2, "Connection %d: TEARDOWN mdns_update on %s.", conn->connection_number, - get_category_string(conn->airplay_stream_category)); - mdns_update(NULL, secondary_txt_records); if (conn->dacp_active_remote != NULL) { free(conn->dacp_active_remote); conn->dacp_active_remote = NULL; @@ -2701,13 +2715,11 @@ void teardown_phase_two(rtsp_conn_info *conn) { void handle_teardown_2(rtsp_conn_info *conn, __attribute__((unused)) rtsp_message *req, rtsp_message *resp) { - debug(2, "Connection %d: TEARDOWN %s.", conn->connection_number, + debug(1, "Connection %d: TEARDOWN %s.", conn->connection_number, get_category_string(conn->airplay_stream_category)); debug_log_rtsp_message(2, "TEARDOWN: ", req); - // if (have_player(conn)) { resp->respcode = 200; msg_add_header(resp, "Connection", "close"); - plist_t messagePlist = plist_from_rtsp_content(req); if (messagePlist != NULL) { // now see if the incoming plist contains a "streams" array @@ -2726,6 +2738,18 @@ void handle_teardown_2(rtsp_conn_info *conn, __attribute__((unused)) rtsp_messag get_category_string(conn->airplay_stream_category)); teardown_phase_one(conn); // try to do phase one anyway teardown_phase_two(conn); + + // only update these things if you're (still) the principal conn + pthread_rwlock_rdlock(&principal_conn_lock); // don't let the principal_conn be changed + pthread_cleanup_push(rwlock_unlock,(void *)&principal_conn_lock); + if ((principal_conn == conn) && (conn->airplay_stream_category == ptp_stream)) { + config.airplay_statusflags &= (0xffffffff - (1 << 11)); // DeviceSupportsRelay + build_bonjour_strings(conn); + debug(2, "Connection %d: TEARDOWN mdns_update on %s.", conn->connection_number, + get_category_string(conn->airplay_stream_category)); + mdns_update(NULL, secondary_txt_records); + } + pthread_cleanup_pop(1); // release the principal_conn lock debug(2, "Connection %d: TEARDOWN %s -- close the connection complete", conn->connection_number, get_category_string(conn->airplay_stream_category)); release_play_lock(conn); @@ -2737,6 +2761,7 @@ void handle_teardown_2(rtsp_conn_info *conn, __attribute__((unused)) rtsp_messag debug(1, "Connection %d: missing plist!", conn->connection_number); resp->respcode = 451; // don't know what to do here } + // debug(1,"Bogus exit for valgrind -- remember to comment it out!."); // exit(EXIT_SUCCESS); // } @@ -3081,12 +3106,18 @@ void handle_setup_2(rtsp_conn_info *conn, rtsp_message *req, rtsp_message *resp) conn->connection_number); // kill all the other listeners */ + // only update these things if you're (still) the principal conn + pthread_rwlock_wrlock(&principal_conn_lock); // don't let the principal_conn be changed + pthread_cleanup_push(rwlock_unlock,(void *)&principal_conn_lock); + if (principal_conn == conn) { + config.airplay_statusflags |= 1 << 11; // DeviceSupportsRelay + build_bonjour_strings(conn); + debug(2, "Connection %d: SETUP mdns_update on %s.", conn->connection_number, + get_category_string(conn->airplay_stream_category)); + mdns_update(NULL, secondary_txt_records); + } + pthread_cleanup_pop(1); // release the principal_conn lock - config.airplay_statusflags |= 1 << 11; // DeviceSupportsRelay - build_bonjour_strings(conn); - debug(2, "Connection %d: SETUP mdns_update on %s.", conn->connection_number, - get_category_string(conn->airplay_stream_category)); - mdns_update(NULL, secondary_txt_records); resp->respcode = 200; } else { debug(1, "SETUP on Connection %d: PTP setup -- no timingPeerInfo plist.", @@ -3580,8 +3611,8 @@ void handle_set_parameter_parameter(rtsp_conn_info *conn, rtsp_message *req, shairport_sync_set_volume(shairportSyncSkeleton, volume); } else { #endif - pthread_cleanup_debug_mutex_lock(&principal_conn_lock, 100000, - 1); // don't let the principal_conn be changed + pthread_rwlock_rdlock(&principal_conn_lock); // don't let the principal_conn be changed + pthread_cleanup_push(rwlock_unlock,(void *)&principal_conn_lock); if (principal_conn == conn) { player_volume(volume, conn); } From eed22e415439fa140b31bf642d3e078b8c4ab9c9 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Thu, 21 Sep 2023 16:33:19 +0100 Subject: [PATCH 16/61] update to correspond with the move of principal_conn_lock to a pthread_rwlock. --- dbus-service.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/dbus-service.c b/dbus-service.c index a25d9962f..8acf23922 100644 --- a/dbus-service.c +++ b/dbus-service.c @@ -635,7 +635,8 @@ gboolean notify_volume_callback(ShairportSync *skeleton, if (((iv >= -30.0) && (iv <= 0.0)) || (iv == -144.0)) { debug(2, ">> setting volume to %7.4f.", iv); - pthread_cleanup_debug_mutex_lock(&principal_conn_lock, 100000, 1); + pthread_rwlock_rdlock(&principal_conn_lock); // don't let the principal_conn be changed + pthread_cleanup_push(rwlock_unlock,(void *)&principal_conn_lock); if (principal_conn != NULL) { player_volume(iv, principal_conn); From 66163c1a71f4db7ddccaffcb59317a325503d97c Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Thu, 21 Sep 2023 16:34:40 +0100 Subject: [PATCH 17/61] Remove redundant (and now faulty) check to see if the player is also principal conn. --- player.c | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/player.c b/player.c index 3a2fb3825..0cfb35540 100644 --- a/player.c +++ b/player.c @@ -1778,14 +1778,9 @@ double suggested_volume(rtsp_conn_info *conn) { void player_thread_cleanup_handler(void *arg) { rtsp_conn_info *conn = (rtsp_conn_info *)arg; - if ((principal_conn == conn) && (conn != NULL)) { - if (config.output->stop) { - debug(2, "Connection %d: Stop the output backend.", conn->connection_number); - config.output->stop(); - } - } else { - if (conn != NULL) - debug(1, "Connection %d: this conn is not the principal_conn.", conn->connection_number); + if (config.output->stop) { + debug(1, "Connection %d: Stop the output backend.", conn->connection_number); + config.output->stop(); } int oldState; From 2987478f4cfbae4bd1b1d4909100f41b55539084 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Thu, 21 Sep 2023 16:36:21 +0100 Subject: [PATCH 18/61] Set and reset bonjour flags correctly when it's a Classic Airplay session. Reintroduce a 1 second delay for the prempted session to go away. --- rtsp.c | 45 +++++++++++++++++++++++++++++++++++++-------- 1 file changed, 37 insertions(+), 8 deletions(-) diff --git a/rtsp.c b/rtsp.c index 5c750a2ab..2ae1c9390 100644 --- a/rtsp.c +++ b/rtsp.c @@ -597,7 +597,7 @@ int get_play_lock(rtsp_conn_info *conn, int allow_session_interruption) { rtsp_conn_info *previous_principal_conn = principal_conn; principal_conn = NULL; // no longer the principal conn pthread_cancel(previous_principal_conn->thread); - // usleep(1000000); // don't know why this delay is needed. + usleep(1000000); // don't know why this delay is needed. principal_conn = conn; // make the conn the new principal_conn response = 1; // interrupted an existing session } else { @@ -700,7 +700,7 @@ void cleanup_threads(void) { debug(2, "Found RTSP connection thread %d in a non-running state.", conns[i]->connection_number); pthread_join(conns[i]->thread, &retval); - debug(1, "Connection %d: deleted.", conns[i]->connection_number); + debug(2, "Connection %d: deleted.", conns[i]->connection_number); free(conns[i]); conns[i] = NULL; } @@ -2715,7 +2715,7 @@ void teardown_phase_two(rtsp_conn_info *conn) { void handle_teardown_2(rtsp_conn_info *conn, __attribute__((unused)) rtsp_message *req, rtsp_message *resp) { - debug(1, "Connection %d: TEARDOWN %s.", conn->connection_number, + debug(1, "Connection %d: TEARDOWN 2 %s.", conn->connection_number, get_category_string(conn->airplay_stream_category)); debug_log_rtsp_message(2, "TEARDOWN: ", req); resp->respcode = 200; @@ -2768,6 +2768,7 @@ void handle_teardown_2(rtsp_conn_info *conn, __attribute__((unused)) rtsp_messag #endif void teardown(rtsp_conn_info *conn) { + debug(2, "Connection %d: TEARDOWN (Classic AirPlay).", conn->connection_number); player_stop(conn); activity_monitor_signify_activity(0); // inactive, and should be after command_stop() if (conn->dacp_active_remote != NULL) { @@ -2780,11 +2781,23 @@ void handle_teardown(rtsp_conn_info *conn, __attribute__((unused)) rtsp_message rtsp_message *resp) { debug_log_rtsp_message(2, "TEARDOWN request", req); debug(2, "Connection %d: TEARDOWN", conn->connection_number); - // if (have_play_lock(conn)) { debug(3, "TEARDOWN: synchronously terminating the player thread of RTSP conversation thread %d (2).", conn->connection_number); teardown(conn); + +#ifdef CONFIG_AIRPLAY_2 + // only update these things if you're (still) the principal conn + pthread_rwlock_rdlock(&principal_conn_lock); // don't let the principal_conn be changed + pthread_cleanup_push(rwlock_unlock,(void *)&principal_conn_lock); + if (principal_conn == conn) { + config.airplay_statusflags &= (0xffffffff - (1 << 11)); // DeviceSupportsRelay + build_bonjour_strings(conn); + mdns_update(NULL, secondary_txt_records); + } + pthread_cleanup_pop(1); // release the principal_conn lock +#endif + release_play_lock(conn); resp->respcode = 200; @@ -4514,9 +4527,25 @@ static void handle_announce(rtsp_conn_info *conn, rtsp_message *req, rtsp_messag #ifdef CONFIG_AIRPLAY_2 conn->airplay_type = ap_1; conn->timing_type = ts_ntp; + if (conn->airplay_gid != NULL) { + free(conn->airplay_gid); + conn->airplay_gid = NULL; + } + + // only update these things if you're (still) the principal conn + pthread_rwlock_rdlock(&principal_conn_lock); // don't let the principal_conn be changed + pthread_cleanup_push(rwlock_unlock,(void *)&principal_conn_lock); + if (principal_conn == conn) { + config.airplay_statusflags |= 1 << 11; // DeviceSupportsRelay -- should this be on? + build_bonjour_strings(conn); + get_category_string(conn->airplay_stream_category)); + mdns_update(NULL, secondary_txt_records); + } + pthread_cleanup_pop(1); // release the principal_conn lock + debug(1, "Connection %d: Classic AirPlay connection from %s:%u to self at %s:%u.", conn->connection_number, conn->client_ip_string, conn->client_rtsp_port, - conn->self_ip_string, conn->self_rtsp_port); + conn->self_ip_string, conn->self_rtsp_port); #endif conn->stream.type = ast_unknown; @@ -5201,7 +5230,7 @@ static void *rtsp_conversation_thread_func(void *pconn) { int i; for (i = 0; i < nconns; i++) { if ((conns[i] != NULL) && (conns[i]->connection_number == 0)) { - debug(1, "conns[%d] at %" PRIxPTR " has a Connection Number of 0.", conns[i]); + debug(1, "conns[%d] at %" PRIxPTR " has a Connection Number of 0!", conns[i]); } } debug_mutex_unlock(&conns_lock, 3); @@ -5507,7 +5536,7 @@ void *rtsp_listen_loop(__attribute((unused)) void *arg) { maxfd = sockfd[i]; } - char **t1 = txt_records; // ap1 test records + char **t1 = txt_records; // ap1 text records char **t2 = NULL; // possibly two text records #ifdef CONFIG_AIRPLAY_2 // make up a secondary set of text records @@ -5561,7 +5590,7 @@ void *rtsp_listen_loop(__attribute((unused)) void *arg) { pthread_cleanup_push(malloc_cleanup, conn); memset(conn, 0, sizeof(rtsp_conn_info)); conn->connection_number = RTSP_connection_index++; - debug(1, "Connection %d is at: 0x%" PRIxPTR ".", conn->connection_number, conn); + debug(2, "Connection %d is at: 0x%" PRIxPTR ".", conn->connection_number, conn); #ifdef CONFIG_AIRPLAY_2 conn->airplay_type = ap_2; // changed if an ANNOUNCE is received conn->timing_type = ts_ptp; // changed if an ANNOUNCE is received From 2e023fb13bd8e28ef4691936045f49d2ab79d04a Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Thu, 21 Sep 2023 16:41:48 +0100 Subject: [PATCH 19/61] Remove a few debug messages. --- player.c | 1 - rtsp.c | 1 - 2 files changed, 2 deletions(-) diff --git a/player.c b/player.c index 0cfb35540..42e0b86c3 100644 --- a/player.c +++ b/player.c @@ -1779,7 +1779,6 @@ void player_thread_cleanup_handler(void *arg) { rtsp_conn_info *conn = (rtsp_conn_info *)arg; if (config.output->stop) { - debug(1, "Connection %d: Stop the output backend.", conn->connection_number); config.output->stop(); } diff --git a/rtsp.c b/rtsp.c index 2ae1c9390..5469a571c 100644 --- a/rtsp.c +++ b/rtsp.c @@ -4538,7 +4538,6 @@ static void handle_announce(rtsp_conn_info *conn, rtsp_message *req, rtsp_messag if (principal_conn == conn) { config.airplay_statusflags |= 1 << 11; // DeviceSupportsRelay -- should this be on? build_bonjour_strings(conn); - get_category_string(conn->airplay_stream_category)); mdns_update(NULL, secondary_txt_records); } pthread_cleanup_pop(1); // release the principal_conn lock From 5abf393f7ec94ed6dbce11ba38f931e997f61f27 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Thu, 21 Sep 2023 16:45:22 +0100 Subject: [PATCH 20/61] clang format --- dbus-service.c | 2 +- nqptp-shm-structures.h | 2 +- rtsp.c | 60 +++++++++++++++++++----------------------- 3 files changed, 29 insertions(+), 35 deletions(-) diff --git a/dbus-service.c b/dbus-service.c index 8acf23922..91e030d67 100644 --- a/dbus-service.c +++ b/dbus-service.c @@ -636,7 +636,7 @@ gboolean notify_volume_callback(ShairportSync *skeleton, debug(2, ">> setting volume to %7.4f.", iv); pthread_rwlock_rdlock(&principal_conn_lock); // don't let the principal_conn be changed - pthread_cleanup_push(rwlock_unlock,(void *)&principal_conn_lock); + pthread_cleanup_push(rwlock_unlock, (void *)&principal_conn_lock); if (principal_conn != NULL) { player_volume(iv, principal_conn); diff --git a/nqptp-shm-structures.h b/nqptp-shm-structures.h index 85b2c9ddd..feeed5a15 100644 --- a/nqptp-shm-structures.h +++ b/nqptp-shm-structures.h @@ -63,7 +63,7 @@ typedef struct { } shm_structure_set; // The actual interface comprises a shared memory region of type struct shm_structure. -// This comprises two records of type shm_structure_set. +// This comprises two records of type shm_structure_set. // The secondary record is written strictly after all writes to the main record are // complete. This is ensured using the __sync_synchronize() construct. // The reader should ensure that both copies match for a read to be valid. diff --git a/rtsp.c b/rtsp.c index 5469a571c..ebf4667e6 100644 --- a/rtsp.c +++ b/rtsp.c @@ -141,7 +141,6 @@ rtsp_conn_info **conns; int metadata_running = 0; - // always lock this when accessing the principal conn value // use a read lock when consulting and holding it // use a write lock if you want to change it @@ -541,7 +540,7 @@ void cancel_all_RTSP_threads(airplay_stream_c stream_category, int except_this_o (stream_category == conns[i]->airplay_stream_category))) { pthread_join(conns[i]->thread, NULL); debug(1, "Connection %d: joined.", conns[i]->connection_number); - + free(conns[i]); conns[i] = NULL; } @@ -566,7 +565,7 @@ void cancel_all_RTSP_threads(airplay_stream_c stream_category, int except_this_o void release_play_lock(rtsp_conn_info *conn) { // no need thread cancellation points in here pthread_rwlock_wrlock(&principal_conn_lock); - if (principal_conn == conn) { // if we have the player + if (principal_conn == conn) { // if we have the player if (conn != NULL) debug(2, "Connection %d: principal_conn released.", conn->connection_number); principal_conn = NULL; // let it go @@ -579,7 +578,7 @@ void release_play_lock(rtsp_conn_info *conn) { int get_play_lock(rtsp_conn_info *conn, int allow_session_interruption) { int response = 0; pthread_rwlock_wrlock(&principal_conn_lock); - pthread_cleanup_push(rwlock_unlock,(void *)&principal_conn_lock); + pthread_cleanup_push(rwlock_unlock, (void *)&principal_conn_lock); if (principal_conn != NULL) debug(1, "Connection %d: is requested to relinquish principal_conn.", principal_conn->connection_number); @@ -781,7 +780,7 @@ rtsp_message *msg_init(void) { die("msg_init -- can not allocate memory for rtsp_message %d.", msg_indexes); } // debug(1,"msg_init -- create item %d.", msg->index_number); - + rc = pthread_mutex_unlock(&reference_counter_lock); if (rc) debug(1, "Error %d unlocking reference counter lock"); @@ -1717,14 +1716,10 @@ void handle_get_info(__attribute((unused)) rtsp_conn_info *conn, rtsp_message *r } else { void *qualifier_response_data = NULL; size_t qualifier_response_data_length = 0; - - + pthread_rwlock_rdlock(&principal_conn_lock); // don't let the principal_conn be changed - pthread_cleanup_push(rwlock_unlock,(void *)&principal_conn_lock); + pthread_cleanup_push(rwlock_unlock, (void *)&principal_conn_lock); - - - if (add_pstring_to_malloc("acl=0", &qualifier_response_data, &qualifier_response_data_length) == 0) debug(1, "Problem"); @@ -2738,10 +2733,10 @@ void handle_teardown_2(rtsp_conn_info *conn, __attribute__((unused)) rtsp_messag get_category_string(conn->airplay_stream_category)); teardown_phase_one(conn); // try to do phase one anyway teardown_phase_two(conn); - + // only update these things if you're (still) the principal conn pthread_rwlock_rdlock(&principal_conn_lock); // don't let the principal_conn be changed - pthread_cleanup_push(rwlock_unlock,(void *)&principal_conn_lock); + pthread_cleanup_push(rwlock_unlock, (void *)&principal_conn_lock); if ((principal_conn == conn) && (conn->airplay_stream_category == ptp_stream)) { config.airplay_statusflags &= (0xffffffff - (1 << 11)); // DeviceSupportsRelay build_bonjour_strings(conn); @@ -2789,7 +2784,7 @@ void handle_teardown(rtsp_conn_info *conn, __attribute__((unused)) rtsp_message #ifdef CONFIG_AIRPLAY_2 // only update these things if you're (still) the principal conn pthread_rwlock_rdlock(&principal_conn_lock); // don't let the principal_conn be changed - pthread_cleanup_push(rwlock_unlock,(void *)&principal_conn_lock); + pthread_cleanup_push(rwlock_unlock, (void *)&principal_conn_lock); if (principal_conn == conn) { config.airplay_statusflags &= (0xffffffff - (1 << 11)); // DeviceSupportsRelay build_bonjour_strings(conn); @@ -2797,7 +2792,7 @@ void handle_teardown(rtsp_conn_info *conn, __attribute__((unused)) rtsp_message } pthread_cleanup_pop(1); // release the principal_conn lock #endif - + release_play_lock(conn); resp->respcode = 200; @@ -3120,14 +3115,15 @@ void handle_setup_2(rtsp_conn_info *conn, rtsp_message *req, rtsp_message *resp) listeners */ // only update these things if you're (still) the principal conn - pthread_rwlock_wrlock(&principal_conn_lock); // don't let the principal_conn be changed - pthread_cleanup_push(rwlock_unlock,(void *)&principal_conn_lock); + pthread_rwlock_wrlock( + &principal_conn_lock); // don't let the principal_conn be changed + pthread_cleanup_push(rwlock_unlock, (void *)&principal_conn_lock); if (principal_conn == conn) { - config.airplay_statusflags |= 1 << 11; // DeviceSupportsRelay - build_bonjour_strings(conn); - debug(2, "Connection %d: SETUP mdns_update on %s.", conn->connection_number, - get_category_string(conn->airplay_stream_category)); - mdns_update(NULL, secondary_txt_records); + config.airplay_statusflags |= 1 << 11; // DeviceSupportsRelay + build_bonjour_strings(conn); + debug(2, "Connection %d: SETUP mdns_update on %s.", conn->connection_number, + get_category_string(conn->airplay_stream_category)); + mdns_update(NULL, secondary_txt_records); } pthread_cleanup_pop(1); // release the principal_conn lock @@ -3625,7 +3621,7 @@ void handle_set_parameter_parameter(rtsp_conn_info *conn, rtsp_message *req, } else { #endif pthread_rwlock_rdlock(&principal_conn_lock); // don't let the principal_conn be changed - pthread_cleanup_push(rwlock_unlock,(void *)&principal_conn_lock); + pthread_cleanup_push(rwlock_unlock, (void *)&principal_conn_lock); if (principal_conn == conn) { player_volume(volume, conn); } @@ -4531,10 +4527,10 @@ static void handle_announce(rtsp_conn_info *conn, rtsp_message *req, rtsp_messag free(conn->airplay_gid); conn->airplay_gid = NULL; } - + // only update these things if you're (still) the principal conn pthread_rwlock_rdlock(&principal_conn_lock); // don't let the principal_conn be changed - pthread_cleanup_push(rwlock_unlock,(void *)&principal_conn_lock); + pthread_cleanup_push(rwlock_unlock, (void *)&principal_conn_lock); if (principal_conn == conn) { config.airplay_statusflags |= 1 << 11; // DeviceSupportsRelay -- should this be on? build_bonjour_strings(conn); @@ -4544,7 +4540,7 @@ static void handle_announce(rtsp_conn_info *conn, rtsp_message *req, rtsp_messag debug(1, "Connection %d: Classic AirPlay connection from %s:%u to self at %s:%u.", conn->connection_number, conn->client_ip_string, conn->client_rtsp_port, - conn->self_ip_string, conn->self_rtsp_port); + conn->self_ip_string, conn->self_rtsp_port); #endif conn->stream.type = ast_unknown; @@ -5221,9 +5217,8 @@ static void *rtsp_conversation_thread_func(void *pconn) { while (conn->stop == 0) { int debug_level = 2; // for printing the request and response - -// check to see if a conn has been zeroed + // check to see if a conn has been zeroed debug_mutex_lock(&conns_lock, 1000000, 3); int i; @@ -5233,7 +5228,7 @@ static void *rtsp_conversation_thread_func(void *pconn) { } } debug_mutex_unlock(&conns_lock, 3); - + reply = rtsp_read_request(conn, &req); if (reply == rtsp_read_request_response_ok) { pthread_cleanup_push(msg_cleanup_function, (void *)&req); @@ -5580,9 +5575,8 @@ void *rtsp_listen_loop(__attribute((unused)) void *arg) { if (acceptfd < 0) // timeout continue; - int release_conn = 1; // on exit, deallocate the buffer unless everything was okay - - + int release_conn = 1; // on exit, deallocate the buffer unless everything was okay + rtsp_conn_info *conn = malloc(sizeof(rtsp_conn_info)); if (conn == 0) die("Couldn't allocate memory for an rtsp_conn_info record."); @@ -5601,7 +5595,7 @@ void *rtsp_listen_loop(__attribute((unused)) void *arg) { debug(1, "Connection %d: New connection on port %d not accepted:", conn->connection_number, config.port); perror("failed to accept connection"); - } else { + } else { size_of_reply = sizeof(SOCKADDR); if (getsockname(conn->fd, (struct sockaddr *)&conn->local, &size_of_reply) == 0) { From 91c0803d04da6455ab6bcf80a4e8e89bcb644ad5 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Thu, 21 Sep 2023 17:28:03 +0100 Subject: [PATCH 21/61] Quieten a debug message. --- rtsp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rtsp.c b/rtsp.c index ebf4667e6..84ecad06b 100644 --- a/rtsp.c +++ b/rtsp.c @@ -2710,7 +2710,7 @@ void teardown_phase_two(rtsp_conn_info *conn) { void handle_teardown_2(rtsp_conn_info *conn, __attribute__((unused)) rtsp_message *req, rtsp_message *resp) { - debug(1, "Connection %d: TEARDOWN 2 %s.", conn->connection_number, + debug(2, "Connection %d: TEARDOWN 2 %s.", conn->connection_number, get_category_string(conn->airplay_stream_category)); debug_log_rtsp_message(2, "TEARDOWN: ", req); resp->respcode = 200; From b3986ed6e54211a90f2bde063e003fdba0b73bfa Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Fri, 22 Sep 2023 15:08:47 +0100 Subject: [PATCH 22/61] Update RELEASENOTES-DEVELOPMENT.md --- RELEASENOTES-DEVELOPMENT.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/RELEASENOTES-DEVELOPMENT.md b/RELEASENOTES-DEVELOPMENT.md index 59e939ed6..86abe9d71 100644 --- a/RELEASENOTES-DEVELOPMENT.md +++ b/RELEASENOTES-DEVELOPMENT.md @@ -1,3 +1,9 @@ +Version 4.3.2-dev-19-g91c0803d +=== +* Lock access to the `principal_conn` (i.e. the connection that may be playing) using a read-write lock rather than a mutex. Use read locking when checking that a connection is currently the principal connection before altering any system-wide values such as mDNS flags. This eliminates a number of data race conditions. +* As soon as it has been decided that the current playing session (as specified in the `principal conn`) is to be preempted, demote it from `principal_conn` status before stopping play, closing the connection, etc. +* Set the status flags correctly for a Classic Airplay session on an AirPlay 2 system. + Version 4.3.2-dev-4-g20a45def === * Bump version information. From a3f12d685ac6082d817fd1924e5b31d51acd2b2c Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Sun, 24 Sep 2023 17:22:22 +0100 Subject: [PATCH 23/61] When a connection termiates abruptly while is it the principal_conn, make sure it sets the principal_conn to NULL and cleans up the bonjour flags, if appropriate. Simplify the TEARDOWN handlers and the thress teardown functions by incporporating the above code in the teardown_phase_two (for AP2) and teardown (fpr AP1) functions. It means that closing a connection will block on the principal_conn_lock, so if you have the principal_conn_lock, closing will not complete until you release it. Maybe we need a principal_conn_acquisition_lock for that... --- rtsp.c | 69 ++++++++++++++++++++++++++++------------------------------ 1 file changed, 33 insertions(+), 36 deletions(-) diff --git a/rtsp.c b/rtsp.c index 84ecad06b..e81e398ae 100644 --- a/rtsp.c +++ b/rtsp.c @@ -596,6 +596,9 @@ int get_play_lock(rtsp_conn_info *conn, int allow_session_interruption) { rtsp_conn_info *previous_principal_conn = principal_conn; principal_conn = NULL; // no longer the principal conn pthread_cancel(previous_principal_conn->thread); + // the previous principal thread will block on the principal conn lock when exiting + // so it's important not to wait for it here, e.g. don't put in a pthread_join here. + // threads are garbage-collected later usleep(1000000); // don't know why this delay is needed. principal_conn = conn; // make the conn the new principal_conn response = 1; // interrupted an existing session @@ -2705,6 +2708,23 @@ void teardown_phase_two(rtsp_conn_info *conn) { } clear_ptp_clock(); } + + // only update these things if you're (still) the principal conn + pthread_rwlock_rdlock(&principal_conn_lock); // don't let the principal_conn be changed + pthread_cleanup_push(rwlock_unlock, (void *)&principal_conn_lock); + if (principal_conn == conn) { + if (conn->airplay_stream_category == ptp_stream) { + config.airplay_statusflags &= (0xffffffff - (1 << 11)); // DeviceSupportsRelay + build_bonjour_strings(conn); + debug(2, "Connection %d: TEARDOWN mdns_update on %s.", conn->connection_number, + get_category_string(conn->airplay_stream_category)); + mdns_update(NULL, secondary_txt_records); + } + principal_conn = NULL; // stop being principal_conn + } + pthread_cleanup_pop(1); // release the principal_conn lock + debug(2, "Connection %d: TEARDOWN %s -- close the connection complete", conn->connection_number, + get_category_string(conn->airplay_stream_category)); } void handle_teardown_2(rtsp_conn_info *conn, __attribute__((unused)) rtsp_message *req, @@ -2733,23 +2753,7 @@ void handle_teardown_2(rtsp_conn_info *conn, __attribute__((unused)) rtsp_messag get_category_string(conn->airplay_stream_category)); teardown_phase_one(conn); // try to do phase one anyway teardown_phase_two(conn); - - // only update these things if you're (still) the principal conn - pthread_rwlock_rdlock(&principal_conn_lock); // don't let the principal_conn be changed - pthread_cleanup_push(rwlock_unlock, (void *)&principal_conn_lock); - if ((principal_conn == conn) && (conn->airplay_stream_category == ptp_stream)) { - config.airplay_statusflags &= (0xffffffff - (1 << 11)); // DeviceSupportsRelay - build_bonjour_strings(conn); - debug(2, "Connection %d: TEARDOWN mdns_update on %s.", conn->connection_number, - get_category_string(conn->airplay_stream_category)); - mdns_update(NULL, secondary_txt_records); - } - pthread_cleanup_pop(1); // release the principal_conn lock - debug(2, "Connection %d: TEARDOWN %s -- close the connection complete", - conn->connection_number, get_category_string(conn->airplay_stream_category)); - release_play_lock(conn); } - plist_free(messagePlist); resp->respcode = 200; } else { @@ -2770,40 +2774,33 @@ void teardown(rtsp_conn_info *conn) { free(conn->dacp_active_remote); conn->dacp_active_remote = NULL; } -} -void handle_teardown(rtsp_conn_info *conn, __attribute__((unused)) rtsp_message *req, - rtsp_message *resp) { - debug_log_rtsp_message(2, "TEARDOWN request", req); - debug(2, "Connection %d: TEARDOWN", conn->connection_number); - debug(3, - "TEARDOWN: synchronously terminating the player thread of RTSP conversation thread %d (2).", - conn->connection_number); - teardown(conn); - -#ifdef CONFIG_AIRPLAY_2 // only update these things if you're (still) the principal conn pthread_rwlock_rdlock(&principal_conn_lock); // don't let the principal_conn be changed pthread_cleanup_push(rwlock_unlock, (void *)&principal_conn_lock); if (principal_conn == conn) { +#ifdef CONFIG_AIRPLAY_2 config.airplay_statusflags &= (0xffffffff - (1 << 11)); // DeviceSupportsRelay build_bonjour_strings(conn); mdns_update(NULL, secondary_txt_records); +#endif + principal_conn = NULL; // stop being principal_conn } pthread_cleanup_pop(1); // release the principal_conn lock -#endif - - release_play_lock(conn); +} +void handle_teardown(rtsp_conn_info *conn, __attribute__((unused)) rtsp_message *req, + rtsp_message *resp) { + debug_log_rtsp_message(2, "TEARDOWN request", req); + debug(2, "Connection %d: TEARDOWN", conn->connection_number); + debug(3, + "TEARDOWN: synchronously terminating the player thread of RTSP conversation thread %d (2).", + conn->connection_number); + teardown(conn); resp->respcode = 200; msg_add_header(resp, "Connection", "close"); debug(3, "TEARDOWN: successful termination of playing thread of RTSP conversation thread %d.", conn->connection_number); - //} else { - // warn("Connection %d TEARDOWN received without having the player (no ANNOUNCE?)", - // conn->connection_number); - // resp->respcode = 451; - // } // debug(1,"Bogus exit for valgrind -- remember to comment it out!."); // exit(EXIT_SUCCESS); } @@ -5163,7 +5160,7 @@ void rtsp_conversation_thread_cleanup_function(void *arg) { pthread_mutex_destroy(&conn->watchdog_mutex); debug(2, "Connection %d: Closed.", conn->connection_number); - conn->running = 0; + conn->running = 0; // for the garbage collector pthread_setcancelstate(oldState, NULL); } } From d236cfc5247676bc4039735d2a3c43497ab027bd Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Sun, 24 Sep 2023 17:35:15 +0100 Subject: [PATCH 24/61] Update RELEASENOTES-DEVELOPMENT.md --- RELEASENOTES-DEVELOPMENT.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/RELEASENOTES-DEVELOPMENT.md b/RELEASENOTES-DEVELOPMENT.md index 86abe9d71..2afb61986 100644 --- a/RELEASENOTES-DEVELOPMENT.md +++ b/RELEASENOTES-DEVELOPMENT.md @@ -1,3 +1,7 @@ +Version 4.3.2-dev-21-ga3f12d68 +=== +* When a connection terminates abruptly while is it the `principal_conn`, make sure it sets the `principal_conn` to `NULL` and cleans up the Bonjour flags, if appropriate. + Version 4.3.2-dev-19-g91c0803d === * Lock access to the `principal_conn` (i.e. the connection that may be playing) using a read-write lock rather than a mutex. Use read locking when checking that a connection is currently the principal connection before altering any system-wide values such as mDNS flags. This eliminates a number of data race conditions. From c4975d5c3b959a0cd3b3c159b41bded88afd0faa Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Mon, 25 Sep 2023 09:37:47 +0100 Subject: [PATCH 25/61] Add missing arguments to some debug messages. They could potentially cause crashes just when you need them to work properly, sigh! --- rtsp.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/rtsp.c b/rtsp.c index e81e398ae..3c193d3dd 100644 --- a/rtsp.c +++ b/rtsp.c @@ -771,7 +771,7 @@ rtsp_message *msg_init(void) { // no thread cancellation points here int rc = pthread_mutex_lock(&reference_counter_lock); if (rc) - debug(1, "Error %d locking reference counter lock"); + debug(1, "Error %d locking reference counter lock", rc); rtsp_message *msg = malloc(sizeof(rtsp_message)); if (msg) { @@ -786,7 +786,7 @@ rtsp_message *msg_init(void) { rc = pthread_mutex_unlock(&reference_counter_lock); if (rc) - debug(1, "Error %d unlocking reference counter lock"); + debug(1, "Error %d unlocking reference counter lock", rc); return msg; } @@ -5221,7 +5221,7 @@ static void *rtsp_conversation_thread_func(void *pconn) { int i; for (i = 0; i < nconns; i++) { if ((conns[i] != NULL) && (conns[i]->connection_number == 0)) { - debug(1, "conns[%d] at %" PRIxPTR " has a Connection Number of 0!", conns[i]); + debug(1, "conns[%d] at %" PRIxPTR " has a Connection Number of 0!", i, conns[i]); } } debug_mutex_unlock(&conns_lock, 3); From 6bba7e0e440ca6acdd46a7633d3fb42262391ebf Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Mon, 25 Sep 2023 09:42:23 +0100 Subject: [PATCH 26/61] Update RELEASENOTES-DEVELOPMENT.md --- RELEASENOTES-DEVELOPMENT.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/RELEASENOTES-DEVELOPMENT.md b/RELEASENOTES-DEVELOPMENT.md index 2afb61986..efd9d6422 100644 --- a/RELEASENOTES-DEVELOPMENT.md +++ b/RELEASENOTES-DEVELOPMENT.md @@ -1,3 +1,7 @@ +Version 4.3.2-dev-23-gc4975d5c +=== +* Fixed some debug messages that were missing arguments. In principle, these could cause crashes just when you need them to work properly, sigh. As far as is known, however, they did not cause problems. Many thanks to [Nathan Gray](https://github.com/n8gray) for the [report](https://github.com/mikebrady/shairport-sync/issues/1734). + Version 4.3.2-dev-21-ga3f12d68 === * When a connection terminates abruptly while is it the `principal_conn`, make sure it sets the `principal_conn` to `NULL` and cleans up the Bonjour flags, if appropriate. From c04674683243b987de4a6da6760efd7416c5b224 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Mon, 25 Sep 2023 16:19:32 +0100 Subject: [PATCH 27/61] Make the pipewire latency 200000 (uS?) instead of 20000. --- audio_pw.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/audio_pw.c b/audio_pw.c index 4271ba09d..ee28fa513 100644 --- a/audio_pw.c +++ b/audio_pw.c @@ -359,11 +359,11 @@ static void start(int sample_rate, int sample_format) { data.rate = sample_rate; data.channels = 2; data.stride = spa_format_samplesize(data.format) * data.channels; - data.latency = 20000; + data.latency = 20000; // looks like microseconds nom = nearbyint((data.latency * data.rate) / 1000000.0); - pw_properties_setf(data.props, PW_KEY_NODE_LATENCY, "%u/%u", nom, data.rate); + pw_properties_setf(data.props, PW_KEY_NODE_LATENCY, "%u/%u", nom, data.rate); // looks like samples in the data.latency period debug(1, "pw: rate: %d", data.rate); debug(1, "pw: channgels: %d", data.channels); From be4887713fa1f6ce337b16d0087a8b34d6d3355c Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Mon, 25 Sep 2023 16:23:34 +0100 Subject: [PATCH 28/61] Update RELEASENOTES-DEVELOPMENT.md --- RELEASENOTES-DEVELOPMENT.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/RELEASENOTES-DEVELOPMENT.md b/RELEASENOTES-DEVELOPMENT.md index efd9d6422..e450a0484 100644 --- a/RELEASENOTES-DEVELOPMENT.md +++ b/RELEASENOTES-DEVELOPMENT.md @@ -1,3 +1,7 @@ +Version 4.3.2-dev-25-gc0467468 +=== +* Make the latency in the PipeWire backend 200,000 instead of 20,000. Should help with DAC underrun. + Version 4.3.2-dev-23-gc4975d5c === * Fixed some debug messages that were missing arguments. In principle, these could cause crashes just when you need them to work properly, sigh. As far as is known, however, they did not cause problems. Many thanks to [Nathan Gray](https://github.com/n8gray) for the [report](https://github.com/mikebrady/shairport-sync/issues/1734). From 04e3f891301f993154e8e84b7f89e53d3d693b77 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Wed, 27 Sep 2023 10:36:28 +0100 Subject: [PATCH 29/61] Very very rarely, the avahi client becomes disconnected. This usually means there are problems in the system as a whole rahter than with Shairport Sync. However, Shairport Sync was not cleaning up properly before deleting the now-disconnected avahi client and creating a new one in an attempt to reconnect. That caused Shairport Sync to crash even if the new avahi client was created successfully. So this commit has code to delete the avahi group and broswer callback before deleting the disconnected avahi client. --- mdns_avahi.c | 40 +++++++++++++++++++++++++++------------- 1 file changed, 27 insertions(+), 13 deletions(-) diff --git a/mdns_avahi.c b/mdns_avahi.c index c154ec60a..2204592ab 100644 --- a/mdns_avahi.c +++ b/mdns_avahi.c @@ -206,7 +206,7 @@ static void egroup_callback(AvahiEntryGroup *g, AvahiEntryGroupState state, debug(2, "avahi: service name collision, renaming service to '%s'", service_name); - /* And recreate the services */ + /* And try to recreate the services */ register_service(avahi_entry_group_get_client(g)); break; } @@ -230,6 +230,15 @@ static void egroup_callback(AvahiEntryGroup *g, AvahiEntryGroupState state, } } +static int deregister_service(AVAHI_GCC_UNUSED AvahiClient *c) { + int response = 0; + if (group != NULL) { + response = avahi_entry_group_free(group); + group = NULL; + } + return response; +} + static void register_service(AvahiClient *c) { if (!group) group = avahi_entry_group_new(c, egroup_callback, NULL); @@ -293,21 +302,26 @@ static void client_callback(AvahiClient *c, AvahiClientState state, case AVAHI_CLIENT_FAILURE: err = avahi_client_errno(c); - debug(1, "avahi: client failure: %s", avahi_strerror(err)); - if (err == AVAHI_ERR_DISCONNECTED) { - debug(1, "avahi client -- we have been disconnected, so let's reconnect."); - /* We have been disconnected, so lets reconnect */ - if (c) - avahi_client_free(c); - else - debug(1, "Attempt to free NULL avahi client"); - c = NULL; - group = NULL; - + debug(1, "avahi client disconnected -- reconnection attempted."); + if (c) { + // it seems that the avahi_threaded_poll thread is still running and locked here + deregister_service(c); // delete the group + dacp_browser_struct *dbs = &private_dbs; + if (dbs->service_browser) { + int rc = avahi_service_browser_free(dbs->service_browser); // delete the service browser + if (rc != 0) + debug(1, + "Error %d freeing the Avahi service browser after the Avahi client has been " + "disconnected.", + rc); + dbs->service_browser = NULL; + } + avahi_client_free(c); // delete the client + } if (!(client = avahi_client_new(avahi_threaded_poll_get(tpoll), AVAHI_CLIENT_NO_FAIL, client_callback, userdata, &err))) { - warn("avahi: failed to create client object: %s", avahi_strerror(err)); + warn("avahi: failed to create a replacement client object: %s", avahi_strerror(err)); avahi_threaded_poll_quit(tpoll); } } else { From 7fe48618742244b3dcb254b3d832e52013380de3 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Wed, 27 Sep 2023 10:53:07 +0100 Subject: [PATCH 30/61] Update RELEASENOTES-DEVELOPMENT.md --- RELEASENOTES-DEVELOPMENT.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/RELEASENOTES-DEVELOPMENT.md b/RELEASENOTES-DEVELOPMENT.md index e450a0484..5d4ce3c9e 100644 --- a/RELEASENOTES-DEVELOPMENT.md +++ b/RELEASENOTES-DEVELOPMENT.md @@ -1,3 +1,10 @@ +Version 4.3.2-dev-27-g04e3f891 +=== +**Bug Fixes** +* Fix bugs in the handling of an Avahi client disconnection event. Due to its rarity, Shairport Sync was not handling Avahi client disconnection events correctly. Specificially, it was not cleaning up before deleting the disconnected Avahi client and attempting to create a new one. This has now been fixed. + + It is worth noting that these disconnection events are very rare and often indicate that there is something wrong with the computer as a whole. + Version 4.3.2-dev-25-gc0467468 === * Make the latency in the PipeWire backend 200,000 instead of 20,000. Should help with DAC underrun. From a1dd3ad33e52254ef931545536c26cd89dbedfc9 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Wed, 27 Sep 2023 12:38:02 +0100 Subject: [PATCH 31/61] Actually _do_ change the latency to 200000 in the PipeWire backend as previously promied, duh. --- audio_pw.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/audio_pw.c b/audio_pw.c index ee28fa513..18820042e 100644 --- a/audio_pw.c +++ b/audio_pw.c @@ -359,7 +359,7 @@ static void start(int sample_rate, int sample_format) { data.rate = sample_rate; data.channels = 2; data.stride = spa_format_samplesize(data.format) * data.channels; - data.latency = 20000; // looks like microseconds + data.latency = 200000; // looks like microseconds nom = nearbyint((data.latency * data.rate) / 1000000.0); From c36d322a9195406ab66f6f710a636e4bd6603936 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Fri, 29 Sep 2023 09:41:07 +0100 Subject: [PATCH 32/61] Don't mute if the volume control is at minimum (-144.0) and ignore_volume_control is seelcted. --- player.c | 27 ++++++++++++++------------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/player.c b/player.c index 42e0b86c3..8c1752722 100644 --- a/player.c +++ b/player.c @@ -3375,20 +3375,21 @@ void player_volume_without_notification(double airplay_volume, rtsp_conn_info *c // we have to consider the settings ignore_volume_control and mute. if (airplay_volume == -144.0) { - - if ((config.output->mute) && (config.output->mute(1) == 0)) - debug(2, - "player_volume_without_notification: volume mode is %d, airplay_volume is %f, " - "hardware mute is enabled.", - volume_mode, airplay_volume); - else { - conn->software_mute_enabled = 1; - debug(2, - "player_volume_without_notification: volume mode is %d, airplay_volume is %f, " - "software mute is enabled.", - volume_mode, airplay_volume); + // only mute if you're not ignoring the volume control + if (config.ignore_volume_control == 0) { + if ((config.output->mute) && (config.output->mute(1) == 0)) + debug(2, + "player_volume_without_notification: volume mode is %d, airplay_volume is %f, " + "hardware mute is enabled.", + volume_mode, airplay_volume); + else { + conn->software_mute_enabled = 1; + debug(2, + "player_volume_without_notification: volume mode is %d, airplay_volume is %f, " + "software mute is enabled.", + volume_mode, airplay_volume); + } } - } else { int32_t max_db = 0, min_db = 0; switch (volume_mode) { From 5916d2397be6de2f96794eda2cbd76f6f0cb4ef6 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Fri, 29 Sep 2023 09:48:32 +0100 Subject: [PATCH 33/61] Update RELEASENOTES-DEVELOPMENT.md --- RELEASENOTES-DEVELOPMENT.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/RELEASENOTES-DEVELOPMENT.md b/RELEASENOTES-DEVELOPMENT.md index 5d4ce3c9e..d0555d1cf 100644 --- a/RELEASENOTES-DEVELOPMENT.md +++ b/RELEASENOTES-DEVELOPMENT.md @@ -1,3 +1,8 @@ +Version 4.3.2-dev-30-gc36d322a +=== +**Bug Fix** +* If the volume control was at its absolute minimum (-144.0), audio would be muted even if `ignore_volume_control` was enabled in the configuration file. The fix was to mute only if `ignore_volume_control` is not enabled. Thanks to [mc510](https://github.com/mc510) for reporting the [issue](https://github.com/mikebrady/shairport-sync/issues/1737). + Version 4.3.2-dev-27-g04e3f891 === **Bug Fixes** From afaf94b73584d22d4401596d1a26dc57c6279492 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Fri, 29 Sep 2023 16:32:12 +0100 Subject: [PATCH 34/61] Initial tone-generator based pipewire backend compiling and apparently working, valgrind is happy too. --- audio_pw.c | 675 +++++++++++++++++++---------------------------------- 1 file changed, 240 insertions(+), 435 deletions(-) diff --git a/audio_pw.c b/audio_pw.c index 18820042e..344371a45 100644 --- a/audio_pw.c +++ b/audio_pw.c @@ -1,6 +1,6 @@ /* - * Asynchronous Pipewire Backend. This file is part of Shairport Sync. - * Copyright (c) Shairport Sync 2021--2022 + * Asynchronous PipeWire Backend. This file is part of Shairport Sync. + * Copyright (c) Mike Brady 2017-2023 * All rights reserved. * * Permission is hereby granted, free of charge, to any person @@ -24,490 +24,295 @@ * OTHER DEALINGS IN THE SOFTWARE. */ + #include "audio.h" #include "common.h" +#include +#include +#include +#include +#include -#include -#include -#include -#include -#include - -#include - -#define PW_TIMEOUT_S 5 -#define SECONDS_TO_NANOSECONDS 1000000000L -#define PW_TIMEOUT_NS (PW_TIMEOUT_S * SECONDS_TO_NANOSECONDS) +#include // many not need this after development -struct pw_data { - struct pw_thread_loop *mainloop; - struct pw_context *context; +#include +#include - struct pw_core *core; - struct spa_hook core_listener; +// note -- these are hacked and hardwired into this code. +#define DEFAULT_FORMAT SPA_AUDIO_FORMAT_S16_LE +#define DEFAULT_RATE 44100 +#define DEFAULT_CHANNELS 2 +#define DEFAULT_VOLUME 0.7 - struct pw_registry *registry; - struct spa_hook registry_listener; +// Four seconds buffer -- should be plenty +#define buffer_allocation 44100 * 4 * 2 * 2 - struct pw_stream *stream; - struct spa_hook stream_listener; +// static pthread_mutex_t buffer_mutex = PTHREAD_MUTEX_INITIALIZER; - struct pw_buffer *pw_buffer; - - struct pw_properties *props; - int sync; +char *audio_lmb, *audio_umb, *audio_toq, *audio_eoq; +size_t audio_size = buffer_allocation; +size_t audio_occupancy; - enum spa_audio_format format; - uint32_t rate; - uint32_t channels; - uint32_t stride; - uint32_t latency; +#define M_PI_M2 (M_PI + M_PI) -} data; +uint64_t starting_time; -static void on_core_info(__attribute__((unused)) void *userdata, const struct pw_core_info *info) { - debug(1, "pw: remote %" PRIu32 " is named \"%s\"", info->id, info->name); -} - -static void on_core_error(__attribute__((unused)) void *userdata, uint32_t id, int seq, int res, - const char *message) { - warn("pw: remote error: id=%" PRIu32 " seq:%d res:%d (%s): %s", id, seq, res, spa_strerror(res), - message); -} +struct data { + struct pw_thread_loop *loop; + struct pw_stream *stream; -static const struct pw_core_events core_events = { - PW_VERSION_CORE_EVENTS, - .info = on_core_info, - .error = on_core_error, + double accumulator; }; -static void registry_event_global(__attribute__((unused)) void *userdata, uint32_t id, - __attribute__((unused)) uint32_t permissions, const char *type, - __attribute__((unused)) uint32_t version, - const struct spa_dict *props) { - const struct spa_dict_item *item; - const char *name, *media_class; - - if (strcmp(type, PW_TYPE_INTERFACE_Node) == 0) { - name = spa_dict_lookup(props, PW_KEY_NODE_NAME); - media_class = spa_dict_lookup(props, PW_KEY_MEDIA_CLASS); - - if (!name || !media_class) - return; - - debug(1, "pw: registry: id=%" PRIu32 " type=%s name=\"%s\" media_class=\"%s\"", id, type, name, - media_class); - - spa_dict_for_each(item, props) { debug(1, "pw: \t\t%s = \"%s\"", item->key, item->value); } - } -} - -static void registry_event_global_remove(__attribute__((unused)) void *userdata, uint32_t id) { - debug(1, "pw: registry: remove id=%" PRIu32 "", id); -} - -static const struct pw_registry_events registry_events = { - PW_VERSION_REGISTRY_EVENTS, - .global = registry_event_global, - .global_remove = registry_event_global_remove, +// the pipewire global data structure +struct data data = { + 0, }; -static void on_state_changed(void *userdata, enum pw_stream_state old, enum pw_stream_state state, - const char *error) { - struct pw_data *pipewire = userdata; - - debug(1, "pw: stream state changed %s -> %s", pw_stream_state_as_string(old), - pw_stream_state_as_string(state)); - - if (state == PW_STREAM_STATE_STREAMING) - debug(1, "pw: stream node %" PRIu32 "", pw_stream_get_node_id(pipewire->stream)); - - if (state == PW_STREAM_STATE_ERROR) - debug(1, "pw: stream node %" PRIu32 " error: %s", pw_stream_get_node_id(pipewire->stream), - error); - - pw_thread_loop_signal(pipewire->mainloop, 0); +static void fill_le16(struct data *d, void *dest, int n_frames) { + //float *dst = dest, val; + float val; + int16_t *dst = dest, le16val; + int i, c; + + for (i = 0; i < n_frames; i++) { + d->accumulator += M_PI_M2 * 440 / DEFAULT_RATE; + if (d->accumulator >= M_PI_M2) + d->accumulator -= M_PI_M2; + + val = sin(d->accumulator) * DEFAULT_VOLUME; + le16val = INT16_MAX * val; + for (c = 0; c < DEFAULT_CHANNELS; c++) + *dst++ = le16val; + } } +/* our data processing function is in general: + * + * struct pw_buffer *b; + * b = pw_stream_dequeue_buffer(stream); + * + * .. generate stuff in the buffer ... + * + * pw_stream_queue_buffer(stream, b); + */ static void on_process(void *userdata) { - struct pw_data *pipewire = userdata; + int wait; + do { + uint64_t time_now = get_absolute_time_in_ns(); + int64_t elapsed_time = time_now - starting_time; + double elapsed_time_seconds = fmod(elapsed_time * 0.000000001, 10.0); + wait = (elapsed_time_seconds > 4.0) && (elapsed_time_seconds < 6.0); + if (wait != 0) { + // debug(1, "wait..."); + usleep(1000); + } + } while (wait != 0); + struct data *data = userdata; + struct pw_buffer *b; + struct spa_buffer *buf; + int n_frames, stride; + uint8_t *p; + + if ((b = pw_stream_dequeue_buffer(data->stream)) == NULL) { + pw_log_warn("out of buffers: %m"); + return; + } + + struct pw_time time_info; + memset(&time_info, 0, sizeof(time_info)); + + int response = pw_stream_get_time_n(data->stream, &time_info, sizeof(time_info)); + if (response == 0) { + struct timespec ts; + clock_gettime(CLOCK_MONOTONIC, &ts); + int64_t diff = SPA_TIMESPEC_TO_NSEC(&ts) - time_info.now; + int64_t elapsed = (time_info.rate.denom * diff) / (time_info.rate.num * SPA_NSEC_PER_SEC); + debug(1, "rate.num: %" PRId64 ", rate.denom: %" PRId64 ", diff: %" PRId64 "ns, %" PRId64 " frames, delay: %" PRId64 ", queued: %" PRId64 ", buffered: %" PRId64 ".", time_info.rate.num, time_info.rate.denom, diff, elapsed, time_info.delay, time_info.queued, time_info.buffered); + } else { + debug(1, "can't get time info: %d.", response); + } - pw_thread_loop_signal(pipewire->mainloop, 0); -} + buf = b->buffer; + if ((p = buf->datas[0].data) == NULL) + return; -static void on_drained(void *userdata) { - struct pw_data *pipewire = userdata; + stride = sizeof(int16_t) * DEFAULT_CHANNELS; + n_frames = SPA_MIN(b->requested, buf->datas[0].maxsize / stride); - pw_stream_set_active(pipewire->stream, false); + fill_le16(data, p, n_frames); - pw_thread_loop_signal(pipewire->mainloop, 0); + buf->datas[0].chunk->offset = 0; + buf->datas[0].chunk->stride = stride; + buf->datas[0].chunk->size = n_frames * stride; + pw_stream_queue_buffer(data->stream, b); } static const struct pw_stream_events stream_events = { PW_VERSION_STREAM_EVENTS, - .state_changed = on_state_changed, .process = on_process, - .drained = on_drained, }; -static void deinit() { - pw_thread_loop_stop(data.mainloop); - - if (data.stream) { - pw_stream_destroy(data.stream); - data.stream = NULL; - } - - if (data.registry) { - pw_proxy_destroy((struct pw_proxy *)data.registry); - data.registry = NULL; - } - - if (data.core) { - pw_core_disconnect(data.core); - data.core = NULL; - } - - if (data.context) { - pw_context_destroy(data.context); - data.context = NULL; - } - - if (data.mainloop) { - pw_thread_loop_destroy(data.mainloop); - data.mainloop = NULL; - } - - if (data.props) { - pw_properties_free(data.props); - data.props = NULL; - } - - pthread_setcancelstate(PTHREAD_CANCEL_ENABLE, NULL); -} - static int init(__attribute__((unused)) int argc, __attribute__((unused)) char **argv) { - - struct pw_loop *loop; - struct pw_properties *props; - + debug(1, "pw_init"); // set up default values first config.audio_backend_buffer_desired_length = 0.35; - config.audio_backend_buffer_interpolation_threshold_in_seconds = 0.02; - config.audio_backend_latency_offset = 0; + config.audio_backend_buffer_interpolation_threshold_in_seconds = + 0.02; // below this, soxr interpolation will not occur -- it'll be basic interpolation + // instead. - pw_init(NULL, NULL); - - debug(1, "pw: compiled with libpipewire %s", pw_get_headers_version()); - debug(1, "pw: linked with libpipewire: %s", pw_get_library_version()); - - data.props = pw_properties_new(PW_KEY_MEDIA_TYPE, "Audio", PW_KEY_MEDIA_CATEGORY, "Playback", - PW_KEY_MEDIA_ROLE, "Music", PW_KEY_APP_NAME, "shairport-sync", - PW_KEY_NODE_NAME, "shairport-sync", NULL); - - if (!data.props) { - deinit(); - die("pw: pw_properties_new() failed: %m"); - } - - data.mainloop = pw_thread_loop_new("pipewire", NULL); - if (!data.mainloop) { - deinit(); - die("pw: pw_thread_loop_new_full() failed: %m"); - } - - props = pw_properties_new(PW_KEY_CONFIG_NAME, "client-rt.conf", NULL); - if (!props) { - deinit(); - die("pw: pw_properties_new() failed: %m"); - } - - loop = pw_thread_loop_get_loop(data.mainloop); - - data.context = pw_context_new(loop, props, 0); - if (!data.context) { - deinit(); - die("pw: pw_context_new() failed: %m"); - } - - props = pw_properties_new(PW_KEY_REMOTE_NAME, NULL, NULL); - if (!props) { - deinit(); - die("pw: pw_properties_new() failed: %m"); - } - - pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, NULL); - pw_thread_loop_lock(data.mainloop); - - if (pw_thread_loop_start(data.mainloop) != 0) { - deinit(); - die("pw: pw_thread_loop_start() failed: %m"); - } + config.audio_backend_latency_offset = 0; - data.core = pw_context_connect(data.context, props, 0); - if (!data.core) { - deinit(); - die("pw: pw_context_connect() failed: %m"); - } + // get settings from settings file - pw_core_add_listener(data.core, &data.core_listener, &core_events, &data); + // do the "general" audio options. Note, these options are in the "general" stanza! + parse_general_audio_options(); - data.registry = pw_core_get_registry(data.core, PW_VERSION_REGISTRY, 0); - if (!data.registry) { - deinit(); - die("pw: pw_core_get_registry() failed: %m"); +/* + // now any PipeWire-specific options + if (config.cfg != NULL) { + const char *str; } +*/ + // finished collecting settings - pw_registry_add_listener(data.registry, &data.registry_listener, ®istry_events, &data); - - data.sync = pw_core_sync(data.core, 0, data.sync); - - pw_thread_loop_unlock(data.mainloop); - pthread_setcancelstate(PTHREAD_CANCEL_ENABLE, NULL); + // allocate space for the audio buffer + audio_lmb = malloc(audio_size); + if (audio_lmb == NULL) + die("Can't allocate %d bytes for pulseaudio buffer.", audio_size); + audio_toq = audio_eoq = audio_lmb; + audio_umb = audio_lmb + audio_size; + audio_occupancy = 0; + const struct spa_pod *params[1]; + uint8_t buffer[1024]; + struct pw_properties *props; + struct spa_pod_builder b = SPA_POD_BUILDER_INIT(buffer, sizeof(buffer)); + + int largc = 0; + pw_init(&largc, NULL); + + /* make a main loop. If you already have another main loop, you can add + * the fd of this pipewire mainloop to it. */ + data.loop = pw_thread_loop_new("tone-generator", NULL); + + pw_thread_loop_lock(data.loop); + + pw_thread_loop_start(data.loop); + + /* Create a simple stream, the simple stream manages the core and remote + * objects for you if you don't need to deal with them. + * + * If you plan to autoconnect your stream, you need to provide at least + * media, category and role properties. + * + * Pass your events and a user_data pointer as the last arguments. This + * will inform you about the stream state. The most important event + * you need to listen to is the process event where you need to produce + * the data. + */ + + props = pw_properties_new(PW_KEY_MEDIA_TYPE, "Audio", PW_KEY_MEDIA_CATEGORY, "Playback", + PW_KEY_MEDIA_ROLE, "Music", NULL); + if (argc > 1) + /* Set stream target if given on command line */ + pw_properties_set(props, PW_KEY_TARGET_OBJECT, argv[1]); + data.stream = pw_stream_new_simple(pw_thread_loop_get_loop(data.loop), "audio-src-tg", props, + &stream_events, &data); + + /* Make one parameter with the supported formats. The SPA_PARAM_EnumFormat + * id means that this is a format enumeration (of 1 value). */ + params[0] = spa_format_audio_raw_build(&b, SPA_PARAM_EnumFormat, + &SPA_AUDIO_INFO_RAW_INIT(.format = SPA_AUDIO_FORMAT_S16_LE, + .channels = DEFAULT_CHANNELS, + .rate = DEFAULT_RATE)); + + /* Now connect this stream. We ask that our process function is + * called in a realtime thread. */ + pw_stream_connect(data.stream, PW_DIRECTION_OUTPUT, PW_ID_ANY, + PW_STREAM_FLAG_AUTOCONNECT | PW_STREAM_FLAG_MAP_BUFFERS | + PW_STREAM_FLAG_RT_PROCESS, + params, 1); + + pw_thread_loop_unlock(data.loop); + debug(1, "pa_init done"); return 0; } -static enum spa_audio_format sps_format_to_spa_format(sps_format_t sps_format) { - switch (sps_format) { - case SPS_FORMAT_S8: - return SPA_AUDIO_FORMAT_S8; - case SPS_FORMAT_U8: - return SPA_AUDIO_FORMAT_U8; - case SPS_FORMAT_S16: - return SPA_AUDIO_FORMAT_S16; - case SPS_FORMAT_S16_LE: - return SPA_AUDIO_FORMAT_S16_LE; - case SPS_FORMAT_S16_BE: - return SPA_AUDIO_FORMAT_S16_BE; - case SPS_FORMAT_S24: - return SPA_AUDIO_FORMAT_S24_32; - case SPS_FORMAT_S24_LE: - return SPA_AUDIO_FORMAT_S24_32_LE; - case SPS_FORMAT_S24_BE: - return SPA_AUDIO_FORMAT_S24_32_BE; - case SPS_FORMAT_S24_3LE: - return SPA_AUDIO_FORMAT_S24_LE; - case SPS_FORMAT_S24_3BE: - return SPA_AUDIO_FORMAT_S24_BE; - case SPS_FORMAT_S32: - return SPA_AUDIO_FORMAT_S32; - case SPS_FORMAT_S32_LE: - return SPA_AUDIO_FORMAT_S32_LE; - case SPS_FORMAT_S32_BE: - return SPA_AUDIO_FORMAT_S32_BE; - - case SPS_FORMAT_UNKNOWN: - case SPS_FORMAT_AUTO: - case SPS_FORMAT_INVALID: - default: - return SPA_AUDIO_FORMAT_S16; - } -} - -static int spa_format_samplesize(enum spa_audio_format audio_format) { - switch (audio_format) { - case SPA_AUDIO_FORMAT_S8: - case SPA_AUDIO_FORMAT_U8: - return 1; - case SPA_AUDIO_FORMAT_S16: - return 2; - case SPA_AUDIO_FORMAT_S24: - return 3; - case SPA_AUDIO_FORMAT_S24_32: - case SPA_AUDIO_FORMAT_S32: - return 4; - default: - die("pw: unhandled spa_audio_format: %d", audio_format); - return -1; - } +static void deinit(void) { + debug (1, "pw_deinit"); + pw_thread_loop_stop(data.loop); + pw_stream_destroy(data.stream); + pw_thread_loop_destroy(data.loop); + pw_deinit(); + debug(1, "pa_deinit done"); } -static const char *spa_format_to_str(enum spa_audio_format audio_format) { - switch (audio_format) { - case SPA_AUDIO_FORMAT_U8: - return "u8"; - case SPA_AUDIO_FORMAT_S8: - return "s8"; - case SPA_AUDIO_FORMAT_S16: - return "s16"; - case SPA_AUDIO_FORMAT_S24: - case SPA_AUDIO_FORMAT_S24_32: - return "s24"; - case SPA_AUDIO_FORMAT_S32: - return "s32"; - default: - die("pw: unhandled spa_audio_format: %d", audio_format); - return "(invalid)"; +static int play(__attribute__((unused)) void *buf, int samples, __attribute__((unused)) int sample_type, + __attribute__((unused)) uint32_t timestamp, + __attribute__((unused)) uint64_t playtime) { + debug(1,"pw_play of %d samples.",samples); + /* + // copy the samples into the queue + check_pa_stream_status(stream, "audio_pw play."); + size_t bytes_to_transfer = samples * 2 * 2; + size_t space_to_end_of_buffer = audio_umb - audio_eoq; + if (space_to_end_of_buffer >= bytes_to_transfer) { + memcpy(audio_eoq, buf, bytes_to_transfer); + audio_occupancy += bytes_to_transfer; + pthread_mutex_lock(&buffer_mutex); + audio_eoq += bytes_to_transfer; + pthread_mutex_unlock(&buffer_mutex); + } else { + memcpy(audio_eoq, buf, space_to_end_of_buffer); + buf += space_to_end_of_buffer; + memcpy(audio_lmb, buf, bytes_to_transfer - space_to_end_of_buffer); + pthread_mutex_lock(&buffer_mutex); + audio_occupancy += bytes_to_transfer; + pthread_mutex_unlock(&buffer_mutex); + audio_eoq = audio_lmb + bytes_to_transfer - space_to_end_of_buffer; } + // maybe goose it if it's stopped? + */ + return 0; } -static void start(int sample_rate, int sample_format) { - - pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, NULL); - - pw_thread_loop_lock(data.mainloop); - - const struct spa_pod *params[1]; - uint8_t buffer[1024]; - struct spa_pod_builder pod_builder = SPA_POD_BUILDER_INIT(buffer, sizeof(buffer)); - struct spa_audio_info_raw info; - uint32_t nom; - int ret; - - data.format = sps_format_to_spa_format(sample_format); - data.rate = sample_rate; - data.channels = 2; - data.stride = spa_format_samplesize(data.format) * data.channels; - data.latency = 200000; // looks like microseconds - - nom = nearbyint((data.latency * data.rate) / 1000000.0); - - pw_properties_setf(data.props, PW_KEY_NODE_LATENCY, "%u/%u", nom, data.rate); // looks like samples in the data.latency period - - debug(1, "pw: rate: %d", data.rate); - debug(1, "pw: channgels: %d", data.channels); - debug(1, "pw: format: %s", spa_format_to_str(data.format)); - debug(1, "pw: samplesize: %d", spa_format_samplesize(data.format)); - debug(1, "pw: stride: %d", data.stride); - if (data.rate != 0) - debug(1, "pw: latency: %d samples (%.3fs)", nom, (double)nom / data.rate); - - info = SPA_AUDIO_INFO_RAW_INIT(.flags = SPA_AUDIO_FLAG_NONE, .format = data.format, - .rate = data.rate, .channels = data.channels); - - params[0] = spa_format_audio_raw_build(&pod_builder, SPA_PARAM_EnumFormat, &info); - - data.stream = pw_stream_new(data.core, "shairport-sync", data.props); - - if (!data.stream) { - deinit(); - die("pw: pw_stream_new() failed: %m"); - } - - debug(1, "pw: connecting stream: target_id=%" PRIu32 "", PW_ID_ANY); - - pw_stream_add_listener(data.stream, &data.stream_listener, &stream_events, &data); - - ret = pw_stream_connect( - data.stream, PW_DIRECTION_OUTPUT, PW_ID_ANY, - PW_STREAM_FLAG_INACTIVE | PW_STREAM_FLAG_AUTOCONNECT | PW_STREAM_FLAG_MAP_BUFFERS, params, 1); - - if (ret < 0) { - deinit(); - die("pw: pw_stream_connect() failed: %s", spa_strerror(ret)); - } - - const struct pw_properties *props; - void *pstate; - const char *key, *val; - - if ((props = pw_stream_get_properties(data.stream)) != NULL) { - debug(1, "pw: stream properties:"); - pstate = NULL; - while ((key = pw_properties_iterate(props, &pstate)) != NULL && - (val = pw_properties_get(props, key)) != NULL) { - debug(1, "pw: \t%s = \"%s\"", key, val); - } - } - - while (1) { - enum pw_stream_state stream_state = pw_stream_get_state(data.stream, NULL); - if (stream_state == PW_STREAM_STATE_PAUSED) - break; - - struct timespec abstime; - - pw_thread_loop_get_time(data.mainloop, &abstime, PW_TIMEOUT_NS); - - ret = pw_thread_loop_timed_wait_full(data.mainloop, &abstime); - if (ret == -ETIMEDOUT) { - deinit(); - die("pw: pw_thread_loop_timed_wait_full timed out: %s", strerror(ret)); - } +/* +int pa_delay(long *the_delay) { + check_pa_stream_status(stream, "audio_pa delay."); + // debug(1,"pa_delay"); + long result = 0; + int reply = 0; + pa_usec_t latency; + int negative; + pa_threaded_mainloop_lock(mainloop); + int gl = pa_stream_get_latency(stream, &latency, &negative); + pa_threaded_mainloop_unlock(mainloop); + if (gl == PA_ERR_NODATA) { + // debug(1, "No latency data yet."); + reply = -ENODEV; + } else if (gl != 0) { + // debug(1,"Error %d getting latency.",gl); + reply = -EIO; + } else { + result = (audio_occupancy / (2 * 2)) + (latency * 44100) / 1000000; + reply = 0; } - - pw_thread_loop_unlock(data.mainloop); - - pthread_setcancelstate(PTHREAD_CANCEL_ENABLE, NULL); -} - -static void stop() { - pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, NULL); - pw_thread_loop_lock(data.mainloop); - - pw_stream_flush(data.stream, true); - - pw_thread_loop_unlock(data.mainloop); - pthread_setcancelstate(PTHREAD_CANCEL_ENABLE, NULL); + *the_delay = result; + return reply; } +*/ -static void flush() { - pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, NULL); - pw_thread_loop_lock(data.mainloop); - - pw_stream_flush(data.stream, false); - - pw_thread_loop_unlock(data.mainloop); - pthread_setcancelstate(PTHREAD_CANCEL_ENABLE, NULL); +void flush(void) { + audio_toq = audio_eoq = audio_lmb; + audio_umb = audio_lmb + audio_size; + audio_occupancy = 0; } -static int play(void *buf, int samples, __attribute__((unused)) int sample_type, - __attribute__((unused)) uint32_t timestamp, - __attribute__((unused)) uint64_t playtime) { - struct pw_buffer *pw_buffer = NULL; - struct spa_buffer *spa_buffer; - struct spa_data *spa_data; - int ret; - - pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, NULL); - pw_thread_loop_lock(data.mainloop); - - if (pw_stream_get_state(data.stream, NULL) == PW_STREAM_STATE_PAUSED) - pw_stream_set_active(data.stream, true); - - while (pw_buffer == NULL) { - pw_buffer = pw_stream_dequeue_buffer(data.stream); - if (pw_buffer) - break; - - struct timespec abstime; - - pw_thread_loop_get_time(data.mainloop, &abstime, PW_TIMEOUT_NS); - - ret = pw_thread_loop_timed_wait_full(data.mainloop, &abstime); - if (ret == -ETIMEDOUT) { - pw_thread_loop_unlock(data.mainloop); - pthread_setcancelstate(PTHREAD_CANCEL_ENABLE, NULL); - return ret; - } - } - - spa_buffer = pw_buffer->buffer; - spa_data = &spa_buffer->datas[0]; - - size_t bytes_to_copy = samples * data.stride; - - debug(3, "pw: bytes_to_copy: %d", bytes_to_copy); - - if (spa_data->maxsize < bytes_to_copy) - bytes_to_copy = spa_data->maxsize; - - debug(3, "pw: spa_data->maxsize: %d", spa_data->maxsize); - - memcpy(spa_data->data, buf, bytes_to_copy); - - spa_data->chunk->offset = 0; - spa_data->chunk->stride = data.stride; - spa_data->chunk->size = bytes_to_copy; - - pw_stream_queue_buffer(data.stream, pw_buffer); - - pw_thread_loop_unlock(data.mainloop); - - pthread_setcancelstate(PTHREAD_CANCEL_ENABLE, NULL); - - return 0; +static void stop(void) { + audio_toq = audio_eoq = audio_lmb; + audio_umb = audio_lmb + audio_size; + audio_occupancy = 0; } audio_output audio_pw = {.name = "pw", @@ -515,7 +320,7 @@ audio_output audio_pw = {.name = "pw", .init = &init, .deinit = &deinit, .prepare = NULL, - .start = &start, + .start = NULL, .stop = &stop, .is_running = NULL, .flush = &flush, @@ -524,4 +329,4 @@ audio_output audio_pw = {.name = "pw", .play = &play, .volume = NULL, .parameters = NULL, - .mute = NULL}; \ No newline at end of file + .mute = NULL}; From ccddc37c1168165c8463fcb158bc6a3ad9777a33 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Sat, 30 Sep 2023 15:40:40 +0100 Subject: [PATCH 35/61] Fix potential bug if the timing data is inconsistent for 10 tries. --- ptp-utilities.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ptp-utilities.c b/ptp-utilities.c index 34184c146..958f65420 100644 --- a/ptp-utilities.c +++ b/ptp-utilities.c @@ -86,7 +86,7 @@ int get_nqptp_data(struct shm_structure *nqptp_data) { } } while ( (memcmp(&nqptp_data->main, &local_nqptp_data.secondary, sizeof(shm_structure_set)) != 0) && - (loop_count < 100)); + (loop_count < 10)); if (loop_count == 10) { debug(1, "get_nqptp_data -- main and secondary records don't match after %d attempts!", loop_count); From 786481d855c2f63165d492fdacc255e3d2fe0847 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Sat, 30 Sep 2023 15:41:40 +0100 Subject: [PATCH 36/61] mutex lock was protecting the wrong variable, duh. --- audio_pa.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/audio_pa.c b/audio_pa.c index 1ba82b343..bd13cfefd 100644 --- a/audio_pa.c +++ b/audio_pa.c @@ -262,10 +262,10 @@ static int play(void *buf, int samples, __attribute__((unused)) int sample_type, size_t space_to_end_of_buffer = audio_umb - audio_eoq; if (space_to_end_of_buffer >= bytes_to_transfer) { memcpy(audio_eoq, buf, bytes_to_transfer); - audio_occupancy += bytes_to_transfer; pthread_mutex_lock(&buffer_mutex); - audio_eoq += bytes_to_transfer; + audio_occupancy += bytes_to_transfer; pthread_mutex_unlock(&buffer_mutex); + audio_eoq += bytes_to_transfer; } else { memcpy(audio_eoq, buf, space_to_end_of_buffer); buf += space_to_end_of_buffer; From 27e4ba09a1bb07defabb9e6d2e68e76312e0ac33 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Sat, 30 Sep 2023 15:45:41 +0100 Subject: [PATCH 37/61] Replace the pipewire backend completely. Play writes to a buffer. The on_process() function reads from the buffer and updates timing information for the delay() function. Barebones -- can't set the app name or the volume. Assumes no further delays and no buffers when on_process is called. But it works! --- audio_pw.c | 240 +++++++++++++++++++++++++++++++---------------------- 1 file changed, 142 insertions(+), 98 deletions(-) diff --git a/audio_pw.c b/audio_pw.c index 344371a45..33b27c7e8 100644 --- a/audio_pw.c +++ b/audio_pw.c @@ -24,7 +24,6 @@ * OTHER DEALINGS IN THE SOFTWARE. */ - #include "audio.h" #include "common.h" #include @@ -33,10 +32,8 @@ #include #include -#include // many not need this after development - -#include #include +#include // note -- these are hacked and hardwired into this code. #define DEFAULT_FORMAT SPA_AUDIO_FORMAT_S16_LE @@ -47,16 +44,25 @@ // Four seconds buffer -- should be plenty #define buffer_allocation 44100 * 4 * 2 * 2 -// static pthread_mutex_t buffer_mutex = PTHREAD_MUTEX_INITIALIZER; +static pthread_mutex_t buffer_mutex = PTHREAD_MUTEX_INITIALIZER; -char *audio_lmb, *audio_umb, *audio_toq, *audio_eoq; -size_t audio_size = buffer_allocation; -size_t audio_occupancy; - -#define M_PI_M2 (M_PI + M_PI) +static char *audio_lmb, *audio_umb, *audio_toq, *audio_eoq; +static size_t audio_size = buffer_allocation; +static size_t audio_occupancy; uint64_t starting_time; +struct timing_data { + int pw_time_is_valid; //set when the pw_time has been set + struct pw_time time_info; // information about the last time a process callback occurred + size_t frames; // the number of frames sent at that time +}; + +// to avoid using a mutex, write the same data twice and check they are the same +// to ensure they are consistent. Make sure the first is written strictly before the second +// using __sync_synchronize(); +struct timing_data timing_data_1, timing_data_2; + struct data { struct pw_thread_loop *loop; struct pw_stream *stream; @@ -69,22 +75,34 @@ struct data data = { 0, }; -static void fill_le16(struct data *d, void *dest, int n_frames) { - //float *dst = dest, val; - float val; - int16_t *dst = dest, le16val; - int i, c; - - for (i = 0; i < n_frames; i++) { - d->accumulator += M_PI_M2 * 440 / DEFAULT_RATE; - if (d->accumulator >= M_PI_M2) - d->accumulator -= M_PI_M2; - - val = sin(d->accumulator) * DEFAULT_VOLUME; - le16val = INT16_MAX * val; - for (c = 0; c < DEFAULT_CHANNELS; c++) - *dst++ = le16val; +static int fill(void *dest, int max_frames, int stride) { + size_t bytes_we_can_transfer = max_frames * stride; + pthread_mutex_lock(&buffer_mutex); + if (bytes_we_can_transfer > audio_occupancy) + bytes_we_can_transfer = audio_occupancy; + pthread_mutex_unlock(&buffer_mutex); + if (bytes_we_can_transfer > 0) { + size_t bytes_to_end_of_buffer = (size_t)(audio_umb - audio_toq); // must be zero or positive + if (bytes_we_can_transfer <= bytes_to_end_of_buffer) { + // the bytes are all in a row in the audio buffer + memcpy(dest, audio_toq, bytes_we_can_transfer); + audio_toq += bytes_we_can_transfer; + } else { + // the bytes are in two places in the audio buffer + size_t first_portion_to_write = audio_umb - audio_toq; + if (first_portion_to_write != 0) + memcpy(dest, audio_toq, first_portion_to_write); + uint8_t *new_dest = dest + first_portion_to_write; + memcpy(new_dest, audio_lmb, bytes_we_can_transfer - first_portion_to_write); + audio_toq = audio_lmb + bytes_we_can_transfer - first_portion_to_write; + } + // lock + pthread_mutex_lock(&buffer_mutex); + audio_occupancy -= bytes_we_can_transfer; + pthread_mutex_unlock(&buffer_mutex); + // unlock } + return bytes_we_can_transfer / stride; // back to numbers of frames } /* our data processing function is in general: @@ -97,57 +115,52 @@ static void fill_le16(struct data *d, void *dest, int n_frames) { * pw_stream_queue_buffer(stream, b); */ static void on_process(void *userdata) { - int wait; - do { - uint64_t time_now = get_absolute_time_in_ns(); - int64_t elapsed_time = time_now - starting_time; - double elapsed_time_seconds = fmod(elapsed_time * 0.000000001, 10.0); - wait = (elapsed_time_seconds > 4.0) && (elapsed_time_seconds < 6.0); - if (wait != 0) { - // debug(1, "wait..."); - usleep(1000); - } - } while (wait != 0); struct data *data = userdata; + + struct pw_time time_info; + memset(&time_info, 0, sizeof(time_info)); + struct pw_buffer *b; struct spa_buffer *buf; - int n_frames, stride; + int max_possible_frames, n_frames, stride; uint8_t *p; if ((b = pw_stream_dequeue_buffer(data->stream)) == NULL) { pw_log_warn("out of buffers: %m"); return; } - - struct pw_time time_info; - memset(&time_info, 0, sizeof(time_info)); - - int response = pw_stream_get_time_n(data->stream, &time_info, sizeof(time_info)); - if (response == 0) { - struct timespec ts; - clock_gettime(CLOCK_MONOTONIC, &ts); - int64_t diff = SPA_TIMESPEC_TO_NSEC(&ts) - time_info.now; - int64_t elapsed = (time_info.rate.denom * diff) / (time_info.rate.num * SPA_NSEC_PER_SEC); - debug(1, "rate.num: %" PRId64 ", rate.denom: %" PRId64 ", diff: %" PRId64 "ns, %" PRId64 " frames, delay: %" PRId64 ", queued: %" PRId64 ", buffered: %" PRId64 ".", time_info.rate.num, time_info.rate.denom, diff, elapsed, time_info.delay, time_info.queued, time_info.buffered); - } else { - debug(1, "can't get time info: %d.", response); - } buf = b->buffer; - if ((p = buf->datas[0].data) == NULL) + if ((p = buf->datas[0].data) == NULL) // the first data block does not contain a data pointer return; stride = sizeof(int16_t) * DEFAULT_CHANNELS; - n_frames = SPA_MIN(b->requested, buf->datas[0].maxsize / stride); - - fill_le16(data, p, n_frames); + max_possible_frames = SPA_MIN(b->requested, buf->datas[0].maxsize / stride); + + do { + n_frames = fill(p, max_possible_frames, stride); + if (n_frames == 0) { + usleep(1000); + } + } while(n_frames == 0); buf->datas[0].chunk->offset = 0; buf->datas[0].chunk->stride = stride; buf->datas[0].chunk->size = n_frames * stride; + debug(3, "Queueing %d frames for output.", n_frames); + + if (pw_stream_get_time_n(data->stream, &timing_data_1.time_info, sizeof(time_info)) == 0) + timing_data_1.pw_time_is_valid = 1; + else + timing_data_1.pw_time_is_valid = 0; + __sync_synchronize(); + memcpy((char *)&timing_data_2, (char *)&timing_data_1, sizeof(struct timing_data)); + __sync_synchronize(); pw_stream_queue_buffer(data->stream, b); + } + static const struct pw_stream_events stream_events = { PW_VERSION_STREAM_EVENTS, .process = on_process, @@ -156,6 +169,8 @@ static const struct pw_stream_events stream_events = { static int init(__attribute__((unused)) int argc, __attribute__((unused)) char **argv) { debug(1, "pw_init"); // set up default values first + memset(&timing_data_1,0,sizeof(struct timing_data)); + memset(&timing_data_2,0,sizeof(struct timing_data)); config.audio_backend_buffer_desired_length = 0.35; config.audio_backend_buffer_interpolation_threshold_in_seconds = 0.02; // below this, soxr interpolation will not occur -- it'll be basic interpolation @@ -168,12 +183,12 @@ static int init(__attribute__((unused)) int argc, __attribute__((unused)) char * // do the "general" audio options. Note, these options are in the "general" stanza! parse_general_audio_options(); -/* - // now any PipeWire-specific options - if (config.cfg != NULL) { - const char *str; - } -*/ + /* + // now any PipeWire-specific options + if (config.cfg != NULL) { + const char *str; + } + */ // finished collecting settings // allocate space for the audio buffer @@ -194,10 +209,10 @@ static int init(__attribute__((unused)) int argc, __attribute__((unused)) char * /* make a main loop. If you already have another main loop, you can add * the fd of this pipewire mainloop to it. */ - data.loop = pw_thread_loop_new("tone-generator", NULL); + data.loop = pw_thread_loop_new("tone-generator", NULL); pw_thread_loop_lock(data.loop); - + pw_thread_loop_start(data.loop); /* Create a simple stream, the simple stream manages the core and remote @@ -211,7 +226,7 @@ static int init(__attribute__((unused)) int argc, __attribute__((unused)) char * * you need to listen to is the process event where you need to produce * the data. */ - + props = pw_properties_new(PW_KEY_MEDIA_TYPE, "Audio", PW_KEY_MEDIA_CATEGORY, "Playback", PW_KEY_MEDIA_ROLE, "Music", NULL); if (argc > 1) @@ -240,29 +255,31 @@ static int init(__attribute__((unused)) int argc, __attribute__((unused)) char * } static void deinit(void) { - debug (1, "pw_deinit"); + debug(1, "pw_deinit"); pw_thread_loop_stop(data.loop); pw_stream_destroy(data.stream); pw_thread_loop_destroy(data.loop); pw_deinit(); + free(audio_lmb); // deallocate that buffer debug(1, "pa_deinit done"); } -static int play(__attribute__((unused)) void *buf, int samples, __attribute__((unused)) int sample_type, - __attribute__((unused)) uint32_t timestamp, - __attribute__((unused)) uint64_t playtime) { - debug(1,"pw_play of %d samples.",samples); - /* +static void start(__attribute__((unused)) int sample_rate, + __attribute__((unused)) int sample_format) { +} + +static int play(__attribute__((unused)) void *buf, int samples, + __attribute__((unused)) int sample_type, __attribute__((unused)) uint32_t timestamp, + __attribute__((unused)) uint64_t playtime) { // copy the samples into the queue - check_pa_stream_status(stream, "audio_pw play."); size_t bytes_to_transfer = samples * 2 * 2; size_t space_to_end_of_buffer = audio_umb - audio_eoq; if (space_to_end_of_buffer >= bytes_to_transfer) { memcpy(audio_eoq, buf, bytes_to_transfer); - audio_occupancy += bytes_to_transfer; pthread_mutex_lock(&buffer_mutex); - audio_eoq += bytes_to_transfer; + audio_occupancy += bytes_to_transfer; pthread_mutex_unlock(&buffer_mutex); + audio_eoq += bytes_to_transfer; } else { memcpy(audio_eoq, buf, space_to_end_of_buffer); buf += space_to_end_of_buffer; @@ -272,47 +289,74 @@ static int play(__attribute__((unused)) void *buf, int samples, __attribute__((u pthread_mutex_unlock(&buffer_mutex); audio_eoq = audio_lmb + bytes_to_transfer - space_to_end_of_buffer; } - // maybe goose it if it's stopped? - */ + debug(3, "play %d samples; %d bytes in buffer.", samples, audio_occupancy); return 0; } -/* -int pa_delay(long *the_delay) { - check_pa_stream_status(stream, "audio_pa delay."); - // debug(1,"pa_delay"); - long result = 0; - int reply = 0; - pa_usec_t latency; - int negative; - pa_threaded_mainloop_lock(mainloop); - int gl = pa_stream_get_latency(stream, &latency, &negative); - pa_threaded_mainloop_unlock(mainloop); - if (gl == PA_ERR_NODATA) { - // debug(1, "No latency data yet."); - reply = -ENODEV; - } else if (gl != 0) { - // debug(1,"Error %d getting latency.",gl); - reply = -EIO; +int delay(long *the_delay) { + // find out what's already in the PipeWire system and when + struct timing_data timing_data; + int loop_count = 1; + do { + memcpy(&timing_data, (char *)&timing_data_1, sizeof(struct timing_data)); + __sync_synchronize(); + if (memcmp(&timing_data, (char *)&timing_data_2, sizeof(struct timing_data)) != 0) { + usleep(2); // microseconds + loop_count++; + __sync_synchronize(); + } + } while ((memcmp(&timing_data, (char *)&timing_data_2, sizeof(struct timing_data)) != 0) && (loop_count < 10)); + long total_delay_now_frames_long = 0; + if ((loop_count < 10) && (timing_data.pw_time_is_valid != 0)) { + struct timespec time_now; + clock_gettime(CLOCK_MONOTONIC, &time_now); + int64_t interval_from_process_time_to_now = SPA_TIMESPEC_TO_NSEC(&time_now) - timing_data.time_info.now; + int64_t delay_in_ns = timing_data.time_info.delay + timing_data.time_info.buffered; + delay_in_ns = delay_in_ns * 1000000000; + delay_in_ns = delay_in_ns * timing_data.time_info.rate.num; + delay_in_ns = delay_in_ns / timing_data.time_info.rate.denom; + + int64_t total_delay_now_ns = delay_in_ns - interval_from_process_time_to_now; + int64_t total_delay_now_frames = (total_delay_now_ns * 44100)/1000000000 + timing_data.frames; + total_delay_now_frames_long = total_delay_now_frames; + debug(3, "total delay in frames: % " PRId64 ", %ld.", total_delay_now_frames, total_delay_now_frames_long); + + debug(3, + "interval_from_process_time_to_now: %" PRId64 " ns, " + "delay_in_ns: %" PRId64 ", queued: %" PRId64 ", buffered: %" PRId64 ".", + // delay_timing_data.time_info.rate.num, delay_timing_data.time_info.rate.denom, + interval_from_process_time_to_now, delay_in_ns, + timing_data.time_info.queued, timing_data.time_info.buffered); + + } else { - result = (audio_occupancy / (2 * 2)) + (latency * 44100) / 1000000; - reply = 0; + debug(1, "can't get time info."); } + + long result = 0; + int reply = 0; + pthread_mutex_lock(&buffer_mutex); + result = total_delay_now_frames_long + audio_occupancy / (2 * 2); + pthread_mutex_unlock(&buffer_mutex); *the_delay = result; return reply; } -*/ -void flush(void) { + +static void flush(void) { audio_toq = audio_eoq = audio_lmb; audio_umb = audio_lmb + audio_size; + pthread_mutex_lock(&buffer_mutex); audio_occupancy = 0; + pthread_mutex_unlock(&buffer_mutex); } static void stop(void) { audio_toq = audio_eoq = audio_lmb; audio_umb = audio_lmb + audio_size; + pthread_mutex_lock(&buffer_mutex); audio_occupancy = 0; + pthread_mutex_unlock(&buffer_mutex); } audio_output audio_pw = {.name = "pw", @@ -320,11 +364,11 @@ audio_output audio_pw = {.name = "pw", .init = &init, .deinit = &deinit, .prepare = NULL, - .start = NULL, + .start = &start, .stop = &stop, .is_running = NULL, .flush = &flush, - .delay = NULL, + .delay = &delay, .stats = NULL, .play = &play, .volume = NULL, From d1294c335ee0c27c53e24332a33514d07c5778ef Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Sat, 30 Sep 2023 17:25:42 +0100 Subject: [PATCH 38/61] stop linker errors if built alongside audio_pa --- audio_pa.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/audio_pa.c b/audio_pa.c index bd13cfefd..2880d4efe 100644 --- a/audio_pa.c +++ b/audio_pa.c @@ -49,9 +49,9 @@ pa_threaded_mainloop *mainloop; pa_mainloop_api *mainloop_api; pa_context *context; pa_stream *stream; -char *audio_lmb, *audio_umb, *audio_toq, *audio_eoq; -size_t audio_size = buffer_allocation; -size_t audio_occupancy; +static char *audio_lmb, *audio_umb, *audio_toq, *audio_eoq; +static size_t audio_size = buffer_allocation; +static size_t audio_occupancy; void context_state_cb(pa_context *context, void *mainloop); void stream_state_cb(pa_stream *s, void *mainloop); @@ -308,7 +308,7 @@ int pa_delay(long *the_delay) { return reply; } -void flush(void) { +static void flush(void) { check_pa_stream_status(stream, "audio_pa flush."); pa_threaded_mainloop_lock(mainloop); if (pa_stream_is_corked(stream) == 0) { From afdf0d8632350ca9ab3149b9f9d9ca20f3ce8452 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Sun, 1 Oct 2023 09:28:41 +0100 Subject: [PATCH 39/61] Stability improvements -- take more care when the fifi is full, fix a logical error in the use of the buffer mutex. --- audio_pa.c | 59 ++++++++++++++++++++++++++++-------------------------- audio_pw.c | 18 +++++++++++------ 2 files changed, 43 insertions(+), 34 deletions(-) diff --git a/audio_pa.c b/audio_pa.c index 2880d4efe..6982c8fb0 100644 --- a/audio_pa.c +++ b/audio_pa.c @@ -259,27 +259,33 @@ static int play(void *buf, int samples, __attribute__((unused)) int sample_type, // copy the samples into the queue check_pa_stream_status(stream, "audio_pa play."); size_t bytes_to_transfer = samples * 2 * 2; - size_t space_to_end_of_buffer = audio_umb - audio_eoq; - if (space_to_end_of_buffer >= bytes_to_transfer) { - memcpy(audio_eoq, buf, bytes_to_transfer); - pthread_mutex_lock(&buffer_mutex); - audio_occupancy += bytes_to_transfer; - pthread_mutex_unlock(&buffer_mutex); - audio_eoq += bytes_to_transfer; - } else { - memcpy(audio_eoq, buf, space_to_end_of_buffer); - buf += space_to_end_of_buffer; - memcpy(audio_lmb, buf, bytes_to_transfer - space_to_end_of_buffer); - pthread_mutex_lock(&buffer_mutex); + + pthread_mutex_lock(&buffer_mutex); + size_t bytes_available = audio_size - audio_occupancy; + if (bytes_available < bytes_to_transfer) + bytes_to_transfer = bytes_available; + if (bytes_to_transfer > 0) { + size_t space_to_end_of_buffer = audio_umb - audio_eoq; + if (space_to_end_of_buffer >= bytes_to_transfer) { + memcpy(audio_eoq, buf, bytes_to_transfer); + audio_eoq += bytes_to_transfer; + } else { + memcpy(audio_eoq, buf, space_to_end_of_buffer); + buf += space_to_end_of_buffer; + memcpy(audio_lmb, buf, bytes_to_transfer - space_to_end_of_buffer); + audio_eoq = audio_lmb + bytes_to_transfer - space_to_end_of_buffer; + } audio_occupancy += bytes_to_transfer; - pthread_mutex_unlock(&buffer_mutex); - audio_eoq = audio_lmb + bytes_to_transfer - space_to_end_of_buffer; } + if ((audio_occupancy >= 11025 * 2 * 2) && (pa_stream_is_corked(stream))) { // debug(1,"Uncorked"); + pthread_mutex_unlock(&buffer_mutex); pa_threaded_mainloop_lock(mainloop); pa_stream_cork(stream, 0, stream_success_cb, mainloop); pa_threaded_mainloop_unlock(mainloop); + } else { + pthread_mutex_unlock(&buffer_mutex); } return 0; } @@ -316,10 +322,12 @@ static void flush(void) { pa_stream_flush(stream, stream_success_cb, NULL); pa_stream_cork(stream, 1, stream_success_cb, mainloop); } + pa_threaded_mainloop_unlock(mainloop); + pthread_mutex_lock(&buffer_mutex); audio_toq = audio_eoq = audio_lmb; audio_umb = audio_lmb + audio_size; audio_occupancy = 0; - pa_threaded_mainloop_unlock(mainloop); + pthread_mutex_unlock(&buffer_mutex); } static void stop(void) { @@ -331,10 +339,12 @@ static void stop(void) { pa_stream_flush(stream, stream_success_cb, NULL); pa_stream_cork(stream, 1, stream_success_cb, mainloop); } + pa_threaded_mainloop_unlock(mainloop); + pthread_mutex_lock(&buffer_mutex); audio_toq = audio_eoq = audio_lmb; audio_umb = audio_lmb + audio_size; audio_occupancy = 0; - pa_threaded_mainloop_unlock(mainloop); + pthread_mutex_unlock(&buffer_mutex); } audio_output audio_pa = {.name = "pa", @@ -370,6 +380,8 @@ void stream_write_cb(pa_stream *stream, size_t requested_bytes, int bytes_transferred = 0; uint8_t *buffer = NULL; int ret = 0; + pthread_mutex_lock(&buffer_mutex); + pthread_cleanup_push(mutex_unlock, (void *)&buffer_mutex); while ((bytes_to_transfer > 0) && (audio_occupancy > 0) && (ret == 0)) { if (pa_stream_is_suspended(stream)) debug(1, "stream is suspended"); @@ -390,13 +402,7 @@ void stream_write_cb(pa_stream *stream, size_t requested_bytes, // the bytes are all in a row in the audo buffer memcpy(buffer, audio_toq, bytes_we_can_transfer); audio_toq += bytes_we_can_transfer; - // lock - pthread_mutex_lock(&buffer_mutex); - audio_occupancy -= bytes_we_can_transfer; - pthread_mutex_unlock(&buffer_mutex); - // unlock ret = pa_stream_write(stream, buffer, bytes_we_can_transfer, NULL, 0LL, PA_SEEK_RELATIVE); - bytes_transferred += bytes_we_can_transfer; } else { // the bytes are in two places in the audio buffer size_t first_portion_to_write = audio_umb - audio_toq; @@ -405,17 +411,14 @@ void stream_write_cb(pa_stream *stream, size_t requested_bytes, uint8_t *new_buffer = buffer + first_portion_to_write; memcpy(new_buffer, audio_lmb, bytes_we_can_transfer - first_portion_to_write); ret = pa_stream_write(stream, buffer, bytes_we_can_transfer, NULL, 0LL, PA_SEEK_RELATIVE); - bytes_transferred += bytes_we_can_transfer; audio_toq = audio_lmb + bytes_we_can_transfer - first_portion_to_write; - // lock - pthread_mutex_lock(&buffer_mutex); - audio_occupancy -= bytes_we_can_transfer; - pthread_mutex_unlock(&buffer_mutex); - // unlock } + bytes_transferred += bytes_we_can_transfer; + audio_occupancy -= bytes_we_can_transfer; bytes_to_transfer -= bytes_we_can_transfer; } } + pthread_cleanup_pop(1); // release the mutex if (ret != 0) debug(1, "error writing to pa buffer"); // debug(1,"<< 1) - /* Set stream target if given on command line */ - pw_properties_set(props, PW_KEY_TARGET_OBJECT, argv[1]); + PW_KEY_MEDIA_ROLE, "Music", PW_KEY_APP_NAME, "Shairport Sync", NULL); + data.stream = pw_stream_new_simple(pw_thread_loop_get_loop(data.loop), "audio-src-tg", props, &stream_events, &data); @@ -255,13 +253,21 @@ static int init(__attribute__((unused)) int argc, __attribute__((unused)) char * } static void deinit(void) { - debug(1, "pw_deinit"); + pw_thread_loop_unlock(data.loop); + debug(1, "pipewire: deinit deactivated the stream"); + pw_thread_loop_signal(data.loop, false); + pw_thread_loop_wait(data.loop); + debug(1, "pipewire: deinit loop wait done"); pw_thread_loop_stop(data.loop); + debug(1, "pipewire: deinit loop stopped"); pw_stream_destroy(data.stream); + debug(1, "pipewire: deinit stream destroyed"); pw_thread_loop_destroy(data.loop); + debug(1, "pipewire: deinit loop destroyed"); pw_deinit(); + debug(1, "pipewire: pw_deinit terminated"); free(audio_lmb); // deallocate that buffer - debug(1, "pa_deinit done"); + debug(1, "pipewire: deinit done"); } static void start(__attribute__((unused)) int sample_rate, From bde2bcc8b4322f612474ac7a4bc252101dfbf663 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Sun, 1 Oct 2023 09:30:29 +0100 Subject: [PATCH 40/61] Complete replacement of the PipeWire backend with a new implementation with full synchrnoisation. Works well, but still preliminary. --- audio_pw.c | 260 +++++++++++++++++++++++------------------------------ 1 file changed, 114 insertions(+), 146 deletions(-) diff --git a/audio_pw.c b/audio_pw.c index c4677dc9c..663dba12e 100644 --- a/audio_pw.c +++ b/audio_pw.c @@ -1,6 +1,6 @@ /* * Asynchronous PipeWire Backend. This file is part of Shairport Sync. - * Copyright (c) Mike Brady 2017-2023 + * Copyright (c) Mike Brady 2023 * All rights reserved. * * Permission is hereby granted, free of charge, to any person @@ -24,6 +24,10 @@ * OTHER DEALINGS IN THE SOFTWARE. */ +// This uses ideas from the tone generator sample code at: +// https://github.com/PipeWire/pipewire/blob/master/src/examples/audio-src.c +// Thanks to the Wim Taymans. + #include "audio.h" #include "common.h" #include @@ -39,7 +43,6 @@ #define DEFAULT_FORMAT SPA_AUDIO_FORMAT_S16_LE #define DEFAULT_RATE 44100 #define DEFAULT_CHANNELS 2 -#define DEFAULT_VOLUME 0.7 // Four seconds buffer -- should be plenty #define buffer_allocation 44100 * 4 * 2 * 2 @@ -53,9 +56,9 @@ static size_t audio_occupancy; uint64_t starting_time; struct timing_data { - int pw_time_is_valid; //set when the pw_time has been set + int pw_time_is_valid; // set when the pw_time has been set struct pw_time time_info; // information about the last time a process callback occurred - size_t frames; // the number of frames sent at that time + size_t frames; // the number of frames sent at that time }; // to avoid using a mutex, write the same data twice and check they are the same @@ -66,8 +69,6 @@ struct timing_data timing_data_1, timing_data_2; struct data { struct pw_thread_loop *loop; struct pw_stream *stream; - - double accumulator; }; // the pipewire global data structure @@ -75,14 +76,46 @@ struct data data = { 0, }; -static int fill(void *dest, int max_frames, int stride) { - size_t bytes_we_can_transfer = max_frames * stride; +static void on_state_changed(__attribute__((unused)) void *userdata, enum pw_stream_state old, + enum pw_stream_state state, + __attribute__((unused)) const char *error) { + // struct pw_data *pw = userdata; + debug(3, "pw: stream state changed %s -> %s", pw_stream_state_as_string(old), + pw_stream_state_as_string(state)); +} + +static void on_process(void *userdata) { + + struct data *data = userdata; + int n_frames = 0; + pthread_mutex_lock(&buffer_mutex); - if (bytes_we_can_transfer > audio_occupancy) - bytes_we_can_transfer = audio_occupancy; - pthread_mutex_unlock(&buffer_mutex); - if (bytes_we_can_transfer > 0) { + + if (audio_occupancy > 0) { + + // get a buffer to see how big it can be + struct pw_buffer *b = pw_stream_dequeue_buffer(data->stream); + if (b == NULL) { + pw_log_warn("out of buffers: %m"); + return; + } + struct spa_buffer *buf = b->buffer; + uint8_t *dest = buf->datas[0].data; + if (dest == NULL) // the first data block does not contain a data pointer + return; + + int stride = sizeof(int16_t) * DEFAULT_CHANNELS; + int max_possible_frames = SPA_MIN(b->requested, buf->datas[0].maxsize / stride); + + size_t bytes_we_can_transfer = max_possible_frames * stride; + + if (bytes_we_can_transfer > audio_occupancy) + bytes_we_can_transfer = audio_occupancy; + + n_frames = bytes_we_can_transfer / stride; + size_t bytes_to_end_of_buffer = (size_t)(audio_umb - audio_toq); // must be zero or positive + if (bytes_we_can_transfer <= bytes_to_end_of_buffer) { // the bytes are all in a row in the audio buffer memcpy(dest, audio_toq, bytes_we_can_transfer); @@ -96,81 +129,42 @@ static int fill(void *dest, int max_frames, int stride) { memcpy(new_dest, audio_lmb, bytes_we_can_transfer - first_portion_to_write); audio_toq = audio_lmb + bytes_we_can_transfer - first_portion_to_write; } - // lock - pthread_mutex_lock(&buffer_mutex); - audio_occupancy -= bytes_we_can_transfer; - pthread_mutex_unlock(&buffer_mutex); - // unlock - } - return bytes_we_can_transfer / stride; // back to numbers of frames -} -/* our data processing function is in general: - * - * struct pw_buffer *b; - * b = pw_stream_dequeue_buffer(stream); - * - * .. generate stuff in the buffer ... - * - * pw_stream_queue_buffer(stream, b); - */ -static void on_process(void *userdata) { - struct data *data = userdata; - - struct pw_time time_info; - memset(&time_info, 0, sizeof(time_info)); - - struct pw_buffer *b; - struct spa_buffer *buf; - int max_possible_frames, n_frames, stride; - uint8_t *p; - - if ((b = pw_stream_dequeue_buffer(data->stream)) == NULL) { - pw_log_warn("out of buffers: %m"); - return; - } + buf->datas[0].chunk->offset = 0; + buf->datas[0].chunk->stride = stride; + buf->datas[0].chunk->size = n_frames * stride; + pw_stream_queue_buffer(data->stream, b); + debug(3, "Queueing %d frames for output.", n_frames); - buf = b->buffer; - if ((p = buf->datas[0].data) == NULL) // the first data block does not contain a data pointer - return; + audio_occupancy -= bytes_we_can_transfer; + } + pthread_mutex_unlock(&buffer_mutex); - stride = sizeof(int16_t) * DEFAULT_CHANNELS; - max_possible_frames = SPA_MIN(b->requested, buf->datas[0].maxsize / stride); - - do { - n_frames = fill(p, max_possible_frames, stride); - if (n_frames == 0) { - usleep(1000); - } - } while(n_frames == 0); - - buf->datas[0].chunk->offset = 0; - buf->datas[0].chunk->stride = stride; - buf->datas[0].chunk->size = n_frames * stride; - debug(3, "Queueing %d frames for output.", n_frames); - - if (pw_stream_get_time_n(data->stream, &timing_data_1.time_info, sizeof(time_info)) == 0) + timing_data_1.frames = n_frames; + if (pw_stream_get_time_n(data->stream, &timing_data_1.time_info, sizeof(struct timing_data)) == 0) timing_data_1.pw_time_is_valid = 1; else timing_data_1.pw_time_is_valid = 0; __sync_synchronize(); memcpy((char *)&timing_data_2, (char *)&timing_data_1, sizeof(struct timing_data)); __sync_synchronize(); - pw_stream_queue_buffer(data->stream, b); - } - static const struct pw_stream_events stream_events = { - PW_VERSION_STREAM_EVENTS, - .process = on_process, -}; + PW_VERSION_STREAM_EVENTS, .process = on_process, .state_changed = on_state_changed}; + +static void deinit(void) { + pw_thread_loop_stop(data.loop); + pw_stream_destroy(data.stream); + pw_thread_loop_destroy(data.loop); + pw_deinit(); + free(audio_lmb); // deallocate that buffer +} static int init(__attribute__((unused)) int argc, __attribute__((unused)) char **argv) { - debug(1, "pw_init"); // set up default values first - memset(&timing_data_1,0,sizeof(struct timing_data)); - memset(&timing_data_2,0,sizeof(struct timing_data)); + memset(&timing_data_1, 0, sizeof(struct timing_data)); + memset(&timing_data_2, 0, sizeof(struct timing_data)); config.audio_backend_buffer_desired_length = 0.35; config.audio_backend_buffer_interpolation_threshold_in_seconds = 0.02; // below this, soxr interpolation will not occur -- it'll be basic interpolation @@ -207,30 +201,17 @@ static int init(__attribute__((unused)) int argc, __attribute__((unused)) char * int largc = 0; pw_init(&largc, NULL); - /* make a main loop. If you already have another main loop, you can add - * the fd of this pipewire mainloop to it. */ - data.loop = pw_thread_loop_new("tone-generator", NULL); + /* make a threaded loop. */ + data.loop = pw_thread_loop_new("shairport-sync", NULL); pw_thread_loop_lock(data.loop); pw_thread_loop_start(data.loop); - /* Create a simple stream, the simple stream manages the core and remote - * objects for you if you don't need to deal with them. - * - * If you plan to autoconnect your stream, you need to provide at least - * media, category and role properties. - * - * Pass your events and a user_data pointer as the last arguments. This - * will inform you about the stream state. The most important event - * you need to listen to is the process event where you need to produce - * the data. - */ - props = pw_properties_new(PW_KEY_MEDIA_TYPE, "Audio", PW_KEY_MEDIA_CATEGORY, "Playback", PW_KEY_MEDIA_ROLE, "Music", PW_KEY_APP_NAME, "Shairport Sync", NULL); - data.stream = pw_stream_new_simple(pw_thread_loop_get_loop(data.loop), "audio-src-tg", props, + data.stream = pw_stream_new_simple(pw_thread_loop_get_loop(data.loop), "shairport-sync", props, &stream_events, &data); /* Make one parameter with the supported formats. The SPA_PARAM_EnumFormat @@ -248,58 +229,42 @@ static int init(__attribute__((unused)) int argc, __attribute__((unused)) char * params, 1); pw_thread_loop_unlock(data.loop); - debug(1, "pa_init done"); return 0; } -static void deinit(void) { - pw_thread_loop_unlock(data.loop); - debug(1, "pipewire: deinit deactivated the stream"); - pw_thread_loop_signal(data.loop, false); - pw_thread_loop_wait(data.loop); - debug(1, "pipewire: deinit loop wait done"); - pw_thread_loop_stop(data.loop); - debug(1, "pipewire: deinit loop stopped"); - pw_stream_destroy(data.stream); - debug(1, "pipewire: deinit stream destroyed"); - pw_thread_loop_destroy(data.loop); - debug(1, "pipewire: deinit loop destroyed"); - pw_deinit(); - debug(1, "pipewire: pw_deinit terminated"); - free(audio_lmb); // deallocate that buffer - debug(1, "pipewire: deinit done"); -} - static void start(__attribute__((unused)) int sample_rate, - __attribute__((unused)) int sample_format) { -} + __attribute__((unused)) int sample_format) {} static int play(__attribute__((unused)) void *buf, int samples, __attribute__((unused)) int sample_type, __attribute__((unused)) uint32_t timestamp, - __attribute__((unused)) uint64_t playtime) { + __attribute__((unused)) uint64_t playtime) { // copy the samples into the queue + debug(3, "play %u samples; %u bytes already in the buffer.", samples, audio_occupancy); size_t bytes_to_transfer = samples * 2 * 2; - size_t space_to_end_of_buffer = audio_umb - audio_eoq; - if (space_to_end_of_buffer >= bytes_to_transfer) { - memcpy(audio_eoq, buf, bytes_to_transfer); - pthread_mutex_lock(&buffer_mutex); - audio_occupancy += bytes_to_transfer; - pthread_mutex_unlock(&buffer_mutex); - audio_eoq += bytes_to_transfer; - } else { - memcpy(audio_eoq, buf, space_to_end_of_buffer); - buf += space_to_end_of_buffer; - memcpy(audio_lmb, buf, bytes_to_transfer - space_to_end_of_buffer); - pthread_mutex_lock(&buffer_mutex); + pthread_mutex_lock(&buffer_mutex); + size_t bytes_available = audio_size - audio_occupancy; + if (bytes_available < bytes_to_transfer) + bytes_to_transfer = bytes_available; + if (bytes_to_transfer > 0) { + size_t space_to_end_of_buffer = audio_umb - audio_eoq; + if (space_to_end_of_buffer >= bytes_to_transfer) { + memcpy(audio_eoq, buf, bytes_to_transfer); + audio_eoq += bytes_to_transfer; + } else { + memcpy(audio_eoq, buf, space_to_end_of_buffer); + buf += space_to_end_of_buffer; + memcpy(audio_lmb, buf, bytes_to_transfer - space_to_end_of_buffer); + audio_eoq = audio_lmb + bytes_to_transfer - space_to_end_of_buffer; + } audio_occupancy += bytes_to_transfer; - pthread_mutex_unlock(&buffer_mutex); - audio_eoq = audio_lmb + bytes_to_transfer - space_to_end_of_buffer; } - debug(3, "play %d samples; %d bytes in buffer.", samples, audio_occupancy); + pthread_mutex_unlock(&buffer_mutex); return 0; } int delay(long *the_delay) { + long result = 0; + int reply = 0; // find out what's already in the PipeWire system and when struct timing_data timing_data; int loop_count = 1; @@ -307,40 +272,44 @@ int delay(long *the_delay) { memcpy(&timing_data, (char *)&timing_data_1, sizeof(struct timing_data)); __sync_synchronize(); if (memcmp(&timing_data, (char *)&timing_data_2, sizeof(struct timing_data)) != 0) { - usleep(2); // microseconds - loop_count++; - __sync_synchronize(); + usleep(2); // microseconds + loop_count++; + __sync_synchronize(); } - } while ((memcmp(&timing_data, (char *)&timing_data_2, sizeof(struct timing_data)) != 0) && (loop_count < 10)); + } while ((memcmp(&timing_data, (char *)&timing_data_2, sizeof(struct timing_data)) != 0) && + (loop_count < 10)); long total_delay_now_frames_long = 0; if ((loop_count < 10) && (timing_data.pw_time_is_valid != 0)) { - struct timespec time_now; + struct timespec time_now; clock_gettime(CLOCK_MONOTONIC, &time_now); - int64_t interval_from_process_time_to_now = SPA_TIMESPEC_TO_NSEC(&time_now) - timing_data.time_info.now; + int64_t interval_from_process_time_to_now = + SPA_TIMESPEC_TO_NSEC(&time_now) - timing_data.time_info.now; int64_t delay_in_ns = timing_data.time_info.delay + timing_data.time_info.buffered; delay_in_ns = delay_in_ns * 1000000000; delay_in_ns = delay_in_ns * timing_data.time_info.rate.num; delay_in_ns = delay_in_ns / timing_data.time_info.rate.denom; - + int64_t total_delay_now_ns = delay_in_ns - interval_from_process_time_to_now; - int64_t total_delay_now_frames = (total_delay_now_ns * 44100)/1000000000 + timing_data.frames; + int64_t total_delay_now_frames = (total_delay_now_ns * 44100) / 1000000000 + timing_data.frames; total_delay_now_frames_long = total_delay_now_frames; - debug(3, "total delay in frames: % " PRId64 ", %ld.", total_delay_now_frames, total_delay_now_frames_long); - - debug(3, - "interval_from_process_time_to_now: %" PRId64 " ns, " - "delay_in_ns: %" PRId64 ", queued: %" PRId64 ", buffered: %" PRId64 ".", - // delay_timing_data.time_info.rate.num, delay_timing_data.time_info.rate.denom, - interval_from_process_time_to_now, delay_in_ns, - timing_data.time_info.queued, timing_data.time_info.buffered); + debug(3, "total delay in frames: %ld.", total_delay_now_frames_long); + if (timing_data.time_info.queued != 0) { + debug(1, "buffers queued: %d", timing_data.time_info.queued); + } + /* + debug(3, + "interval_from_process_time_to_now: %" PRId64 " ns, " + "delay_in_ns: %" PRId64 ", queued: %" PRId64 ", buffered: %" PRId64 ".", + // delay_timing_data.time_info.rate.num, delay_timing_data.time_info.rate.denom, + interval_from_process_time_to_now, delay_in_ns, + timing_data.time_info.queued, timing_data.time_info.buffered); + */ } else { debug(1, "can't get time info."); } - long result = 0; - int reply = 0; pthread_mutex_lock(&buffer_mutex); result = total_delay_now_frames_long + audio_occupancy / (2 * 2); pthread_mutex_unlock(&buffer_mutex); @@ -348,19 +317,18 @@ int delay(long *the_delay) { return reply; } - static void flush(void) { + pthread_mutex_lock(&buffer_mutex); audio_toq = audio_eoq = audio_lmb; audio_umb = audio_lmb + audio_size; - pthread_mutex_lock(&buffer_mutex); audio_occupancy = 0; pthread_mutex_unlock(&buffer_mutex); } static void stop(void) { + pthread_mutex_lock(&buffer_mutex); audio_toq = audio_eoq = audio_lmb; audio_umb = audio_lmb + audio_size; - pthread_mutex_lock(&buffer_mutex); audio_occupancy = 0; pthread_mutex_unlock(&buffer_mutex); } From 4447b25a05b648b28c002cd6c7d519e793f4434f Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Sun, 1 Oct 2023 09:37:07 +0100 Subject: [PATCH 41/61] Update RELEASENOTES-DEVELOPMENT.md --- RELEASENOTES-DEVELOPMENT.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/RELEASENOTES-DEVELOPMENT.md b/RELEASENOTES-DEVELOPMENT.md index d0555d1cf..9b31bdff0 100644 --- a/RELEASENOTES-DEVELOPMENT.md +++ b/RELEASENOTES-DEVELOPMENT.md @@ -1,3 +1,11 @@ +Version 4.3.2-dev-38-gbde2bcc8 +=== +**Enhancement** +* The PipeWire backend (`pw`) has been completely rewritten and now provides full sychronisation. + +**Bug Fixes** +* Stability improvements to the PulseAudio (`pa`) backend. some bugs with the use of mutexes and the treatment of a full FIFO buffer were identified and fixed. + Version 4.3.2-dev-30-gc36d322a === **Bug Fix** From 29a5589be6d354e086991a34b9d9942ac0fa491c Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Mon, 2 Oct 2023 10:05:21 +0100 Subject: [PATCH 42/61] Quieten a debug message. --- rtsp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rtsp.c b/rtsp.c index 3c193d3dd..44058fda3 100644 --- a/rtsp.c +++ b/rtsp.c @@ -580,7 +580,7 @@ int get_play_lock(rtsp_conn_info *conn, int allow_session_interruption) { pthread_rwlock_wrlock(&principal_conn_lock); pthread_cleanup_push(rwlock_unlock, (void *)&principal_conn_lock); if (principal_conn != NULL) - debug(1, "Connection %d: is requested to relinquish principal_conn.", + debug(2, "Connection %d: is requested to relinquish principal_conn.", principal_conn->connection_number); if (conn != NULL) debug(2, "Connection %d: request to acquire principal_conn.", conn->connection_number); From dc197ae711f0f1ac7593f66f20135e2ec3aefd30 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Mon, 2 Oct 2023 10:06:55 +0100 Subject: [PATCH 43/61] Send silence when no audio is coming through from Shairport Sync. This eliminates (?) xrun errors. Clean up a few imprefections. --- audio_pw.c | 121 +++++++++++++++++++++++++++++++---------------------- 1 file changed, 71 insertions(+), 50 deletions(-) diff --git a/audio_pw.c b/audio_pw.c index 663dba12e..a053edc7c 100644 --- a/audio_pw.c +++ b/audio_pw.c @@ -26,7 +26,7 @@ // This uses ideas from the tone generator sample code at: // https://github.com/PipeWire/pipewire/blob/master/src/examples/audio-src.c -// Thanks to the Wim Taymans. +// Thanks to Wim Taymans. #include "audio.h" #include "common.h" @@ -39,7 +39,7 @@ #include #include -// note -- these are hacked and hardwired into this code. +// note -- these are hardwired into this code. #define DEFAULT_FORMAT SPA_AUDIO_FORMAT_S16_LE #define DEFAULT_RATE 44100 #define DEFAULT_CHANNELS 2 @@ -52,8 +52,7 @@ static pthread_mutex_t buffer_mutex = PTHREAD_MUTEX_INITIALIZER; static char *audio_lmb, *audio_umb, *audio_toq, *audio_eoq; static size_t audio_size = buffer_allocation; static size_t audio_occupancy; - -uint64_t starting_time; +static int enable_fill; struct timing_data { int pw_time_is_valid; // set when the pw_time has been set @@ -72,10 +71,9 @@ struct data { }; // the pipewire global data structure -struct data data = { - 0, -}; +struct data data = {NULL, NULL}; +/* static void on_state_changed(__attribute__((unused)) void *userdata, enum pw_stream_state old, enum pw_stream_state state, __attribute__((unused)) const char *error) { @@ -83,6 +81,7 @@ static void on_state_changed(__attribute__((unused)) void *userdata, enum pw_str debug(3, "pw: stream state changed %s -> %s", pw_stream_state_as_string(old), pw_stream_state_as_string(state)); } +*/ static void on_process(void *userdata) { @@ -91,52 +90,63 @@ static void on_process(void *userdata) { pthread_mutex_lock(&buffer_mutex); - if (audio_occupancy > 0) { + if ((audio_occupancy > 0) || (enable_fill)) { // get a buffer to see how big it can be struct pw_buffer *b = pw_stream_dequeue_buffer(data->stream); if (b == NULL) { pw_log_warn("out of buffers: %m"); - return; + die("PipeWire failue -- out of buffers!"); } struct spa_buffer *buf = b->buffer; uint8_t *dest = buf->datas[0].data; - if (dest == NULL) // the first data block does not contain a data pointer - return; - - int stride = sizeof(int16_t) * DEFAULT_CHANNELS; - int max_possible_frames = SPA_MIN(b->requested, buf->datas[0].maxsize / stride); - - size_t bytes_we_can_transfer = max_possible_frames * stride; - - if (bytes_we_can_transfer > audio_occupancy) - bytes_we_can_transfer = audio_occupancy; - - n_frames = bytes_we_can_transfer / stride; - - size_t bytes_to_end_of_buffer = (size_t)(audio_umb - audio_toq); // must be zero or positive - - if (bytes_we_can_transfer <= bytes_to_end_of_buffer) { - // the bytes are all in a row in the audio buffer - memcpy(dest, audio_toq, bytes_we_can_transfer); - audio_toq += bytes_we_can_transfer; - } else { - // the bytes are in two places in the audio buffer - size_t first_portion_to_write = audio_umb - audio_toq; - if (first_portion_to_write != 0) - memcpy(dest, audio_toq, first_portion_to_write); - uint8_t *new_dest = dest + first_portion_to_write; - memcpy(new_dest, audio_lmb, bytes_we_can_transfer - first_portion_to_write); - audio_toq = audio_lmb + bytes_we_can_transfer - first_portion_to_write; - } - - buf->datas[0].chunk->offset = 0; - buf->datas[0].chunk->stride = stride; - buf->datas[0].chunk->size = n_frames * stride; - pw_stream_queue_buffer(data->stream, b); - debug(3, "Queueing %d frames for output.", n_frames); - - audio_occupancy -= bytes_we_can_transfer; + if (dest != NULL) { + int stride = sizeof(int16_t) * DEFAULT_CHANNELS; + + // note: the requested field is the number of frames, not bytes, requested + int max_possible_frames = SPA_MIN(b->requested, buf->datas[0].maxsize / stride); + + size_t bytes_we_can_transfer = max_possible_frames * stride; + + if (audio_occupancy > 0) { + // if (enable_fill == 1)) { + // debug(1, "got audio -- disable_fill"); + // } + enable_fill = 0; + + if (bytes_we_can_transfer > audio_occupancy) + bytes_we_can_transfer = audio_occupancy; + + n_frames = bytes_we_can_transfer / stride; + + size_t bytes_to_end_of_buffer = (size_t)(audio_umb - audio_toq); // must be zero or positive + if (bytes_we_can_transfer <= bytes_to_end_of_buffer) { + // the bytes are all in a row in the audio buffer + memcpy(dest, audio_toq, bytes_we_can_transfer); + audio_toq += bytes_we_can_transfer; + } else { + // the bytes are in two places in the audio buffer + size_t first_portion_to_write = audio_umb - audio_toq; + if (first_portion_to_write != 0) + memcpy(dest, audio_toq, first_portion_to_write); + uint8_t *new_dest = dest + first_portion_to_write; + memcpy(new_dest, audio_lmb, bytes_we_can_transfer - first_portion_to_write); + audio_toq = audio_lmb + bytes_we_can_transfer - first_portion_to_write; + } + audio_occupancy -= bytes_we_can_transfer; + + } else { + debug(3, "send silence"); + // this should really be dithered silence + memset(dest, 0, bytes_we_can_transfer); + n_frames = max_possible_frames; + } + buf->datas[0].chunk->offset = 0; + buf->datas[0].chunk->stride = stride; + buf->datas[0].chunk->size = n_frames * stride; + pw_stream_queue_buffer(data->stream, b); + debug(3, "Queueing %d frames for output.", n_frames); + } // (else the first data block does not contain a data pointer) } pthread_mutex_unlock(&buffer_mutex); @@ -150,8 +160,9 @@ static void on_process(void *userdata) { __sync_synchronize(); } -static const struct pw_stream_events stream_events = { - PW_VERSION_STREAM_EVENTS, .process = on_process, .state_changed = on_state_changed}; +static const struct pw_stream_events stream_events = {PW_VERSION_STREAM_EVENTS, + .process = on_process}; +// PW_VERSION_STREAM_EVENTS, .process = on_process, .state_changed = on_state_changed}; static void deinit(void) { pw_thread_loop_stop(data.loop); @@ -173,7 +184,6 @@ static int init(__attribute__((unused)) int argc, __attribute__((unused)) char * config.audio_backend_latency_offset = 0; // get settings from settings file - // do the "general" audio options. Note, these options are in the "general" stanza! parse_general_audio_options(); @@ -188,10 +198,12 @@ static int init(__attribute__((unused)) int argc, __attribute__((unused)) char * // allocate space for the audio buffer audio_lmb = malloc(audio_size); if (audio_lmb == NULL) - die("Can't allocate %d bytes for pulseaudio buffer.", audio_size); + die("Can't allocate %d bytes for PipeWire buffer.", audio_size); audio_toq = audio_eoq = audio_lmb; audio_umb = audio_lmb + audio_size; audio_occupancy = 0; + // debug(1, "init enable_fill"); + enable_fill = 1; const struct spa_pod *params[1]; uint8_t buffer[1024]; @@ -233,7 +245,8 @@ static int init(__attribute__((unused)) int argc, __attribute__((unused)) char * } static void start(__attribute__((unused)) int sample_rate, - __attribute__((unused)) int sample_format) {} + __attribute__((unused)) int sample_format) { +} static int play(__attribute__((unused)) void *buf, int samples, __attribute__((unused)) int sample_type, __attribute__((unused)) uint32_t timestamp, @@ -322,6 +335,10 @@ static void flush(void) { audio_toq = audio_eoq = audio_lmb; audio_umb = audio_lmb + audio_size; audio_occupancy = 0; + // if (enable_fill == 0) { + // debug(1, "flush enable_fill"); + // } + enable_fill = 1; pthread_mutex_unlock(&buffer_mutex); } @@ -330,6 +347,10 @@ static void stop(void) { audio_toq = audio_eoq = audio_lmb; audio_umb = audio_lmb + audio_size; audio_occupancy = 0; + // if (enable_fill == 0) { + // debug(1, "stop enable_fill"); + // } + enable_fill = 1; pthread_mutex_unlock(&buffer_mutex); } From 57382cdecb1c8e5cc679583d5654abd42b51b421 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Mon, 2 Oct 2023 10:12:31 +0100 Subject: [PATCH 44/61] Update RELEASENOTES-DEVELOPMENT.md --- RELEASENOTES-DEVELOPMENT.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/RELEASENOTES-DEVELOPMENT.md b/RELEASENOTES-DEVELOPMENT.md index 9b31bdff0..0f2d9aad6 100644 --- a/RELEASENOTES-DEVELOPMENT.md +++ b/RELEASENOTES-DEVELOPMENT.md @@ -1,3 +1,8 @@ +Version 4.3.2-dev-41-gdc197ae7 +=== +**Bug Fix** +* Fix underrun ("xrun") errors. These errors seem harmless, and the only place they were noticed was in the listing from `pw-top` on recent versions of PipeWire (e.g. PipeWire 0.3.80 in Fedora 38). Anyway, they are gone now. + Version 4.3.2-dev-38-gbde2bcc8 === **Enhancement** From d9c7f0aeb13112ebe3dd1ffa1425a966574eed02 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Mon, 2 Oct 2023 10:13:01 +0100 Subject: [PATCH 45/61] Update RELEASENOTES-DEVELOPMENT.md --- RELEASENOTES-DEVELOPMENT.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/RELEASENOTES-DEVELOPMENT.md b/RELEASENOTES-DEVELOPMENT.md index 0f2d9aad6..b2978f554 100644 --- a/RELEASENOTES-DEVELOPMENT.md +++ b/RELEASENOTES-DEVELOPMENT.md @@ -1,7 +1,7 @@ Version 4.3.2-dev-41-gdc197ae7 === **Bug Fix** -* Fix underrun ("xrun") errors. These errors seem harmless, and the only place they were noticed was in the listing from `pw-top` on recent versions of PipeWire (e.g. PipeWire 0.3.80 in Fedora 38). Anyway, they are gone now. +* Fix underrun ("xrun") errors in the new PipeWire backend. These errors seem harmless, and the only place they were noticed was in the listing from `pw-top` on recent versions of PipeWire (e.g. PipeWire 0.3.80 in Fedora 38). Anyway, they are gone now. Version 4.3.2-dev-38-gbde2bcc8 === From 3db689f9e8205143bced74f3d3c086887f2dd219 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Thu, 5 Oct 2023 09:39:22 +0100 Subject: [PATCH 46/61] Deallocate settings strings on exit. --- audio_pa.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/audio_pa.c b/audio_pa.c index 6982c8fb0..c0fe1bd2c 100644 --- a/audio_pa.c +++ b/audio_pa.c @@ -244,6 +244,14 @@ static void deinit(void) { pa_stream_disconnect(stream); pa_threaded_mainloop_stop(mainloop); pa_threaded_mainloop_free(mainloop); + if (config.pa_server) + free(config.pa_server); + if (config.pa_application_name) + free(config.pa_application_name); + if (config.pa_sink) + free(config.pa_sink); + config.pa_server = config.pa_application_name = config.pa_sink = NULL; + // debug(1, "pa deinit done"); } From 3763b321b8bd122eb340985c3c52471638bf384b Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Thu, 5 Oct 2023 09:41:21 +0100 Subject: [PATCH 47/61] Add two PipeWire backend ("pw") settings -- application_name (default: "Shairport Sync") and node_name (default: "shairport-sync"). --- audio_pw.c | 47 ++++++++++++++++++++++++++++++------- common.h | 9 ++++++- scripts/shairport-sync.conf | 11 ++++++++- 3 files changed, 57 insertions(+), 10 deletions(-) diff --git a/audio_pw.c b/audio_pw.c index a053edc7c..0dd389393 100644 --- a/audio_pw.c +++ b/audio_pw.c @@ -41,11 +41,14 @@ // note -- these are hardwired into this code. #define DEFAULT_FORMAT SPA_AUDIO_FORMAT_S16_LE +#define DEFAULT_BYTES_PER_SAMPLE 2 + #define DEFAULT_RATE 44100 #define DEFAULT_CHANNELS 2 +#define DEFAULT_BUFFER_SIZE_IN_SECONDS 4 // Four seconds buffer -- should be plenty -#define buffer_allocation 44100 * 4 * 2 * 2 +#define buffer_allocation DEFAULT_RATE * DEFAULT_BUFFER_SIZE_IN_SECONDS * DEFAULT_BYTES_PER_SAMPLE * DEFAULT_CHANNELS static pthread_mutex_t buffer_mutex = PTHREAD_MUTEX_INITIALIZER; @@ -101,7 +104,7 @@ static void on_process(void *userdata) { struct spa_buffer *buf = b->buffer; uint8_t *dest = buf->datas[0].data; if (dest != NULL) { - int stride = sizeof(int16_t) * DEFAULT_CHANNELS; + int stride = DEFAULT_BYTES_PER_SAMPLE * DEFAULT_CHANNELS; // note: the requested field is the number of frames, not bytes, requested int max_possible_frames = SPA_MIN(b->requested, buf->datas[0].maxsize / stride); @@ -170,6 +173,13 @@ static void deinit(void) { pw_thread_loop_destroy(data.loop); pw_deinit(); free(audio_lmb); // deallocate that buffer + if (config.pw_application_name) + free(config.pw_application_name); + if (config.pw_node_name) + free(config.pw_node_name); + config.pw_application_name = config.pw_node_name = NULL; + + } static int init(__attribute__((unused)) int argc, __attribute__((unused)) char **argv) { @@ -187,12 +197,22 @@ static int init(__attribute__((unused)) int argc, __attribute__((unused)) char * // do the "general" audio options. Note, these options are in the "general" stanza! parse_general_audio_options(); - /* + // now any PipeWire-specific options if (config.cfg != NULL) { const char *str; + + /* Get the Application Name. */ + if (config_lookup_string(config.cfg, "pw.application_name", &str)) { + config.pw_application_name = (char *)str; + } + + /* Get the PulseAudio node name. */ + if (config_lookup_string(config.cfg, "pa.node_name", &str)) { + config.pw_node_name = (char *)str; + } } - */ + // finished collecting settings // allocate space for the audio buffer @@ -219,17 +239,28 @@ static int init(__attribute__((unused)) int argc, __attribute__((unused)) char * pw_thread_loop_lock(data.loop); pw_thread_loop_start(data.loop); + + char* appname = config.pw_application_name; + if (appname == NULL) + appname = "Shairport Sync"; + + char* nodename = config.pw_node_name; + if (nodename == NULL) + nodename = config.appName; + + props = pw_properties_new(PW_KEY_MEDIA_TYPE, "Audio", PW_KEY_MEDIA_CATEGORY, "Playback", - PW_KEY_MEDIA_ROLE, "Music", PW_KEY_APP_NAME, "Shairport Sync", NULL); + PW_KEY_MEDIA_ROLE, "Music", PW_KEY_APP_NAME, appname, + PW_KEY_NODE_NAME, nodename, NULL); - data.stream = pw_stream_new_simple(pw_thread_loop_get_loop(data.loop), "shairport-sync", props, + data.stream = pw_stream_new_simple(pw_thread_loop_get_loop(data.loop), config.appName, props, &stream_events, &data); /* Make one parameter with the supported formats. The SPA_PARAM_EnumFormat * id means that this is a format enumeration (of 1 value). */ params[0] = spa_format_audio_raw_build(&b, SPA_PARAM_EnumFormat, - &SPA_AUDIO_INFO_RAW_INIT(.format = SPA_AUDIO_FORMAT_S16_LE, + &SPA_AUDIO_INFO_RAW_INIT(.format = DEFAULT_FORMAT, .channels = DEFAULT_CHANNELS, .rate = DEFAULT_RATE)); @@ -303,7 +334,7 @@ int delay(long *the_delay) { delay_in_ns = delay_in_ns / timing_data.time_info.rate.denom; int64_t total_delay_now_ns = delay_in_ns - interval_from_process_time_to_now; - int64_t total_delay_now_frames = (total_delay_now_ns * 44100) / 1000000000 + timing_data.frames; + int64_t total_delay_now_frames = (total_delay_now_ns * DEFAULT_RATE) / 1000000000 + timing_data.frames; total_delay_now_frames_long = total_delay_now_frames; debug(3, "total delay in frames: %ld.", total_delay_now_frames_long); diff --git a/common.h b/common.h index 1b9e2f03d..3a392f684 100644 --- a/common.h +++ b/common.h @@ -139,10 +139,17 @@ typedef struct { char *pa_server; // the pulseaudio server address that Shairport Sync will play on. char *pa_application_name; // the name under which Shairport Sync shows up as an "Application" in // the Sound Preferences in most desktop Linuxes. - // Defaults to "Shairport Sync". Shairport Sync must be playing to see it. + // Defaults to "Shairport Sync". char *pa_sink; // the name (or id) of the sink that Shairport Sync will play on. #endif +#ifdef CONFIG_PW + char *pw_application_name; // the name under which Shairport Sync shows up as an "Application" in + // the Sound Preferences in most desktop Linuxes. + // Defaults to "Shairport Sync". + + char *pw_node_name; // defaults to the application's name, usually "shairport-sync". +#endif #ifdef CONFIG_METADATA int metadata_enabled; char *metadata_pipename; diff --git a/scripts/shairport-sync.conf b/scripts/shairport-sync.conf index 3f4c152f5..001a88d26 100644 --- a/scripts/shairport-sync.conf +++ b/scripts/shairport-sync.conf @@ -154,6 +154,15 @@ sndio = // bufsz = ; // advanced optional setting to set the buffer size near to this value }; +// Parameters for the "pw" PipeWire backend. +// For this section to be operative, Shairport Sync must be built with the following configuration flag: +// --with-pw +pw = +{ +// application_name = "Shairport Sync"; // Set this to the name that should appear in the Sounds "Applications". +// node_name = ""; // This is normally "shairport-sync". +}; + // Parameters for the "pa" PulseAudio backend. // For this section to be operative, Shairport Sync must be built with the following configuration flag: // --with-pa @@ -181,7 +190,7 @@ jack = // bufsz = ; // advanced optional setting to set the buffer size to this value }; -// Parameters for the "pipe" audio back end, a back end that directs raw CD-style audio output to a pipe. No interpolation is done. +// Parameters for the "pipe" audio back end, a back end that directs raw CD-format audio output to a pipe. No interpolation is done. // For this section to be operative, Shairport Sync must have been built with the following configuration flag: // --with-pipe pipe = From 16dd54e80aa20bfabdc97a5bd5411a1797f17ead Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Thu, 5 Oct 2023 09:50:17 +0100 Subject: [PATCH 48/61] Update RELEASENOTES-DEVELOPMENT.md --- RELEASENOTES-DEVELOPMENT.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/RELEASENOTES-DEVELOPMENT.md b/RELEASENOTES-DEVELOPMENT.md index b2978f554..f9d557378 100644 --- a/RELEASENOTES-DEVELOPMENT.md +++ b/RELEASENOTES-DEVELOPMENT.md @@ -1,3 +1,11 @@ +Version 4.3.2-dev-45-g3763b321 +=== +**Enhancement** +* Add two `pw` (PipeWire) backend settings -- `application_name` (default: "Shairport Sync") and `node_name` (default: "shairport-sync"). The `application_name` is used when referring to Shairport Sync in the GUI and the `node_name` is used in, for example, output from `pw-link`. Thanks to [alexsarmiento](https://github.com/alexsarmiento) for the [suggestion](https://github.com/mikebrady/shairport-sync/issues/1742). + +**Bug Fixes** +* Minor bug fixes to the PulseAudio backend. + Version 4.3.2-dev-41-gdc197ae7 === **Bug Fix** From 288e5eb69590ba65d84d83f935829e4cdc270757 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Thu, 5 Oct 2023 16:40:42 +0100 Subject: [PATCH 49/61] Change node_name default to "Shairport Sync", parameterise a few settings. --- audio_pw.c | 19 ++++++++++--------- scripts/shairport-sync.conf | 4 ++-- 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/audio_pw.c b/audio_pw.c index 0dd389393..f77b8766c 100644 --- a/audio_pw.c +++ b/audio_pw.c @@ -207,7 +207,7 @@ static int init(__attribute__((unused)) int argc, __attribute__((unused)) char * config.pw_application_name = (char *)str; } - /* Get the PulseAudio node name. */ + /* Get the PipeWire node name. */ if (config_lookup_string(config.cfg, "pa.node_name", &str)) { config.pw_node_name = (char *)str; } @@ -246,7 +246,7 @@ static int init(__attribute__((unused)) int argc, __attribute__((unused)) char * char* nodename = config.pw_node_name; if (nodename == NULL) - nodename = config.appName; + nodename = "Shairport Sync"; @@ -257,15 +257,15 @@ static int init(__attribute__((unused)) int argc, __attribute__((unused)) char * data.stream = pw_stream_new_simple(pw_thread_loop_get_loop(data.loop), config.appName, props, &stream_events, &data); - /* Make one parameter with the supported formats. The SPA_PARAM_EnumFormat - * id means that this is a format enumeration (of 1 value). */ + // Make one parameter with the supported formats. The SPA_PARAM_EnumFormat + // id means that this is a format enumeration (of 1 value). params[0] = spa_format_audio_raw_build(&b, SPA_PARAM_EnumFormat, &SPA_AUDIO_INFO_RAW_INIT(.format = DEFAULT_FORMAT, .channels = DEFAULT_CHANNELS, .rate = DEFAULT_RATE)); - /* Now connect this stream. We ask that our process function is - * called in a realtime thread. */ + // Now connect this stream. We ask that our process function is + // called in a realtime thread. pw_stream_connect(data.stream, PW_DIRECTION_OUTPUT, PW_ID_ANY, PW_STREAM_FLAG_AUTOCONNECT | PW_STREAM_FLAG_MAP_BUFFERS | PW_STREAM_FLAG_RT_PROCESS, @@ -284,7 +284,7 @@ static int play(__attribute__((unused)) void *buf, int samples, __attribute__((unused)) uint64_t playtime) { // copy the samples into the queue debug(3, "play %u samples; %u bytes already in the buffer.", samples, audio_occupancy); - size_t bytes_to_transfer = samples * 2 * 2; + size_t bytes_to_transfer = samples * DEFAULT_CHANNELS * DEFAULT_BYTES_PER_SAMPLE; pthread_mutex_lock(&buffer_mutex); size_t bytes_available = audio_size - audio_occupancy; if (bytes_available < bytes_to_transfer) @@ -351,11 +351,12 @@ int delay(long *the_delay) { */ } else { - debug(1, "can't get time info."); + warn("Shairport Sync's PipeWire backend can not get timing information from the PipeWire " + "system. Is PipeWire running?"); } pthread_mutex_lock(&buffer_mutex); - result = total_delay_now_frames_long + audio_occupancy / (2 * 2); + result = total_delay_now_frames_long + audio_occupancy / (DEFAULT_BYTES_PER_SAMPLE * DEFAULT_CHANNELS); pthread_mutex_unlock(&buffer_mutex); *the_delay = result; return reply; diff --git a/scripts/shairport-sync.conf b/scripts/shairport-sync.conf index 001a88d26..cddffeb72 100644 --- a/scripts/shairport-sync.conf +++ b/scripts/shairport-sync.conf @@ -159,8 +159,8 @@ sndio = // --with-pw pw = { -// application_name = "Shairport Sync"; // Set this to the name that should appear in the Sounds "Applications". -// node_name = ""; // This is normally "shairport-sync". +// application_name = "Shairport Sync"; // Set this to the name that should appear in the Sounds "Applications" or "Volume Levels". +// node_name = "Shairport Sync"; // This appears in some PipeWire CLI tool outputs }; // Parameters for the "pa" PulseAudio backend. From b0a2fe14ec52a24b48939ba0ea4683dd7d54ae40 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Thu, 5 Oct 2023 16:42:34 +0100 Subject: [PATCH 50/61] Update RELEASENOTES-DEVELOPMENT.md --- RELEASENOTES-DEVELOPMENT.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/RELEASENOTES-DEVELOPMENT.md b/RELEASENOTES-DEVELOPMENT.md index f9d557378..7a9c86134 100644 --- a/RELEASENOTES-DEVELOPMENT.md +++ b/RELEASENOTES-DEVELOPMENT.md @@ -1,3 +1,7 @@ +Version 4.3.2-dev-47-g288e5eb6 +=== +* Change the default for `node_name` from "shairport-sync" to "Shairport Sync". + Version 4.3.2-dev-45-g3763b321 === **Enhancement** From d7b365597a792f1bce6dd586362d414756a67adc Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Thu, 5 Oct 2023 16:43:09 +0100 Subject: [PATCH 51/61] Update RELEASENOTES-DEVELOPMENT.md --- RELEASENOTES-DEVELOPMENT.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/RELEASENOTES-DEVELOPMENT.md b/RELEASENOTES-DEVELOPMENT.md index 7a9c86134..f1c531387 100644 --- a/RELEASENOTES-DEVELOPMENT.md +++ b/RELEASENOTES-DEVELOPMENT.md @@ -1,6 +1,6 @@ Version 4.3.2-dev-47-g288e5eb6 === -* Change the default for `node_name` from "shairport-sync" to "Shairport Sync". +* In the PipeWire backend, change the default for `node_name` from "shairport-sync" to "Shairport Sync". Version 4.3.2-dev-45-g3763b321 === From 420fe4ca7ecb5d4fd252d6664961180d1ad7c6ad Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Fri, 6 Oct 2023 10:50:30 +0100 Subject: [PATCH 52/61] Remove code that was double-deallocating settings strings. --- audio_pa.c | 8 -------- 1 file changed, 8 deletions(-) diff --git a/audio_pa.c b/audio_pa.c index c0fe1bd2c..6982c8fb0 100644 --- a/audio_pa.c +++ b/audio_pa.c @@ -244,14 +244,6 @@ static void deinit(void) { pa_stream_disconnect(stream); pa_threaded_mainloop_stop(mainloop); pa_threaded_mainloop_free(mainloop); - if (config.pa_server) - free(config.pa_server); - if (config.pa_application_name) - free(config.pa_application_name); - if (config.pa_sink) - free(config.pa_sink); - config.pa_server = config.pa_application_name = config.pa_sink = NULL; - // debug(1, "pa deinit done"); } From 98679bbb54f5aaeda859e34aa28425647b8d179e Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Fri, 6 Oct 2023 10:51:49 +0100 Subject: [PATCH 53/61] Add PipeWire sink target setting. Fix PipeWire node name code, duh, remove code that was double-deallocating settings strings. --- audio_pw.c | 28 ++++++++++++++-------------- common.h | 1 + scripts/shairport-sync.conf | 3 ++- 3 files changed, 17 insertions(+), 15 deletions(-) diff --git a/audio_pw.c b/audio_pw.c index f77b8766c..1bd9c5ab2 100644 --- a/audio_pw.c +++ b/audio_pw.c @@ -173,13 +173,6 @@ static void deinit(void) { pw_thread_loop_destroy(data.loop); pw_deinit(); free(audio_lmb); // deallocate that buffer - if (config.pw_application_name) - free(config.pw_application_name); - if (config.pw_node_name) - free(config.pw_node_name); - config.pw_application_name = config.pw_node_name = NULL; - - } static int init(__attribute__((unused)) int argc, __attribute__((unused)) char **argv) { @@ -196,21 +189,25 @@ 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(); - - + // now any PipeWire-specific options if (config.cfg != NULL) { const char *str; - /* Get the Application Name. */ + // Get the optional Application Name, if provided. if (config_lookup_string(config.cfg, "pw.application_name", &str)) { config.pw_application_name = (char *)str; } - /* Get the PipeWire node name. */ - if (config_lookup_string(config.cfg, "pa.node_name", &str)) { + // Get the optional PipeWire node name, if provided. + if (config_lookup_string(config.cfg, "pw.node_name", &str)) { config.pw_node_name = (char *)str; } + + // Get the optional PipeWire sink target name, if provided. + if (config_lookup_string(config.cfg, "pw.sink_target", &str)) { + config.pw_sink_target = (char *)str; + } } // finished collecting settings @@ -247,12 +244,15 @@ static int init(__attribute__((unused)) int argc, __attribute__((unused)) char * char* nodename = config.pw_node_name; if (nodename == NULL) nodename = "Shairport Sync"; - - props = pw_properties_new(PW_KEY_MEDIA_TYPE, "Audio", PW_KEY_MEDIA_CATEGORY, "Playback", PW_KEY_MEDIA_ROLE, "Music", PW_KEY_APP_NAME, appname, PW_KEY_NODE_NAME, nodename, NULL); + + if (config.pw_sink_target != NULL) { + debug(3, "setting sink target to \"%s\".", config.pw_sink_target); + pw_properties_set(props, PW_KEY_TARGET_OBJECT, config.pw_sink_target); + } data.stream = pw_stream_new_simple(pw_thread_loop_get_loop(data.loop), config.appName, props, &stream_events, &data); diff --git a/common.h b/common.h index 3a392f684..a1d50d448 100644 --- a/common.h +++ b/common.h @@ -149,6 +149,7 @@ typedef struct { // Defaults to "Shairport Sync". char *pw_node_name; // defaults to the application's name, usually "shairport-sync". + char *pw_sink_target; // leave this unset if you don't want to change the sink_target. #endif #ifdef CONFIG_METADATA int metadata_enabled; diff --git a/scripts/shairport-sync.conf b/scripts/shairport-sync.conf index cddffeb72..79ef16cf4 100644 --- a/scripts/shairport-sync.conf +++ b/scripts/shairport-sync.conf @@ -160,7 +160,8 @@ sndio = pw = { // application_name = "Shairport Sync"; // Set this to the name that should appear in the Sounds "Applications" or "Volume Levels". -// node_name = "Shairport Sync"; // This appears in some PipeWire CLI tool outputs +// node_name = "Shairport Sync"; // This appears in some PipeWire CLI tool outputs. +// sink_target = ""; // Leave this commented out to get the sink target already chosen by the PipeWire system. }; // Parameters for the "pa" PulseAudio backend. From 07c9cc2d64509219b831fd9bc0a24bbaa18092e3 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Fri, 6 Oct 2023 10:58:24 +0100 Subject: [PATCH 54/61] Update RELEASENOTES-DEVELOPMENT.md --- RELEASENOTES-DEVELOPMENT.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/RELEASENOTES-DEVELOPMENT.md b/RELEASENOTES-DEVELOPMENT.md index f1c531387..64460da2e 100644 --- a/RELEASENOTES-DEVELOPMENT.md +++ b/RELEASENOTES-DEVELOPMENT.md @@ -1,3 +1,13 @@ +Version 4.3.2-dev-51-g98679bbb +=== +**Enhancement** +* Add a new `pw` (PipeWire) backend setting -- `sink_target`. Leave the setting commented out to use the sink target already selected by the PipeWire system. + +**Bug Fixes** +* Fix a bug in picking up the PipeWire `node_name` -- it was being sought in the `pa` stanza instead of the `pw` stanza of the configuration file. +* Fix memory management bugs in the PipeWire backend -- settings strings were being deallocated twice on exit. +* Fix memory management bugs in the PulseAudio backend -- settings strings were being deallocated twice on exit. + Version 4.3.2-dev-47-g288e5eb6 === * In the PipeWire backend, change the default for `node_name` from "shairport-sync" to "Shairport Sync". From 7aaaf0c96d7f120e15c2ae16a859eae0ee04f10f Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Fri, 6 Oct 2023 11:13:30 +0100 Subject: [PATCH 55/61] Move the PipeWire stanza up in the configuration file. --- scripts/shairport-sync.conf | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/scripts/shairport-sync.conf b/scripts/shairport-sync.conf index 79ef16cf4..78a79c7e1 100644 --- a/scripts/shairport-sync.conf +++ b/scripts/shairport-sync.conf @@ -142,6 +142,16 @@ alsa = // disable_standby_mode_silence_scan_interval = 0.004; // Use this optional advanced setting to control how often the amount of audio remaining in the output buffer should be checked. }; +// Parameters for the "pw" PipeWire backend. +// For this section to be operative, Shairport Sync must be built with the following configuration flag: +// --with-pw +pw = +{ +// application_name = "Shairport Sync"; // Set this to the name that should appear in the Sounds "Applications" or "Volume Levels". +// node_name = "Shairport Sync"; // This appears in some PipeWire CLI tool outputs. +// sink_target = ""; // Leave this commented out to get the sink target already chosen by the PipeWire system. +}; + // Parameters for the "sndio" audio back end. All are optional. // For this section to be operative, Shairport Sync must be built with the following configuration flag: // --with-sndio @@ -154,16 +164,6 @@ sndio = // bufsz = ; // advanced optional setting to set the buffer size near to this value }; -// Parameters for the "pw" PipeWire backend. -// For this section to be operative, Shairport Sync must be built with the following configuration flag: -// --with-pw -pw = -{ -// application_name = "Shairport Sync"; // Set this to the name that should appear in the Sounds "Applications" or "Volume Levels". -// node_name = "Shairport Sync"; // This appears in some PipeWire CLI tool outputs. -// sink_target = ""; // Leave this commented out to get the sink target already chosen by the PipeWire system. -}; - // Parameters for the "pa" PulseAudio backend. // For this section to be operative, Shairport Sync must be built with the following configuration flag: // --with-pa From f3e0ddf2971da1a8a78398724ea42291bb26dff2 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Sun, 8 Oct 2023 17:40:43 +0100 Subject: [PATCH 56/61] Typo --- audio_pw.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/audio_pw.c b/audio_pw.c index 1bd9c5ab2..76764d2f3 100644 --- a/audio_pw.c +++ b/audio_pw.c @@ -99,7 +99,7 @@ static void on_process(void *userdata) { struct pw_buffer *b = pw_stream_dequeue_buffer(data->stream); if (b == NULL) { pw_log_warn("out of buffers: %m"); - die("PipeWire failue -- out of buffers!"); + die("PipeWire failure -- out of buffers!"); } struct spa_buffer *buf = b->buffer; uint8_t *dest = buf->datas[0].data; From 4cd3c1da4bca050d092a54706dc2ddd128fd9a05 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Sun, 8 Oct 2023 17:43:02 +0100 Subject: [PATCH 57/61] Return code 200 for a POST of type /feedback, and continue to return 500 for everything else. --- rtsp.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/rtsp.c b/rtsp.c index 44058fda3..217fe0a6b 100644 --- a/rtsp.c +++ b/rtsp.c @@ -2055,12 +2055,17 @@ void handle_get(__attribute((unused)) rtsp_conn_info *conn, __attribute((unused) resp->respcode = 500; } -void handle_post(rtsp_conn_info *conn, rtsp_message *req, - __attribute((unused)) rtsp_message *resp) { - debug(1, "Connection %d: POST %s Content-Length %d", conn->connection_number, req->path, - req->contentlength); +void handle_post(rtsp_conn_info *conn, rtsp_message *req, rtsp_message *resp) { resp->respcode = 500; + if (strcmp(req->path, "/feedback") == 0) { + resp->respcode = 200; + } else { + 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); + } } + #endif #ifdef CONFIG_AIRPLAY_2 From 3298f42a52a1682b71f8ef1aab5bf23e172cfaa8 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Sun, 8 Oct 2023 17:49:35 +0100 Subject: [PATCH 58/61] Update RELEASENOTES-DEVELOPMENT.md --- RELEASENOTES-DEVELOPMENT.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/RELEASENOTES-DEVELOPMENT.md b/RELEASENOTES-DEVELOPMENT.md index 64460da2e..cf4e5bcda 100644 --- a/RELEASENOTES-DEVELOPMENT.md +++ b/RELEASENOTES-DEVELOPMENT.md @@ -1,3 +1,8 @@ +Version 4.3.2-dev-56-g4cd3c1da +=== +**Investigation** +* Return `200` -- "OK" in response to a `POST` message with the argument `/feedback` on a Classic AirPlay connection. Continue to return `500` -- "Internal Server Error" for all other `POST` messages. This is part of an investigation into [Issue #1745](https://github.com/mikebrady/shairport-sync/issues/1745). + Version 4.3.2-dev-51-g98679bbb === **Enhancement** From b70dd4633fa2e366eb09482e27ee791c58894996 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Fri, 13 Oct 2023 07:52:51 +0100 Subject: [PATCH 59/61] Change response to a /feedback message in a Classic AirPlay session to 501 Not Implmemented instead of 200 OK. --- rtsp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rtsp.c b/rtsp.c index 217fe0a6b..26883471d 100644 --- a/rtsp.c +++ b/rtsp.c @@ -2058,7 +2058,7 @@ void handle_get(__attribute((unused)) rtsp_conn_info *conn, __attribute((unused) void handle_post(rtsp_conn_info *conn, rtsp_message *req, rtsp_message *resp) { resp->respcode = 500; if (strcmp(req->path, "/feedback") == 0) { - resp->respcode = 200; + resp->respcode = 501; } else { debug(1, "Connection %d: Airplay 1. Unhandled POST %s Content-Length %d", conn->connection_number, req->path, req->contentlength); From a65ec2d7f1f380bbae196d7f8f1cd6a88ef5777b Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Fri, 13 Oct 2023 07:56:30 +0100 Subject: [PATCH 60/61] Update RELEASENOTES-DEVELOPMENT.md --- RELEASENOTES-DEVELOPMENT.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/RELEASENOTES-DEVELOPMENT.md b/RELEASENOTES-DEVELOPMENT.md index cf4e5bcda..9d81e0244 100644 --- a/RELEASENOTES-DEVELOPMENT.md +++ b/RELEASENOTES-DEVELOPMENT.md @@ -1,3 +1,8 @@ +Version 4.3.2-dev-58-gb70dd463 +=== +**Investigation -- continued** +* Return `501` ("Not Implemented") instead of `200` ("OK") in response to a `POST` message with the argument `/feedback` on a Classic AirPlay connection. + Version 4.3.2-dev-56-g4cd3c1da === **Investigation** From 9732afb58f517220f75a964af5d5824a1e28c26b Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Sun, 15 Oct 2023 08:32:02 +0100 Subject: [PATCH 61/61] Update CAR INSTALL.md Add note about Bookworm --- CAR INSTALL.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/CAR INSTALL.md b/CAR INSTALL.md index db4c2a774..767410eea 100644 --- a/CAR INSTALL.md +++ b/CAR INSTALL.md @@ -9,12 +9,14 @@ Note that Android devices can not, so far, do this trick of using the two networ ## Example +**Important Note** This guide can not be used for the latest (time of writing: October 2023) version of Rasberry Pi OS (Bookworm) because the `dhcpd` package is no longer part of the system. It does work for Rasberry Pi OS (Bullseye) + In this example, a Raspberry Pi Zero 2 W and a Pimoroni PHAT DAC are used. Shairport Sync will be built for AirPlay 2 operation, but you can build it for "classic" AirPlay (aka AirPlay 1) operation if you prefer. A Pi Zero W is powerful enough for classic AirPlay. -Please note that some of the details of setting up networks are specific to the version of Linux used – Rasberry Pi OS (Bullseye) Lite or later. +Please note that some of the details of setting up networks are specific to the version of Linux used. ### Prepare the initial SD Image -* Download the latest version of Raspberry Pi OS (Lite) – Bullseye (Lite) of 2022-04-04 at the time of writing – and install it onto an SD Card using `Raspberry Pi Imager`. The Lite version is preferable to the Desktop version as it doesn't include a sound server like PulseAudio or PipeWire that can prevent direct access to the audio output device. +* Download Raspberry Pi OS Bullseye (Lite) and install it onto an SD Card using `Raspberry Pi Imager`. The Lite version is preferable to the Desktop version as it doesn't include a sound server like PulseAudio or PipeWire that can prevent direct access to the audio output device. * Before writing the image to the card, use the Settings control on `Raspberry Pi Imager` to set hostname, enable SSH and provide a username and password to use while building the system. Similarly, you can specify a wireless network the Pi will connect to while building the system. Later on, the Pi will be configured to start its own isolated network. * The next few steps are to add the overlay needed for the sound card. This may not be necessary in your case, but in this example a Pimoroni PHAT is being used. If you do not need to add an overlay, skip these steps. * Mount the card on a Linux machine. Two drives should appear – a `boot` drive and a `rootfs` drive.