Merge branch 'development' to 'master' 5.2.2.

Summary of changes:
* If the output rate is different to the input rate (e.g. 48,000 vs 44,100), look for the deepest output bit depth so that the transcoder has the maximum dynamic range available to minimise transcoding inaccuracies.
* Fix a bug whereby the audio backend latency offset was mishandled in AirPlay 2 operation.
* Fix an output format selection bug.
* Update the README reference to the MPRIS specification to the latest version.
* Remove the explicit `-` for `xxd` to read from `STDIN`. It's not necessary, and removing it makes it easier to package Shairport Sync on other build systems, such as OpenWrt.

.github/workflows/docker-on-push-tag-or-pr.yaml

@@ -59,7 +59,7 @@ jobs:
           fi
       
       - name: Login to Docker Registry
-        uses: docker/login-action@v4.5.1
+        uses: docker/login-action@v4.6.0
         with:
           registry: ${{ secrets.DOCKER_REGISTRY }}
           username: ${{ secrets.DOCKER_REGISTRY_USER }}

.github/workflows/stale.yaml

@@ -11,7 +11,7 @@ jobs:
   stale:
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/stale@v10.4.0
+      - uses: actions/stale@v11.0.0
         with:
           stale-issue-message: 'This issue has been inactive for 28 days so will be closed 7 days from now. To prevent this, please remove the "stale" label or post a comment.'
           stale-pr-message: 'This PR has been inactive for 28 days so will be closed 7 days from now. To prevent this, please remove the "stale" label or post a comment.'

README.md

