Squashed commit of the following:

    Use a  gentler flush process for resyncing.
    Potentially fix a flaw in resyncing. The problem was that a flush is set up but not triggered.
    Open metadata and audio pipes with 666 permissions for easier integration with other audio sources to give them permission to write to the audio pipe later.
    Ensure metadata and cover art are enabled if metadata support is included at compilation.
    Reword some misleading error messages.
    Set convolution defaults even if no configuration file is found.
    Quieten a few debug messages.

diff --git a/audio_pipe.c b/audio_pipe.c
index 6dedc464b9b34d45d07d39266fbefbf4c26875fa..45e0f06008b43ec282517822d851a91c5983ecd3 100644 (file)
--- a/audio_pipe.c
+++ b/audio_pipe.c
@@ -121,7 +121,7 @@ static int init(int argc, char **argv) {
 
   // here, create the pipe
   mode_t oldumask = umask(000);
-  if (mkfifo(pipename, 0644) && errno != EEXIST)
+  if (mkfifo(pipename, 0666) && errno != EEXIST)
     die("Could not create audio pipe \"%s\"", pipename);
   umask(oldumask);
 

diff --git a/configure.ac b/configure.ac
index 3d8b6e94cde12aad9fdc02d62976f03f12cdd138..7f25369996a5255f85a14683fb41a619438d68b8 100644 (file)
--- 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], [3.3.7rc1], [4265913+mikebrady@users.noreply.github.com])
+AC_INIT([shairport-sync], [3.3.7rc2], [4265913+mikebrady@users.noreply.github.com])
 AM_INIT_AUTOMAKE
 AC_CONFIG_SRCDIR([shairport.c])
 AC_CONFIG_HEADERS([config.h])

diff --git a/dacp.c b/dacp.c
index c6b9968fde5c2334437b085232973bfa1ca8a5bf..a25bf9e48257ccc64bedc59df91681432916f45d 100644 (file)
--- a/dacp.c
+++ b/dacp.c
@@ -154,7 +154,7 @@ int dacp_send_command(const char *command, char **body, ssize_t *bodysize) {
   // debug(1,"dacp_send_command: command is: \"%s\".",command);
 
   if (dacp_server.port == 0) {
-    debug(3, "No DACP port specified yet");
+    // debug(3, "No DACP port specified yet");
     result = 490; // no port specified
   } else {
 
@@ -504,11 +504,13 @@ void *dacp_monitor_thread_code(__attribute__((unused)) void *na) {
                (metadata_store.advanced_dacp_server_active != 0);
       metadata_store.dacp_server_active = 0;
       metadata_store.advanced_dacp_server_active = 0;
+      /*
       debug(3,
             "setting metadata_store.dacp_server_active and "
             "metadata_store.advanced_dacp_server_active to 0 with an update "
             "flag value of %d",
             ch);
+      */
       metadata_hub_modify_epilog(ch);
 
       uint64_t time_to_wait_for_wakeup_ns =
@@ -1247,9 +1249,9 @@ int dacp_get_volume(int32_t *the_actual_volume) {
       // metadata_store.speaker_volume = actual_volume;
       // metadata_hub_modify_epilog(1);
     } else {
-      debug(1, "Unexpected return code %d from dacp_get_speaker_list.", http_response);
+      debug(2, "Unexpected return code %d from dacp_get_speaker_list.", http_response);
     }
-  } else if (http_response != 400) {
+  } else if ((http_response != 400) && (http_response != 490)) {
     debug(3, "Unexpected return code %d from dacp_get_client_volume.", http_response);
   }
   if (the_actual_volume) {

diff --git a/metadata_hub.c b/metadata_hub.c
index 190895c0a432cb0bfda3869b20370950a0bb1ca3..7b20ae18d2f6c99fddbeb61a3e1850eba092c62c 100644 (file)
--- a/metadata_hub.c
+++ b/metadata_hub.c
@@ -148,7 +148,7 @@ void _metadata_hub_modify_prolog(const char *filename, const int linenumber) {
        }
        last_metadata_hub_modify_prolog_file = strdup(filename);
        last_metadata_hub_modify_prolog_line = linenumber;
-    debug(3, "Metadata_hub write lock acquired.");
+    // debug(3, "Metadata_hub write lock acquired.");
   }
   metadata_hub_re_lock_access_is_delayed = 0;
 }
@@ -169,7 +169,7 @@ void _metadata_hub_modify_epilog(int modified, const char *filename, const int l
                }
   }
   pthread_rwlock_unlock(&metadata_hub_re_lock);
