From: Mike Brady <4265913+mikebrady@users.noreply.github.com> Date: Fri, 21 Aug 2020 09:43:09 +0000 (+0100) Subject: Squashed commit of the following: X-Git-Tag: 3.3.7rc2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=27faeaf69667e9625e3e7c8ac67fb1f88f3368cb;p=thirdparty%2Fshairport-sync.git 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 6dedc464..45e0f060 100644 --- 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 3d8b6e94..7f253699 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], [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 c6b9968f..a25bf9e4 100644 --- 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 190895c0..7b20ae18 100644 --- 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 dcef6c4e..edca685f 100644 --- 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 2febacd0..1f1cb9f9 100644 --- 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 14e50725..975b71fc 100644 --- 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':