@@ -24,7 +24,7 @@ Shairport Sync does not support AirPlay video or photo streaming.
 * Metadata — Shairport Sync can deliver metadata supplied by the source, such as Album Name, Artist Name, Cover Art, etc. through a pipe or UDP socket to a recipient application program — see https://github.com/mikebrady/shairport-sync-metadata-reader for a sample recipient. Sources that supply metadata include iTunes and the Music app in macOS and iOS.
 * An interface to [MQTT](https://en.wikipedia.org/wiki/MQTT), a popular protocol for Inter Process Communication, Machine-to-Machine, Internet of Things and Home Automation projects. The interface provides access to metadata and artwork, with remote control for Classic AirPlay clients.
 * Digital Signal Processing facilities – please see the [DSP Wiki Page Guide](https://github.com/mikebrady/shairport-sync/wiki/Digital-Signal-Processing-with-Shairport-Sync). (Thanks to [Yann Pomarède](https://github.com/yannpom) for the code and to [Paul Wieland](https://github.com/PaulWieland) for the guide.)
-* An [MPRIS](https://specifications.freedesktop.org/mpris-spec/2.2/)-like interface, partially complete and very functional, including access to metadata and artwork, and partial remote control for Classic AirPlay clients.
+* An [MPRIS](https://specifications.freedesktop.org/mpris/latest/)-like interface, partially complete and very functional, including access to metadata and artwork, and partial remote control for Classic AirPlay clients.
 * A native D-Bus interface, including access to metadata, artwork and system settings. Remote control is also available for Classic AirPlay clients.
 * Better Volume Control — Shairport Sync offers finer control at very top and very bottom of the volume range. See http://tangentsoft.net/audio/atten.html for a good discussion of audio "attenuators", upon which volume control in Shairport Sync is modelled. See also the diagram of the volume transfer function in the documents folder. In addition, Shairport Sync can offer an extended volume control range on devices with a restricted range.
 * Flexible output rates, formats and channels with built-in transcoding.

audio.c

@@ -959,21 +959,30 @@ int32_t search_for_suitable_configuration(unsigned int channels, unsigned int ra
         local_rate = rate; // begin with the requested rate
         debug(3, "check for an exact multiple of %u with %u channels.", rate, local_channels);
         while ((local_rate <= 384000) && (local_format == SPS_FORMAT_UNKNOWN)) {
+          // first, if we in a circumstance where no further audio processing,
+          // e.g. volume adjustment or convolution is required, check_
+          // for an exact rate and bit depth match with the same or more channels
           if (
             // clang-format off
             // check for the exact format only under these conditions, otherwise look for the best
             (config.ignore_volume_control != 0) &&
             (config.volume_max_db_set == 0) &&
+#ifdef CONFIG_CONVOLUTION
+            (config.convolution_enabled == 0) &&
+#endif
+            (config.loudness_enabled == 0) &&
             (local_rate == rate) &&
             (local_channels >= channels) &&
             (config.playback_mode != ST_mono)
             // clang-format on
           ) {
-            // debug(1, "check exact");
+            // debug(1, "first, check exact rate and format");
             local_format = check_configuration_with_formats(
                 local_channels, local_rate, (sps_format_t)format, check_configuration);
-          } else {
+          }
-            // debug(1, "check best");
+          
+          if (local_format == SPS_FORMAT_UNKNOWN) {
+            // debug(1, "check best, with any format");
             local_format = check_configuration_with_formats(local_channels, local_rate,
                                                             SPS_FORMAT_S32, check_configuration);
           }
@@ -985,7 +994,7 @@ int32_t search_for_suitable_configuration(unsigned int channels, unsigned int ra
         }
 #ifdef CONFIG_FFMPEG
         if (local_format == SPS_FORMAT_UNKNOWN) {
-          debug(3, "check for the next highest rate above %u with %u channels.", rate,
+          debug(3, "check for the next highest rate above %u with %u channels and the best format.", rate,
                 local_channels);
           unsigned int rate_pointer = 0;
           while ((rate_pointer < sizeof(rates) / sizeof(unsigned int)) &&
@@ -993,7 +1002,7 @@ int32_t search_for_suitable_configuration(unsigned int channels, unsigned int ra
             local_rate = rates[rate_pointer];
             if (local_rate > rate) {
               local_format = check_configuration_with_formats(
-                  local_channels, local_rate, (sps_format_t)format, check_configuration);
+                  local_channels, local_rate, SPS_FORMAT_S32, check_configuration);
             }
             if (local_format == SPS_FORMAT_UNKNOWN) {
               rate_pointer++;
@@ -1003,13 +1012,13 @@ int32_t search_for_suitable_configuration(unsigned int channels, unsigned int ra
 
         if (local_format == SPS_FORMAT_UNKNOWN) {
           int rate_pointer = (int)(sizeof(rates) / sizeof(unsigned int) - 1);
-          debug(3, "check for the next lowest rate below %u with %u channels.", rate,
+          debug(3, "check for the next lowest rate below %u with %u channels and the best format.", rate,
                 local_channels);
           while ((rate_pointer >= 0) && (local_format == SPS_FORMAT_UNKNOWN)) {
             local_rate = rates[rate_pointer];
             if (local_rate < rate) {
               local_format = check_configuration_with_formats(
-                  local_channels, local_rate, (sps_format_t)format, check_configuration);
+                  local_channels, local_rate, SPS_FORMAT_S32, check_configuration);
             }
             if (local_format == SPS_FORMAT_UNKNOWN) {
               rate_pointer--;

common.h

@@ -213,6 +213,7 @@ typedef struct {
 
   char *pa_sink; // the name (or id) of the sink that Shairport Sync will play on.
 #endif
+
 #ifdef CONFIG_PIPEWIRE
   char *pw_application_name; // the name under which Shairport Sync shows up as an "Application" in
                              // the Sound Preferences in most desktop Linuxes.
@@ -221,6 +222,7 @@ typedef struct {
   char *pw_node_name;   // defaults to the application's name, usually "shairport-sync".
   char *pw_sink_target; // leave this unset if you don't want to change the sink_target.
 #endif
+
 #ifdef CONFIG_METADATA
   int metadata_enabled;
   char *metadata_pipename;
@@ -230,6 +232,7 @@ typedef struct {
   int get_coverart;
   double metadata_progress_interval; // 0 means no progress reports
 #endif
+
 #ifdef CONFIG_MQTT
   int mqtt_enabled;
   char *mqtt_hostname;
@@ -250,6 +253,7 @@ typedef struct {
   char *mqtt_autodiscovery_prefix;
   char *mqtt_empty_payload_substitute;
 #endif
+
   uint8_t ap1_prefix[6];
   uint8_t hw_addr[8]; // only needs 6 but 8 is handy when converting this to a number
   int port;
@@ -302,8 +306,6 @@ typedef struct {
                             // to be enabled under the auto setting
   int decoders_supported;
   int decoder_in_use;
-  // char *logfile;
-  // char *errfile;
   char *configfile;
   char *regtype; // The regtype is the service type followed by the protocol, separated by a dot, by
                  // default “_raop._tcp.” for AirPlay 1.
@@ -348,17 +350,11 @@ typedef struct {
 
 #ifdef CONFIG_CONVOLUTION
   int convolution_enabled;
-  unsigned int convolution_rate; // 0 means the convolver has never been initialised, so ignore
-                                 // convolver_valid.
-  // but if this is the same as the current rate and convolver_valid is false, it means that an
-  // attempt to initialise the convolver has failed.
-  size_t convolution_block_size;
   unsigned int convolution_ir_file_count;
   ir_file_info_t *convolution_ir_files; // NULL or an array of information about all the impulse
                                         // response files loaded
   int convolution_ir_files_updated; // set to true if the convolution_ir_files are changed. Cleared
                                     // when the convolver has been initialised
-  int convolver_valid;              // set to true if the convolver can be initialised
   unsigned int convolution_threads; // number of threads in the convolver thread pool
   float convolution_gain;
   double convolution_max_length_in_seconds;
@@ -385,6 +381,7 @@ typedef struct {
   int scan_max_inactive_count;     // number of scans to do before stopping if not made active again
                                    // (about 15 minutes worth)
 #endif
+
   int disable_resend_requests; // set this to stop resend request being made for missing packets
   double diagnostic_drop_packet_fraction; // pseudo randomly drop this fraction of packets, for
                                           // debugging. Currently audio packets only...
@@ -395,10 +392,11 @@ typedef struct {
   int jack_soxr_resample_quality;
 #endif
 #endif
+
   void *gradients; // a linked list of the clock gradients discovered for all DACP IDs
                    // can't use IP numbers as they might be given to different devices
                    // can't get hold of MAC addresses.
-                   // can't define the nvll linked list struct here
+                   // can't define the null linked list struct here
   char *firmware_version;
   // use these in information requests
   char *model;
@@ -440,6 +438,7 @@ typedef struct {
 #if defined(CONFIG_DBUS_INTERFACE)
   dbus_message_bus_t dbus_service_bus_type;
 #endif
+
 #if defined(CONFIG_MPRIS_INTERFACE)
   dbus_message_bus_t mpris_service_bus_type;
 #endif
@@ -452,21 +451,6 @@ uint32_t nctohl(const uint8_t *p);  // read 4 characters from *p and do ntohl on
 uint16_t nctohs(const uint8_t *p);  // read 2 characters from *p and do ntohs on them
 uint64_t nctoh64(const uint8_t *p); // read 8 characters from *p to a uint64_t
 
-// void memory_barrier();
-
-/*
-void log_to_stderr(); // call this to direct logging to stderr;
-void log_to_stdout(); // call this to direct logging to stdout;
-void log_to_syslog(); // call this to direct logging to the system log;
-void log_to_file();   // call this to direct logging to a file or (pre-existing) pipe;
-*/
-
-// true if Shairport Sync is supposed to be sending output to the output device, false otherwise
-
-//int get_requested_connection_state_to_output();
-
-// void set_requested_connection_state_to_output(int v);
-
 int try_to_open_pipe_for_writing(
     const char *pathname); // open it without blocking if it's not hooked up
 

configure.ac

@@ -1,7 +1,7 @@
 # Process this file with autoconf to produce a configure script.
 
 AC_PREREQ([2.50])
-AC_INIT([shairport-sync], [5.2.1], [4265913+mikebrady@users.noreply.github.com])
+AC_INIT([shairport-sync], [5.2.2], [4265913+mikebrady@users.noreply.github.com])
 : ${CFLAGS="-O3"}
 : ${CXXFLAGS="-O3"}
 AM_INIT_AUTOMAKE([subdir-objects])

rtp.c

@@ -1488,8 +1488,11 @@ int get_ptp_anchor_local_time_info(rtsp_conn_info *conn, uint32_t *anchorRTP,
     }
 
     if (conn->last_anchor_info_is_valid != 0) {
-      if (anchorRTP != NULL)
+      if (anchorRTP != NULL) {
-        *anchorRTP = conn->last_anchor_rtptime;
+        // Use the current rate in case the stream format has changed.
+        int32_t added_latency = (int32_t)(config.audio_backend_latency_offset * conn->input_rate);
+        *anchorRTP = conn->last_anchor_rtptime - added_latency;
+      }
       if (anchorLocalTime != NULL)
         *anchorLocalTime = conn->last_anchor_local_time;
     }
@@ -1616,6 +1619,7 @@ int32_t decipher_player_put_packet(uint8_t *ciphered_audio_alt, ssize_t nread,
 }
 
 void *rtp_ap2_control_receiver(void *arg) {
+  const int32_t ap2_realttime_stream_latency_fudge_factor = 11025; // seems to bring everything into sync
   //  #include <syscall.h>
   //  debug(1, "rtp_ap2_control_receiver PID %d", syscall(SYS_gettid));
   pthread_cleanup_push(rtp_ap2_control_handler_cleanup_handler, arg);
@@ -1674,22 +1678,6 @@ void *rtp_ap2_control_receiver(void *arg) {
             switch (packet[1]) {
             case 215: // code 215, effectively an anchoring announcement
             {
-              // struct timespec tnr;
-              // clock_gettime(CLOCK_REALTIME, &tnr);
-              // uint64_t local_realtime_now = timespec_to_ns(&tnr);
-
-              /*
-                        char obf[4096];
-                        char *obfp = obf;
-                        int obfc;
-                        for (obfc=0;obfc<nread;obfc++) {
-                          snprintf(obfp, 3, "%02X", packet[obfc]);
-                          obfp+=2;
-                        };
-                        *obfp=0;
-                        debug(1,"AP2 Timing Control Received: \"%s\"",obf);
-              */
-
               uint64_t remote_packet_time_ns = nctoh64(packet + 8);
               check64conversion("remote_packet_time_ns", packet + 8, remote_packet_time_ns);
               uint64_t clock_id = nctoh64(packet + 20);
@@ -1699,49 +1687,45 @@ void *rtp_ap2_control_receiver(void *arg) {
               // debug(1,"remote_packet_time_ns: %" PRIx64 ", local_realtime_now_ns: %" PRIx64
               // ".", remote_packet_time_ns, local_realtime_now);
               uint32_t frame_1 =
-                  nctohl(packet + 4); // this seems to be the frame with latency of 77165 included
+                  nctohl(packet + 4); // this seems to be the frame with latency of 77175 included
               check32conversion("frame_1", packet + 4, frame_1);
               uint32_t frame_2 =
                   nctohl(packet + 16); // this seems to be the frame the time refers to
               check32conversion("frame_2", packet + 16, frame_2);
-              // this just updates the anchor information contained in the packet
+              // this updates the anchor information contained in the packet
-              // the frame and its remote time
+              int32_t stream_specified_latency = frame_2 - frame_1; // this is the latency expected
-              // add in the audio_backend_latency_offset;
+              if (stream_specified_latency != 77175)
-              int32_t notified_latency = frame_2 - frame_1;
+                debug(1, "Stream-specified latency is %d frames. Normally it is 77175.", stream_specified_latency);
-              if (notified_latency != 77175)
+              int32_t net_source_latency = stream_specified_latency + ap2_realttime_stream_latency_fudge_factor;
-                debug(1, "Notified latency is %d frames.", notified_latency);
-              int32_t added_latency =
-                  (int32_t)(config.audio_backend_latency_offset * conn->input_rate);
-              // the actual latency is the notified latency plus the fixed latency + the added
-              // latency
 
-              int32_t net_latency =
+              // Now to accommodate a backend buffer of the desired length.
-                  notified_latency + 11035 +
+              // Note that it's in input-rate frames, not output-rate frames!
-                  added_latency; // this is the latency between incoming frames and the DAC
+              
-              net_latency = net_latency - (int32_t)(config.audio_backend_buffer_desired_length *
+              net_source_latency = net_source_latency - (int32_t)(config.audio_backend_buffer_desired_length *
                                                     conn->input_rate);
-              // debug(1, "Net latency is %d frames.", net_latency);
+                                                                               
+              // Now we want to check the user-specified latency offset.
+
+              // We want to warn the user if they have asked for a negative latency that is too great --
+              // one that would require packets to arrive before they actually do,
+              // (which is about two seconds before they are to be played).
+              
+              int32_t net_latency = net_source_latency + (int32_t)(config.audio_backend_latency_offset * conn->input_rate);
               
               if (net_latency <= 0) {
                 if (conn->latency_warning_issued == 0) {
-                  warn("The stream latency (%f seconds) it too short to accommodate an offset of "
+                  warn("The stream latency (%g seconds) is too short to accommodate an audio backend latency offset of "
-                       "%f "
+                       "%g seconds and a backend buffer of %g seconds. The audio_backend_latency_offset has been set to zero.",
-                       "seconds and a backend buffer of %f seconds.",
+                       ((stream_specified_latency + ap2_realttime_stream_latency_fudge_factor) * 1.0) / conn->input_rate,
-                       ((notified_latency + 11035) * 1.0) / conn->input_rate,
                        config.audio_backend_latency_offset,
                        config.audio_backend_buffer_desired_length);
-                  warn("(FYI the stream latency needed would be %f seconds.)",
+                  config.audio_backend_latency_offset = 0.0;
-                       config.audio_backend_buffer_desired_length -
+                  net_latency = net_source_latency;
-                           config.audio_backend_latency_offset);
                   conn->latency_warning_issued = 1;
                 }
-                conn->latency = notified_latency + 11035;
-              } else {
-                conn->latency = notified_latency + 11035 + added_latency;
               }
-
+              conn->latency = net_latency; // this is the time window within which packets can be accepted without being too late
-              set_ptp_anchor_info(conn, clock_id, frame_1 - 11035 - added_latency,
+              set_ptp_anchor_info(conn, clock_id, frame_1 - ap2_realttime_stream_latency_fudge_factor, remote_packet_time_ns);
-                                  remote_packet_time_ns);
               if (conn->anchor_clock != clock_id) {
                 debug(2, "Connection %d: Change Anchor Clock: %" PRIx64 ".",
                       conn->connection_number, clock_id);

rtsp.c

@@ -1453,11 +1453,8 @@ void handle_setrateanchori(rtsp_conn_info *conn, rtsp_message *req, rtsp_message
       // debug(1, "anchor rtpTime is %" PRId64 ".", rtpTime);
       uint32_t anchorRTPTime = rtpTime;
 
-      int32_t added_latency = (int32_t)(config.audio_backend_latency_offset * conn->input_rate);
+      // Store the raw anchor; apply the latency offset when it is used.
-      // debug(1,"anchorRTPTime: %" PRIu32 ", added latency: %" PRId32 ".", anchorRTPTime,
+      set_ptp_anchor_info(conn, conn->networkTimeTimelineID, anchorRTPTime, anchorTimeNanoseconds);
-      // added_latency);
-      set_ptp_anchor_info(conn, conn->networkTimeTimelineID, anchorRTPTime - added_latency,
-                          anchorTimeNanoseconds);
     }
 
     item = plist_dict_get_item(messagePlist, "rate");

shairport.c

@@ -1955,7 +1955,7 @@ void exit_function() {
         }
         if (glib_worker_thread != NULL) {
           g_thread_join(glib_worker_thread);
-          debug(1, "GLib worker thread joined");
+          debug(2, "GLib worker thread joined");
         }
 
 #endif

xml_plist_codegen.sh

@@ -23,6 +23,6 @@ printf "// Please do not edit this file!\n" > $2/$BASE_FILENAME.c
 printf "// This file was automatically generated from $INPUT_FILE_NAME.\n\n" >> $2/$BASE_FILENAME.c
 printf "#include \"$BASE_FILENAME.h\"\n\n" >> $2/$BASE_FILENAME.c
 printf "unsigned char ${BASE_FILENAME}_plist[] = {\n" >> $2/$BASE_FILENAME.c
-plistutil -i $1 | xxd -i - >> $2/$BASE_FILENAME.c
+plistutil -i $1 | xxd -i >> $2/$BASE_FILENAME.c
 printf "  };\n\n" >> $2/$BASE_FILENAME.c
 printf "size_t ${BASE_FILENAME}_plist_len = sizeof(${BASE_FILENAME}_plist);\n" >> $2/$BASE_FILENAME.c