-  debug(3, "Metadata_hub write lock unlocked.");
+  // debug(3, "Metadata_hub write lock unlocked.");
 }
 
 /*

diff --git a/player.c b/player.c
index dcef6c4e3f1cdb5504a4727c5e2d6bc89107daee..edca685fd6b58e0caf0b66efb7a3ca77581d2439 100644 (file)
--- a/player.c
+++ b/player.c
@@ -950,71 +950,71 @@ static abuf_t *buffer_get_frame(rtsp_conn_info *conn) {
          debug(2, "flush request: flush output device.");
         }
       conn->flush_output_flushed = 1;
-      // now check to see it the flush request is for frames in the buffer or not
-      // if the first_packet_timestamp is zero, don't check
-      int flush_needed = 0;
-      int drop_request = 0;
-      if (conn->flush_rtp_timestamp == 0) {
-       debug(1, "flush request: flush frame 0 -- flush assumed to be needed.");
-       flush_needed = 1;
-       drop_request = 1;
-      } else {
-                               if ((conn->ab_synced) && ((conn->ab_write - conn->ab_read) > 0)) {
-                                       abuf_t *firstPacket = conn->audio_buffer + BUFIDX(conn->ab_read);
-                                       abuf_t *lastPacket = conn->audio_buffer + BUFIDX(conn->ab_write - 1);
-                                       if ((firstPacket != NULL) && (firstPacket->ready)) {
-                                               // discard flushes more than 10 seconds into the future -- they are probably bogus
-                                               uint32_t first_frame_in_buffer = firstPacket->given_timestamp;
-                                               int32_t offset_from_first_frame = (int32_t)(conn->flush_rtp_timestamp - first_frame_in_buffer);
-                                               if (offset_from_first_frame > (int)conn->input_rate * 10) {
-                                                       debug(1, "flush request: sanity check -- flush frame %u is too far into the future from the first frame %u -- discarded.", conn->flush_rtp_timestamp, first_frame_in_buffer);
-                                                       drop_request = 1;
-                                               } else {
-                                                       if ((lastPacket != NULL) && (lastPacket->ready)) {
-                                                               // we have enough information to check if the flush is needed or can be discarded
-                                                               uint32_t last_frame_in_buffer = lastPacket->given_timestamp + lastPacket->length - 1;
-                                                               // now we have to work out if the flush frame is in the buffer
-                                                               // if it is later than the end of the buffer, flush everything and keep the request active.
-                                                               // if it is in the buffer, we need to flush part of the buffer. Actually we flush the entire buffer and drop the request.
-                                                               // if it is before the buffer, no flush is needed. Drop the request.
-                                                               if (offset_from_first_frame > 0) {
-                                                                       int32_t offset_to_last_frame = (int32_t)(last_frame_in_buffer - conn->flush_rtp_timestamp);
-                                                                       if (offset_to_last_frame >= 0) {
-                                                                               debug(2,"flush request: flush frame %u active -- buffer contains %u frames, from %u to %u", conn->flush_rtp_timestamp, last_frame_in_buffer - first_frame_in_buffer + 1, first_frame_in_buffer, last_frame_in_buffer);
-                                                                               drop_request = 1;
-                                                                               flush_needed = 1;
-                                                                       } else {
-                                                                               debug(2,"flush request: flush frame %u pending -- buffer contains %u frames, from %u to %u", conn->flush_rtp_timestamp, last_frame_in_buffer - first_frame_in_buffer + 1, first_frame_in_buffer, last_frame_in_buffer);
-                                                                               flush_needed = 1;
-                                                                       }
+    }
+               // now check to see it the flush request is for frames in the buffer or not
+               // if the first_packet_timestamp is zero, don't check
+               int flush_needed = 0;
+               int drop_request = 0;
+               if ((conn->flush_requested == 1) && (conn->flush_rtp_timestamp == 0)) {
+                       debug(1, "flush request: flush frame 0 -- flush assumed to be needed.");
+                       flush_needed = 1;
+                       drop_request = 1;
+               } else if (conn->flush_rtp_timestamp != 0) {
+                       if ((conn->ab_synced) && ((conn->ab_write - conn->ab_read) > 0)) {
+                               abuf_t *firstPacket = conn->audio_buffer + BUFIDX(conn->ab_read);
+                               abuf_t *lastPacket = conn->audio_buffer + BUFIDX(conn->ab_write - 1);
+                               if ((firstPacket != NULL) && (firstPacket->ready)) {
+                                       // discard flushes more than 10 seconds into the future -- they are probably bogus
+                                       uint32_t first_frame_in_buffer = firstPacket->given_timestamp;
+                                       int32_t offset_from_first_frame = (int32_t)(conn->flush_rtp_timestamp - first_frame_in_buffer);
+                                       if (offset_from_first_frame > (int)conn->input_rate * 10) {
+                                               debug(1, "flush request: sanity check -- flush frame %u is too far into the future from the first frame %u -- discarded.", conn->flush_rtp_timestamp, first_frame_in_buffer);
+                                               drop_request = 1;
+                                       } else {
+                                               if ((lastPacket != NULL) && (lastPacket->ready)) {
+                                                       // we have enough information to check if the flush is needed or can be discarded
+                                                       uint32_t last_frame_in_buffer = lastPacket->given_timestamp + lastPacket->length - 1;
+                                                       // now we have to work out if the flush frame is in the buffer
+                                                       // if it is later than the end of the buffer, flush everything and keep the request active.
+                                                       // if it is in the buffer, we need to flush part of the buffer. Actually we flush the entire buffer and drop the request.
+                                                       // if it is before the buffer, no flush is needed. Drop the request.
+                                                       if (offset_from_first_frame > 0) {
+                                                               int32_t offset_to_last_frame = (int32_t)(last_frame_in_buffer - conn->flush_rtp_timestamp);
+                                                               if (offset_to_last_frame >= 0) {
+                                                                       debug(2,"flush request: flush frame %u active -- buffer contains %u frames, from %u to %u", conn->flush_rtp_timestamp, last_frame_in_buffer - first_frame_in_buffer + 1, first_frame_in_buffer, last_frame_in_buffer);
+                                                                       drop_request = 1;
+                                                                       flush_needed = 1;
                                                                } else {
-                                                                               debug(2,"flush request: flush frame %u expired -- buffer contains %u frames, from %u to %u", conn->flush_rtp_timestamp, last_frame_in_buffer - first_frame_in_buffer + 1, first_frame_in_buffer, last_frame_in_buffer);
-                                                                               drop_request = 1;
+                                                                       debug(2,"flush request: flush frame %u pending -- buffer contains %u frames, from %u to %u", conn->flush_rtp_timestamp, last_frame_in_buffer - first_frame_in_buffer + 1, first_frame_in_buffer, last_frame_in_buffer);
+                                                                       flush_needed = 1;
                                                                }
+                                                       } else {
+                                                                       debug(2,"flush request: flush frame %u expired -- buffer contains %u frames, from %u to %u", conn->flush_rtp_timestamp, last_frame_in_buffer - first_frame_in_buffer + 1, first_frame_in_buffer, last_frame_in_buffer);
+                                                                       drop_request = 1;
                                                        }
                                                }
                                        }
-                               } else {
-                                       debug(3, "flush request: flush frame %u  -- buffer not synced or empty: synced: %d, ab_read: %u, ab_write: %u", conn->flush_rtp_timestamp, conn->ab_synced, conn->ab_read, conn->ab_write);
-                                       // leave flush request pending and don't do a buffer flush, because there isn't one
                                }
-      }
-                       if (flush_needed) {
-                               debug(2, "flush request: flush done.");
-                               ab_resync(conn);          // no cancellation points
-                               conn->first_packet_timestamp = 0;
-                               conn->first_packet_time_to_play = 0;
-                               conn->time_since_play_started = 0;
-                               have_sent_prefiller_silence = 0;
-                               dac_delay = 0;
-                       }
-                       if (drop_request) {
-                               debug(2, "flush request: request dropped.");
-                               conn->flush_requested = 0;
-                               conn->flush_rtp_timestamp = 0;
-                               conn->flush_output_flushed = 0;
+                       } else {
+                               debug(3, "flush request: flush frame %u  -- buffer not synced or empty: synced: %d, ab_read: %u, ab_write: %u", conn->flush_rtp_timestamp, conn->ab_synced, conn->ab_read, conn->ab_write);
+                               // leave flush request pending and don't do a buffer flush, because there isn't one
                        }
-    }
+               }
+               if (flush_needed) {
+                       debug(2, "flush request: flush done.");
+                       ab_resync(conn);          // no cancellation points
+                       conn->first_packet_timestamp = 0;
+                       conn->first_packet_time_to_play = 0;
+                       conn->time_since_play_started = 0;
+                       have_sent_prefiller_silence = 0;
+                       dac_delay = 0;
+               }
+               if (drop_request) {
+                       debug(2, "flush request: request dropped.");
+                       conn->flush_requested = 0;
+                       conn->flush_rtp_timestamp = 0;
+                       conn->flush_output_flushed = 0;
+               }
     debug_mutex_unlock(&conn->flush_mutex, 0);
     if (conn->ab_synced) {
       curframe = conn->audio_buffer + BUFIDX(conn->ab_read);
@@ -1997,7 +1997,7 @@ void *player_thread_func(void *arg) {
         //          debug(3, "Play frame %d.", play_number);
         conn->play_number_after_flush++;
         if (inframe->given_timestamp == 0) {
-          debug(1,
+          debug(2,
                 "Player has supplied a silent frame, (possibly frame %u) for play number %d, "
                 "status 0x%X after %u resend requests.",
                 SUCCESSOR(conn->last_seqno_read), play_number, inframe->status,

diff --git a/rtsp.c b/rtsp.c
index 2febacd08b9628dfc0b2789b463831cb054a358b..1f1cb9f9e5d0049f8f50354b8c1861dc4cbcd547 100644 (file)
--- a/rtsp.c
+++ b/rtsp.c
@@ -1572,7 +1572,7 @@ void *metadata_hub_thread_function(__attribute__((unused)) void *ignore) {
   snprintf(path, pl + 1, "%s", config.metadata_pipename);
 
   mode_t oldumask = umask(000);
-  if (mkfifo(path, 0644) && errno != EEXIST)
+  if (mkfifo(path, 0666) && errno != EEXIST)
     die("Could not create metadata pipe \"%s\".", path);
   umask(oldumask);
   debug(1, "metadata pipe name is \"%s\".", path);

diff --git a/shairport.c b/shairport.c
index 14e5072575c32f6da842f359a84f4e8143bfdc15..975b71fc6100c51b17ad265f6340d635c2cde852 100644 (file)
--- a/shairport.c
+++ b/shairport.c
@@ -266,7 +266,7 @@ void usage(char *progname) {
   printf("    --metadata-pipename=PIPE send metadata to PIPE, e.g. "
          "--metadata-pipename=/tmp/shairport-sync-metadata.\n");
   printf("                            The default is /tmp/shairport-sync-metadata.\n");
-  printf("    --get-coverart          send cover art through the metadata pipe.\n");
+  printf("    -g, --get-coverart      send cover art through the metadata pipe.\n");
 #endif
   printf("    -u, --use-stderr        log messages through STDERR rather than the system log.\n");
   printf("\n");
@@ -418,6 +418,17 @@ int parse_options(int argc, char **argv) {
   // seconds then we can add an offset of 5.17 seconds and still leave a second's worth of buffers
   // for unexpected circumstances
 
+#ifdef CONFIG_METADATA
+      /* Get the metadata setting. */
+      config.metadata_enabled = 1; // if metadata support is included, then enable it by default
+      config.get_coverart = 1; // if metadata support is included, then enable it by default
+#endif
+
+#ifdef CONFIG_CONVOLUTION
+      config.convolution_max_length = 8192;
+#endif
+      config.loudness_reference_volume_db = -20;
+
 #ifdef CONFIG_METADATA_HUB
   config.cover_art_cache_dir = "/tmp/shairport-sync/.cache/coverart";
   config.scan_interval_when_active =
