From a0e9c0ac34fd350d9179d6fc39c47e64bb07e875 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Sun, 21 May 2023 13:42:34 +0100 Subject: [PATCH 01/63] add --broken to the git describe call --- Makefile.am | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Makefile.am b/Makefile.am index 1634c6af..6cf318d3 100644 --- a/Makefile.am +++ b/Makefile.am @@ -38,14 +38,14 @@ endif endif endif -# include information generated by 'git describe --tags --dirty' if requested +# include information generated by 'git describe --tags --dirty --broken' if requested if USE_GIT_VERSION shairport_sync_SOURCES += gitversion.c gitversion.h: .git/index printf "// Do not edit!\n" > gitversion.h - printf "// This file is automatically generated by 'git describe --tags --dirty', if available.\n" >> gitversion.h + printf "// This file is automatically generated by 'git describe --tags --dirty --broken', if available.\n" >> gitversion.h printf " char git_version_string[] = \"" >> gitversion.h - git describe --tags --dirty | tr -d '[[:space:]]' >> gitversion.h + git describe --tags --dirty --broken | tr -d '[[:space:]]' >> gitversion.h printf "\";\n" >> gitversion.h gitversion.c: gitversion.h touch gitversion.c From 1ff669d4d2b8516b857830db1194b418452a529c Mon Sep 17 00:00:00 2001 From: Dan George Date: Wed, 24 May 2023 15:15:09 +1000 Subject: [PATCH 02/63] Specify libao input channel ordering --- audio_ao.c | 1 + 1 file changed, 1 insertion(+) diff --git a/audio_ao.c b/audio_ao.c index 64299659..61746487 100644 --- a/audio_ao.c +++ b/audio_ao.c @@ -131,6 +131,7 @@ static int init(int argc, char **argv) { fmt.rate = 44100; fmt.channels = 2; fmt.byte_format = AO_FMT_NATIVE; + fmt.matrix = strdup("L,R"); } return 0; } From beb195eb46a78f620f1e8f22f5d9a724920e6d4a Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Sun, 28 May 2023 08:10:13 +0100 Subject: [PATCH 03/63] Uncheck "require password" Added hint thanks to [ajones45](https://github.com/ajones45). --- ADDINGTOHOME.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ADDINGTOHOME.md b/ADDINGTOHOME.md index df5f4d5a..1703332b 100644 --- a/ADDINGTOHOME.md +++ b/ADDINGTOHOME.md @@ -5,7 +5,7 @@ Here is how to add a Shairport Sync device to the Apple Home application on iOS Step 0: Speaker & TV access. --- -In the Home app > Home settings "Allow Speaker & TV access", select the option "Anyone On the Same Network". +In the Home app > Home settings "Allow Speaker & TV access", select the option "Anyone On the Same Network". If a "require password" option below the "anyone on same network" is available, make sure it is unchecked. Step 1: Open the "Home" app --- From c3d99f96c9e0f810304efa84498b84e6ef2fa342 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Mon, 5 Jun 2023 14:22:40 +0100 Subject: [PATCH 04/63] increase the debug output for RTSP messages and responses at verbosity 2. --- rtsp.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/rtsp.c b/rtsp.c index 02c63cce..10b48109 100644 --- a/rtsp.c +++ b/rtsp.c @@ -5172,7 +5172,7 @@ static void *rtsp_conversation_thread_func(void *pconn) { #endif while (conn->stop == 0) { - int debug_level = 3; // for printing the request and response + int debug_level = 2; // for printing the request and response reply = rtsp_read_request(conn, &req); if (reply == rtsp_read_request_response_ok) { pthread_cleanup_push(msg_cleanup_function, (void *)&req); @@ -5180,10 +5180,10 @@ static void *rtsp_conversation_thread_func(void *pconn) { pthread_cleanup_push(msg_cleanup_function, (void *)&resp); resp->respcode = 501; // Not Implemented int dl = debug_level; - if ((strcmp(req->method, "OPTIONS") == 0) || - (strcmp(req->method, "POST") == - 0)) // the options message is very common, so don't log it until level 3 - dl = 3; + //if ((strcmp(req->method, "OPTIONS") == 0) || + // (strcmp(req->method, "POST") == + // 0)) // the options message is very common, so don't log it until level 3 + // dl = 3; if (conn->airplay_stream_category == remote_control_stream) { debug(dl, "Connection %d (RC): Received an RTSP Packet of type \"%s\":", From d633a82e3f27ae49e8e48c1f7ff9cb0fe487707a Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Mon, 5 Jun 2023 14:40:20 +0100 Subject: [PATCH 05/63] Update RELEASENOTES-DEVELOPMENT.md Enhanced debug messages --- RELEASENOTES-DEVELOPMENT.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/RELEASENOTES-DEVELOPMENT.md b/RELEASENOTES-DEVELOPMENT.md index 7a7157ff..22130751 100644 --- a/RELEASENOTES-DEVELOPMENT.md +++ b/RELEASENOTES-DEVELOPMENT.md @@ -1,5 +1,12 @@ +Version 4.2.1d0-5-gc3d99f96 +==== +**Enhancement** +* Increase the level of detail in the debug log at verbosity level 2 (i.e. `-vv`). + Version 4.1-dev-922-gffeeacc4 ==== +This is (approximately) Version 4.2 release. + **Enhancement** * When built for AirPlay 2, include the Shared Memory Interface (SMI) version number in the version string, in the form: `smi*` where `*` is the version number, e.g. `smi9`. This must be same as the SMI version number of NQPTP, which can be listed by entering `$ nqptp -V`. From eef619863936863b03baf23abc9b5ae1547cf3b0 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Mon, 5 Jun 2023 14:42:34 +0100 Subject: [PATCH 06/63] Bump configure.ac version number... --- configure.ac | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index 7797c074..61f42589 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.2], [4265913+mikebrady@users.noreply.github.com]) +AC_INIT([shairport-sync], [4.2.1-dev], [4265913+mikebrady@users.noreply.github.com]) AM_INIT_AUTOMAKE([subdir-objects]) AC_CONFIG_SRCDIR([shairport.c]) AC_CONFIG_HEADERS([config.h]) From fbeba7e360d9828269dcc2d1056912b13fcdaaa9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20K=C3=B6nig?= Date: Wed, 7 Jun 2023 23:32:24 +0200 Subject: [PATCH 07/63] Fix some openssl 3.0 deprecation warnings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Christian König --- metadata_hub.c | 14 +++++++----- rtsp.c | 58 +++++++++++++++++++++++++++++++------------------- 2 files changed, 45 insertions(+), 27 deletions(-) diff --git a/metadata_hub.c b/metadata_hub.c index 796e0927..27176058 100644 --- a/metadata_hub.c +++ b/metadata_hub.c @@ -56,7 +56,7 @@ #endif #ifdef CONFIG_OPENSSL -#include +#include #endif struct metadata_bundle metadata_store; @@ -209,10 +209,14 @@ char *metadata_write_image_file(const char *buf, int len) { // uint8_t ap_md5[16]; #ifdef CONFIG_OPENSSL - MD5_CTX ctx; - MD5_Init(&ctx); - MD5_Update(&ctx, buf, len); - MD5_Final(img_md5, &ctx); + EVP_MD_CTX *ctx; + unsigned int img_md5_len = EVP_MD_size(EVP_md5()); + + ctx = EVP_MD_CTX_new(); + EVP_DigestInit_ex(ctx, EVP_md5(), NULL); + EVP_DigestUpdate(ctx, buf, len); + EVP_DigestFinal_ex(ctx, img_md5, &img_md5_len); + EVP_MD_CTX_free(ctx); #endif #ifdef CONFIG_MBEDTLS diff --git a/rtsp.c b/rtsp.c index 10b48109..4db9c0b4 100644 --- a/rtsp.c +++ b/rtsp.c @@ -55,7 +55,7 @@ #include "config.h" #ifdef CONFIG_OPENSSL -#include +#include #endif #ifdef CONFIG_MBEDTLS @@ -4882,22 +4882,31 @@ static int rtsp_auth(char **nonce, rtsp_message *req, rtsp_message *resp) { uint8_t digest_urp[16], digest_mu[16], digest_total[16]; #ifdef CONFIG_OPENSSL - MD5_CTX ctx; - + EVP_MD_CTX *ctx; + unsigned int digest_urp_len = EVP_MD_size(EVP_md5()); + unsigned int digest_mu_len = EVP_MD_size(EVP_md5()); int oldState; pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &oldState); - MD5_Init(&ctx); - MD5_Update(&ctx, username, strlen(username)); - MD5_Update(&ctx, ":", 1); - MD5_Update(&ctx, realm, strlen(realm)); - MD5_Update(&ctx, ":", 1); - MD5_Update(&ctx, config.password, strlen(config.password)); - MD5_Final(digest_urp, &ctx); - MD5_Init(&ctx); - MD5_Update(&ctx, req->method, strlen(req->method)); - MD5_Update(&ctx, ":", 1); - MD5_Update(&ctx, uri, strlen(uri)); - MD5_Final(digest_mu, &ctx); + ctx = EVP_MD_CTX_new(); + EVP_DigestInit_ex(ctx, EVP_md5(), NULL); + + EVP_DigestUpdate(ctx, username, strlen(username)); + EVP_DigestUpdate(ctx, ":", 1); + EVP_DigestUpdate(ctx, realm, strlen(realm)); + EVP_DigestUpdate(ctx, ":", 1); + EVP_DigestUpdate(ctx, config.password, strlen(config.password)); + EVP_DigestFinal_ex(ctx, digest_urp, &digest_urp_len); + EVP_MD_CTX_free(ctx); + + ctx = EVP_MD_CTX_new(); + EVP_DigestInit_ex(ctx, EVP_md5(), NULL); + + EVP_DigestUpdate(ctx, req->method, strlen(req->method)); + EVP_DigestUpdate(ctx, ":", 1); + EVP_DigestUpdate(ctx, uri, strlen(uri)); + + EVP_DigestFinal_ex(ctx, digest_mu, &digest_mu_len); + EVP_MD_CTX_free(ctx); pthread_setcancelstate(oldState, NULL); #endif @@ -4956,15 +4965,20 @@ static int rtsp_auth(char **nonce, rtsp_message *req, rtsp_message *resp) { #ifdef CONFIG_OPENSSL pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &oldState); - MD5_Init(&ctx); - MD5_Update(&ctx, buf, 32); - MD5_Update(&ctx, ":", 1); - MD5_Update(&ctx, *nonce, strlen(*nonce)); - MD5_Update(&ctx, ":", 1); + unsigned int digest_total_len = EVP_MD_size(EVP_md5()); + + ctx = EVP_MD_CTX_new(); + EVP_DigestInit_ex(ctx, EVP_md5(), NULL); + + EVP_DigestUpdate(ctx, buf, 32); + EVP_DigestUpdate(ctx, ":", 1); + EVP_DigestUpdate(ctx, *nonce, strlen(*nonce)); + EVP_DigestUpdate(ctx, ":", 1); for (i = 0; i < 16; i++) snprintf((char *)buf + 2 * i, 3, "%02x", digest_mu[i]); - MD5_Update(&ctx, buf, 32); - MD5_Final(digest_total, &ctx); + EVP_DigestUpdate(ctx, buf, 32); + EVP_DigestFinal_ex(ctx, digest_total, &digest_total_len); + EVP_MD_CTX_free(ctx); pthread_setcancelstate(oldState, NULL); #endif From b4b8d2638b6c1558938496c5aab276693f9793e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20K=C3=B6nig?= Date: Fri, 9 Jun 2023 19:55:36 +0200 Subject: [PATCH 08/63] Run workflows on pull requests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Christian König --- .github/workflows/check_ap2_systemd_basic.yml | 2 + .github/workflows/check_ap2_systemd_full.yml | 4 +- .../check_ap2_systemd_full_build_folder.yml | 2 + .github/workflows/check_ap2_systemv_full.yml | 4 +- .github/workflows/check_classic_mac_basic.yml | 4 +- .../workflows/check_classic_systemd_basic.yml | 4 +- .../workflows/check_classic_systemd_full.yml | 2 + ...ocker-build-on-push_and_pull_request.yaml} | 49 ++++++++++++++++++- 8 files changed, 65 insertions(+), 6 deletions(-) rename .github/workflows/{docker-build-on-push.yaml => docker-build-on-push_and_pull_request.yaml} (60%) diff --git a/.github/workflows/check_ap2_systemd_basic.yml b/.github/workflows/check_ap2_systemd_basic.yml index cb1ecdc8..23e155be 100644 --- a/.github/workflows/check_ap2_systemd_basic.yml +++ b/.github/workflows/check_ap2_systemd_basic.yml @@ -3,6 +3,8 @@ name: Basic ALSA configuration for systemd, using a build folder. on: push: branches: [ "development" ] + pull_request: + types: [opened, synchronize, reopened, ready_for_review] jobs: build: diff --git a/.github/workflows/check_ap2_systemd_full.yml b/.github/workflows/check_ap2_systemd_full.yml index 94239fe0..e99e0a23 100644 --- a/.github/workflows/check_ap2_systemd_full.yml +++ b/.github/workflows/check_ap2_systemd_full.yml @@ -3,7 +3,9 @@ name: Full configuration (but without apple-alac) for systemd. on: push: branches: [ "development", "danger" ] - + pull_request: + types: [opened, synchronize, reopened, ready_for_review] + jobs: build: diff --git a/.github/workflows/check_ap2_systemd_full_build_folder.yml b/.github/workflows/check_ap2_systemd_full_build_folder.yml index cad5bbe4..bdab8195 100644 --- a/.github/workflows/check_ap2_systemd_full_build_folder.yml +++ b/.github/workflows/check_ap2_systemd_full_build_folder.yml @@ -3,6 +3,8 @@ name: Full configuration (but without apple-alac) for systemd, using a build fol on: push: branches: [ "development" ] + pull_request: + types: [opened, synchronize, reopened, ready_for_review] jobs: build: diff --git a/.github/workflows/check_ap2_systemv_full.yml b/.github/workflows/check_ap2_systemv_full.yml index 4e514430..1f8dab07 100644 --- a/.github/workflows/check_ap2_systemv_full.yml +++ b/.github/workflows/check_ap2_systemv_full.yml @@ -3,7 +3,9 @@ name: Full configuration (but without apple-alac) for a System V system. on: push: branches: [ "development" ] - + pull_request: + types: [opened, synchronize, reopened, ready_for_review] + jobs: build: diff --git a/.github/workflows/check_classic_mac_basic.yml b/.github/workflows/check_classic_mac_basic.yml index f53de4a2..b06e046a 100644 --- a/.github/workflows/check_classic_mac_basic.yml +++ b/.github/workflows/check_classic_mac_basic.yml @@ -3,7 +3,9 @@ name: Basic libao configuration for macOS with BREW -- classic only, because mac on: push: branches: [ "development", "danger" ] - + pull_request: + types: [opened, synchronize, reopened, ready_for_review] + jobs: build: diff --git a/.github/workflows/check_classic_systemd_basic.yml b/.github/workflows/check_classic_systemd_basic.yml index 82525484..95f9d904 100644 --- a/.github/workflows/check_classic_systemd_basic.yml +++ b/.github/workflows/check_classic_systemd_basic.yml @@ -3,7 +3,9 @@ name: Basic ALSA classic configuration for systemd, using a build folder. on: push: branches: [ "development" ] - + pull_request: + types: [opened, synchronize, reopened, ready_for_review] + jobs: build: diff --git a/.github/workflows/check_classic_systemd_full.yml b/.github/workflows/check_classic_systemd_full.yml index 57bb7ac5..75229469 100644 --- a/.github/workflows/check_classic_systemd_full.yml +++ b/.github/workflows/check_classic_systemd_full.yml @@ -3,6 +3,8 @@ name: Full classic configuration (but without apple-alac) for systemd, using a b on: push: branches: [ "development", "danger" ] + pull_request: + types: [opened, synchronize, reopened, ready_for_review] jobs: build: diff --git a/.github/workflows/docker-build-on-push.yaml b/.github/workflows/docker-build-on-push_and_pull_request.yaml similarity index 60% rename from .github/workflows/docker-build-on-push.yaml rename to .github/workflows/docker-build-on-push_and_pull_request.yaml index 79ac8eac..fdbeb810 100644 --- a/.github/workflows/docker-build-on-push.yaml +++ b/.github/workflows/docker-build-on-push_and_pull_request.yaml @@ -4,20 +4,65 @@ # 'master' - rolling, rolling-classic # 'development' - development, development-classic -name: Build and push docker (commit) +name: Build and push docker (push/pull request) on: push: branches: - master - development + pull_request: + types: [opened, synchronize, reopened, ready_for_review] env: DOCKER_PLATFORMS: linux/386,linux/amd64,linux/arm/v6,linux/arm/v7,linux/arm64 NQPTP_BRANCH: main jobs: - main: + test-build-on-pull-request: + if: github.event_name == 'pull_request' + runs-on: ubuntu-latest + steps: + - name: Checkout Repo + uses: actions/checkout@v3 + with: + fetch-depth: 0 + ref: ${{github.event.pull_request.head.ref}} + repository: ${{github.event.pull_request.head.repo.full_name}} + + - name: Set SHAIRPORT_SYNC_BRANCH env + run: | + SHAIRPORT_SYNC_BRANCH=$(git rev-parse --abbrev-ref HEAD) + echo "Current SHAIRPORT_SYNC_BRANCH set to ${SHAIRPORT_SYNC_BRANCH}" + echo "SHAIRPORT_SYNC_BRANCH=${SHAIRPORT_SYNC_BRANCH}" >> $GITHUB_ENV + + - name: Set up QEMU + uses: docker/setup-qemu-action@v2.1.0 + + - name: Set up Docker Buildx + uses: docker/setup-buildx-action@v2.5.0 + + - name: Build (classic) + uses: docker/build-push-action@v4.0.0 + with: + context: ./ + file: ./docker/classic/Dockerfile + push: false + build-args: | + SHAIRPORT_SYNC_BRANCH=${{ env.SHAIRPORT_SYNC_BRANCH }} + + - name: Build + uses: docker/build-push-action@v4.0.0 + with: + context: ./ + file: ./docker/Dockerfile + push: false + build-args: | + SHAIRPORT_SYNC_BRANCH=${{ env.SHAIRPORT_SYNC_BRANCH }} + NQPTP_BRANCH=${{ env.NQPTP_BRANCH }} + + build-and-publish: + if: github.event_name != 'pull_request' runs-on: ubuntu-latest steps: - name: Checkout From b3af03ebc0c30221361ff65ee33dc288ffb24d1f Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Sat, 10 Jun 2023 15:36:40 +0100 Subject: [PATCH 09/63] fix a warning of discarding a const property --- audio_pa.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/audio_pa.c b/audio_pa.c index eef771db..1ba82b34 100644 --- a/audio_pa.c +++ b/audio_pa.c @@ -59,7 +59,7 @@ void stream_success_cb(pa_stream *stream, int success, void *userdata); void stream_write_cb(pa_stream *stream, size_t requested_bytes, void *userdata); int status_error_notifications = 0; -static void check_pa_stream_status(const pa_stream *p, const char *message) { +static void check_pa_stream_status(pa_stream *p, const char *message) { if (status_error_notifications < 10) { if (p == NULL) { warn("%s No pulseaudio stream!", message); From 1d858d6866f9c97e990bb0b613a66bae87b9a7f2 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Sat, 10 Jun 2023 15:52:37 +0100 Subject: [PATCH 10/63] replace code making deprecated OpenSSL calls --- common.c | 103 +++++++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 81 insertions(+), 22 deletions(-) diff --git a/common.c b/common.c index f60f1ea5..6bac822a 100644 --- a/common.c +++ b/common.c @@ -71,11 +71,12 @@ #endif #ifdef CONFIG_OPENSSL -#include -#include -#include -#include -#include +#include // needed for BIO_new_mem_buf +#include // needed for older AES stuff +#include // needed for ERR_error_string, ERR_get_error +#include // needed for EVP_PKEY_CTX_new, EVP_PKEY_sign_init, EVP_PKEY_sign +#include // needed for PEM_read_bio_RSAPrivateKey, EVP_PKEY_CTX_set_rsa_padding +#include // needed for EVP_PKEY_CTX_set_rsa_padding #endif #ifdef CONFIG_POLARSSL @@ -808,25 +809,83 @@ static char super_secret_key[] = uint8_t *rsa_apply(uint8_t *input, int inlen, int *outlen, int mode) { int oldState; pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &oldState); - RSA *rsa = NULL; - if (!rsa) { - BIO *bmem = BIO_new_mem_buf(super_secret_key, -1); - rsa = PEM_read_bio_RSAPrivateKey(bmem, NULL, NULL, NULL); - BIO_free(bmem); - } + uint8_t *out = NULL; + BIO *bmem = BIO_new_mem_buf(super_secret_key, -1); // 1.0.2 + EVP_PKEY *rsaKey = PEM_read_bio_PrivateKey(bmem, NULL, NULL, NULL); // 1.0.2 + BIO_free(bmem); + size_t ol = 0; + if (rsaKey != NULL) { + EVP_PKEY_CTX *ctx = EVP_PKEY_CTX_new(rsaKey, NULL); // 1.0.2 + if (ctx != NULL) { - uint8_t *out = malloc(RSA_size(rsa)); - switch (mode) { - case RSA_MODE_AUTH: - *outlen = RSA_private_encrypt(inlen, input, out, rsa, RSA_PKCS1_PADDING); - break; - case RSA_MODE_KEY: - *outlen = RSA_private_decrypt(inlen, input, out, rsa, RSA_PKCS1_OAEP_PADDING); - break; - default: - die("bad rsa mode"); + switch (mode) { + case RSA_MODE_AUTH: { + if (EVP_PKEY_sign_init(ctx) > 0) { // 1.0.2 + if (EVP_PKEY_CTX_set_rsa_padding(ctx, RSA_PKCS1_PADDING) > 0) { // 1.0.2 + if (EVP_PKEY_sign(ctx, NULL, &ol, (const unsigned char *)input, inlen) > 0) { // 1.0.2 + debug(1, "output size = %lu.", ol); + out = (unsigned char *)malloc(ol); + if (EVP_PKEY_sign(ctx, out, &ol, (const unsigned char *)input, inlen) > 0) { // 1.0.2 + debug(1, "success with output length of %lu.", ol); + } else { + debug(1, "error 2 \"%s\" with EVP_PKEY_sign:", + ERR_error_string(ERR_get_error(), NULL)); + } + } else { + debug(1, + "error 1 \"%s\" with EVP_PKEY_sign:", ERR_error_string(ERR_get_error(), NULL)); + } + } else { + debug(1, "error \"%s\" with EVP_PKEY_CTX_set_rsa_padding:", + ERR_error_string(ERR_get_error(), NULL)); + } + } else { + debug(1, + "error \"%s\" with EVP_PKEY_sign_init:", ERR_error_string(ERR_get_error(), NULL)); + } + } break; + case RSA_MODE_KEY: { + if (EVP_PKEY_decrypt_init(ctx) > 0) { + if (EVP_PKEY_CTX_set_rsa_padding(ctx, RSA_PKCS1_OAEP_PADDING) > 0) { + /* Determine buffer length */ + if (EVP_PKEY_decrypt(ctx, NULL, &ol, (const unsigned char *)input, inlen) > 0) { + out = OPENSSL_malloc(ol); + if (out != NULL) { + if (EVP_PKEY_decrypt(ctx, out, &ol, (const unsigned char *)input, inlen) > 0) { + debug(1, "decrypt success"); + } else { + debug(1, "error \"%s\" with EVP_PKEY_decrypt:", + ERR_error_string(ERR_get_error(), NULL)); + } + } else { + debug(1, "OPENSSL_malloc failed"); + } + } else { + debug(1, + "error \"%s\" with EVP_PKEY_decrypt:", ERR_error_string(ERR_get_error(), NULL)); + } + } else { + debug(1, "error \"%s\" with EVP_PKEY_CTX_set_rsa_padding:", + ERR_error_string(ERR_get_error(), NULL)); + } + } else { + debug(1, "error \"%s\" with EVP_PKEY_decrypt_init:", + ERR_error_string(ERR_get_error(), NULL)); + } + } break; + default: + debug(1, "Unknown mode"); + break; + } + EVP_PKEY_CTX_free(ctx); // 1.0.2 + } else { + printf("error \"%s\" with EVP_PKEY_CTX_new:\n", ERR_error_string(ERR_get_error(), NULL)); + } + EVP_PKEY_free(rsaKey); // 1.0.2 + } else { + printf("error \"%s\" with EVP_PKEY_new:\n", ERR_error_string(ERR_get_error(), NULL)); } - RSA_free(rsa); + *outlen = ol; pthread_setcancelstate(oldState, NULL); return out; } From 78dbfe6a8809eddd623045c9edc9ea282ee62b5b Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Sat, 10 Jun 2023 16:06:25 +0100 Subject: [PATCH 11/63] replace some deprecated OpenSSL calls --- common.c | 5 ++--- shairport.c | 12 +++++++----- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/common.c b/common.c index 6bac822a..2c902e3b 100644 --- a/common.c +++ b/common.c @@ -823,10 +823,9 @@ uint8_t *rsa_apply(uint8_t *input, int inlen, int *outlen, int mode) { if (EVP_PKEY_sign_init(ctx) > 0) { // 1.0.2 if (EVP_PKEY_CTX_set_rsa_padding(ctx, RSA_PKCS1_PADDING) > 0) { // 1.0.2 if (EVP_PKEY_sign(ctx, NULL, &ol, (const unsigned char *)input, inlen) > 0) { // 1.0.2 - debug(1, "output size = %lu.", ol); out = (unsigned char *)malloc(ol); if (EVP_PKEY_sign(ctx, out, &ol, (const unsigned char *)input, inlen) > 0) { // 1.0.2 - debug(1, "success with output length of %lu.", ol); + debug(3, "success with output length of %lu.", ol); } else { debug(1, "error 2 \"%s\" with EVP_PKEY_sign:", ERR_error_string(ERR_get_error(), NULL)); @@ -852,7 +851,7 @@ uint8_t *rsa_apply(uint8_t *input, int inlen, int *outlen, int mode) { out = OPENSSL_malloc(ol); if (out != NULL) { if (EVP_PKEY_decrypt(ctx, out, &ol, (const unsigned char *)input, inlen) > 0) { - debug(1, "decrypt success"); + debug(3, "decrypt success"); } else { debug(1, "error \"%s\" with EVP_PKEY_decrypt:", ERR_error_string(ERR_get_error(), NULL)); diff --git a/shairport.c b/shairport.c index db54c8c6..8aafa269 100644 --- a/shairport.c +++ b/shairport.c @@ -62,6 +62,7 @@ #ifdef CONFIG_OPENSSL #include +#include #endif #if defined(CONFIG_DBUS_INTERFACE) @@ -2525,11 +2526,12 @@ int main(int argc, char **argv) { // debug(1, "size of hw_addr is %u.", sizeof(config.hw_addr)); #ifdef CONFIG_OPENSSL - MD5_CTX ctx; - MD5_Init(&ctx); - MD5_Update(&ctx, config.service_name, strlen(config.service_name)); - MD5_Update(&ctx, config.hw_addr, sizeof(config.hw_addr)); - MD5_Final(ap_md5, &ctx); + EVP_MD_CTX *mdctx = EVP_MD_CTX_new(); + EVP_DigestInit_ex(mdctx, EVP_md5(), NULL); + EVP_DigestUpdate(mdctx, config.service_name, strlen(config.service_name)); + EVP_DigestUpdate(mdctx, config.hw_addr, sizeof(config.hw_addr)); + unsigned int md5_digest_len = EVP_MD_size(EVP_md5()); + EVP_DigestFinal_ex(mdctx, ap_md5, &md5_digest_len); #endif #ifdef CONFIG_MBEDTLS From 97dd0b1eacb9e2855fb733a176e39fbef288896e Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Sat, 10 Jun 2023 16:25:03 +0100 Subject: [PATCH 12/63] replace some deprecated OpenSSL calls --- player.c | 49 +++++++++++++++++++++++++++++++++++++++++++------ player.h | 4 ---- 2 files changed, 43 insertions(+), 10 deletions(-) diff --git a/player.c b/player.c index 9d52c6ec..86837da6 100644 --- a/player.c +++ b/player.c @@ -57,7 +57,13 @@ #endif #ifdef CONFIG_OPENSSL -#include +#include // needed for BIO_new_mem_buf +// #include +#include // needed for older AES stuff +#include // needed for ERR_error_string, ERR_get_error +#include // needed for EVP_PKEY_CTX_new, EVP_PKEY_sign_init, EVP_PKEY_sign +#include // needed for PEM_read_bio_RSAPrivateKey, EVP_PKEY_CTX_set_rsa_padding +#include // needed for EVP_PKEY_CTX_set_rsa_padding #endif #ifdef CONFIG_SOXR @@ -190,6 +196,41 @@ void unencrypted_packet_decode(unsigned char *packet, int length, short *dest, i } } +#ifdef CONFIG_OPENSSL +// Thanks to +// https://stackoverflow.com/questions/27558625/how-do-i-use-aes-cbc-encrypt-128-openssl-properly-in-ubuntu +// for inspiration. Changed to a 128-bit key and no padding. + +int openssl_aes_decrypt_cbc(unsigned char *ciphertext, int ciphertext_len, unsigned char *key, + unsigned char *iv, unsigned char *plaintext) { + EVP_CIPHER_CTX *ctx; + int len; + int plaintext_len = 0; + ctx = EVP_CIPHER_CTX_new(); + if (ctx != NULL) { + if (EVP_DecryptInit_ex(ctx, EVP_aes_128_cbc(), NULL, key, iv) == 1) { + EVP_CIPHER_CTX_set_padding(ctx, 0); // no padding -- always returns 1 + // no need to allow space for padding in the output, as padding is disabled + if (EVP_DecryptUpdate(ctx, plaintext, &len, ciphertext, ciphertext_len) == 1) { + plaintext_len = len; + if (EVP_DecryptFinal_ex(ctx, plaintext + len, &len) == 1) { + plaintext_len += len; + } else { + debug(1, "EVP_DecryptFinal_ex error \"%s\".", ERR_error_string(ERR_get_error(), NULL)); + } + } else { + debug(1, "EVP_DecryptUpdate error \"%s\".", ERR_error_string(ERR_get_error(), NULL)); + } + } else { + debug(1, "EVP_DecryptInit_ex error \"%s\".", ERR_error_string(ERR_get_error(), NULL)); + } + EVP_CIPHER_CTX_free(ctx); + } else { + debug(1, "EVP_CIPHER_CTX_new error \"%s\".", ERR_error_string(ERR_get_error(), NULL)); + } + return plaintext_len; +} +#endif int audio_packet_decode(short *dest, int *destlen, uint8_t *buf, int len, rtsp_conn_info *conn) { // parameters: where the decoded stuff goes, its length in samples, // the incoming packet, the length of the incoming packet in bytes @@ -218,7 +259,7 @@ int audio_packet_decode(short *dest, int *destlen, uint8_t *buf, int len, rtsp_c aes_crypt_cbc(&conn->dctx, AES_DECRYPT, aeslen, iv, buf, packet); #endif #ifdef CONFIG_OPENSSL - AES_cbc_encrypt(buf, packet, aeslen, &conn->aes, iv, AES_DECRYPT); + openssl_aes_decrypt_cbc(buf, aeslen, conn->stream.aeskey, iv, packet); #endif memcpy(packet + aeslen, buf + aeslen, len - aeslen); unencrypted_packet_decode(packet, len, dest, &outsize, maximum_possible_outsize, conn); @@ -1923,10 +1964,6 @@ void *player_thread_func(void *arg) { memset(&conn->dctx, 0, sizeof(aes_context)); aes_setkey_dec(&conn->dctx, conn->stream.aeskey, 128); #endif - -#ifdef CONFIG_OPENSSL - AES_set_decrypt_key(conn->stream.aeskey, 128, &conn->aes); -#endif } conn->timestamp_epoch = 0; // indicate that the next timestamp will be the first one. diff --git a/player.h b/player.h index a35b54e6..55f77e2e 100644 --- a/player.h +++ b/player.h @@ -246,10 +246,6 @@ typedef struct { aes_context dctx; #endif -#ifdef CONFIG_OPENSSL - AES_KEY aes; -#endif - int amountStuffed; int32_t framesProcessedInThisEpoch; From 8f620b4580c1cdb4c709c07f7f957e9aeb06fb2b Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Sat, 10 Jun 2023 16:35:33 +0100 Subject: [PATCH 13/63] Update RELEASENOTES-DEVELOPMENT.md --- RELEASENOTES-DEVELOPMENT.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/RELEASENOTES-DEVELOPMENT.md b/RELEASENOTES-DEVELOPMENT.md index 22130751..34a5fce1 100644 --- a/RELEASENOTES-DEVELOPMENT.md +++ b/RELEASENOTES-DEVELOPMENT.md @@ -1,3 +1,9 @@ +Version 4.2.1d0-15-g97dd0b1e +==== +**Enhancements** +* Stop using deprecated OpenSSL calls. Thanks to [yubiuser](https://github.com/yubiuser) for their [PR](https://github.com/mikebrady/shairport-sync/pull/1684) -- which did some of the updating -- and for their guidance. +* Run workflow-based tests on PRs automatically. Thanks to [yubiuser](https://github.com/yubiuser) for their [PR](https://github.com/mikebrady/shairport-sync/pull/1687). + Version 4.2.1d0-5-gc3d99f96 ==== **Enhancement** From 17414f576db69b9bb6ad1708ebe42765c2da6414 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Sat, 10 Jun 2023 17:10:02 +0100 Subject: [PATCH 14/63] stop two FFmpeg deprecation warnings --- rtp.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/rtp.c b/rtp.c index 7f9ebe83..ed64ea74 100644 --- a/rtp.c +++ b/rtp.c @@ -2916,14 +2916,25 @@ void *rtp_buffered_audio_processor(void *arg) { else if (ret < 0) { debug(1, "error %d during decoding", ret); } else { +#if LIBAVUTIL_VERSION_MAJOR >= 57 + av_samples_alloc(&pcm_audio, &dst_linesize, codec_context->ch_layout.nb_channels, + decoded_frame->nb_samples, av_format, 1); +#else av_samples_alloc(&pcm_audio, &dst_linesize, codec_context->channels, decoded_frame->nb_samples, av_format, 1); +#endif // remember to free pcm_audio ret = swr_convert(swr, &pcm_audio, decoded_frame->nb_samples, (const uint8_t **)decoded_frame->extended_data, decoded_frame->nb_samples); +#if LIBAVUTIL_VERSION_MAJOR >= 57 + dst_bufsize = av_samples_get_buffer_size( + &dst_linesize, codec_context->ch_layout.nb_channels, ret, av_format, 1); +#else dst_bufsize = av_samples_get_buffer_size( &dst_linesize, codec_context->channels, ret, av_format, 1); +#endif + // debug(1,"generated %d bytes of PCM", dst_bufsize); // copy the PCM audio into the PCM buffer. // make sure it's big enough first From cafc922d4a89636d8bfb7f093bded742d3a479a1 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Sat, 10 Jun 2023 17:13:25 +0100 Subject: [PATCH 15/63] Update RELEASENOTES-DEVELOPMENT.md --- RELEASENOTES-DEVELOPMENT.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/RELEASENOTES-DEVELOPMENT.md b/RELEASENOTES-DEVELOPMENT.md index 34a5fce1..f598ecfa 100644 --- a/RELEASENOTES-DEVELOPMENT.md +++ b/RELEASENOTES-DEVELOPMENT.md @@ -1,6 +1,7 @@ -Version 4.2.1d0-15-g97dd0b1e +Version 4.2.1d0-17-g17414f57 ==== **Enhancements** +* Stop using a deprecated FFmpeg data structure reference. * Stop using deprecated OpenSSL calls. Thanks to [yubiuser](https://github.com/yubiuser) for their [PR](https://github.com/mikebrady/shairport-sync/pull/1684) -- which did some of the updating -- and for their guidance. * Run workflow-based tests on PRs automatically. Thanks to [yubiuser](https://github.com/yubiuser) for their [PR](https://github.com/mikebrady/shairport-sync/pull/1687). From 48b433ef9306c31482a9b367233a4b23b1cc67e9 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Sat, 10 Jun 2023 19:35:33 +0100 Subject: [PATCH 16/63] minor corrections --- player.c | 1 - shairport.c | 4 +++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/player.c b/player.c index 86837da6..869ac484 100644 --- a/player.c +++ b/player.c @@ -58,7 +58,6 @@ #ifdef CONFIG_OPENSSL #include // needed for BIO_new_mem_buf -// #include #include // needed for older AES stuff #include // needed for ERR_error_string, ERR_get_error #include // needed for EVP_PKEY_CTX_new, EVP_PKEY_sign_init, EVP_PKEY_sign diff --git a/shairport.c b/shairport.c index 8aafa269..efcfaa0f 100644 --- a/shairport.c +++ b/shairport.c @@ -2531,7 +2531,9 @@ int main(int argc, char **argv) { EVP_DigestUpdate(mdctx, config.service_name, strlen(config.service_name)); EVP_DigestUpdate(mdctx, config.hw_addr, sizeof(config.hw_addr)); unsigned int md5_digest_len = EVP_MD_size(EVP_md5()); - EVP_DigestFinal_ex(mdctx, ap_md5, &md5_digest_len); + EVP_DigestFinal_ex(mdctx, ap_md5, &md5_digest_len); + EVP_MD_CTX_free(mdctx); + #endif #ifdef CONFIG_MBEDTLS From ef01dc541e2218d2c1226f2c5235ae4ddcdd14e7 Mon Sep 17 00:00:00 2001 From: Michiel De Witte Date: Tue, 20 Jun 2023 11:10:54 +0200 Subject: [PATCH 17/63] Add missing format string argument --- audio_jack.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/audio_jack.c b/audio_jack.c index 4218d245..abdf09d9 100644 --- a/audio_jack.c +++ b/audio_jack.c @@ -298,7 +298,8 @@ static int jack_init(__attribute__((unused)) int argc, __attribute__((unused)) c } while (port_list[i++] != NULL) { inform( - "Additional matching port %s found. Check that the connections are what you intended."); + "Additional matching port %s found. Check that the connections are what you intended.", + port_list[i-1]); } jack_free(port_list); } From cf33fb48b1a9b95ef907707ff18ab5a3ee63b774 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Tue, 20 Jun 2023 10:25:15 +0100 Subject: [PATCH 18/63] Update RELEASENOTES-DEVELOPMENT.md --- RELEASENOTES-DEVELOPMENT.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/RELEASENOTES-DEVELOPMENT.md b/RELEASENOTES-DEVELOPMENT.md index f598ecfa..8821f44d 100644 --- a/RELEASENOTES-DEVELOPMENT.md +++ b/RELEASENOTES-DEVELOPMENT.md @@ -1,3 +1,8 @@ +Version 4.2.1d0-21-g2f52e699 +==== +**Bug Fix** +* Add an important missing format string argument to a call in the Jack Audio backend. Many thanks to [michieldwitte](https://github.com/michieldwitte) for their [PR](https://github.com/mikebrady/shairport-sync/pull/1693). + Version 4.2.1d0-17-g17414f57 ==== **Enhancements** From 46a215b630e43ec97448971f516c9b7e1fadbbd3 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Wed, 21 Jun 2023 19:24:18 +0100 Subject: [PATCH 19/63] activity monitor should send an active_end on shutdown, provided it was in the active state. --- activity_monitor.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/activity_monitor.c b/activity_monitor.c index 49f99d13..361bf517 100644 --- a/activity_monitor.c +++ b/activity_monitor.c @@ -50,6 +50,7 @@ enum am_state state; enum ps_state { ps_inactive, ps_active } player_state; int activity_monitor_running = 0; +int activity_monitor_is_shutting_down = 0; pthread_t activity_monitor_thread; pthread_mutex_t activity_monitor_mutex; @@ -120,7 +121,7 @@ void activity_monitor_signify_activity(int active) { pthread_mutex_unlock(&activity_monitor_mutex); going_active(config.cmd_blocking); } else if ((state == am_active) && (player_state == ps_inactive) && - (config.active_state_timeout == 0.0)) { + ((config.active_state_timeout == 0.0) || (activity_monitor_is_shutting_down != 0))) { state = am_inactive; pthread_mutex_unlock(&activity_monitor_mutex); going_inactive(config.cmd_blocking); @@ -240,7 +241,9 @@ void activity_monitor_start() { void activity_monitor_stop() { if (activity_monitor_running) { + activity_monitor_is_shutting_down = 1; debug(3, "activity_monitor_stop start..."); + activity_monitor_signify_activity(0); // tell it to go inactive... pthread_cancel(activity_monitor_thread); pthread_join(activity_monitor_thread, NULL); debug(2, "activity_monitor_stop complete"); From bb69630cc224b9bb7656e8b1fa820f1de9bafd6b Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Wed, 21 Jun 2023 19:28:30 +0100 Subject: [PATCH 20/63] Update RELEASENOTES-DEVELOPMENT.md --- RELEASENOTES-DEVELOPMENT.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/RELEASENOTES-DEVELOPMENT.md b/RELEASENOTES-DEVELOPMENT.md index 8821f44d..064d0612 100644 --- a/RELEASENOTES-DEVELOPMENT.md +++ b/RELEASENOTES-DEVELOPMENT.md @@ -1,3 +1,8 @@ +Version 4.2.1d0-23-g46a215b6 +==== +**Enhancement** +* On graceful shutdown, an `active_end` signal should now be generated if the system was in the active state. Addresses issue [#1647](https://github.com/mikebrady/shairport-sync/issues/1647). Thanks to [Tucker Kern](https://github.com/mill1000) for raising the issue. + Version 4.2.1d0-21-g2f52e699 ==== **Bug Fix** From b29e92b8cd273fa16699c1bb6750dfbda186bfda Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Thu, 22 Jun 2023 09:53:09 +0100 Subject: [PATCH 21/63] Always use revision number 1 when looking for status updates on the DACP remote control port. This is ia hacky change to try out a (good) suggestion in Issue #1658. --- dacp.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/dacp.c b/dacp.c index 50cf0e17..c7adac58 100644 --- a/dacp.c +++ b/dacp.c @@ -411,6 +411,7 @@ void set_dacp_server_information(rtsp_conn_info *conn) { debug(2, "set_dacp_server_information set IP to \"%s\" and DACP id to \"%s\".", dacp_server.ip_string, dacp_server.dacp_id); +/* // If the client is forked-daapd, then we always use revision number 1 // because otherwise the return read will hang in a "long poll" if there // are no changes. @@ -424,6 +425,10 @@ void set_dacp_server_information(rtsp_conn_info *conn) { dacp_server.always_use_revision_number_1 = 1; } } +*/ + + // always use revision number 1 + dacp_server.always_use_revision_number_1 = 1; metadata_hub_modify_prolog(); int ch = metadata_store.dacp_server_active != dacp_server.scan_enable; From 0af12c1452d1eb4ef91dde69365dbac89b3c53e0 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Thu, 22 Jun 2023 09:58:32 +0100 Subject: [PATCH 22/63] Update RELEASENOTES-DEVELOPMENT.md --- RELEASENOTES-DEVELOPMENT.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/RELEASENOTES-DEVELOPMENT.md b/RELEASENOTES-DEVELOPMENT.md index 064d0612..ed60b08a 100644 --- a/RELEASENOTES-DEVELOPMENT.md +++ b/RELEASENOTES-DEVELOPMENT.md @@ -1,3 +1,8 @@ +Version 4.2.1d0-26-g716aa051 +==== +**Bug Fix** +* Classic Airplay Remote Control. Always use a revision number of 1 when looking for status updates on the DACP remote control port. This is ia quick-and-dirty change to try out a (good) suggestion in [Issue #1658](https://github.com/mikebrady/shairport-sync/issues/1658) -- if it works change will be made permanent. Thanks to [ejurgensen](https://github.com/ejurgensen), as ever, for the report and the suggested fix. + Version 4.2.1d0-23-g46a215b6 ==== **Enhancement** From 0ca5fd3caac3ff44e1e1c9249cee836bd1a78c83 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Sat, 24 Jun 2023 22:20:14 +0100 Subject: [PATCH 23/63] Fix a statistics bug and also tidy up the statistics logging interval logic for resetting min and max counters. --- player.c | 36 +++++++++++++++++++++--------------- 1 file changed, 21 insertions(+), 15 deletions(-) diff --git a/player.c b/player.c index 869ac484..726c7d12 100644 --- a/player.c +++ b/player.c @@ -57,8 +57,8 @@ #endif #ifdef CONFIG_OPENSSL -#include // needed for BIO_new_mem_buf #include // needed for older AES stuff +#include // needed for BIO_new_mem_buf #include // needed for ERR_error_string, ERR_get_error #include // needed for EVP_PKEY_CTX_new, EVP_PKEY_sign_init, EVP_PKEY_sign #include // needed for PEM_read_bio_RSAPrivateKey, EVP_PKEY_CTX_set_rsa_padding @@ -201,7 +201,7 @@ void unencrypted_packet_decode(unsigned char *packet, int length, short *dest, i // for inspiration. Changed to a 128-bit key and no padding. int openssl_aes_decrypt_cbc(unsigned char *ciphertext, int ciphertext_len, unsigned char *key, - unsigned char *iv, unsigned char *plaintext) { + unsigned char *iv, unsigned char *plaintext) { EVP_CIPHER_CTX *ctx; int len; int plaintext_len = 0; @@ -2065,14 +2065,14 @@ void *player_thread_func(void *arg) { #define trend_interval 1003 int number_of_statistics, oldest_statistic, newest_statistic; - int at_least_one_frame_seen = 0; + int frames_seen_in_this_logging_interval = 0; int at_least_one_frame_seen_this_session = 0; int64_t tsum_of_sync_errors, tsum_of_corrections, tsum_of_insertions_and_deletions, tsum_of_drifts; int64_t previous_sync_error = 0, previous_correction = 0; - uint64_t minimum_dac_queue_size = UINT64_MAX; - int32_t minimum_buffer_occupancy = INT32_MAX; - int32_t maximum_buffer_occupancy = INT32_MIN; + uint64_t minimum_dac_queue_size; + int32_t minimum_buffer_occupancy; + int32_t maximum_buffer_occupancy; #ifdef CONFIG_AIRPLAY_2 conn->ap2_audio_buffer_minimum_size = -1; @@ -2481,7 +2481,7 @@ void *player_thread_func(void *arg) { // now, go back as far as the total latency less, say, 100 ms, and check the presence of // frames from then onwards - at_least_one_frame_seen = 1; + frames_seen_in_this_logging_interval++; // This is the timing error for the next audio frame in the DAC, if applicable int64_t sync_error = 0; @@ -2508,10 +2508,12 @@ void *player_thread_func(void *arg) { int16_t bo = conn->ab_write - conn->ab_read; // do this in 16 bits conn->buffer_occupancy = bo; // 32 bits - if (conn->buffer_occupancy < minimum_buffer_occupancy) + if ((frames_seen_in_this_logging_interval == 1) || + (conn->buffer_occupancy < minimum_buffer_occupancy)) minimum_buffer_occupancy = conn->buffer_occupancy; - if (conn->buffer_occupancy > maximum_buffer_occupancy) + if ((frames_seen_in_this_logging_interval == 1) || + (conn->buffer_occupancy > maximum_buffer_occupancy)) maximum_buffer_occupancy = conn->buffer_occupancy; // now, before outputting anything to the output device, check the stats @@ -2600,7 +2602,7 @@ void *player_thread_func(void *arg) { if (config.statistics_requested) { - if (at_least_one_frame_seen) { + if (frames_seen_in_this_logging_interval) { do { line_of_stats[0] = '\0'; statistics_column = 0; @@ -2668,13 +2670,9 @@ void *player_thread_func(void *arg) { inform("No frames received in the last sampling interval."); } } - minimum_dac_queue_size = UINT64_MAX; // hack reset - maximum_buffer_occupancy = INT32_MIN; // can't be less than this - minimum_buffer_occupancy = INT32_MAX; // can't be more than this #ifdef CONFIG_AIRPLAY_2 conn->ap2_audio_buffer_minimum_size = -1; #endif - at_least_one_frame_seen = 0; } // here, we want to check (a) if we are meant to do synchronisation, @@ -2697,7 +2695,8 @@ void *player_thread_func(void *arg) { current_delay = 0; // could get a negative value if there was underrun, but ignore it. } - if (current_delay < minimum_dac_queue_size) { + if ((frames_seen_in_this_logging_interval == 1) || + (current_delay < minimum_dac_queue_size)) { minimum_dac_queue_size = current_delay; // update for display later } } else { @@ -3241,6 +3240,13 @@ void *player_thread_func(void *arg) { inframe->resend_time = 0; inframe->initialisation_time = 0; + // if we've just printed out statistics, note that in the next interval + // we haven't seen any frames yet + + if (play_number % print_interval == 0) { + frames_seen_in_this_logging_interval = 0; + } + // update the watchdog if ((config.dont_check_timeout == 0) && (config.timeout != 0)) { uint64_t time_now = get_absolute_time_in_ns(); From 257a84dd4ccfa8f8f98d24ab2c11a6b8697350c1 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Sat, 24 Jun 2023 22:22:51 +0100 Subject: [PATCH 24/63] Update RELEASENOTES-DEVELOPMENT.md --- RELEASENOTES-DEVELOPMENT.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/RELEASENOTES-DEVELOPMENT.md b/RELEASENOTES-DEVELOPMENT.md index ed60b08a..9ff94bfe 100644 --- a/RELEASENOTES-DEVELOPMENT.md +++ b/RELEASENOTES-DEVELOPMENT.md @@ -1,3 +1,8 @@ +Version 4.2.1d0-28-g0ca5fd3c +==== +**Tiny Bug Fix** +* Fix a `statistics` bug (the minimum buffer size was incorrectly logged) and also tidy up the statistics logging interval logic for resetting min and max counters. + Version 4.2.1d0-26-g716aa051 ==== **Bug Fix** From 01937965d7eead02272f15c548cc3ba0eae03121 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Sun, 25 Jun 2023 11:39:32 +0100 Subject: [PATCH 25/63] quieten a few debug messages --- player.c | 2 +- rtsp.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/player.c b/player.c index 726c7d12..c1739ee1 100644 --- a/player.c +++ b/player.c @@ -430,7 +430,7 @@ static void free_audio_buffers(rtsp_conn_info *conn) { free(conn->audio_buffer[i].data); buffers_released++; } - debug(1, "%" PRId64 " buffers allocated, %" PRId64 " buffers released.", buffers_allocated, + debug(2, "%" PRId64 " buffers allocated, %" PRId64 " buffers released.", buffers_allocated, buffers_released); } diff --git a/rtsp.c b/rtsp.c index 4db9c0b4..0b680f8c 100644 --- a/rtsp.c +++ b/rtsp.c @@ -1373,7 +1373,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(1, "Connection %d: Connection closed by client.", conn->connection_number); + debug(3, "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 b0f2fb9a104f461a32c85a4aab73d60f73e3fd81 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Sun, 25 Jun 2023 17:51:32 +0100 Subject: [PATCH 26/63] Fix a bug when interrupting a Classic Airplay session -- don't reset the UDP port suggestion when you interrupt a session. IOW use different UDP ports for the new session to avoid poisoning the new session. --- rtsp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rtsp.c b/rtsp.c index 0b680f8c..baf90eec 100644 --- a/rtsp.c +++ b/rtsp.c @@ -4487,7 +4487,7 @@ static void handle_announce(rtsp_conn_info *conn, rtsp_message *req, rtsp_messag // now, if this new session did not break in, then it's okay to reset the next UDP ports // to the start of the range - if (get_play_status == 1) { // will be zero if it wasn't waiting to break in + if (get_play_status == 0) { // will be zero if it wasn't waiting to break in resetFreeUDPPort(); } From 6e6b2a59591c97588f4f37cc3b2c91bf68cb0d7b Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Sun, 25 Jun 2023 18:03:08 +0100 Subject: [PATCH 27/63] Update RELEASENOTES-DEVELOPMENT.md --- RELEASENOTES-DEVELOPMENT.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/RELEASENOTES-DEVELOPMENT.md b/RELEASENOTES-DEVELOPMENT.md index 9ff94bfe..91f0b260 100644 --- a/RELEASENOTES-DEVELOPMENT.md +++ b/RELEASENOTES-DEVELOPMENT.md @@ -1,3 +1,8 @@ +Version 4.2.1d0-31-gb0f2fb9a +==== +**Bug Fix** +* Fix a bug that could ocassionally cause a crash if `allow_session_interruption` is set to `"yes"` and a Classic AirPlay session is interrupted by a play session request from another device. + Version 4.2.1d0-28-g0ca5fd3c ==== **Tiny Bug Fix** From d4e003805c4129cc7db8a217f64c62b79b541716 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Mon, 26 Jun 2023 07:13:45 +0100 Subject: [PATCH 28/63] Ensure the metadata associated with active_end is sent before quitting the app gracefully. (Up to now, the metadata systems would be shutdown before the active_end was done. --- shairport.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/shairport.c b/shairport.c index efcfaa0f..db5bf2c0 100644 --- a/shairport.c +++ b/shairport.c @@ -1593,6 +1593,16 @@ void exit_function() { #endif */ + debug(2, "Stopping the activity monitor."); + activity_monitor_stop(0); + debug(2, "Stopping the activity monitor done."); + +#ifdef CONFIG_DACP_CLIENT + debug(2, "Stopping DACP Monitor"); + dacp_monitor_stop(); + debug(2, "Stopping DACP Monitor Done"); +#endif + #if defined(CONFIG_DBUS_INTERFACE) || defined(CONFIG_MPRIS_INTERFACE) /* Actually, there is no stop_mpris_service() function. @@ -1618,11 +1628,6 @@ void exit_function() { } #endif -#ifdef CONFIG_DACP_CLIENT - debug(2, "Stopping DACP Monitor"); - dacp_monitor_stop(); - debug(2, "Stopping DACP Monitor Done"); -#endif #ifdef CONFIG_METADATA_HUB debug(2, "Stopping metadata hub"); @@ -1635,9 +1640,6 @@ void exit_function() { metadata_stop(); // close down the metadata pipe debug(2, "Stopping metadata done"); #endif - debug(2, "Stopping the activity monitor."); - activity_monitor_stop(0); - debug(2, "Stopping the activity monitor done."); if ((config.output) && (config.output->deinit)) { debug(2, "Deinitialise the audio backend."); From c2abee35d08b15ad57d6d0747f80a72d7b77618b Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Mon, 26 Jun 2023 07:17:41 +0100 Subject: [PATCH 29/63] Update RELEASENOTES-DEVELOPMENT.md --- RELEASENOTES-DEVELOPMENT.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/RELEASENOTES-DEVELOPMENT.md b/RELEASENOTES-DEVELOPMENT.md index 91f0b260..d2cb7774 100644 --- a/RELEASENOTES-DEVELOPMENT.md +++ b/RELEASENOTES-DEVELOPMENT.md @@ -1,3 +1,8 @@ +Version 4.2.1d0-33-gd4e00380 +==== +**Bug Fix** +* If an `active_end` signal is generated when the system is shutdown gracefully, ensure it is sent before shutting down the metadata transmission system, duh. + Version 4.2.1d0-31-gb0f2fb9a ==== **Bug Fix** From baf51cf2a7a34736252b3b6adedddce9ea1a0c9e Mon Sep 17 00:00:00 2001 From: dasl- Date: Sun, 2 Jul 2023 09:27:53 -0400 Subject: [PATCH 30/63] Add logarithmic volume_control_profile mode --- common.c | 27 +++++++++++++++++++++++++++ common.h | 5 +++++ dbus-service.c | 7 +++++++ player.c | 3 +++ scripts/shairport-sync.conf | 1 + shairport.c | 6 ++++-- 6 files changed, 47 insertions(+), 2 deletions(-) diff --git a/common.c b/common.c index 2c902e3b..f9b83b8d 100644 --- a/common.c +++ b/common.c @@ -37,6 +37,7 @@ #include #include // PRIdPTR #include +#include #include #include #include @@ -1205,6 +1206,32 @@ double flat_vol2attn(double vol, long max_db, long min_db) { } return vol_setting; } + +double logarithmic_vol2attn(double vol, long max_db, long min_db) { + double vol_setting = min_db; // if all else fails, set this, for safety + + if ((vol <= 0.0) && (vol >= -30.0)) { + double vol_pct = 1 - (vol / -30.0); // This will be in the range [0, 1] + if (vol_pct <= 0) { + return min_db; + } + + vol_setting = 1000 * log10(vol_pct) / log10(2); // This will be in the range [-inf, 0] + if (vol_setting < min_db) { + return min_db; + } + if (vol_setting > max_db) { + return max_db; + } + return vol_setting; + } else if (vol != -144.0) { + debug(1, + "Logarithmic volume request value %f is out of range: should be from 0.0 to -30.0 or -144.0.", + vol); + } + return vol_setting; +} + // Given a volume (0 to -30) and high and low attenuations available in the mixer in dB, return an // attenuation depending on the volume and the function's transfer function // See http://tangentsoft.net/audio/atten.html for data on good attenuators. diff --git a/common.h b/common.h index e152db68..ceea2206 100644 --- a/common.h +++ b/common.h @@ -73,6 +73,7 @@ typedef enum { typedef enum { VCP_standard = 0, VCP_flat, + VCP_logarithmic, } volume_control_profile_type; typedef enum { @@ -390,6 +391,10 @@ uint8_t *rsa_apply(uint8_t *input, int inlen, int *outlen, int mode); // dB), return an attenuation depending on a linear interpolation along along the range double flat_vol2attn(double vol, long max_db, long min_db); +// given a volume (0 to -30) and high and low attenuations in dB*100 (e.g. 0 to -6000 for 0 to -60 +// dB), return an attenuation depending on a logarithmic interpolation along along the range +double logarithmic_vol2attn(double vol, long max_db, long min_db); + // given a volume (0 to -30) and high and low attenuations in dB*100 (e.g. 0 to -6000 for 0 to -60 // dB), return an attenuation depending on the transfer function double vol2attn(double vol, long max_db, long min_db); diff --git a/dbus-service.c b/dbus-service.c index 6a3b544f..4795896e 100644 --- a/dbus-service.c +++ b/dbus-service.c @@ -765,6 +765,8 @@ gboolean notify_volume_control_profile_callback(ShairportSync *skeleton, config.volume_control_profile = VCP_standard; else if (strcasecmp(th, "flat") == 0) config.volume_control_profile = VCP_flat; + else if (strcasecmp(th, "logarithmic") == 0) + config.volume_control_profile = VCP_logarithmic; else { warn("Unrecognised Volume Control Profile: \"%s\".", th); switch (config.volume_control_profile) { @@ -774,6 +776,9 @@ gboolean notify_volume_control_profile_callback(ShairportSync *skeleton, case VCP_flat: shairport_sync_set_volume_control_profile(skeleton, "flat"); break; + case VCP_logarithmic: + shairport_sync_set_volume_control_profile(skeleton, "logarithmic"); + break; default: debug(1, "This should never happen!"); shairport_sync_set_volume_control_profile(skeleton, "standard"); @@ -1065,6 +1070,8 @@ static void on_dbus_name_acquired(GDBusConnection *connection, const gchar *name if (config.volume_control_profile == VCP_standard) shairport_sync_set_volume_control_profile(SHAIRPORT_SYNC(shairportSyncSkeleton), "standard"); + else if (config.volume_control_profile == VCP_logarithmic) + shairport_sync_set_volume_control_profile(SHAIRPORT_SYNC(shairportSyncSkeleton), "logarithmic"); else shairport_sync_set_volume_control_profile(SHAIRPORT_SYNC(shairportSyncSkeleton), "flat"); diff --git a/player.c b/player.c index c1739ee1..3aa29943 100644 --- a/player.c +++ b/player.c @@ -3432,6 +3432,9 @@ void player_volume_without_notification(double airplay_volume, rtsp_conn_info *c else if (config.volume_control_profile == VCP_flat) scaled_attenuation = flat_vol2attn(airplay_volume, max_db, min_db); // no cancellation points + else if (config.volume_control_profile == VCP_logarithmic) + scaled_attenuation = + logarithmic_vol2attn(airplay_volume, max_db, min_db); // no cancellation points else debug(1, "player_volume_without_notification: unrecognised volume control profile"); } diff --git a/scripts/shairport-sync.conf b/scripts/shairport-sync.conf index 03cb4339..e2521985 100644 --- a/scripts/shairport-sync.conf +++ b/scripts/shairport-sync.conf @@ -42,6 +42,7 @@ general = // volume_control_profile = "standard" ; // use this advanced setting to specify how the airplay volume is transferred to the mixer volume. // "standard" makes the volume change more quickly at lower volumes and slower at higher volumes. // "flat" makes the volume change at the same rate at all volumes. +// "logarithmic" makes the volume change logarithmically. // volume_control_combined_hardware_priority = "no"; // when extending the volume range by combining the built-in software attenuator with the hardware mixer attenuator, set this to "yes" to reduce volume by using the hardware mixer first, then the built-in software attenuator. // default_airplay_volume = -24.0; // this is the suggested volume after a reset or after the high_volume_threshold has been exceed and the high_volume_idle_timeout_in_minutes has passed diff --git a/shairport.c b/shairport.c index db5bf2c0..7d200313 100644 --- a/shairport.c +++ b/shairport.c @@ -905,9 +905,11 @@ int parse_options(int argc, char **argv) { config.volume_control_profile = VCP_standard; else if (strcasecmp(str, "flat") == 0) config.volume_control_profile = VCP_flat; + else if (strcasecmp(str, "logarithmic") == 0) + config.volume_control_profile = VCP_logarithmic; else - die("Invalid volume_control_profile choice \"%s\". It should be \"standard\" (default) " - "or \"flat\"", + die("Invalid volume_control_profile choice \"%s\". It should be \"standard\" (default), " + "\"logarithmic\", or \"flat\"", str); } From 6acd73da5778a0153907075ec3884a502fe28562 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Tue, 4 Jul 2023 19:28:05 +0100 Subject: [PATCH 31/63] Make the change to short DACP polling (semi-)permanent. Maye look at this again in the future. --- dacp.c | 21 +++++++-------------- 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/dacp.c b/dacp.c index c7adac58..14154263 100644 --- a/dacp.c +++ b/dacp.c @@ -412,19 +412,12 @@ void set_dacp_server_information(rtsp_conn_info *conn) { dacp_server.ip_string, dacp_server.dacp_id); /* - // If the client is forked-daapd, then we always use revision number 1 - // because otherwise the return read will hang in a "long poll" if there - // are no changes. - // This is different to other AirPlay clients - // which return immediately with a 403 code if there are no changes. - dacp_server.always_use_revision_number_1 = 0; - if (conn->UserAgent != NULL) { - char *p = strstr(conn->UserAgent, "forked-daapd"); - if ((p != 0) && - (p == conn->UserAgent)) { // must exist and be at the start of the UserAgent string - dacp_server.always_use_revision_number_1 = 1; - } - } + +"long polling" is not implemented by Shairport Sync, whereby by sending the client the +last-received revision number, the link will hang until a change occurs. + +Instead, at present, Shairport Sync always uses a revision number of 1. + */ // always use revision number 1 @@ -658,7 +651,7 @@ void *dacp_monitor_thread_code(__attribute__((unused)) void *na) { char *response = NULL; int32_t item_size; char command[1024] = ""; - if (always_use_revision_number_1 != 0) // for forked-daapd + if (always_use_revision_number_1 != 0) // see the "long polling" note above revision_number = 1; snprintf(command, sizeof(command) - 1, "playstatusupdate?revision-number=%d", revision_number); From 1fd863f360bb0e81751cfc785c8dfbb5b7512a27 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Tue, 4 Jul 2023 19:35:27 +0100 Subject: [PATCH 32/63] Update RELEASENOTES-DEVELOPMENT.md --- RELEASENOTES-DEVELOPMENT.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/RELEASENOTES-DEVELOPMENT.md b/RELEASENOTES-DEVELOPMENT.md index d2cb7774..5c2352c2 100644 --- a/RELEASENOTES-DEVELOPMENT.md +++ b/RELEASENOTES-DEVELOPMENT.md @@ -1,3 +1,8 @@ +Version 4.2.1d0-35-g6acd73da +==== +**Update** +* Confirm the bug fix made in [4.2.1d0-26](#version-421d0-26-g716aa051) to Classic Airplay Remote Control: always use a revision number of 1 when looking for status updates on the DACP remote control port. + Version 4.2.1d0-33-gd4e00380 ==== **Bug Fix** From 9a8f1ba12be6191f264e96e66d53c9995908e50b Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Thu, 6 Jul 2023 09:13:58 +0100 Subject: [PATCH 33/63] Update the mDNS strings for the Classic AirPlay feature of AP2 --- rtsp.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/rtsp.c b/rtsp.c index baf90eec..a61ade1b 100644 --- a/rtsp.c +++ b/rtsp.c @@ -270,10 +270,15 @@ void build_bonjour_strings(__attribute((unused)) rtsp_conn_info *conn) { txt_records[entry_number++] = "cn=0,1"; txt_records[entry_number++] = "da=true"; - txt_records[entry_number++] = "et=0,4"; + txt_records[entry_number++] = "et=0,1"; txt_records[entry_number++] = ap1_featuresString; txt_records[entry_number++] = firmware_version; - txt_records[entry_number++] = "md=2"; +#ifdef CONFIG_METADATA + if (config.get_coverart == 0) + txt_records[entry_number++] = "md=0,2"; + else + txt_records[entry_number++] = "md=0,1,2"; +#endif txt_records[entry_number++] = "am=Shairport Sync"; txt_records[entry_number++] = "sf=0x4"; txt_records[entry_number++] = "tp=UDP"; From e509849d742256a5e77fd2efbe84759df159d5e0 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Thu, 6 Jul 2023 09:17:26 +0100 Subject: [PATCH 34/63] Update RELEASENOTES-DEVELOPMENT.md --- RELEASENOTES-DEVELOPMENT.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/RELEASENOTES-DEVELOPMENT.md b/RELEASENOTES-DEVELOPMENT.md index 5c2352c2..848383cc 100644 --- a/RELEASENOTES-DEVELOPMENT.md +++ b/RELEASENOTES-DEVELOPMENT.md @@ -1,3 +1,8 @@ +Version 4.2.1d0-37-g9a8f1ba1 +==== +**Bug Fix** +* Update the mDNS strings for the Classic AirPlay feature of AP2, so that it does not appear to provide MFi authentication. Addresses [this discussion](https://github.com/mikebrady/shairport-sync/discussions/1691). + Version 4.2.1d0-35-g6acd73da ==== **Update** From 3479d8d7d4aad9221d417246489221befcc52131 Mon Sep 17 00:00:00 2001 From: David Leibovic Date: Fri, 7 Jul 2023 20:19:34 -0400 Subject: [PATCH 35/63] make use of max_db --- common.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common.c b/common.c index f9b83b8d..6e3e4ab0 100644 --- a/common.c +++ b/common.c @@ -1216,7 +1216,7 @@ double logarithmic_vol2attn(double vol, long max_db, long min_db) { return min_db; } - vol_setting = 1000 * log10(vol_pct) / log10(2); // This will be in the range [-inf, 0] + vol_setting = max_db + 1000 * log10(vol_pct) / log10(2); // This will be in the range [-inf, max_db] if (vol_setting < min_db) { return min_db; } From 9f8b360c63a52d3a0d25e2667f3deca56f4a9fc9 Mon Sep 17 00:00:00 2001 From: sternenseemann Date: Sun, 9 Jul 2023 21:16:03 +0200 Subject: [PATCH 36/63] configure.ac: check $AR for ar to use Resolves #1705. Tested against https://github.com/NixOS/nixpkgs/pull/242350. --- configure.ac | 1 + 1 file changed, 1 insertion(+) diff --git a/configure.ac b/configure.ac index 61f42589..743b4743 100644 --- a/configure.ac +++ b/configure.ac @@ -33,6 +33,7 @@ esac # Checks for programs. AC_PROG_CC AC_PROG_CXX +AC_CHECK_TOOL(AR, ar, :) AC_PROG_INSTALL PKG_PROG_PKG_CONFIG([0.9.0]) From d242aab65a8b40134cb372f837ea77a0295953b8 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Sun, 9 Jul 2023 20:49:11 +0100 Subject: [PATCH 37/63] Update RELEASENOTES-DEVELOPMENT.md --- RELEASENOTES-DEVELOPMENT.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/RELEASENOTES-DEVELOPMENT.md b/RELEASENOTES-DEVELOPMENT.md index 848383cc..5a9cb9c8 100644 --- a/RELEASENOTES-DEVELOPMENT.md +++ b/RELEASENOTES-DEVELOPMENT.md @@ -1,3 +1,8 @@ +Version 4.2.1d0-40-g9ef4e351 +==== +**Bug Fix** +* Fix a cross-compliation error caused by not looking for the correct version of the `ar` tool. The fix was to substitute the correct version during the `autoreconf` phase. Thanks to [sternenseemann](https://github.com/sternenseemann) for raising the [issue](https://github.com/mikebrady/shairport-sync/issues/1705) and the [PR](https://github.com/mikebrady/shairport-sync/pull/1706) containing the fix. + Version 4.2.1d0-37-g9a8f1ba1 ==== **Bug Fix** From ebe1ca17f2eb0e3434c249958316c50624c20d4a Mon Sep 17 00:00:00 2001 From: dasl- Date: Thu, 13 Jul 2023 21:43:25 -0400 Subject: [PATCH 38/63] rename to dasl_tapered --- common.c | 4 ++-- common.h | 9 ++++++--- dbus-service.c | 12 ++++++------ player.c | 4 ++-- scripts/shairport-sync.conf | 4 +++- shairport.c | 6 +++--- 6 files changed, 22 insertions(+), 17 deletions(-) diff --git a/common.c b/common.c index 6e3e4ab0..edd4351a 100644 --- a/common.c +++ b/common.c @@ -1207,7 +1207,7 @@ double flat_vol2attn(double vol, long max_db, long min_db) { return vol_setting; } -double logarithmic_vol2attn(double vol, long max_db, long min_db) { +double dasl_tapered_vol2attn(double vol, long max_db, long min_db) { double vol_setting = min_db; // if all else fails, set this, for safety if ((vol <= 0.0) && (vol >= -30.0)) { @@ -1226,7 +1226,7 @@ double logarithmic_vol2attn(double vol, long max_db, long min_db) { return vol_setting; } else if (vol != -144.0) { debug(1, - "Logarithmic volume request value %f is out of range: should be from 0.0 to -30.0 or -144.0.", + "dasl_tapered volume request value %f is out of range: should be from 0.0 to -30.0 or -144.0.", vol); } return vol_setting; diff --git a/common.h b/common.h index ceea2206..c65b4c1f 100644 --- a/common.h +++ b/common.h @@ -73,7 +73,7 @@ typedef enum { typedef enum { VCP_standard = 0, VCP_flat, - VCP_logarithmic, + VCP_dasl_tapered, } volume_control_profile_type; typedef enum { @@ -392,8 +392,11 @@ uint8_t *rsa_apply(uint8_t *input, int inlen, int *outlen, int mode); double flat_vol2attn(double vol, long max_db, long min_db); // given a volume (0 to -30) and high and low attenuations in dB*100 (e.g. 0 to -6000 for 0 to -60 -// dB), return an attenuation depending on a logarithmic interpolation along along the range -double logarithmic_vol2attn(double vol, long max_db, long min_db); +// dB), return an attenuation depending on a logarithmic interpolation along along the range. +// The intention behind this attenuation function is that a given percentage change in volume +// should result in the same percentage change in perceived loudness. For instance, a doubling +// (100% increase) of volume level should result in a doubling of perceived loudness. +double dasl_tapered_vol2attn(double vol, long max_db, long min_db); // given a volume (0 to -30) and high and low attenuations in dB*100 (e.g. 0 to -6000 for 0 to -60 // dB), return an attenuation depending on the transfer function diff --git a/dbus-service.c b/dbus-service.c index 4795896e..b4a9cc9e 100644 --- a/dbus-service.c +++ b/dbus-service.c @@ -765,8 +765,8 @@ gboolean notify_volume_control_profile_callback(ShairportSync *skeleton, config.volume_control_profile = VCP_standard; else if (strcasecmp(th, "flat") == 0) config.volume_control_profile = VCP_flat; - else if (strcasecmp(th, "logarithmic") == 0) - config.volume_control_profile = VCP_logarithmic; + else if (strcasecmp(th, "dasl_tapered") == 0) + config.volume_control_profile = VCP_dasl_tapered; else { warn("Unrecognised Volume Control Profile: \"%s\".", th); switch (config.volume_control_profile) { @@ -776,8 +776,8 @@ gboolean notify_volume_control_profile_callback(ShairportSync *skeleton, case VCP_flat: shairport_sync_set_volume_control_profile(skeleton, "flat"); break; - case VCP_logarithmic: - shairport_sync_set_volume_control_profile(skeleton, "logarithmic"); + case VCP_dasl_tapered: + shairport_sync_set_volume_control_profile(skeleton, "dasl_tapered"); break; default: debug(1, "This should never happen!"); @@ -1070,8 +1070,8 @@ static void on_dbus_name_acquired(GDBusConnection *connection, const gchar *name if (config.volume_control_profile == VCP_standard) shairport_sync_set_volume_control_profile(SHAIRPORT_SYNC(shairportSyncSkeleton), "standard"); - else if (config.volume_control_profile == VCP_logarithmic) - shairport_sync_set_volume_control_profile(SHAIRPORT_SYNC(shairportSyncSkeleton), "logarithmic"); + else if (config.volume_control_profile == VCP_dasl_tapered) + shairport_sync_set_volume_control_profile(SHAIRPORT_SYNC(shairportSyncSkeleton), "dasl_tapered"); else shairport_sync_set_volume_control_profile(SHAIRPORT_SYNC(shairportSyncSkeleton), "flat"); diff --git a/player.c b/player.c index 3aa29943..a5b0681b 100644 --- a/player.c +++ b/player.c @@ -3432,9 +3432,9 @@ void player_volume_without_notification(double airplay_volume, rtsp_conn_info *c else if (config.volume_control_profile == VCP_flat) scaled_attenuation = flat_vol2attn(airplay_volume, max_db, min_db); // no cancellation points - else if (config.volume_control_profile == VCP_logarithmic) + else if (config.volume_control_profile == VCP_dasl_tapered) scaled_attenuation = - logarithmic_vol2attn(airplay_volume, max_db, min_db); // no cancellation points + dasl_tapered_vol2attn(airplay_volume, max_db, min_db); // no cancellation points else debug(1, "player_volume_without_notification: unrecognised volume control profile"); } diff --git a/scripts/shairport-sync.conf b/scripts/shairport-sync.conf index e2521985..5528712d 100644 --- a/scripts/shairport-sync.conf +++ b/scripts/shairport-sync.conf @@ -42,7 +42,9 @@ general = // volume_control_profile = "standard" ; // use this advanced setting to specify how the airplay volume is transferred to the mixer volume. // "standard" makes the volume change more quickly at lower volumes and slower at higher volumes. // "flat" makes the volume change at the same rate at all volumes. -// "logarithmic" makes the volume change logarithmically. +// "dasl_tapered" is similar to "standard" - it makes the volume change more quickly at lower volumes and slower at higher volumes. +// The intention behind dasl_tapered is that a given percentage change in volume should result in the same percentage change in +// perceived loudness. For instance, a doubling (100% increase) of volume level should result in a doubling of perceived loudness. // volume_control_combined_hardware_priority = "no"; // when extending the volume range by combining the built-in software attenuator with the hardware mixer attenuator, set this to "yes" to reduce volume by using the hardware mixer first, then the built-in software attenuator. // default_airplay_volume = -24.0; // this is the suggested volume after a reset or after the high_volume_threshold has been exceed and the high_volume_idle_timeout_in_minutes has passed diff --git a/shairport.c b/shairport.c index 7d200313..6d9fb959 100644 --- a/shairport.c +++ b/shairport.c @@ -905,11 +905,11 @@ int parse_options(int argc, char **argv) { config.volume_control_profile = VCP_standard; else if (strcasecmp(str, "flat") == 0) config.volume_control_profile = VCP_flat; - else if (strcasecmp(str, "logarithmic") == 0) - config.volume_control_profile = VCP_logarithmic; + else if (strcasecmp(str, "dasl_tapered") == 0) + config.volume_control_profile = VCP_dasl_tapered; else die("Invalid volume_control_profile choice \"%s\". It should be \"standard\" (default), " - "\"logarithmic\", or \"flat\"", + "\"dasl_tapered\", or \"flat\"", str); } From 1a555377900592f41999b8397444a57bfea534e9 Mon Sep 17 00:00:00 2001 From: dasl- Date: Thu, 13 Jul 2023 21:49:29 -0400 Subject: [PATCH 39/63] fix typo --- common.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/common.h b/common.h index c65b4c1f..0c4d92ec 100644 --- a/common.h +++ b/common.h @@ -388,11 +388,11 @@ char *base64_enc(uint8_t *input, int length); uint8_t *rsa_apply(uint8_t *input, int inlen, int *outlen, int mode); // given a volume (0 to -30) and high and low attenuations in dB*100 (e.g. 0 to -6000 for 0 to -60 -// dB), return an attenuation depending on a linear interpolation along along the range +// dB), return an attenuation depending on a linear interpolation along the range double flat_vol2attn(double vol, long max_db, long min_db); // given a volume (0 to -30) and high and low attenuations in dB*100 (e.g. 0 to -6000 for 0 to -60 -// dB), return an attenuation depending on a logarithmic interpolation along along the range. +// dB), return an attenuation depending on a logarithmic interpolation along the range. // The intention behind this attenuation function is that a given percentage change in volume // should result in the same percentage change in perceived loudness. For instance, a doubling // (100% increase) of volume level should result in a doubling of perceived loudness. From 0319cf388aa8fef3b7731f21d15de6a0fcde9941 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Mon, 17 Jul 2023 15:31:19 +0200 Subject: [PATCH 40/63] Update RELEASENOTES-DEVELOPMENT.md Add note about the `dasl-tapered` volume control profile --- RELEASENOTES-DEVELOPMENT.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/RELEASENOTES-DEVELOPMENT.md b/RELEASENOTES-DEVELOPMENT.md index 5a9cb9c8..670c9670 100644 --- a/RELEASENOTES-DEVELOPMENT.md +++ b/RELEASENOTES-DEVELOPMENT.md @@ -1,3 +1,10 @@ +Version 4.2.1d0-46-g57061dfb +==== +**Enhancement** +* Add a new volume-control profile called `dasl-tapered`. Shairport Sync already has two volume control profiles -- the `standard` profile -- which is the default -- and a `flat profile. A feature of the `dasl-tapered` profile is that halving the volume control setting (e.g. moving the volume slider from full to half, or from half to a quarter) reduces the output level by 10dB, which roughly corresponds with a perceived halving of the audio volume level. Many thanks to David Leibovic, aka [dasl-](https://github.com/dasl-) for this. +To activate the `dasl-tapered` profile, set the `volume_control_profile` to `"dasl_tapered"` in the configuration file and restart Shairport Sync. +There may be some more minor adjustments to this profile for devices with a restricted attenuation range... + Version 4.2.1d0-40-g9ef4e351 ==== **Bug Fix** From 89ec0f871bc40c52b837b2f066ba8a8fd24f43f8 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Mon, 17 Jul 2023 15:31:38 +0200 Subject: [PATCH 41/63] 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 670c9670..e6ec317a 100644 --- a/RELEASENOTES-DEVELOPMENT.md +++ b/RELEASENOTES-DEVELOPMENT.md @@ -1,7 +1,7 @@ Version 4.2.1d0-46-g57061dfb ==== **Enhancement** -* Add a new volume-control profile called `dasl-tapered`. Shairport Sync already has two volume control profiles -- the `standard` profile -- which is the default -- and a `flat profile. A feature of the `dasl-tapered` profile is that halving the volume control setting (e.g. moving the volume slider from full to half, or from half to a quarter) reduces the output level by 10dB, which roughly corresponds with a perceived halving of the audio volume level. Many thanks to David Leibovic, aka [dasl-](https://github.com/dasl-) for this. +* Add a new volume-control profile called `dasl-tapered`. Shairport Sync already has two volume control profiles -- the `standard` profile -- which is the default -- and a `flat` profile. A feature of the `dasl-tapered` profile is that halving the volume control setting (e.g. moving the volume slider from full to half, or from half to a quarter) reduces the output level by 10dB, which roughly corresponds with a perceived halving of the audio volume level. Many thanks to David Leibovic, aka [dasl-](https://github.com/dasl-) for this. To activate the `dasl-tapered` profile, set the `volume_control_profile` to `"dasl_tapered"` in the configuration file and restart Shairport Sync. There may be some more minor adjustments to this profile for devices with a restricted attenuation range... From 869a66d24773c100503103d8492a4c7876d1d782 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Mon, 17 Jul 2023 15:32:19 +0200 Subject: [PATCH 42/63] 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 e6ec317a..769f9efd 100644 --- a/RELEASENOTES-DEVELOPMENT.md +++ b/RELEASENOTES-DEVELOPMENT.md @@ -1,7 +1,7 @@ Version 4.2.1d0-46-g57061dfb ==== **Enhancement** -* Add a new volume-control profile called `dasl-tapered`. Shairport Sync already has two volume control profiles -- the `standard` profile -- which is the default -- and a `flat` profile. A feature of the `dasl-tapered` profile is that halving the volume control setting (e.g. moving the volume slider from full to half, or from half to a quarter) reduces the output level by 10dB, which roughly corresponds with a perceived halving of the audio volume level. Many thanks to David Leibovic, aka [dasl-](https://github.com/dasl-) for this. +* Add a new volume-control profile called `dasl-tapered`. Shairport Sync already has two volume control profiles -- the `standard` profile -- which is the default -- and a `flat` profile. A feature of the `dasl-tapered` profile is that halving the volume control setting (e.g. moving the volume slider from full to half, or from half to a quarter) reduces the output level by 10dB, which roughly corresponds with a perceived halving of the audio volume level. Many thanks to David Leibovic, aka [dasl-](https://github.com/dasl-), for this. To activate the `dasl-tapered` profile, set the `volume_control_profile` to `"dasl_tapered"` in the configuration file and restart Shairport Sync. There may be some more minor adjustments to this profile for devices with a restricted attenuation range... From 9aa03008c2f5250c16d8c6e5c1b1f3e70369c33b Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Mon, 17 Jul 2023 15:32:55 +0200 Subject: [PATCH 43/63] 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 769f9efd..73e35fb6 100644 --- a/RELEASENOTES-DEVELOPMENT.md +++ b/RELEASENOTES-DEVELOPMENT.md @@ -1,7 +1,7 @@ Version 4.2.1d0-46-g57061dfb ==== **Enhancement** -* Add a new volume-control profile called `dasl-tapered`. Shairport Sync already has two volume control profiles -- the `standard` profile -- which is the default -- and a `flat` profile. A feature of the `dasl-tapered` profile is that halving the volume control setting (e.g. moving the volume slider from full to half, or from half to a quarter) reduces the output level by 10dB, which roughly corresponds with a perceived halving of the audio volume level. Many thanks to David Leibovic, aka [dasl-](https://github.com/dasl-), for this. +* Add a new volume control profile called `dasl-tapered`. Shairport Sync already has two volume control profiles -- the `standard` profile -- which is the default -- and a `flat` profile. A feature of the `dasl-tapered` profile is that halving the volume control setting (e.g. moving the volume slider from full to half, or from half to a quarter) reduces the output level by 10dB, which roughly corresponds with a perceived halving of the audio volume level. Many thanks to David Leibovic, aka [dasl-](https://github.com/dasl-), for this. To activate the `dasl-tapered` profile, set the `volume_control_profile` to `"dasl_tapered"` in the configuration file and restart Shairport Sync. There may be some more minor adjustments to this profile for devices with a restricted attenuation range... From bc2fda520715153f8d8c7cd081f55987f1e15f82 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Thu, 20 Jul 2023 16:27:07 +0200 Subject: [PATCH 44/63] Modify the dasl_tapered volume control transfer function to use the entire AirPlay volume range with mixers having a restricted attenuation range. --- common.c | 69 +++++++++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 56 insertions(+), 13 deletions(-) diff --git a/common.c b/common.c index edd4351a..85235a59 100644 --- a/common.c +++ b/common.c @@ -1192,7 +1192,25 @@ uint32_t uatoi(const char *nptr) { return r; } +// clang-format off + +// Given an AirPlay volume (0 to -30) and the highest and lowest attenuations available in the mixer, +// the *vol2attn functions return anmattenuation depending on the AirPlay volume +// and the function's transfer function. + +// Note that the max_db and min_db are given as dB*100 + +// clang-format on + double flat_vol2attn(double vol, long max_db, long min_db) { +// clang-format off + +// This "flat" volume control profile has the property that a given change in the AirPlay volume +// always results in the same change in output dB. For example, if a change of AirPlay volume +// from 0 to -4 resulted in a 7 dB change, then a change in AirPlay volume from -20 to -24 +// would also result in a 7 dB change. + +// clang-format on double vol_setting = min_db; // if all else fails, set this, for safety if ((vol <= 0.0) && (vol >= -30.0)) { @@ -1201,13 +1219,39 @@ double flat_vol2attn(double vol, long max_db, long min_db) { // max_db); } else if (vol != -144.0) { debug(1, - "Linear volume request value %f is out of range: should be from 0.0 to -30.0 or -144.0.", + "flat_vol2attn volume request value %f is out of range: should be from 0.0 to -30.0 or -144.0.", vol); } return vol_setting; } + double dasl_tapered_vol2attn(double vol, long max_db, long min_db) { +// clang-format off + +// The "dasl_tapered" volume control profile has the property that halving the AirPlay volume (the "vol" parameter) +// reduces the output level by 10 dB, which corresponds to roughly halving the perceived volume. + +// For example, if the AirPlay volume goes from 0.0 to -15.0, the output level will decrease by 10 dB. +// Halving the AirPlay volume again, from -15 to -22.5, will decrease output by a further 10 dB. +// Reducing the AirPlay volume from rom -22.5 to -25.25 decreases the output by a further 10 dB, +// meaning that at AirPlay volume -25.25, the volume is decreased 30 dB. + +// If the attenuation range of the mixer is restricted -- for example, if it is just 30 dB -- +// the output level would reach its minimum before the AirPlay volume reached its minimum. +// This would result in part of the AirPlay volume control's range where +// changing the AirPlay volume would make no difference to the output level. + +// In the example of an attenuator with a range of 00.dB to -30.0dB, this +// "dead zone" would be from AirPlay volume -30.0 to -25.25, +// i.e. about one sixth of its -30.0 to 0.0 travel. + +// To work around this, the "flat" output level is used if it gives a +// higher output dB value than the calculation described above. +// If the device's attenuation range is over about 50 dB, +// the flat output level will hardly be needed at all. + +// clang-format on double vol_setting = min_db; // if all else fails, set this, for safety if ((vol <= 0.0) && (vol >= -30.0)) { @@ -1215,10 +1259,12 @@ double dasl_tapered_vol2attn(double vol, long max_db, long min_db) { if (vol_pct <= 0) { return min_db; } - + + double flat_setting = min_db + (max_db - min_db) * vol_pct; vol_setting = max_db + 1000 * log10(vol_pct) / log10(2); // This will be in the range [-inf, max_db] - if (vol_setting < min_db) { - return min_db; + if (vol_setting < flat_setting) { + debug(3, "dasl_tapered_vol2attn returning a flat setting of %f for AirPlay volume %f instead of a tapered setting of %f in a range from %f to %f.", flat_setting, vol, vol_setting, 1.0 * min_db, 1.0 * max_db); + return flat_setting; } if (vol_setting > max_db) { return max_db; @@ -1232,16 +1278,13 @@ double dasl_tapered_vol2attn(double vol, long max_db, long min_db) { return vol_setting; } -// Given a volume (0 to -30) and high and low attenuations available in the mixer in dB, return an -// attenuation depending on the volume and the function's transfer function -// See http://tangentsoft.net/audio/atten.html for data on good attenuators. -// We want a smooth attenuation function, like, for example, the ALPS RK27 Potentiometer transfer -// functions referred to at the link above. - -// Note that the max_db and min_db are given as dB*100 - double vol2attn(double vol, long max_db, long min_db) { + // See http://tangentsoft.net/audio/atten.html for data on good attenuators. + + // We want a smooth attenuation function, like, for example, the ALPS RK27 Potentiometer transfer + // functions referred to at the link above. + // We use a little coordinate geometry to build a transfer function from the volume passed in to // the device's dynamic range. (See the diagram in the documents folder.) The x axis is the // "volume in" which will be from -30 to 0. The y axis will be the "volume out" which will be from @@ -1284,7 +1327,7 @@ double vol2attn(double vol, long max_db, long min_db) { } vol_setting += max_db; } else if (vol != -144.0) { - debug(1, "Volume request value %f is out of range: should be from 0.0 to -30.0 or -144.0.", + debug(1, "vol2attn request value %f is out of range: should be from 0.0 to -30.0 or -144.0.", vol); vol_setting = min_db; // for safety, return the lowest setting... } else { From a5ebbc3067ec97ed57e1dde59efe35483757fb8a Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Thu, 20 Jul 2023 16:45:05 +0200 Subject: [PATCH 45/63] Update RELEASENOTES-DEVELOPMENT.md Modify dasl_tapered profile to work better with restricted range attenuators. --- RELEASENOTES-DEVELOPMENT.md | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/RELEASENOTES-DEVELOPMENT.md b/RELEASENOTES-DEVELOPMENT.md index 73e35fb6..02cca84f 100644 --- a/RELEASENOTES-DEVELOPMENT.md +++ b/RELEASENOTES-DEVELOPMENT.md @@ -1,3 +1,22 @@ +Version 4.2.1d0-51-gbc2fda52 +==== +**Enhancement** +* Modify the new `dasl_tapered` volume control profile to work better with mixers with a restricted attenuation range. + + This volume control profile has the property that halving the AirPlay volume reduces the output level by 10 dB, which corresponds to roughly halving the perceived volume. + + For example, if the AirPlay volume goes from 0.0 to -15.0, the output level will decrease by 10 dB. +Halving the AirPlay volume again, from -15 to -22.5, will decrease output by another 10 dB. +Halving the AirPlay volume once more, from -22.5 to -25.25, decreases the output by a further 10 dB, +meaning that at AirPlay volume -25.25, the volume is decreased by 30 dB relative to the level at AirPlay volume 0.0. + + Now, if the attenuation range of the mixer is only 30 dB, then changing the AirPlay volume in the range from -25.25 down to -30.0 +would have no further effect on the output level, since it's already at the lowest possible level permitted by the attenuator. This "dead zone" is about one sixth of the full range of the AirPlay volume control. + + To work around this, the "flat" output level is used if it gives a higher output dB value than the calculation described above. It means that changing the AirPlay volume, even at low levels, will always change the output level, while the original advantage of the `dasl_tapered` profile is retained at higher AirPlay volumes. + +In practice, if the device's attenuation range is over about 50 dB, the flat output level will hardly be needed at all. + Version 4.2.1d0-46-g57061dfb ==== **Enhancement** From 89c637af9b5843401f26551ba5a24a31daba4bd5 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Thu, 20 Jul 2023 16:46:20 +0200 Subject: [PATCH 46/63] Update RELEASENOTES-DEVELOPMENT.md Layout fix. --- RELEASENOTES-DEVELOPMENT.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/RELEASENOTES-DEVELOPMENT.md b/RELEASENOTES-DEVELOPMENT.md index 02cca84f..54d086ed 100644 --- a/RELEASENOTES-DEVELOPMENT.md +++ b/RELEASENOTES-DEVELOPMENT.md @@ -15,7 +15,7 @@ would have no further effect on the output level, since it's already at the lowe To work around this, the "flat" output level is used if it gives a higher output dB value than the calculation described above. It means that changing the AirPlay volume, even at low levels, will always change the output level, while the original advantage of the `dasl_tapered` profile is retained at higher AirPlay volumes. -In practice, if the device's attenuation range is over about 50 dB, the flat output level will hardly be needed at all. + In practice, if the device's attenuation range is over about 50 dB, the flat output level will hardly be needed at all. Version 4.2.1d0-46-g57061dfb ==== From b965add15084b9696b50409d47afdc1fd8a75cb2 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Fri, 21 Jul 2023 13:34:02 +0200 Subject: [PATCH 47/63] Minor comment change. --- common.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common.c b/common.c index 85235a59..bed243ba 100644 --- a/common.c +++ b/common.c @@ -1234,7 +1234,7 @@ double dasl_tapered_vol2attn(double vol, long max_db, long min_db) { // For example, if the AirPlay volume goes from 0.0 to -15.0, the output level will decrease by 10 dB. // Halving the AirPlay volume again, from -15 to -22.5, will decrease output by a further 10 dB. -// Reducing the AirPlay volume from rom -22.5 to -25.25 decreases the output by a further 10 dB, +// Reducing the AirPlay volume by half again, this time from -22.5 to -25.25 decreases the output by a further 10 dB, // meaning that at AirPlay volume -25.25, the volume is decreased 30 dB. // If the attenuation range of the mixer is restricted -- for example, if it is just 30 dB -- From 8821ec286171a67ae61158551f4884c7c926e330 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Sun, 23 Jul 2023 13:18:52 +0200 Subject: [PATCH 48/63] Update some slightly outdated comments in common.h and shairport-sync.conf --- common.h | 11 +++++------ scripts/shairport-sync.conf | 6 +++++- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/common.h b/common.h index 0c4d92ec..c8e8915b 100644 --- a/common.h +++ b/common.h @@ -391,13 +391,12 @@ uint8_t *rsa_apply(uint8_t *input, int inlen, int *outlen, int mode); // dB), return an attenuation depending on a linear interpolation along the range double flat_vol2attn(double vol, long max_db, long min_db); -// given a volume (0 to -30) and high and low attenuations in dB*100 (e.g. 0 to -6000 for 0 to -60 -// dB), return an attenuation depending on a logarithmic interpolation along the range. -// The intention behind this attenuation function is that a given percentage change in volume -// should result in the same percentage change in perceived loudness. For instance, a doubling -// (100% increase) of volume level should result in a doubling of perceived loudness. +// The intention behind dasl_tapered is that a given percentage change in volume should result in the same percentage change in +// perceived loudness. For instance, doubling the volume level should result in doubling the perceived loudness. +// With the range of AirPlay volume being from -30 to 0, doubling the volume from -22.5 to -15 results in an increase of 10 dB. +// Similarly, doubling the volume from -15 to 0 results in an increase of 10 dB. +// For compatibility with mixers having a restricted attenuation range (e.g. 30 dB), "dasl_tapered" will switch to a flat profile at low AirPlay volumes. double dasl_tapered_vol2attn(double vol, long max_db, long min_db); - // given a volume (0 to -30) and high and low attenuations in dB*100 (e.g. 0 to -6000 for 0 to -60 // dB), return an attenuation depending on the transfer function double vol2attn(double vol, long max_db, long min_db); diff --git a/scripts/shairport-sync.conf b/scripts/shairport-sync.conf index 5528712d..3f4c152f 100644 --- a/scripts/shairport-sync.conf +++ b/scripts/shairport-sync.conf @@ -44,7 +44,11 @@ general = // "flat" makes the volume change at the same rate at all volumes. // "dasl_tapered" is similar to "standard" - it makes the volume change more quickly at lower volumes and slower at higher volumes. // The intention behind dasl_tapered is that a given percentage change in volume should result in the same percentage change in -// perceived loudness. For instance, a doubling (100% increase) of volume level should result in a doubling of perceived loudness. +// perceived loudness. For instance, doubling the volume level should result in doubling the perceived loudness. +// With the range of AirPlay volume being from -30 to 0, doubling the volume from -22.5 to -15 results in an increase of 10 dB. +// Similarly, doubling the volume from -15 to 0 results in an increase of 10 dB. +// For compatibility with mixers having a restricted attenuation range (e.g. 30 dB), "dasl_tapered" will switch to a flat profile at low AirPlay volumes. + // volume_control_combined_hardware_priority = "no"; // when extending the volume range by combining the built-in software attenuator with the hardware mixer attenuator, set this to "yes" to reduce volume by using the hardware mixer first, then the built-in software attenuator. // default_airplay_volume = -24.0; // this is the suggested volume after a reset or after the high_volume_threshold has been exceed and the high_volume_idle_timeout_in_minutes has passed From a9000de666816cb91125c93a9d38e008ff7299aa Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Sun, 27 Aug 2023 15:54:16 +0100 Subject: [PATCH 49/63] Try again to ensure an active_end metadata item is emitted if the player exits while playing. --- activity_monitor.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/activity_monitor.c b/activity_monitor.c index 361bf517..0c15f3d2 100644 --- a/activity_monitor.c +++ b/activity_monitor.c @@ -50,7 +50,6 @@ enum am_state state; enum ps_state { ps_inactive, ps_active } player_state; int activity_monitor_running = 0; -int activity_monitor_is_shutting_down = 0; pthread_t activity_monitor_thread; pthread_mutex_t activity_monitor_mutex; @@ -121,7 +120,7 @@ void activity_monitor_signify_activity(int active) { pthread_mutex_unlock(&activity_monitor_mutex); going_active(config.cmd_blocking); } else if ((state == am_active) && (player_state == ps_inactive) && - ((config.active_state_timeout == 0.0) || (activity_monitor_is_shutting_down != 0))) { + (config.active_state_timeout == 0.0)) { state = am_inactive; pthread_mutex_unlock(&activity_monitor_mutex); going_inactive(config.cmd_blocking); @@ -241,9 +240,11 @@ void activity_monitor_start() { void activity_monitor_stop() { if (activity_monitor_running) { - activity_monitor_is_shutting_down = 1; - debug(3, "activity_monitor_stop start..."); - activity_monitor_signify_activity(0); // tell it to go inactive... + debug(2, "activity_monitor_stop begin. state: %d, player_state: %d.", state, player_state); + if ((state == am_active) || (state == am_timing_out)) { + going_inactive(config.cmd_blocking); + state = am_inactive; + } pthread_cancel(activity_monitor_thread); pthread_join(activity_monitor_thread, NULL); debug(2, "activity_monitor_stop complete"); From d31f4b275a9b30c53c9f901b2a34c5d16ae912c3 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Sun, 27 Aug 2023 16:02:26 +0100 Subject: [PATCH 50/63] Update RELEASENOTES-DEVELOPMENT.md Fix for active_end on shutdown. --- RELEASENOTES-DEVELOPMENT.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/RELEASENOTES-DEVELOPMENT.md b/RELEASENOTES-DEVELOPMENT.md index 54d086ed..fab87f9e 100644 --- a/RELEASENOTES-DEVELOPMENT.md +++ b/RELEASENOTES-DEVELOPMENT.md @@ -1,3 +1,9 @@ +Version 4.2.1d0-56-ga9000de6 +==== +**Bug Fixes** +* The `active_end` enhancement in 4.2.1d0-23 didn't work and has been rewritten here. +* A crashing bug in NQPTP has been fixed in the `development` version -- please update. (The updated version will be incorporated automatically in the `development` version of the Docker image.) + Version 4.2.1d0-51-gbc2fda52 ==== **Enhancement** From 4bee50c920256bcf827d720de540ded0df465954 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Sun, 3 Sep 2023 15:10:06 +0100 Subject: [PATCH 51/63] Update README.md Fix typo in command line! --- docker/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docker/README.md b/docker/README.md index 6caf1c4d..51aee15d 100644 --- a/docker/README.md +++ b/docker/README.md @@ -36,7 +36,7 @@ Command line options will be passed to Shairport Sync. Here is an example: ``` $ docker run -d --restart unless-stopped --net host --device /dev/snd \ mikebrady/shairport-sync:latest \ - -v --statistics -a DenSystem -d hw:0 -c PCM + -v --statistics -a DenSystem -- -d hw:0 -c PCM ``` This will send audio to alsa hardware device `hw:0` and make use of the that device's mixer control called `PCM`. The service will be visible as `DenSystem` on the network. From 986f9587de5be3c8273f3fbe6f89052edbf688e7 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Sat, 9 Sep 2023 10:49:32 +0100 Subject: [PATCH 52/63] Rebuild the locking mechanisms for safe interruption of an existing play session. --- dbus-service.c | 18 +- player.c | 83 ++-- player.h | 7 +- rtsp.c | 1046 +++++++++++++++++++++++------------------------- rtsp.h | 2 +- 5 files changed, 567 insertions(+), 589 deletions(-) diff --git a/dbus-service.c b/dbus-service.c index b4a9cc9e..a25d9962 100644 --- a/dbus-service.c +++ b/dbus-service.c @@ -635,13 +635,14 @@ gboolean notify_volume_callback(ShairportSync *skeleton, if (((iv >= -30.0) && (iv <= 0.0)) || (iv == -144.0)) { debug(2, ">> setting volume to %7.4f.", iv); - lock_player(); - if (playing_conn != NULL) { - player_volume(iv, playing_conn); - playing_conn->own_airplay_volume = iv; - playing_conn->own_airplay_volume_set = 1; + pthread_cleanup_debug_mutex_lock(&principal_conn_lock, 100000, 1); + + if (principal_conn != NULL) { + player_volume(iv, principal_conn); + principal_conn->own_airplay_volume = iv; + principal_conn->own_airplay_volume_set = 1; } - unlock_player(); + pthread_cleanup_pop(1); // release the principal_conn lock config.airplay_volume = iv; config.last_access_to_volume_info_time = get_absolute_time_in_ns(); } else { @@ -873,8 +874,6 @@ static gboolean on_handle_remote_command(ShairportSync *skeleton, GDBusMethodInv static gboolean on_handle_drop_session(ShairportSync *skeleton, GDBusMethodInvocation *invocation, __attribute__((unused)) gpointer user_data) { - if (playing_conn != NULL) - debug(1, ">> stopping current play session"); get_play_lock(NULL, 1); // stop any current session and don't replace it shairport_sync_complete_drop_session(skeleton, invocation); return TRUE; @@ -1071,7 +1070,8 @@ static void on_dbus_name_acquired(GDBusConnection *connection, const gchar *name if (config.volume_control_profile == VCP_standard) shairport_sync_set_volume_control_profile(SHAIRPORT_SYNC(shairportSyncSkeleton), "standard"); else if (config.volume_control_profile == VCP_dasl_tapered) - shairport_sync_set_volume_control_profile(SHAIRPORT_SYNC(shairportSyncSkeleton), "dasl_tapered"); + shairport_sync_set_volume_control_profile(SHAIRPORT_SYNC(shairportSyncSkeleton), + "dasl_tapered"); else shairport_sync_set_volume_control_profile(SHAIRPORT_SYNC(shairportSyncSkeleton), "flat"); diff --git a/player.c b/player.c index a5b0681b..3a2fb382 100644 --- a/player.c +++ b/player.c @@ -101,7 +101,7 @@ #include "activity_monitor.h" -// mstop) { - debug(3, "Connection %d: Stop the output backend.", conn->connection_number); - config.output->stop(); - } - } else { - debug(1, "This is not the playing conn."); + if ((principal_conn == conn) && (conn != NULL)) { + if (config.output->stop) { + debug(2, "Connection %d: Stop the output backend.", conn->connection_number); + config.output->stop(); } - pthread_cleanup_pop(1); // unlock the mutex } else { - debug(1, "Can not acquire play lock."); + if (conn != NULL) + debug(1, "Connection %d: this conn is not the principal_conn.", conn->connection_number); } int oldState; @@ -1900,8 +1895,6 @@ void player_thread_cleanup_handler(void *arg) { if (conn->stream.type == ast_apple_lossless) terminate_decoders(conn); - // reset_anchor_info(conn); - // release_play_lock(conn); conn->rtp_running = 0; pthread_setcancelstate(oldState, NULL); debug(2, "Connection %d: player terminated.", conn->connection_number); @@ -3631,14 +3624,21 @@ int player_prepare_to_play(rtsp_conn_info *conn) { } int player_play(rtsp_conn_info *conn) { - pthread_t *pt = malloc(sizeof(pthread_t)); - if (pt == NULL) - die("Couldn't allocate space for pthread_t"); - conn->player_thread = pt; - int rc = pthread_create(pt, NULL, player_thread_func, (void *)conn); - if (rc) - debug(1, "Error creating player_thread: %s", strerror(errno)); - + debug(2, "Connection %d: player_play.", conn->connection_number); + pthread_cleanup_debug_mutex_lock(&conn->player_create_delete_mutex, 5000, 1); + if (conn->player_thread == NULL) { + pthread_t *pt = malloc(sizeof(pthread_t)); + if (pt == NULL) + die("Couldn't allocate space for pthread_t"); + int rc = pthread_create(pt, NULL, player_thread_func, (void *)conn); + if (rc) + debug(1, "Connection %d: error creating player_thread: %s", conn->connection_number, + strerror(errno)); + conn->player_thread = pt; // set _after_ creation of thread + } else { + debug(1, "Connection %d: player thread already exists.", conn->connection_number); + } + pthread_cleanup_pop(1); // release the player_create_delete_mutex #ifdef CONFIG_METADATA send_ssnc_metadata('pbeg', NULL, 0, 1); // contains cancellation points #endif @@ -3647,35 +3647,38 @@ int player_play(rtsp_conn_info *conn) { int player_stop(rtsp_conn_info *conn) { // note -- this may be called from another connection thread. - // int dl = debuglev; - // debuglev = 3; - debug(3, "player_stop"); - if (conn->player_thread) { -#ifdef CONFIG_AIRPLAY_2 - ptp_send_control_message_string("E"); // signify play is "E"nding -#endif + debug(2, "Connection %d: player_stop.", conn->connection_number); + int response = 0; // okay + pthread_cleanup_debug_mutex_lock(&conn->player_create_delete_mutex, 5000, 1); + pthread_t *pt = conn->player_thread; + if (pt) { debug(3, "player_thread cancel..."); - pthread_cancel(*conn->player_thread); + conn->player_thread = NULL; // cleared _before_ cancelling of thread + pthread_cancel(*pt); debug(3, "player_thread join..."); - if (pthread_join(*conn->player_thread, NULL) == -1) { + if (pthread_join(*pt, NULL) == -1) { char errorstring[1024]; strerror_r(errno, (char *)errorstring, sizeof(errorstring)); debug(1, "Connection %d: error %d joining player thread: \"%s\".", conn->connection_number, errno, (char *)errorstring); } else { - debug(3, "player_thread joined."); + debug(2, "Connection %d: player_stop successful.", conn->connection_number); } - free(conn->player_thread); - conn->player_thread = NULL; + free(pt); + response = 0; // deleted + } else { + debug(2, "Connection %d: no player thread.", conn->connection_number); + response = -1; // already deleted or never created... + } + pthread_cleanup_pop(1); // release the player_create_delete_mutex + if (response == 0) { // if the thread was just stopped and deleted... +#ifdef CONFIG_AIRPLAY_2 + ptp_send_control_message_string("E"); // signify play is "E"nding +#endif #ifdef CONFIG_METADATA send_ssnc_metadata('pend', NULL, 0, 1); // contains cancellation points #endif - // debuglev = dl; command_stop(); - return 0; - } else { - debug(3, "Connection %d: player thread already deleted.", conn->connection_number); - // debuglev = dl; - return -1; } + return response; } diff --git a/player.h b/player.h index 55f77e2e..c5cfd002 100644 --- a/player.h +++ b/player.h @@ -107,7 +107,8 @@ typedef enum { unspecified_stream_category = 0, ptp_stream, ntp_stream, - remote_control_stream + remote_control_stream, + classic_airplay_stream } airplay_stream_c; // "c" for category #ifdef CONFIG_AIRPLAY_2 @@ -221,7 +222,7 @@ typedef struct { int32_t last_seqno_read; // mutexes and condition variables pthread_cond_t flowcontrol; - pthread_mutex_t ab_mutex, flush_mutex, volume_control_mutex; + pthread_mutex_t ab_mutex, flush_mutex, volume_control_mutex, player_create_delete_mutex; int fix_volume; double own_airplay_volume; @@ -414,7 +415,7 @@ typedef struct { uint64_t dac_buffer_queue_minimum_length; } rtsp_conn_info; -extern pthread_mutex_t playing_conn_lock; +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.c b/rtsp.c index a61ade1b..dfa1c668 100644 --- a/rtsp.c +++ b/rtsp.c @@ -93,7 +93,8 @@ #include "ptp-utilities.h" #ifdef HAVE_LIBPLIST_GE_2_3_0 -#define plist_from_memory(plist_data, length, plist) plist_from_memory((plist_data), (length), (plist), NULL) +#define plist_from_memory(plist_data, length, plist) \ + plist_from_memory((plist_data), (length), (plist), NULL) #endif #endif @@ -134,13 +135,18 @@ enum rtsp_read_request_response { rtsp_read_request_response_error }; -rtsp_conn_info *playing_conn; +static int nconns = 0; // i.e. the size if the conns array +rtsp_conn_info *principal_conn; rtsp_conn_info **conns; int metadata_running = 0; -// always lock this when accessing the playing conn value -pthread_mutex_t playing_conn_lock = PTHREAD_MUTEX_INITIALIZER; +// 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; // always lock this when accessing the list of connection threads pthread_mutex_t conns_lock = PTHREAD_MUTEX_INITIALIZER; @@ -511,131 +517,94 @@ int pc_queue_get_item(pc_queue *the_queue, void *the_stuff) { #endif -void lock_player() { debug_mutex_lock(&playing_conn_lock, 1000000, 3); } - -void unlock_player() { debug_mutex_unlock(&playing_conn_lock, 3); } - -int have_play_lock(rtsp_conn_info *conn) { - int response = 0; - lock_player(); - if (playing_conn == conn) // this connection definitely has the play lock - response = 1; - unlock_player(); - return response; -} - -/* -// return 0 if the playing_conn isn't already locked by someone else and if -// it belongs to the conn passed in. -// remember to release it! -int try_to_get_and_lock_playing_conn(rtsp_conn_info **conn) { - int response = -1; - if (pthread_mutex_trylock(&playing_conn_lock) == 0) { - response = 0; - *conn = playing_conn; +// note: connection numbers start at 1, so an except_this_one value of zero means "all threads" +void cancel_all_RTSP_threads(airplay_stream_c stream_category, int except_this_one) { + // if the stream category is unspecified_stream_category + // all categories are elegible for cancellation + // otherwise just the category itself + debug_mutex_lock(&conns_lock, 1000000, 3); + int i; + for (i = 0; i < nconns; i++) { + if ((conns[i] != NULL) && (conns[i]->running != 0) && + (conns[i]->connection_number != except_this_one) && + ((stream_category == unspecified_stream_category) || + (stream_category == conns[i]->airplay_stream_category))) { + pthread_cancel(conns[i]->thread); + debug(1, "Connection %d: cancelled.", conns[i]->connection_number); + } else if (conns[i] != NULL) { + debug(1, "Connection %d: not cancelled.", conns[i]->connection_number); + } } - return response; + for (i = 0; i < nconns; i++) { + if ((conns[i] != NULL) && (conns[i]->running != 0) && + (conns[i]->connection_number != except_this_one) && + ((stream_category == unspecified_stream_category) || + (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; + } + } + debug_mutex_unlock(&conns_lock, 3); } +// The principal_conn variable points to the connection that +// controls the mDNS status and flags and that is potentially +// in control of the playing subsystem to output audio to a backend +// the principal_conn variable may be NULL -void release_hold_on_play_lock(__attribute__((unused)) rtsp_conn_info *conn) { - pthread_mutex_unlock(&playing_conn_lock); -} +// the principal_conn is set by an ANNOUNCE message (Classic AirPlay) or +// by the initial SETUP (of a connection, not of a play session) message (AirPlay 2) and cleared +// when a session is terminated (AirPlay 2) -*/ +// In AirPlay 2, only one PTP connection can be live at any time, and it is the principal_conn. +// This is because, in AirPlay 2, the principal_conn connection +// also has control of the mDNS interface, and thus determines the state of the player as seen by +// other devices. -// make conn no longer the playing_conn void release_play_lock(rtsp_conn_info *conn) { - if (conn != NULL) - debug(2, "Connection %d: release play lock.", conn->connection_number); - else - debug(2, "Release play lock."); - lock_player(); - if (playing_conn == conn) { // if we have the player + pthread_cleanup_debug_mutex_lock(&principal_conn_lock, 100000, + 1); // don't let the principal_conn be changed + if (principal_conn == conn) { // if we have the player if (conn != NULL) - debug(2, "Connection %d: play lock released.", conn->connection_number); - else - debug(2, "Play lock released."); - playing_conn = NULL; // let it go + debug(2, "Connection %d: principal_conn released.", conn->connection_number); + principal_conn = NULL; // let it go } - unlock_player(); + pthread_cleanup_pop(1); // release the principal_conn lock } -// make conn the playing_conn, and kill the current session if permitted +// 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); + if (principal_conn != NULL) + debug(2, "Connection %d: is requested to relinquish principal_conn.", + principal_conn->connection_number); if (conn != NULL) - debug(2, "Connection %d: request play lock.", conn->connection_number); - else if (playing_conn != NULL) - debug(2, "Connection %d: request release.", playing_conn->connection_number); - else - debug(2, "Request release of non-existent player."); + debug(2, "Connection %d: request to acquire principal_conn.", conn->connection_number); // returns -1 if it failed, 0 if it succeeded and 1 if it succeeded but // interrupted an existing session - int response = 0; - - int have_the_player = 0; - int should_wait = 0; // this will be true if you're trying to break in to the current session - int interrupting_current_session = 0; - - // try to become the current playing_conn - - lock_player(); // don't let it change while we are looking at it... - - if (playing_conn == NULL) { - playing_conn = conn; - have_the_player = 1; - } else if (playing_conn == conn) { - have_the_player = 1; + if (principal_conn == NULL) { + principal_conn = conn; + } else if (principal_conn == conn) { if (conn != NULL) - warn("Duplicate attempt to acquire the player by the same connection, by the look of it!"); - } else if (playing_conn->stop) { - debug(1, "Connection %d: Waiting for Connection %d to stop playing.", conn->connection_number, - playing_conn->connection_number); - should_wait = 1; + warn("Connection %d: request to re-acquire principal_conn!", + principal_conn->connection_number); } else if (allow_session_interruption != 0) { - debug(2, "Asking Connection %d to stop playing.", playing_conn->connection_number); - playing_conn->stop = 1; - interrupting_current_session = 1; - should_wait = 1; - pthread_cancel(playing_conn->thread); // asking the RTSP thread to exit - } - unlock_player(); - - if (should_wait) { - int time_remaining = 3000000; // must be signed, as it could go negative... - - while ((time_remaining > 0) && (have_the_player == 0)) { - lock_player(); // get it - if (playing_conn == NULL) { - playing_conn = conn; - have_the_player = 1; - } - unlock_player(); - - if (have_the_player == 0) { - usleep(100000); - time_remaining -= 100000; - } - } - if ((have_the_player == 1) && (interrupting_current_session == 1)) { - if (conn != NULL) - debug(2, "Connection %d: Got player lock", conn->connection_number); - else - debug(2, "Player released."); - response = 1; - } else { - debug(2, "Connection %d: failed to get player lock after waiting.", conn->connection_number); - response = -1; - } - } - - if ((have_the_player == 1) && (interrupting_current_session == 0)) { - if (conn != NULL) - debug(2, "Connection %d: Got player lock.", conn->connection_number); - else - debug(2, "Player released."); - response = 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. + principal_conn = conn; // make the conn the new principal_conn + response = 1; // interrupted an existing session + } else { + response = -1; // can't get it... } + if (principal_conn != NULL) + debug(3, "Connection %d has acquired principal_conn.", principal_conn->connection_number); + pthread_cleanup_pop(1); // release the principal_conn lock return response; } @@ -690,8 +659,6 @@ void *player_watchdog_thread_code(void *arg) { pthread_exit(NULL); } -// keep track of the threads we have spawned so we can join() them -static int nconns = 0; static void track_thread(rtsp_conn_info *conn) { debug_mutex_lock(&conns_lock, 1000000, 3); // look for an empty slot first @@ -718,36 +685,6 @@ static void track_thread(rtsp_conn_info *conn) { debug_mutex_unlock(&conns_lock, 3); } -// note: connection numbers start at 1, so an except_this_one value of zero means "all threads" -void cancel_all_RTSP_threads(airplay_stream_c stream_category, int except_this_one) { - // if the stream category is unspecified_stream_category - // all categories are elegible for cancellation - // otherwise just the category itself - debug_mutex_lock(&conns_lock, 1000000, 3); - int i; - for (i = 0; i < nconns; i++) { - if ((conns[i] != NULL) && (conns[i]->running != 0) && - (conns[i]->connection_number != except_this_one) && - ((stream_category == unspecified_stream_category) || - (stream_category == conns[i]->airplay_stream_category))) { - pthread_cancel(conns[i]->thread); - debug(2, "Connection %d: cancelled.", conns[i]->connection_number); - } - } - for (i = 0; i < nconns; i++) { - if ((conns[i] != NULL) && (conns[i]->running != 0) && - (conns[i]->connection_number != except_this_one) && - ((stream_category == unspecified_stream_category) || - (stream_category == conns[i]->airplay_stream_category))) { - pthread_join(conns[i]->thread, NULL); - debug(2, "Connection %d: joined.", conns[i]->connection_number); - free(conns[i]); - conns[i] = NULL; - } - } - debug_mutex_unlock(&conns_lock, 3); -} - int old_connection_count = -1; void cleanup_threads(void) { @@ -759,7 +696,7 @@ void cleanup_threads(void) { debug_mutex_lock(&conns_lock, 1000000, 3); for (i = 0; i < nconns; i++) { if ((conns[i] != NULL) && (conns[i]->running == 0)) { - debug(3, "found RTSP connection thread %d in a non-running state.", + 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); @@ -1368,11 +1305,14 @@ enum rtsp_read_request_response rtsp_read_request(rtsp_conn_info *conn, rtsp_mes int msg_size = -1; while (msg_size < 0) { - if (conn->stop != 0) { - debug(3, "Connection %d: Shutdown requested by client.", conn->connection_number); - reply = rtsp_read_request_response_immediate_shutdown_requested; - goto shutdown; - } + + /* + if (conn->stop != 0) { + debug(3, "Connection %d: Shutdown requested by client.", conn->connection_number); + reply = rtsp_read_request_response_immediate_shutdown_requested; + goto shutdown; + } + */ nread = read_from_rtsp_connection(conn, buf + inbuf, buflen - inbuf); @@ -1475,11 +1415,14 @@ enum rtsp_read_request_response rtsp_read_request(rtsp_conn_info *conn, rtsp_mes } } - if (conn->stop != 0) { - debug(1, "RTSP shutdown requested."); - reply = rtsp_read_request_response_immediate_shutdown_requested; - goto shutdown; - } + /* + if (conn->stop != 0) { + debug(1, "RTSP shutdown requested."); + reply = rtsp_read_request_response_immediate_shutdown_requested; + goto shutdown; + } + */ + size_t read_chunk = msg_size - inbuf; // if (read_chunk > max_read_chunk) // read_chunk = max_read_chunk; @@ -1644,6 +1587,9 @@ char *get_category_string(airplay_stream_c cat) { case remote_control_stream: category = "Remote Control stream"; break; + case classic_airplay_stream: + category = "Classic AirPlay stream"; + break; default: category = "Unexpected stream code"; break; @@ -1661,11 +1607,12 @@ void handle_record_2(rtsp_conn_info *conn, __attribute((unused)) rtsp_message *r void handle_record(rtsp_conn_info *conn, rtsp_message *req, rtsp_message *resp) { debug(2, "Connection %d: RECORD", conn->connection_number); - if (have_play_lock(conn)) { + if ((conn != NULL) && (principal_conn == conn)) { + // if (have_play_lock(conn)) { if (conn->player_thread) warn("Connection %d: RECORD: Duplicate RECORD message -- ignored", conn->connection_number); else { - debug(1, "Connection %d: Classic AirPlay connection from %s:%u to self at %s:%u.", + debug(2, "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); activity_monitor_signify_activity(1); @@ -2037,7 +1984,8 @@ void handle_setrateanchori(rtsp_conn_info *conn, rtsp_message *req, rtsp_message pthread_cleanup_push(mutex_unlock, &conn->flush_mutex); conn->ap2_rate = rate; if ((rate & 1) != 0) { - ptp_send_control_message_string("B"); // signify clock dependability period is "B"eginning (or resuming) + ptp_send_control_message_string( + "B"); // signify clock dependability period is "B"eginning (or resuming) debug(2, "Connection %d: Start playing, with anchor clock %" PRIx64 ".", conn->connection_number, conn->networkTimeTimelineID); activity_monitor_signify_activity(1); @@ -2737,7 +2685,7 @@ void teardown_phase_two(rtsp_conn_info *conn) { } conn->groupContainsGroupLeader = 0; config.airplay_statusflags &= (0xffffffff - (1 << 11)); // DeviceSupportsRelay - build_bonjour_strings(conn); + 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); @@ -2745,7 +2693,6 @@ void teardown_phase_two(rtsp_conn_info *conn) { free(conn->dacp_active_remote); conn->dacp_active_remote = NULL; } - release_play_lock(conn); clear_ptp_clock(); } } @@ -2753,6 +2700,8 @@ 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, + get_category_string(conn->airplay_stream_category)); debug_log_rtsp_message(2, "TEARDOWN: ", req); // if (have_player(conn)) { resp->respcode = 200; @@ -2764,26 +2713,28 @@ void handle_teardown_2(rtsp_conn_info *conn, __attribute__((unused)) rtsp_messag plist_t streams = plist_dict_get_item(messagePlist, "streams"); if (streams) { - debug(2, "Connection %d: TEARDOWN %s Close the stream.", conn->connection_number, + debug(2, "Connection %d: TEARDOWN %s -- close the stream.", conn->connection_number, get_category_string(conn->airplay_stream_category)); // we are being asked to close a stream teardown_phase_one(conn); plist_free(streams); - debug(2, "Connection %d: TEARDOWN %s Close the stream complete", conn->connection_number, + debug(2, "Connection %d: TEARDOWN %s -- close the stream complete", conn->connection_number, get_category_string(conn->airplay_stream_category)); } else { - debug(2, "Connection %d: TEARDOWN %s Close the connection.", conn->connection_number, + debug(2, "Connection %d: TEARDOWN %s -- close the connection.", conn->connection_number, get_category_string(conn->airplay_stream_category)); teardown_phase_one(conn); // try to do phase one anyway teardown_phase_two(conn); - debug(2, "Connection %d: TEARDOWN %s Close the connection complete", conn->connection_number, - get_category_string(conn->airplay_stream_category)); + debug(2, "Connection %d: TEARDOWN %s -- close the connection complete", + conn->connection_number, get_category_string(conn->airplay_stream_category)); } //} else { // warn("Connection %d TEARDOWN received without having the player (no ANNOUNCE?)", // conn->connection_number); // resp->respcode = 451; + release_play_lock(conn); + plist_free(messagePlist); resp->respcode = 200; } else { @@ -2808,22 +2759,22 @@ 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, + // if (have_play_lock(conn)) { + debug(3, "TEARDOWN: synchronously terminating the player thread of RTSP conversation thread %d (2).", conn->connection_number); - teardown(conn); - release_play_lock(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; - } + teardown(conn); + release_play_lock(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); } @@ -2846,7 +2797,8 @@ void handle_flush(rtsp_conn_info *conn, rtsp_message *req, rtsp_message *resp) { } } debug(2, "RTSP Flush Requested: %u.", rtptime); - if (have_play_lock(conn)) { + + if ((conn != NULL) && (conn == principal_conn)) { #ifdef CONFIG_METADATA if (p) send_metadata('ssnc', 'flsr', p + 1, strlen(p + 1), req, 1); @@ -2884,7 +2836,7 @@ static void check_and_send_plist_metadata(plist_t messagePlist, const char *plis void handle_setup_2(rtsp_conn_info *conn, rtsp_message *req, rtsp_message *resp) { int err; debug(2, "Connection %d: SETUP (AirPlay 2)", conn->connection_number); - // debug_log_rtsp_message(1, "SETUP (AirPlay 2) SETUP incoming message", req); + debug_log_rtsp_message(2, "SETUP (AirPlay 2) SETUP incoming message", req); plist_t messagePlist = plist_from_rtsp_content(req); plist_t setupResponsePlist = plist_new_dict(); @@ -2921,15 +2873,6 @@ void handle_setup_2(rtsp_conn_info *conn, rtsp_message *req, rtsp_message *resp) clientNameString, conn->self_ip_string, conn->self_rtsp_port); conn->airplay_stream_category = ptp_stream; conn->timing_type = ts_ptp; -#ifdef CONFIG_METADATA - send_ssnc_metadata('conn', conn->client_ip_string, strlen(conn->client_ip_string), - 1); // before disconnecting an existing play -#endif - get_play_lock(conn, 1); // airplay 2 always allows interruption -#ifdef CONFIG_METADATA - send_ssnc_metadata('clip', conn->client_ip_string, strlen(conn->client_ip_string), 1); - send_ssnc_metadata('svip', conn->self_ip_string, strlen(conn->self_ip_string), 1); -#endif } else if (strcmp(timingProtocolString, "NTP") == 0) { debug(1, "Connection %d: SETUP: NTP setup from %s:%u (\"%s\") to self at %s:%u.", conn->connection_number, conn->client_ip_string, conn->client_rtsp_port, @@ -2949,7 +2892,7 @@ void handle_setup_2(rtsp_conn_info *conn, rtsp_message *req, rtsp_message *resp) plist_get_bool_val(isRemoteControlOnly, &isRemoteControlOnlyBoolean); if (isRemoteControlOnlyBoolean != 0) { debug( - 1, + 2, "Connection %d: Remote Control connection from %s:%u (\"%s\") to self at %s:%u.", conn->connection_number, conn->client_ip_string, conn->client_rtsp_port, clientNameString, conn->self_ip_string, conn->self_rtsp_port); @@ -2972,178 +2915,206 @@ void handle_setup_2(rtsp_conn_info *conn, rtsp_message *req, rtsp_message *resp) // if it's a full service PTP stream, we get groupUUID, groupContainsGroupLeader and // timingPeerList if (conn->airplay_stream_category == ptp_stream) { - if (ptp_shm_interface_open() != - 0) // it should be open already, but just in case it isn't... - die("Can not access the NQPTP service. Has it stopped running?"); - // clear_ptp_clock(); - debug_log_rtsp_message(2, "SETUP \"PTP\" message", req); - plist_t groupUUID = plist_dict_get_item(messagePlist, "groupUUID"); - if (groupUUID) { - char *gid = NULL; - plist_get_string_val(groupUUID, &gid); - if (gid) { - if (conn->airplay_gid) - free(conn->airplay_gid); - conn->airplay_gid = gid; // it'll be free'd later on... - } else { - debug(1, "Invalid groupUUID"); - } - } else { - debug(1, "No groupUUID in SETUP"); - } - // now see if the group contains a group leader - plist_t groupContainsGroupLeader = - plist_dict_get_item(messagePlist, "groupContainsGroupLeader"); - if (groupContainsGroupLeader) { - uint8_t value = 0; - plist_get_bool_val(groupContainsGroupLeader, &value); - conn->groupContainsGroupLeader = value; - debug(2, "Updated groupContainsGroupLeader to %u", conn->groupContainsGroupLeader); - } else { - debug(1, "No groupContainsGroupLeader in SETUP"); - } - - char timing_list_message[4096]; - timing_list_message[0] = 'T'; - timing_list_message[1] = 0; - - // ensure the client itself is first -- it's okay if it's duplicated later - strncat(timing_list_message, " ", - sizeof(timing_list_message) - 1 - strlen(timing_list_message)); - strncat(timing_list_message, (const char *)&conn->client_ip_string, - sizeof(timing_list_message) - 1 - strlen(timing_list_message)); - - plist_t timing_peer_info = plist_dict_get_item(messagePlist, "timingPeerInfo"); - if (timing_peer_info) { - // first, get the incoming plist. - plist_t addresses_array = plist_dict_get_item(timing_peer_info, "Addresses"); - if (addresses_array) { - // iterate through the array of items - uint32_t items = plist_array_get_size(addresses_array); - if (items) { - uint32_t item; - for (item = 0; item < items; item++) { - plist_t n = plist_array_get_item(addresses_array, item); - char *ip_address = NULL; - plist_get_string_val(n, &ip_address); - // debug(1, "Timing peer: %s", ip_address); - strncat(timing_list_message, " ", - sizeof(timing_list_message) - 1 - strlen(timing_list_message)); - strncat(timing_list_message, ip_address, - sizeof(timing_list_message) - 1 - strlen(timing_list_message)); - free(ip_address); - } - } else { - debug(1, "SETUP on Connection %d: No timingPeerInfo addresses in the array.", - conn->connection_number); - } - } else { - debug(1, "SETUP on Connection %d: Can't find timingPeerInfo addresses", - conn->connection_number); - } - // make up the timing peer info list part of the response... - // debug(1,"Create timingPeerInfoPlist"); - plist_t timingPeerInfoPlist = plist_new_dict(); - plist_t addresses = plist_new_array(); // to hold the device's interfaces - plist_array_append_item(addresses, plist_new_string(conn->self_ip_string)); - // debug(1,"self ip: \"%s\"", conn->self_ip_string); - - struct ifaddrs *addrs, *iap; - getifaddrs(&addrs); - for (iap = addrs; iap != NULL; iap = iap->ifa_next) { - // debug(1, "Interface index %d, name: \"%s\"",if_nametoindex(iap->ifa_name), - // iap->ifa_name); - if ((iap->ifa_addr) && (iap->ifa_netmask) && (iap->ifa_flags & IFF_UP) && - ((iap->ifa_flags & IFF_LOOPBACK) == 0)) { - char buf[INET6_ADDRSTRLEN + 1]; // +1 for a NUL - memset(buf, 0, sizeof(buf)); - if (iap->ifa_addr->sa_family == AF_INET6) { - struct sockaddr_in6 *addr6 = (struct sockaddr_in6 *)(iap->ifa_addr); - inet_ntop(AF_INET6, (void *)&addr6->sin6_addr, buf, sizeof(buf)); - plist_array_append_item(addresses, plist_new_string(buf)); - // debug(1, "Own address IPv6: %s", buf); - - // strncat(timing_list_message, " ", - // sizeof(timing_list_message) - 1 - strlen(timing_list_message)); - // strncat(timing_list_message, buf, - // sizeof(timing_list_message) - 1 - strlen(timing_list_message)); - - } else { - struct sockaddr_in *addr = (struct sockaddr_in *)(iap->ifa_addr); - inet_ntop(AF_INET, (void *)&addr->sin_addr, buf, sizeof(buf)); - plist_array_append_item(addresses, plist_new_string(buf)); - // debug(1, "Own address IPv4: %s", buf); - - // strncat(timing_list_message, " ", - // sizeof(timing_list_message) - 1 - strlen(timing_list_message)); - // strncat(timing_list_message, buf, - // sizeof(timing_list_message) - 1 - strlen(timing_list_message)); - } - } - } - freeifaddrs(addrs); - - // debug(1,"initial timing peer command: \"%s\".", timing_list_message); - // ptp_send_control_message_string(timing_list_message); - set_client_as_ptp_clock(conn); - ptp_send_control_message_string("B"); // signify clock dependability period is "B"eginning (or continuing) - plist_dict_set_item(timingPeerInfoPlist, "Addresses", addresses); - plist_dict_set_item(timingPeerInfoPlist, "ID", plist_new_string(conn->self_ip_string)); - plist_dict_set_item(setupResponsePlist, "timingPeerInfo", timingPeerInfoPlist); - // get a port to use as an event port - // bind a new TCP port and get a socket - conn->local_event_port = 0; // any port - int err = bind_socket_and_port(SOCK_STREAM, conn->connection_ip_family, - conn->self_ip_string, conn->self_scope_id, - &conn->local_event_port, &conn->event_socket); - if (err) { - die("SETUP on Connection %d: Error %d: could not find a TCP port to use as an event " - "port", - conn->connection_number, err); - } - - listen(conn->event_socket, 128); // ensure socket is open before telling client - - debug(2, "Connection %d: TCP PTP event port opened: %u.", conn->connection_number, - conn->local_event_port); - - if (conn->rtp_event_thread != NULL) - debug(1, "previous rtp_event_thread allocation not freed, it seems."); - conn->rtp_event_thread = malloc(sizeof(pthread_t)); - if (conn->rtp_event_thread == NULL) - die("Couldn't allocate space for pthread_t"); - - pthread_create(conn->rtp_event_thread, NULL, &rtp_event_receiver, (void *)conn); - - plist_dict_set_item(setupResponsePlist, "eventPort", - plist_new_uint(conn->local_event_port)); - plist_dict_set_item(setupResponsePlist, "timingPort", plist_new_uint(0)); // dummy - cancel_all_RTSP_threads(unspecified_stream_category, - conn->connection_number); // kill all the other listeners - - 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.", - conn->connection_number); - } + // airplay 2 always allows interruption, so should never return -1 + if (get_play_lock(conn, 1) != -1) { #ifdef CONFIG_METADATA - check_and_send_plist_metadata(messagePlist, "name", 'snam'); - check_and_send_plist_metadata(messagePlist, "deviceID", 'cdid'); - check_and_send_plist_metadata(messagePlist, "model", 'cmod'); - check_and_send_plist_metadata(messagePlist, "macAddress", 'cmac'); + send_ssnc_metadata('conn', conn->client_ip_string, strlen(conn->client_ip_string), + 1); // before disconnecting an existing play #endif + +#ifdef CONFIG_METADATA + send_ssnc_metadata('clip', conn->client_ip_string, strlen(conn->client_ip_string), 1); + send_ssnc_metadata('svip', conn->self_ip_string, strlen(conn->self_ip_string), 1); +#endif + + if (ptp_shm_interface_open() != + 0) // it should be open already, but just in case it isn't... + die("Can not access the NQPTP service. Has it stopped running?"); + // clear_ptp_clock(); + debug_log_rtsp_message(2, "SETUP \"PTP\" message", req); + plist_t groupUUID = plist_dict_get_item(messagePlist, "groupUUID"); + if (groupUUID) { + char *gid = NULL; + plist_get_string_val(groupUUID, &gid); + if (gid) { + if (conn->airplay_gid) + free(conn->airplay_gid); + conn->airplay_gid = gid; // it'll be free'd later on... + } else { + debug(1, "Invalid groupUUID"); + } + } else { + debug(1, "No groupUUID in SETUP"); + } + + // now see if the group contains a group leader + plist_t groupContainsGroupLeader = + plist_dict_get_item(messagePlist, "groupContainsGroupLeader"); + if (groupContainsGroupLeader) { + uint8_t value = 0; + plist_get_bool_val(groupContainsGroupLeader, &value); + conn->groupContainsGroupLeader = value; + debug(2, "Updated groupContainsGroupLeader to %u", conn->groupContainsGroupLeader); + } else { + debug(1, "No groupContainsGroupLeader in SETUP"); + } + + char timing_list_message[4096]; + timing_list_message[0] = 'T'; + timing_list_message[1] = 0; + + // ensure the client itself is first -- it's okay if it's duplicated later + strncat(timing_list_message, " ", + sizeof(timing_list_message) - 1 - strlen(timing_list_message)); + strncat(timing_list_message, (const char *)&conn->client_ip_string, + sizeof(timing_list_message) - 1 - strlen(timing_list_message)); + + plist_t timing_peer_info = plist_dict_get_item(messagePlist, "timingPeerInfo"); + if (timing_peer_info) { + // first, get the incoming plist. + plist_t addresses_array = plist_dict_get_item(timing_peer_info, "Addresses"); + if (addresses_array) { + // iterate through the array of items + uint32_t items = plist_array_get_size(addresses_array); + if (items) { + uint32_t item; + for (item = 0; item < items; item++) { + plist_t n = plist_array_get_item(addresses_array, item); + char *ip_address = NULL; + plist_get_string_val(n, &ip_address); + // debug(1, "Timing peer: %s", ip_address); + strncat(timing_list_message, " ", + sizeof(timing_list_message) - 1 - strlen(timing_list_message)); + strncat(timing_list_message, ip_address, + sizeof(timing_list_message) - 1 - strlen(timing_list_message)); + free(ip_address); + } + } else { + debug(1, "SETUP on Connection %d: No timingPeerInfo addresses in the array.", + conn->connection_number); + } + } else { + debug(1, "SETUP on Connection %d: Can't find timingPeerInfo addresses", + conn->connection_number); + } + // make up the timing peer info list part of the response... + // debug(1,"Create timingPeerInfoPlist"); + plist_t timingPeerInfoPlist = plist_new_dict(); + plist_t addresses = plist_new_array(); // to hold the device's interfaces + plist_array_append_item(addresses, plist_new_string(conn->self_ip_string)); + // debug(1,"self ip: \"%s\"", conn->self_ip_string); + + struct ifaddrs *addrs, *iap; + getifaddrs(&addrs); + for (iap = addrs; iap != NULL; iap = iap->ifa_next) { + // debug(1, "Interface index %d, name: \"%s\"",if_nametoindex(iap->ifa_name), + // iap->ifa_name); + if ((iap->ifa_addr) && (iap->ifa_netmask) && (iap->ifa_flags & IFF_UP) && + ((iap->ifa_flags & IFF_LOOPBACK) == 0)) { + char buf[INET6_ADDRSTRLEN + 1]; // +1 for a NUL + memset(buf, 0, sizeof(buf)); + if (iap->ifa_addr->sa_family == AF_INET6) { + struct sockaddr_in6 *addr6 = (struct sockaddr_in6 *)(iap->ifa_addr); + inet_ntop(AF_INET6, (void *)&addr6->sin6_addr, buf, sizeof(buf)); + plist_array_append_item(addresses, plist_new_string(buf)); + // debug(1, "Own address IPv6: %s", buf); + + // strncat(timing_list_message, " ", + // sizeof(timing_list_message) - 1 - strlen(timing_list_message)); + // strncat(timing_list_message, buf, + // sizeof(timing_list_message) - 1 - strlen(timing_list_message)); + + } else { + struct sockaddr_in *addr = (struct sockaddr_in *)(iap->ifa_addr); + inet_ntop(AF_INET, (void *)&addr->sin_addr, buf, sizeof(buf)); + plist_array_append_item(addresses, plist_new_string(buf)); + // debug(1, "Own address IPv4: %s", buf); + + // strncat(timing_list_message, " ", + // sizeof(timing_list_message) - 1 - strlen(timing_list_message)); + // strncat(timing_list_message, buf, + // sizeof(timing_list_message) - 1 - strlen(timing_list_message)); + } + } + } + freeifaddrs(addrs); + + // debug(1,"initial timing peer command: \"%s\".", timing_list_message); + // ptp_send_control_message_string(timing_list_message); + set_client_as_ptp_clock(conn); + ptp_send_control_message_string( + "B"); // signify clock dependability period is "B"eginning (or continuing) + plist_dict_set_item(timingPeerInfoPlist, "Addresses", addresses); + plist_dict_set_item(timingPeerInfoPlist, "ID", + plist_new_string(conn->self_ip_string)); + plist_dict_set_item(setupResponsePlist, "timingPeerInfo", timingPeerInfoPlist); + // get a port to use as an event port + // bind a new TCP port and get a socket + conn->local_event_port = 0; // any port + int err = bind_socket_and_port(SOCK_STREAM, conn->connection_ip_family, + conn->self_ip_string, conn->self_scope_id, + &conn->local_event_port, &conn->event_socket); + if (err) { + die("SETUP on Connection %d: Error %d: could not find a TCP port to use as an " + "event " + "port", + conn->connection_number, err); + } + + listen(conn->event_socket, 128); // ensure socket is open before telling client + + debug(2, "Connection %d: TCP PTP event port opened: %u.", conn->connection_number, + conn->local_event_port); + + if (conn->rtp_event_thread != NULL) + debug(1, "previous rtp_event_thread allocation not freed, it seems."); + conn->rtp_event_thread = malloc(sizeof(pthread_t)); + if (conn->rtp_event_thread == NULL) + die("Couldn't allocate space for pthread_t"); + + pthread_create(conn->rtp_event_thread, NULL, &rtp_event_receiver, (void *)conn); + + plist_dict_set_item(setupResponsePlist, "eventPort", + plist_new_uint(conn->local_event_port)); + plist_dict_set_item(setupResponsePlist, "timingPort", plist_new_uint(0)); // dummy + + /* + cancel_all_RTSP_threads(unspecified_stream_category, + conn->connection_number); // kill all the other + listeners + */ + + 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.", + conn->connection_number); + } + +#ifdef CONFIG_METADATA + check_and_send_plist_metadata(messagePlist, "name", 'snam'); + check_and_send_plist_metadata(messagePlist, "deviceID", 'cdid'); + check_and_send_plist_metadata(messagePlist, "model", 'cmod'); + check_and_send_plist_metadata(messagePlist, "macAddress", 'cmac'); +#endif + } else { + // this should never happen! + debug(1, "SETUP on Connection %d: could not become principal conn.", + conn->connection_number); + resp->respcode = 453; + } } else if (conn->airplay_stream_category == ntp_stream) { debug(1, "SETUP on Connection %d: ntp stream handling is not implemented!", conn->connection_number, req); warn("Shairport Sync can not handle NTP streams."); } else if (conn->airplay_stream_category == remote_control_stream) { + /* debug_log_rtsp_message(2, "SETUP (no stream) \"isRemoteControlOnly\" message", req); // get a port to use as an event port @@ -3160,7 +3131,7 @@ void handle_setup_2(rtsp_conn_info *conn, rtsp_message *req, rtsp_message *resp) listen(conn->event_socket, 128); // ensure socket is open before telling client - debug(2, "Connection %d SETUP (RC): TCP Remote Control event port opened: %u.", + debug(1, "Connection %d SETUP (RC): TCP Remote Control event port opened: %u.", conn->connection_number, conn->local_event_port); if (conn->rtp_event_thread != NULL) debug(1, @@ -3179,6 +3150,7 @@ void handle_setup_2(rtsp_conn_info *conn, rtsp_message *req, rtsp_message *resp) remote_control_stream, conn->connection_number); // kill all the other remote control listeners resp->respcode = 200; + */ } else { debug(1, "SETUP on Connection %d: an unrecognised \"%s\" setup detected.", conn->connection_number, timingProtocolString); @@ -3202,7 +3174,8 @@ void handle_setup_2(rtsp_conn_info *conn, rtsp_message *req, rtsp_message *resp) conn->connection_number, get_category_string(conn->airplay_stream_category)); if (conn->airplay_stream_category == ptp_stream) { // get stream[0] - ptp_send_control_message_string("B"); // signify clock dependability period is "B"eginning (or continuing) + ptp_send_control_message_string( + "B"); // signify clock dependability period is "B"eginning (or continuing) plist_t stream0 = plist_array_get_item(streams, 0); plist_t streams_array = plist_new_array(); // to hold the ports and stuff @@ -3280,7 +3253,6 @@ void handle_setup_2(rtsp_conn_info *conn, rtsp_message *req, rtsp_message *resp) case 96: { debug(1, "Connection %d. AP2 Realtime Audio Stream.", conn->connection_number); debug_log_rtsp_message(2, "Realtime Audio Stream SETUP incoming message", req); - // get_play_lock(conn); conn->airplay_stream_type = realtime_stream; // bind a new UDP port and get a socket conn->local_realtime_audio_port = 0; // any port @@ -3332,9 +3304,8 @@ void handle_setup_2(rtsp_conn_info *conn, rtsp_message *req, rtsp_message *resp) conn->rtp_running = 1; // hack! } break; case 103: { - debug(1, "Connection %d. AP2 Buffered Audio Stream.", conn->connection_number); + debug(2, "Connection %d. AP2 Buffered Audio Stream.", conn->connection_number); debug_log_rtsp_message(2, "Buffered Audio Stream SETUP incoming message", req); - // get_play_lock(conn); conn->airplay_stream_type = buffered_stream; // get needed stuff @@ -3446,9 +3417,10 @@ void handle_setup_2(rtsp_conn_info *conn, rtsp_message *req, rtsp_message *resp) #endif void handle_setup(rtsp_conn_info *conn, rtsp_message *req, rtsp_message *resp) { - debug(3, "Connection %d: SETUP", conn->connection_number); + debug(2, "Connection %d: SETUP", conn->connection_number); resp->respcode = 451; // invalid arguments -- expect them - if (have_play_lock(conn)) { + // check this connection has the principal_conn, obtained during a prior ANNOUNCE + if ((conn != NULL) && (principal_conn == conn)) { uint16_t cport, tport; char *ar = msg_get_header(req, "Active-Remote"); if (ar) { @@ -3554,20 +3526,19 @@ void handle_setup(rtsp_conn_info *conn, rtsp_message *req, rtsp_message *resp) { } else { debug(1, "Connection %d: SETUP doesn't contain a Transport header.", conn->connection_number); } - if (resp->respcode == 200) { -#ifdef CONFIG_METADATA - send_ssnc_metadata('clip', conn->client_ip_string, strlen(conn->client_ip_string), 1); - send_ssnc_metadata('svip', conn->self_ip_string, strlen(conn->self_ip_string), 1); -#endif - } else { - debug(1, "Connection %d: SETUP error -- releasing the player lock.", conn->connection_number); - release_play_lock(conn); - } - } else { warn("Connection %d SETUP received without having the player (no ANNOUNCE?)", conn->connection_number); } + if (resp->respcode == 200) { +#ifdef CONFIG_METADATA + send_ssnc_metadata('clip', conn->client_ip_string, strlen(conn->client_ip_string), 1); + send_ssnc_metadata('svip', conn->self_ip_string, strlen(conn->self_ip_string), 1); +#endif + } else { + debug(1, "Connection %d: SETUP error -- releasing the player lock.", conn->connection_number); + release_play_lock(conn); + } } /* @@ -3613,15 +3584,16 @@ void handle_set_parameter_parameter(rtsp_conn_info *conn, rtsp_message *req, shairport_sync_set_volume(shairportSyncSkeleton, volume); } else { #endif - lock_player(); - if (playing_conn == conn) { + pthread_cleanup_debug_mutex_lock(&principal_conn_lock, 100000, + 1); // don't let the principal_conn be changed + if (principal_conn == conn) { player_volume(volume, conn); } if (conn != NULL) { conn->own_airplay_volume = volume; conn->own_airplay_volume_set = 1; } - unlock_player(); + pthread_cleanup_pop(1); // release the principal_conn lock config.last_access_to_volume_info_time = get_absolute_time_in_ns(); #ifdef CONFIG_DBUS_INTERFACE } @@ -4484,10 +4456,13 @@ static void handle_set_parameter(rtsp_conn_info *conn, rtsp_message *req, rtsp_m } static void handle_announce(rtsp_conn_info *conn, rtsp_message *req, rtsp_message *resp) { - debug(3, "Connection %d: ANNOUNCE", conn->connection_number); + debug(2, "Connection %d: ANNOUNCE", conn->connection_number); + int get_play_status = get_play_lock(conn, config.allow_session_interruption); if (get_play_status != -1) { - debug(3, "Connection %d: ANNOUNCE has acquired play lock.", conn->connection_number); + debug(2, "Connection %d: ANNOUNCE has acquired play lock.", conn->connection_number); + + conn->airplay_stream_category = classic_airplay_stream; // now, if this new session did not break in, then it's okay to reset the next UDP ports // to the start of the range @@ -4512,13 +4487,13 @@ 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(1, "Connection %d: Classic AirPlay connection from %s:%u to self at %s:%u.", + debug(2, "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 conn->stream.type = ast_unknown; - resp->respcode = 456; // 456 - Header Field Not Valid for Resource + resp->respcode = 200; // presumed OK char *pssid = NULL; char *paesiv = NULL; char *prsaaeskey = NULL; @@ -4613,28 +4588,29 @@ static void handle_announce(rtsp_conn_info *conn, rtsp_message *req, rtsp_messag // debug(1,"Encrypted session requested"); } else { warn("Invalid Announce message -- missing paesiv or prsaaeskey."); - goto out; + resp->respcode = 456; // 456 - Header Field Not Valid for Resource + // goto out; } if (conn->stream.encrypted) { int len, keylen; uint8_t *aesiv = base64_dec(paesiv, &len); - if (len != 16) { + if (len == 16) { + memcpy(conn->stream.aesiv, aesiv, 16); + } else { + resp->respcode = 456; // 456 - Header Field Not Valid for Resource warn("client announced aeskey of %d bytes, wanted 16", len); - free(aesiv); - goto out; } - memcpy(conn->stream.aesiv, aesiv, 16); free(aesiv); uint8_t *rsaaeskey = base64_dec(prsaaeskey, &len); uint8_t *aeskey = rsa_apply(rsaaeskey, len, &keylen, RSA_MODE_KEY); free(rsaaeskey); - if (keylen != 16) { + if (keylen == 16) { + memcpy(conn->stream.aeskey, aeskey, 16); + } else { + resp->respcode = 456; // 456 - Header Field Not Valid for Resource warn("client announced rsaaeskey of %d bytes, wanted 16", keylen); - free(aeskey); - goto out; } - memcpy(conn->stream.aeskey, aeskey, 16); free(aeskey); } @@ -4675,7 +4651,37 @@ static void handle_announce(rtsp_conn_info *conn, rtsp_message *req, rtsp_messag conn->input_bytes_per_frame = conn->input_num_channels * ((conn->input_bit_depth + 7) / 8); } - if (conn->stream.type == ast_unknown) { + if ((resp->respcode == 200) && (conn->stream.type != ast_unknown)) { + char *hdr = msg_get_header(req, "X-Apple-Client-Name"); + if (hdr) { + debug(1, "Play connection from device named \"%s\" on RTSP conversation thread %d.", hdr, + conn->connection_number); +#ifdef CONFIG_METADATA + send_metadata('ssnc', 'snam', hdr, strlen(hdr), req, 1); +#endif + } + hdr = msg_get_header(req, "User-Agent"); + if (hdr) { + conn->UserAgent = strdup(hdr); + debug(2, "Play connection from user agent \"%s\" on RTSP conversation thread %d.", hdr, + conn->connection_number); + // if the user agent is AirPlay and has a version number of 353 or less (from iOS 11.1,2) + // use the older way of calculating the latency + + char *p = strstr(hdr, "AirPlay"); + if (p) { + p = strchr(p, '/'); + if (p) { + conn->AirPlayVersion = atoi(p + 1); + debug(2, "AirPlay version %d detected.", conn->AirPlayVersion); + } + } + +#ifdef CONFIG_METADATA + send_metadata('ssnc', 'snua', hdr, strlen(hdr), req, 1); +#endif + } + } else { warn("Can not process the following ANNOUNCE message:"); // print each line of the request content // the problem is that nextline has replace all returns, newlines, etc. by @@ -4688,50 +4694,11 @@ static void handle_announce(rtsp_conn_info *conn, rtsp_message *req, rtsp_messag cp += strlen(cp) + 1; cp_left -= strlen(cp) + 1; } - goto out; } - - char *hdr = msg_get_header(req, "X-Apple-Client-Name"); - if (hdr) { - debug(1, "Play connection from device named \"%s\" on RTSP conversation thread %d.", hdr, - conn->connection_number); -#ifdef CONFIG_METADATA - send_metadata('ssnc', 'snam', hdr, strlen(hdr), req, 1); -#endif - } - hdr = msg_get_header(req, "User-Agent"); - if (hdr) { - conn->UserAgent = strdup(hdr); - debug(2, "Play connection from user agent \"%s\" on RTSP conversation thread %d.", hdr, - conn->connection_number); - // if the user agent is AirPlay and has a version number of 353 or less (from iOS 11.1,2) - // use the older way of calculating the latency - - char *p = strstr(hdr, "AirPlay"); - if (p) { - p = strchr(p, '/'); - if (p) { - conn->AirPlayVersion = atoi(p + 1); - debug(2, "AirPlay version %d detected.", conn->AirPlayVersion); - } - } - -#ifdef CONFIG_METADATA - send_metadata('ssnc', 'snua', hdr, strlen(hdr), req, 1); -#endif - } - resp->respcode = 200; + debug(2, "Connection %d: ANNOUNCE has completed.", conn->connection_number); } else { + // can't get the principal_conn resp->respcode = 453; - debug(1, "Connection %d: ANNOUNCE failed because another connection is already playing.", - conn->connection_number); - } - -out: - if (resp->respcode != 200 && resp->respcode != 453) { - debug(1, "Connection %d: Error in handling ANNOUNCE. Unlocking the play lock.", - conn->connection_number); - release_play_lock(conn); } } @@ -5042,110 +5009,113 @@ authenticate: void rtsp_conversation_thread_cleanup_function(void *arg) { rtsp_conn_info *conn = (rtsp_conn_info *)arg; - int oldState; - pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &oldState); + if (conn != NULL) { + int oldState; + pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &oldState); - debug(2, "Connection %d: rtsp_conversation_thread_func_cleanup_function called.", - conn->connection_number); + 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 - // AP2 - teardown_phase_one(conn); - teardown_phase_two(conn); + // AP2 + teardown_phase_one(conn); + teardown_phase_two(conn); #else - // AP1 - if (have_play_lock(conn)) { + // AP1 teardown(conn); - release_play_lock(conn); - } #endif - debug(3, "Connection %d: terminating -- closing timing, control and audio sockets...", - conn->connection_number); - if (conn->control_socket) { - debug(3, "Connection %d: terminating -- closing control_socket %d.", conn->connection_number, - conn->control_socket); - close(conn->control_socket); - conn->control_socket = 0; - } - if (conn->timing_socket) { - debug(3, "Connection %d: terminating -- closing timing_socket %d.", conn->connection_number, - conn->timing_socket); - close(conn->timing_socket); - conn->timing_socket = 0; - } - if (conn->audio_socket) { - debug(3, "Connection %d: terminating -- closing audio_socket %d.", conn->connection_number, - conn->audio_socket); - close(conn->audio_socket); - conn->audio_socket = 0; - } - if (conn->fd > 0) { - debug(2, + debug(3, "Connection %d: terminating -- closing timing, control and audio sockets...", + conn->connection_number); + if (conn->control_socket) { + debug(3, "Connection %d: terminating -- closing control_socket %d.", conn->connection_number, + conn->control_socket); + close(conn->control_socket); + conn->control_socket = 0; + } + if (conn->timing_socket) { + debug(3, "Connection %d: terminating -- closing timing_socket %d.", conn->connection_number, + conn->timing_socket); + close(conn->timing_socket); + conn->timing_socket = 0; + } + if (conn->audio_socket) { + debug(3, "Connection %d: terminating -- closing audio_socket %d.", conn->connection_number, + conn->audio_socket); + close(conn->audio_socket); + conn->audio_socket = 0; + } + if (conn->fd > 0) { + debug( + 2, "Connection %d: terminating -- closing RTSP connection socket %d: from %s:%u to self at " "%s:%u.", conn->connection_number, conn->fd, conn->client_ip_string, conn->client_rtsp_port, conn->self_ip_string, conn->self_rtsp_port); - close(conn->fd); - conn->fd = 0; - } - if (conn->auth_nonce) { - free(conn->auth_nonce); - conn->auth_nonce = NULL; - } + close(conn->fd); + conn->fd = 0; + } + if (conn->auth_nonce) { + free(conn->auth_nonce); + conn->auth_nonce = NULL; + } #ifdef CONFIG_AIRPLAY_2 - buf_drain(&conn->ap2_pairing_context.control_cipher_bundle.plaintext_read_buffer, -1); - buf_drain(&conn->ap2_pairing_context.control_cipher_bundle.encrypted_read_buffer, -1); - pair_cipher_free(conn->ap2_pairing_context.control_cipher_bundle.cipher_ctx); - pair_setup_free(conn->ap2_pairing_context.setup_ctx); - pair_verify_free(conn->ap2_pairing_context.verify_ctx); - if (conn->airplay_gid) { - free(conn->airplay_gid); - conn->airplay_gid = NULL; - } + buf_drain(&conn->ap2_pairing_context.control_cipher_bundle.plaintext_read_buffer, -1); + buf_drain(&conn->ap2_pairing_context.control_cipher_bundle.encrypted_read_buffer, -1); + pair_cipher_free(conn->ap2_pairing_context.control_cipher_bundle.cipher_ctx); + pair_setup_free(conn->ap2_pairing_context.setup_ctx); + pair_verify_free(conn->ap2_pairing_context.verify_ctx); + if (conn->airplay_gid) { + free(conn->airplay_gid); + conn->airplay_gid = NULL; + } + #endif - rtp_terminate(conn); + rtp_terminate(conn); - if (conn->dacp_id) { - free(conn->dacp_id); - conn->dacp_id = NULL; + if (conn->dacp_id) { + free(conn->dacp_id); + conn->dacp_id = NULL; + } + + if (conn->UserAgent) { + free(conn->UserAgent); + conn->UserAgent = NULL; + } + + // remove flow control and mutexes + + int rc = pthread_mutex_destroy(&conn->player_create_delete_mutex); + if (rc) + debug(1, "Connection %d: error %d destroying player_create_delete_mutex.", + conn->connection_number, rc); + rc = pthread_mutex_destroy(&conn->volume_control_mutex); + if (rc) + debug(1, "Connection %d: error %d destroying volume_control_mutex.", conn->connection_number, + rc); + rc = pthread_cond_destroy(&conn->flowcontrol); + if (rc) + debug(1, "Connection %d: error %d destroying flow control condition variable.", + conn->connection_number, rc); + rc = pthread_mutex_destroy(&conn->ab_mutex); + if (rc) + debug(1, "Connection %d: error %d destroying ab_mutex.", conn->connection_number, rc); + rc = pthread_mutex_destroy(&conn->flush_mutex); + if (rc) + debug(1, "Connection %d: error %d destroying flush_mutex.", conn->connection_number, rc); + + debug(3, "Cancel watchdog thread."); + pthread_cancel(conn->player_watchdog_thread); + debug(3, "Join watchdog thread."); + pthread_join(conn->player_watchdog_thread, NULL); + debug(3, "Delete watchdog mutex."); + pthread_mutex_destroy(&conn->watchdog_mutex); + + debug(2, "Connection %d: Closed.", conn->connection_number); + conn->running = 0; + pthread_setcancelstate(oldState, NULL); } - - if (conn->UserAgent) { - free(conn->UserAgent); - conn->UserAgent = NULL; - } - - // remove flow control and mutexes - int rc = pthread_mutex_destroy(&conn->volume_control_mutex); - if (rc) - debug(1, "Connection %d: error %d destroying volume_control_mutex.", conn->connection_number, - rc); - rc = pthread_cond_destroy(&conn->flowcontrol); - if (rc) - debug(1, "Connection %d: error %d destroying flow control condition variable.", - conn->connection_number, rc); - rc = pthread_mutex_destroy(&conn->ab_mutex); - if (rc) - debug(1, "Connection %d: error %d destroying ab_mutex.", conn->connection_number, rc); - rc = pthread_mutex_destroy(&conn->flush_mutex); - if (rc) - debug(1, "Connection %d: error %d destroying flush_mutex.", conn->connection_number, rc); - - debug(3, "Cancel watchdog thread."); - pthread_cancel(conn->player_watchdog_thread); - debug(3, "Join watchdog thread."); - pthread_join(conn->player_watchdog_thread, NULL); - debug(3, "Delete watchdog mutex."); - pthread_mutex_destroy(&conn->watchdog_mutex); - - // debug(3, "Connection %d: Checking play lock.", conn->connection_number); - // release_play_lock(conn); - - debug(2, "Connection %d: Closed.", conn->connection_number); - conn->running = 0; - pthread_setcancelstate(oldState, NULL); } void msg_cleanup_function(void *arg) { @@ -5175,6 +5145,11 @@ static void *rtsp_conversation_thread_func(void *pconn) { if (rc) die("Connection %d: error %d initialising volume_control_mutex.", conn->connection_number, rc); + rc = pthread_mutex_init(&conn->player_create_delete_mutex, NULL); + if (rc) + die("Connection %d: error %d initialising player_create_delete_mutex.", conn->connection_number, + rc); + // nothing before this is cancellable pthread_cleanup_push(rtsp_conversation_thread_cleanup_function, (void *)conn); @@ -5199,7 +5174,7 @@ static void *rtsp_conversation_thread_func(void *pconn) { pthread_cleanup_push(msg_cleanup_function, (void *)&resp); resp->respcode = 501; // Not Implemented int dl = debug_level; - //if ((strcmp(req->method, "OPTIONS") == 0) || + // if ((strcmp(req->method, "OPTIONS") == 0) || // (strcmp(req->method, "POST") == // 0)) // the options message is very common, so don't log it until level 3 // dl = 3; @@ -5273,24 +5248,23 @@ static void *rtsp_conversation_thread_func(void *pconn) { debug(dl, "Connection %d: RTSP Response:", conn->connection_number); debug_log_rtsp_message(dl, NULL, resp); } - if (conn->stop == 0) { - int err = msg_write_response(conn, resp); - if (err) { - debug(1, - "Connection %d: Unable to write an RTSP message response. Terminating the " - "connection.", - conn->connection_number); - struct linger so_linger; - so_linger.l_onoff = 1; // "true" - so_linger.l_linger = 0; - err = setsockopt(conn->fd, SOL_SOCKET, SO_LINGER, &so_linger, sizeof so_linger); - if (err) - debug(1, "Could not set the RTSP socket to abort due to a write error on closing."); - conn->stop = 1; - // if (debuglev >= 1) - // debuglev = 3; // see what happens next - } + // if (conn->stop == 0) { + int err = msg_write_response(conn, resp); + if (err) { + debug(1, + "Connection %d: Unable to write an RTSP message response. Terminating the " + "connection.", + conn->connection_number); + struct linger so_linger; + so_linger.l_onoff = 1; // "true" + so_linger.l_linger = 0; + err = setsockopt(conn->fd, SOL_SOCKET, SO_LINGER, &so_linger, sizeof so_linger); + if (err) + debug(1, "Could not set the RTSP socket to abort due to a write error on closing."); + conn->stop = 1; + pthread_cancel(conn->thread); } + // } pthread_cleanup_pop(1); pthread_cleanup_pop(1); } else { @@ -5348,8 +5322,8 @@ static void *rtsp_conversation_thread_func(void *pconn) { } } pthread_cleanup_pop(1); + debug(2, "Connection %d: RTSP thread exit.", conn->connection_number); pthread_exit(NULL); - debug(1, "Connection %d: RTSP thread exit.", conn->connection_number); } /* @@ -5397,7 +5371,7 @@ void *rtsp_listen_loop(__attribute((unused)) void *arg) { int nsock = 0; int i, ret; - playing_conn = NULL; // the data structure representing the connection that has the player. + principal_conn = NULL; // the data structure representing the connection that has the player. memset(&hints, 0, sizeof(hints)); hints.ai_family = AF_UNSPEC; diff --git a/rtsp.h b/rtsp.h index 0725f664..59b3ae3d 100644 --- a/rtsp.h +++ b/rtsp.h @@ -3,7 +3,7 @@ #include "player.h" -extern rtsp_conn_info *playing_conn; +extern rtsp_conn_info *principal_conn; extern rtsp_conn_info **conns; void *rtsp_listen_loop(__attribute((unused)) void *arg); From 98e48b3fae7c18175721c58b1c34494c2c32df3b Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Sat, 9 Sep 2023 11:13:12 +0100 Subject: [PATCH 53/63] Update RELEASENOTES-DEVELOPMENT.md --- RELEASENOTES-DEVELOPMENT.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/RELEASENOTES-DEVELOPMENT.md b/RELEASENOTES-DEVELOPMENT.md index fab87f9e..214eb5f5 100644 --- a/RELEASENOTES-DEVELOPMENT.md +++ b/RELEASENOTES-DEVELOPMENT.md @@ -1,3 +1,8 @@ +Version 4.2.1d0-60-g702e8285 +==== +**Bug Fix** +* Fixes a bug that causes the Docker image to crash occasionally when OwnTone attempts to interrupt an existing iOS session. Thanks to [aaronk6](https://github.com/aaronk6) for the report. + Version 4.2.1d0-56-ga9000de6 ==== **Bug Fixes** From 556419e84572e3bf77fb3e83e7bd88ac125ef847 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Sun, 10 Sep 2023 11:02:59 +0100 Subject: [PATCH 54/63] Fix a bug whereby volume control changes would be ignored after a pause in playing of longer than about 8 minutes. --- rtsp.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/rtsp.c b/rtsp.c index dfa1c668..57781ec8 100644 --- a/rtsp.c +++ b/rtsp.c @@ -2727,13 +2727,8 @@ void handle_teardown_2(rtsp_conn_info *conn, __attribute__((unused)) rtsp_messag teardown_phase_two(conn); debug(2, "Connection %d: TEARDOWN %s -- close the connection complete", conn->connection_number, get_category_string(conn->airplay_stream_category)); + release_play_lock(conn); } - //} else { - // warn("Connection %d TEARDOWN received without having the player (no ANNOUNCE?)", - // conn->connection_number); - // resp->respcode = 451; - - release_play_lock(conn); plist_free(messagePlist); resp->respcode = 200; From d1524f2c85fb9908c22bf9b2a149309fc9e09544 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Sun, 10 Sep 2023 11:05:59 +0100 Subject: [PATCH 55/63] Update RELEASENOTES-DEVELOPMENT.md --- RELEASENOTES-DEVELOPMENT.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/RELEASENOTES-DEVELOPMENT.md b/RELEASENOTES-DEVELOPMENT.md index 214eb5f5..b5690573 100644 --- a/RELEASENOTES-DEVELOPMENT.md +++ b/RELEASENOTES-DEVELOPMENT.md @@ -1,3 +1,8 @@ +Version 4.2.1d0-62-g556419e8 +==== +**Bug Fix** +* Fix a bug introducted in the last `development` push, 4.2.1d0-60-g702e8285, whereby volume control changes would be ignored after a pause in playing of longer than about 8 minutes. + Version 4.2.1d0-60-g702e8285 ==== **Bug Fix** From b3db7e88cfbde6fc3d5f6eab970c62b619d5c291 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Mon, 11 Sep 2023 15:58:49 +0100 Subject: [PATCH 56/63] Small bug fixes --- Makefile.am | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Makefile.am b/Makefile.am index 6cf318d3..d9cfbf6e 100644 --- a/Makefile.am +++ b/Makefile.am @@ -262,12 +262,12 @@ endif # INSTALL_CONFIG_FILES INSTALL_GROUP_TARGET = install-group-local $(INSTALL_GROUP_TARGET): - getent group shairport-sync &>/dev/null || groupadd -r shairport-sync >/dev/null + getent group shairport-sync &>/dev/null || groupadd -r shairport-sync &>/dev/null INSTALL_USER_TARGET = install-user-local $(INSTALL_USER_TARGET): $(INSTALL_GROUP_TARGET) - getent passwd shairport-sync &> /dev/null || useradd -r -M -g shairport-sync -s /usr/sbin/nologin -G audio shairport-sync >/dev/null + getent passwd shairport-sync &>/dev/null || useradd -r -M -g shairport-sync -s /usr/sbin/nologin -G audio shairport-sync &>/dev/null if INSTALL_SYSTEMV From b7d9c22de9effda99d2a77849de9b25739e5e1d4 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Mon, 11 Sep 2023 16:01:38 +0100 Subject: [PATCH 57/63] format adjustments --- audio_alsa.c | 3 ++- audio_jack.c | 2 +- common.c | 27 ++++++++++++++++----------- common.h | 12 +++++++----- dacp.c | 10 +++++----- metadata_hub.c | 2 +- rtp.c | 6 ++++-- 7 files changed, 36 insertions(+), 26 deletions(-) diff --git a/audio_alsa.c b/audio_alsa.c index bf8d6ada..78780804 100644 --- a/audio_alsa.c +++ b/audio_alsa.c @@ -176,7 +176,8 @@ void handle_unfixable_error(int errorCode) { command_execute(config.cmd_unfixable, messageString, 1); } else { die("An unrecoverable error, \"output_device_error_%d\", has been " - "detected. Doing an emergency exit, as no run_this_if_an_unfixable_error_is_detected program.", + "detected. Doing an emergency exit, as no run_this_if_an_unfixable_error_is_detected " + "program.", errorCode); } } diff --git a/audio_jack.c b/audio_jack.c index abdf09d9..911fada7 100644 --- a/audio_jack.c +++ b/audio_jack.c @@ -299,7 +299,7 @@ static int jack_init(__attribute__((unused)) int argc, __attribute__((unused)) c while (port_list[i++] != NULL) { inform( "Additional matching port %s found. Check that the connections are what you intended.", - port_list[i-1]); + port_list[i - 1]); } jack_free(port_list); } diff --git a/common.c b/common.c index bed243ba..618293ae 100644 --- a/common.c +++ b/common.c @@ -72,8 +72,8 @@ #endif #ifdef CONFIG_OPENSSL -#include // needed for BIO_new_mem_buf #include // needed for older AES stuff +#include // needed for BIO_new_mem_buf #include // needed for ERR_error_string, ERR_get_error #include // needed for EVP_PKEY_CTX_new, EVP_PKEY_sign_init, EVP_PKEY_sign #include // needed for PEM_read_bio_RSAPrivateKey, EVP_PKEY_CTX_set_rsa_padding @@ -1203,14 +1203,14 @@ uint32_t uatoi(const char *nptr) { // clang-format on double flat_vol2attn(double vol, long max_db, long min_db) { -// clang-format off + // clang-format off // This "flat" volume control profile has the property that a given change in the AirPlay volume // always results in the same change in output dB. For example, if a change of AirPlay volume // from 0 to -4 resulted in a 7 dB change, then a change in AirPlay volume from -20 to -24 // would also result in a 7 dB change. -// clang-format on + // clang-format on double vol_setting = min_db; // if all else fails, set this, for safety if ((vol <= 0.0) && (vol >= -30.0)) { @@ -1219,15 +1219,15 @@ double flat_vol2attn(double vol, long max_db, long min_db) { // max_db); } else if (vol != -144.0) { debug(1, - "flat_vol2attn volume request value %f is out of range: should be from 0.0 to -30.0 or -144.0.", + "flat_vol2attn volume request value %f is out of range: should be from 0.0 to -30.0 or " + "-144.0.", vol); } return vol_setting; } - double dasl_tapered_vol2attn(double vol, long max_db, long min_db) { -// clang-format off + // clang-format off // The "dasl_tapered" volume control profile has the property that halving the AirPlay volume (the "vol" parameter) // reduces the output level by 10 dB, which corresponds to roughly halving the perceived volume. @@ -1251,7 +1251,7 @@ double dasl_tapered_vol2attn(double vol, long max_db, long min_db) { // If the device's attenuation range is over about 50 dB, // the flat output level will hardly be needed at all. -// clang-format on + // clang-format on double vol_setting = min_db; // if all else fails, set this, for safety if ((vol <= 0.0) && (vol >= -30.0)) { @@ -1259,11 +1259,15 @@ double dasl_tapered_vol2attn(double vol, long max_db, long min_db) { if (vol_pct <= 0) { return min_db; } - + double flat_setting = min_db + (max_db - min_db) * vol_pct; - vol_setting = max_db + 1000 * log10(vol_pct) / log10(2); // This will be in the range [-inf, max_db] + vol_setting = + max_db + 1000 * log10(vol_pct) / log10(2); // This will be in the range [-inf, max_db] if (vol_setting < flat_setting) { - debug(3, "dasl_tapered_vol2attn returning a flat setting of %f for AirPlay volume %f instead of a tapered setting of %f in a range from %f to %f.", flat_setting, vol, vol_setting, 1.0 * min_db, 1.0 * max_db); + debug(3, + "dasl_tapered_vol2attn returning a flat setting of %f for AirPlay volume %f instead of " + "a tapered setting of %f in a range from %f to %f.", + flat_setting, vol, vol_setting, 1.0 * min_db, 1.0 * max_db); return flat_setting; } if (vol_setting > max_db) { @@ -1272,7 +1276,8 @@ double dasl_tapered_vol2attn(double vol, long max_db, long min_db) { return vol_setting; } else if (vol != -144.0) { debug(1, - "dasl_tapered volume request value %f is out of range: should be from 0.0 to -30.0 or -144.0.", + "dasl_tapered volume request value %f is out of range: should be from 0.0 to -30.0 or " + "-144.0.", vol); } return vol_setting; diff --git a/common.h b/common.h index c8e8915b..edd48fa8 100644 --- a/common.h +++ b/common.h @@ -391,11 +391,13 @@ uint8_t *rsa_apply(uint8_t *input, int inlen, int *outlen, int mode); // dB), return an attenuation depending on a linear interpolation along the range double flat_vol2attn(double vol, long max_db, long min_db); -// The intention behind dasl_tapered is that a given percentage change in volume should result in the same percentage change in -// perceived loudness. For instance, doubling the volume level should result in doubling the perceived loudness. -// With the range of AirPlay volume being from -30 to 0, doubling the volume from -22.5 to -15 results in an increase of 10 dB. -// Similarly, doubling the volume from -15 to 0 results in an increase of 10 dB. -// For compatibility with mixers having a restricted attenuation range (e.g. 30 dB), "dasl_tapered" will switch to a flat profile at low AirPlay volumes. +// The intention behind dasl_tapered is that a given percentage change in volume should result in +// the same percentage change in perceived loudness. For instance, doubling the volume level should +// result in doubling the perceived loudness. With the range of AirPlay volume being from -30 to 0, +// doubling the volume from -22.5 to -15 results in an increase of 10 dB. Similarly, doubling the +// volume from -15 to 0 results in an increase of 10 dB. For compatibility with mixers having a +// restricted attenuation range (e.g. 30 dB), "dasl_tapered" will switch to a flat profile at low +// AirPlay volumes. double dasl_tapered_vol2attn(double vol, long max_db, long min_db); // given a volume (0 to -30) and high and low attenuations in dB*100 (e.g. 0 to -6000 for 0 to -60 // dB), return an attenuation depending on the transfer function diff --git a/dacp.c b/dacp.c index 14154263..52535cc5 100644 --- a/dacp.c +++ b/dacp.c @@ -411,14 +411,14 @@ void set_dacp_server_information(rtsp_conn_info *conn) { debug(2, "set_dacp_server_information set IP to \"%s\" and DACP id to \"%s\".", dacp_server.ip_string, dacp_server.dacp_id); -/* + /* -"long polling" is not implemented by Shairport Sync, whereby by sending the client the -last-received revision number, the link will hang until a change occurs. + "long polling" is not implemented by Shairport Sync, whereby by sending the client the + last-received revision number, the link will hang until a change occurs. -Instead, at present, Shairport Sync always uses a revision number of 1. + Instead, at present, Shairport Sync always uses a revision number of 1. -*/ + */ // always use revision number 1 dacp_server.always_use_revision_number_1 = 1; diff --git a/metadata_hub.c b/metadata_hub.c index 27176058..235b6fe5 100644 --- a/metadata_hub.c +++ b/metadata_hub.c @@ -211,7 +211,7 @@ char *metadata_write_image_file(const char *buf, int len) { #ifdef CONFIG_OPENSSL EVP_MD_CTX *ctx; unsigned int img_md5_len = EVP_MD_size(EVP_md5()); - + ctx = EVP_MD_CTX_new(); EVP_DigestInit_ex(ctx, EVP_md5(), NULL); EVP_DigestUpdate(ctx, buf, len); diff --git a/rtp.c b/rtp.c index ed64ea74..c7c8efa3 100644 --- a/rtp.c +++ b/rtp.c @@ -2917,7 +2917,8 @@ void *rtp_buffered_audio_processor(void *arg) { debug(1, "error %d during decoding", ret); } else { #if LIBAVUTIL_VERSION_MAJOR >= 57 - av_samples_alloc(&pcm_audio, &dst_linesize, codec_context->ch_layout.nb_channels, + av_samples_alloc(&pcm_audio, &dst_linesize, + codec_context->ch_layout.nb_channels, decoded_frame->nb_samples, av_format, 1); #else av_samples_alloc(&pcm_audio, &dst_linesize, codec_context->channels, @@ -2929,7 +2930,8 @@ void *rtp_buffered_audio_processor(void *arg) { decoded_frame->nb_samples); #if LIBAVUTIL_VERSION_MAJOR >= 57 dst_bufsize = av_samples_get_buffer_size( - &dst_linesize, codec_context->ch_layout.nb_channels, ret, av_format, 1); + &dst_linesize, codec_context->ch_layout.nb_channels, ret, av_format, + 1); #else dst_bufsize = av_samples_get_buffer_size( &dst_linesize, codec_context->channels, ret, av_format, 1); From dabfee5bac93e599cb2e66268bb6c9a91c048eec Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Mon, 11 Sep 2023 16:02:49 +0100 Subject: [PATCH 58/63] Move to smi version 10, read-only interface, no mutex, write and read each record twice to ensure it is not inconsistent when read. --- nqptp-shm-structures.h | 20 +++++++---- ptp-utilities.c | 79 +++++++++++++++++++++++++++++++++--------- ptp-utilities.h | 3 +- shairport.c | 64 +++++++++++++++++++++++----------- 4 files changed, 122 insertions(+), 44 deletions(-) diff --git a/nqptp-shm-structures.h b/nqptp-shm-structures.h index 251d06c1..09f3b600 100644 --- a/nqptp-shm-structures.h +++ b/nqptp-shm-structures.h @@ -22,7 +22,7 @@ #define NQPTP_INTERFACE_NAME "/nqptp" -#define NQPTP_SHM_STRUCTURES_VERSION 9 +#define NQPTP_SHM_STRUCTURES_VERSION 10 #define NQPTP_CONTROL_PORT 9000 // The control port expects a UDP packet with the first character being a command letter @@ -50,20 +50,26 @@ // When the clock is inactive, it can stop running. This causes the offset to decrease. // NQPTP clock smoothing would treat this as a network delay, causing true sync to be lost. // To avoid this, when the clock goes from inactive to active, -// NQPTP resets clock smoothing to the new offset. - +// NQPTP resets clock smoothing to the new offset. #include #include -struct shm_structure { - pthread_mutex_t shm_mutex; // for safely accessing the structure - uint16_t version; // check this is equal to NQPTP_SHM_STRUCTURES_VERSION +typedef struct { uint64_t master_clock_id; // the current master clock - char master_clock_ip[64]; // where it's coming from uint64_t local_time; // the time when the offset was calculated uint64_t local_to_master_time_offset; // add this to the local time to get master clock time uint64_t master_clock_start_time; // this is when the master clock became master +} shm_structure_set; + +// The secondary record must be 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. +struct shm_structure { + uint16_t version; // check this is equal to NQPTP_SHM_STRUCTURES_VERSION + shm_structure_set main; + shm_structure_set secondary; }; #endif diff --git a/ptp-utilities.c b/ptp-utilities.c index 559b62ea..34184c14 100644 --- a/ptp-utilities.c +++ b/ptp-utilities.c @@ -1,6 +1,6 @@ /* * This file is part of Shairport Sync. - * Copyright (c) Mike Brady 2020 -- 2021 + * Copyright (c) Mike Brady 2020 -- 2023 * All rights reserved. * * Permission is hereby granted, free of charge, to any person @@ -49,23 +49,70 @@ void *mapped_addr = NULL; // returns a copy of the shared memory data from the nqptp // shared memory interface, so long as it's open. int get_nqptp_data(struct shm_structure *nqptp_data) { + // uint64_t tn = get_absolute_time_in_ns(); // if interested in timing the function... + struct shm_structure local_nqptp_data; int response = -1; // presume the worst. Fix it on success - // the first part of the shared memory is a mutex lock, so use it to get - // exclusive access while copying + + // We need to ensure that when we read the record, we are not reading it while it is partly + // updated and therefore inconsistent. To achieve this, we do the following: + + // We ensure that the secondary record is written by NQPTP _strictly after_ + // all writes to the main record are complete. + + // Here we read two copies of the entire record, the second + // _strictly after_ all reads from the first are complete. + + // (Strict write and read ordering is ensured using the __sync_synchronize() construct.) + + // We then compare the main record in the first read to the + // secondary record in the second read. + + // If they are equal, we can be sure we have not read a record that has been + // made inconsistent by an interrupted update. if ((mapped_addr != MAP_FAILED) && (mapped_addr != NULL)) { - pthread_cleanup_debug_mutex_lock((pthread_mutex_t *)mapped_addr, 10000, 1); - memcpy(nqptp_data, (char *)mapped_addr, sizeof(struct shm_structure)); - pthread_cleanup_pop(1); // release the mutex - response = 0; + int loop_count = 1; + do { + __sync_synchronize(); + memcpy(nqptp_data, (char *)mapped_addr, sizeof(struct shm_structure)); + __sync_synchronize(); + // read again strictly after a full read -- this is to read the secondary strictly after the + // primary + memcpy(&local_nqptp_data, (char *)mapped_addr, sizeof(struct shm_structure)); + // check that the main and secondary data sets match + if (memcmp(&nqptp_data->main, &local_nqptp_data.secondary, sizeof(shm_structure_set)) != 0) { + usleep(2); // microseconds + loop_count++; + } + } while ( + (memcmp(&nqptp_data->main, &local_nqptp_data.secondary, sizeof(shm_structure_set)) != 0) && + (loop_count < 100)); + if (loop_count == 10) { + debug(1, "get_nqptp_data -- main and secondary records don't match after %d attempts!", + loop_count); + response = -1; + } else { + response = 0; + } } else { if (mapped_addr == NULL) - debug(1, "get_nqptp_data because the mapped_addr is NULL"); + debug(1, "get_nqptp_data failed because the mapped_addr is NULL"); else if (mapped_addr == MAP_FAILED) - debug(1, "get_nqptp_data because the mapped_addr is MAP_FAILED"); + debug(1, "get_nqptp_data failed because the mapped_addr is MAP_FAILED"); else debug(1, "get_nqptp_data failed"); } + // int64_t et = get_absolute_time_in_ns() - tn; + // debug(1, "get_nqptp_data time: %.3f microseconds.", 0.001 * et); + return response; +} + +int ptp_get_clock_version() { + int response = 0; // no version number information available + struct shm_structure nqptp_data; + if (get_nqptp_data(&nqptp_data) == 0) { + response = nqptp_data.version; + } return response; } @@ -85,15 +132,15 @@ int ptp_get_clock_info(uint64_t *actual_clock_id, uint64_t *time_of_sample, uint if (get_nqptp_data(&nqptp_data) == 0) { if (nqptp_data.version == NQPTP_SHM_STRUCTURES_VERSION) { // assuming a clock id can not be zero - if (nqptp_data.master_clock_id != 0) { + if (nqptp_data.main.master_clock_id != 0) { if (actual_clock_id != NULL) - *actual_clock_id = nqptp_data.master_clock_id; + *actual_clock_id = nqptp_data.main.master_clock_id; if (time_of_sample != NULL) - *time_of_sample = nqptp_data.local_time; + *time_of_sample = nqptp_data.main.local_time; if (raw_offset != NULL) - *raw_offset = nqptp_data.local_to_master_time_offset; + *raw_offset = nqptp_data.main.local_to_master_time_offset; if (mastership_start_time != NULL) - *mastership_start_time = nqptp_data.master_clock_start_time; + *mastership_start_time = nqptp_data.main.master_clock_start_time; } else { response = clock_no_master; } @@ -124,12 +171,12 @@ int ptp_shm_interface_open() { if (strcmp(config.nqptp_shared_memory_interface_name, "") != 0) { response = 0; int shared_memory_file_descriptor = - shm_open(config.nqptp_shared_memory_interface_name, O_RDWR, 0); + shm_open(config.nqptp_shared_memory_interface_name, O_RDONLY, 0); if (shared_memory_file_descriptor >= 0) { mapped_addr = // needs to be PROT_READ | PROT_WRITE to allow the mapped memory to be writable for the // mutex to lock and unlock - mmap(NULL, sizeof(struct shm_structure), PROT_READ | PROT_WRITE, MAP_SHARED, + mmap(NULL, sizeof(struct shm_structure), PROT_READ, MAP_SHARED, shared_memory_file_descriptor, 0); if (mapped_addr == MAP_FAILED) { response = -1; diff --git a/ptp-utilities.h b/ptp-utilities.h index 761a5cae..738ed21f 100644 --- a/ptp-utilities.h +++ b/ptp-utilities.h @@ -1,6 +1,6 @@ /* * This file is part of Shairport Sync. - * Copyright (c) Mike Brady 2020 -- 2021 + * Copyright (c) Mike Brady 2020 -- 2023 * All rights reserved. * * Permission is hereby granted, free of charge, to any person @@ -37,6 +37,7 @@ void ptp_send_control_message_string(const char *msg); void ptp_shm_interface_init(); int ptp_shm_interface_open(); +int ptp_get_clock_version(); int ptp_shm_interface_close(); #endif /* __PTP_UTILITIES_H */ diff --git a/shairport.c b/shairport.c index 6d9fb959..66d9028a 100644 --- a/shairport.c +++ b/shairport.c @@ -61,8 +61,8 @@ #endif #ifdef CONFIG_OPENSSL -#include #include +#include #endif #if defined(CONFIG_DBUS_INTERFACE) @@ -1416,7 +1416,7 @@ int parse_options(int argc, char **argv) { for (i = 5; i >= 0; i--) { // In AirPlay 2 mode, the AP1 name prefix must be // the same as the AirPlay 2 device id less the colons. - config.ap1_prefix[i] = temporary_airplay_id & 0xFF; + config.ap1_prefix[i] = temporary_airplay_id & 0xFF; apids[i * 3 + 1] = hexchar[temporary_airplay_id & 0xF]; temporary_airplay_id = temporary_airplay_id >> 4; apids[i * 3] = hexchar[temporary_airplay_id & 0xF]; @@ -1630,7 +1630,6 @@ void exit_function() { } #endif - #ifdef CONFIG_METADATA_HUB debug(2, "Stopping metadata hub"); metadata_hub_stop(); @@ -2521,7 +2520,6 @@ int main(int argc, char **argv) { soxr_time_check_thread_started = 1; #endif - // In AirPlay 2 mode, the AP1 prefix is the same as the device ID less the colons // In AirPlay 1 mode, the AP1 prefix is calculated by hashing the service name. #ifndef CONFIG_AIRPLAY_2 @@ -2600,25 +2598,51 @@ int main(int argc, char **argv) { #endif #ifdef CONFIG_AIRPLAY_2 - ptp_send_control_message_string("T"); // get nqptp to create the named shm interface - int ptp_check_times = 0; - const int ptp_wait_interval_us = 5000; - // wait for up to ten seconds for NQPTP to come online + ptp_send_control_message_string( + "T"); // send this message to get nqptp to create the named shm interface + uint64_t nqptp_start_waiting_time = get_absolute_time_in_ns(); + int continue_waiting = 0; + int response = 0; + int64_t time_spent_waiting = 0; do { - ptp_send_control_message_string("T"); // get nqptp to create the named shm interface - usleep(ptp_wait_interval_us); - ptp_check_times++; - } while ((ptp_shm_interface_open() != 0) && - (ptp_check_times < (10000000 / ptp_wait_interval_us))); + continue_waiting = 0; + response = ptp_shm_interface_open(); + if ((response == -1) && (errno == ENOENT)) { + time_spent_waiting = get_absolute_time_in_ns() - nqptp_start_waiting_time; + if (time_spent_waiting < 10000000000L) { + continue_waiting = 1; + usleep(50000); + } + } + } while (continue_waiting != 0); - if (ptp_shm_interface_open() != 0) { - die("Can't access NQPTP! Is it installed and running?"); - } else { - if (ptp_check_times == 1) - debug(1, "NQPTP is online."); - else - debug(1, "NQPTP is online after %u microseconds.", ptp_check_times * ptp_wait_interval_us); + if ((response == -1) && (errno == ENOENT)) { + die("Shairport Sync can not find the nqptp service on this system. Is nqptp installed and " + "running?"); + } else if ((response == -1) && (errno == EACCES)) { + die("Shairport Sync must have read access to the nqptp shared memory file in /dev/shm/."); + } else if (response != 0) { + die("an error occurred accessing the nqptp service."); } + + int ptp_clock_version = ptp_get_clock_version(); + if (ptp_clock_version == 0) { + die("The nqptp service on this system, which is required for Shairport Sync to operate, does " + "not seem to be initialised."); + } else if (ptp_clock_version < NQPTP_SHM_STRUCTURES_VERSION) { + die("The nqptp service (SMI Version %d) on this system is too old for this version of " + "Shairport Sync, which requires SMI Version %d. Please update.", + ptp_clock_version, NQPTP_SHM_STRUCTURES_VERSION); + } else if (ptp_clock_version > NQPTP_SHM_STRUCTURES_VERSION) { + die("This version of Shairport Sync (SMI Version %d) is too old for the version of nqptp (SMI " + "Version %d) on this system. Please update.", + NQPTP_SHM_STRUCTURES_VERSION, ptp_clock_version); + } + + if (time_spent_waiting == 0) + debug(1, "NQPTP is online."); + else + debug(1, "NQPTP came online after %.3f milliseconds.", 0.000001 * time_spent_waiting); #endif #ifdef CONFIG_METADATA From ef14cacebf4fffe0835f2770ad7526b7073c7380 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Mon, 11 Sep 2023 16:47:14 +0100 Subject: [PATCH 59/63] Quieten a debug message --- rtsp.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rtsp.c b/rtsp.c index 57781ec8..64c3c3bf 100644 --- a/rtsp.c +++ b/rtsp.c @@ -5213,7 +5213,7 @@ static void *rtsp_conversation_thread_func(void *pconn) { } } if (method_selected == 0) { - debug(1, + debug(2, "Connection %d: Unrecognised and unhandled rtsp request \"%s\". HTTP Response Code " "501 (\"Not Implemented\") returned.", conn->connection_number, req->method); @@ -5232,7 +5232,7 @@ static void *rtsp_conversation_thread_func(void *pconn) { obfp += 2; }; *obfp = 0; - debug(1, "Content: \"%s\".", obf); + debug(2, "Content: \"%s\".", obf); } } } From 73cb5b20021811e1218f4305b69d0af94ac13d2c Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Mon, 11 Sep 2023 17:00:20 +0100 Subject: [PATCH 60/63] Update RELEASENOTES-DEVELOPMENT.md --- RELEASENOTES-DEVELOPMENT.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/RELEASENOTES-DEVELOPMENT.md b/RELEASENOTES-DEVELOPMENT.md index b5690573..03a07945 100644 --- a/RELEASENOTES-DEVELOPMENT.md +++ b/RELEASENOTES-DEVELOPMENT.md @@ -1,3 +1,11 @@ +Version 4.2.1d0-67-gef14cace +=== +**IMPORTANT Bug Fixes** +* A recent security audit has identified a number of issues with NQPTP that have been fixed in latest update to NQPTP and in this corresponding update to Shairport Sync. +* Please update to the latest `development` branch version of NQPTP before updating Shairport Sync. (Shairport Sync now requires the Shared Memory Interface version 10, i.e. `smi10`.) +* When updating NQPTP on Linux, ensure you remove the old service file as directed in the README. +* Having completed both updates and installations, remember to restart NQPTP first and then restart Shairport Sync. + Version 4.2.1d0-62-g556419e8 ==== **Bug Fix** From 35fcfafd3c32c5224c35182b29914d92250e2603 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Mon, 11 Sep 2023 17:00:58 +0100 Subject: [PATCH 61/63] 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 03a07945..d4a3b205 100644 --- a/RELEASENOTES-DEVELOPMENT.md +++ b/RELEASENOTES-DEVELOPMENT.md @@ -1,7 +1,7 @@ Version 4.2.1d0-67-gef14cace === **IMPORTANT Bug Fixes** -* A recent security audit has identified a number of issues with NQPTP that have been fixed in latest update to NQPTP and in this corresponding update to Shairport Sync. +* A recent security audit has identified a number of issues with NQPTP that have been fixed in the latest update to NQPTP and in this corresponding update to Shairport Sync. * Please update to the latest `development` branch version of NQPTP before updating Shairport Sync. (Shairport Sync now requires the Shared Memory Interface version 10, i.e. `smi10`.) * When updating NQPTP on Linux, ensure you remove the old service file as directed in the README. * Having completed both updates and installations, remember to restart NQPTP first and then restart Shairport Sync. From 73af29c8238a506b2f349f6702c6e33ccaeb0e60 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Mon, 11 Sep 2023 17:46:00 +0100 Subject: [PATCH 62/63] in the docker build process, skip the make install for nqptp and take the binary of nqptp from where it is built --- docker/Dockerfile | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/docker/Dockerfile b/docker/Dockerfile index 1dc34141..da8688ed 100644 --- a/docker/Dockerfile +++ b/docker/Dockerfile @@ -46,7 +46,6 @@ RUN git checkout "$NQPTP_BRANCH" RUN autoreconf -i RUN ./configure RUN make -RUN make install WORKDIR / ##### NQPTP END ##### @@ -97,7 +96,7 @@ RUN apk -U add \ # Copy build files. COPY --from=builder /shairport-sync/build/install/usr/local/bin/shairport-sync /usr/local/bin/shairport-sync COPY --from=builder /shairport-sync/build/install/usr/local/share/man/man7 /usr/share/man/man7 -COPY --from=builder /usr/local/bin/nqptp /usr/local/bin/nqptp +COPY --from=builder /nqptp/nqptp /usr/local/bin/nqptp COPY --from=builder /usr/local/lib/libalac.* /usr/local/lib/ COPY --from=builder /shairport-sync/build/install/etc/shairport-sync.conf /etc/ COPY --from=builder /shairport-sync/build/install/etc/shairport-sync.conf.sample /etc/ From c76ef97bee9a297a5e7fa9ba55a40b34c5ea7c03 Mon Sep 17 00:00:00 2001 From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Mon, 11 Sep 2023 20:03:16 +0100 Subject: [PATCH 63/63] Comment update --- nqptp-shm-structures.h | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/nqptp-shm-structures.h b/nqptp-shm-structures.h index 09f3b600..85b2c9dd 100644 --- a/nqptp-shm-structures.h +++ b/nqptp-shm-structures.h @@ -1,6 +1,6 @@ /* * This file is part of the nqptp distribution (https://github.com/mikebrady/nqptp). - * Copyright (c) 2021--2022 Mike Brady. + * Copyright (c) 2021--2023 Mike Brady. * * This program is free software: you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -62,10 +62,13 @@ typedef struct { uint64_t master_clock_start_time; // this is when the master clock became master } shm_structure_set; -// The secondary record must be written strictly after all writes to the main record are -// complete. -// This is ensured using the __sync_synchronize() construct. +// The actual interface comprises a shared memory region of type struct shm_structure. +// 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. +// For safety, the secondary record should be read strictly after the first. + struct shm_structure { uint16_t version; // check this is equal to NQPTP_SHM_STRUCTURES_VERSION shm_structure_set main;