@@ -846,7 +857,6 @@ int parse_options(int argc, char **argv) {
 
 #ifdef CONFIG_METADATA
       /* Get the metadata setting. */
-      config.metadata_enabled = 1; // if metadata support is included, then enable it by default
       if (config_lookup_string(config.cfg, "metadata.enabled", &str)) {
         if (strcasecmp(str, "no") == 0)
           config.metadata_enabled = 0;
@@ -856,7 +866,6 @@ int parse_options(int argc, char **argv) {
           die("Invalid metadata enabled option choice \"%s\". It should be \"yes\" or \"no\"");
       }
 
-      config.get_coverart = 1; // if metadata support is included, then enable it by default
       if (config_lookup_string(config.cfg, "metadata.include_cover_art", &str)) {
         if (strcasecmp(str, "no") == 0)
           config.get_coverart = 0;
@@ -988,7 +997,6 @@ int parse_options(int argc, char **argv) {
               dvalue);
       }
 
-      config.convolution_max_length = 8192;
       if (config_lookup_int(config.cfg, "dsp.convolution_max_length", &value)) {
         config.convolution_max_length = value;
 
@@ -1015,7 +1023,6 @@ int parse_options(int argc, char **argv) {
           die("Invalid dsp.convolution. It should be \"yes\" or \"no\"");
       }
 
-      config.loudness_reference_volume_db = -20;
       if (config_lookup_float(config.cfg, "dsp.loudness_reference_volume_db", &dvalue)) {
         config.loudness_reference_volume_db = dvalue;
         if (dvalue > 0 || dvalue < -100)
@@ -1168,7 +1175,7 @@ int parse_options(int argc, char **argv) {
       break;
     case 'g':
       if (config.metadata_enabled == 0)
-        die("If you want to get cover art, you must also select the --metadata-pipename option.");
+        die("If you want to get cover art, ensure metadata_enabled is true.");
       break;
 #endif
     case 'S':