Skip to content

Commit

Permalink
Squashed commit of the following:
Browse files Browse the repository at this point in the history 
    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.
  • Loading branch information
@mikebrady
mikebrady committed Aug 21, 2020
1 parent 153c88a commit 27faeaf
Show file tree
Hide file tree
Showing 7 changed files with 82 additions and 73 deletions.
2 changes: 1 addition & 1 deletion audio_pipe.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
2 changes: 1 addition & 1 deletion configure.ac
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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], [[email protected]])
AC_INIT([shairport-sync], [3.3.7rc2], [[email protected]])
AM_INIT_AUTOMAKE
AC_CONFIG_SRCDIR([shairport.c])
AC_CONFIG_HEADERS([config.h])
Expand Down
8 changes: 5 additions & 3 deletions dacp.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

Expand Down Expand Up @@ -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 =
Expand Down Expand Up @@ -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) {
Expand Down
4 changes: 2 additions & 2 deletions metadata_hub.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -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.");
}

/*
Expand Down
118 changes: 59 additions & 59 deletions player.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion rtsp.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
19 changes: 13 additions & 6 deletions shairport.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down Expand Up @@ -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 =
Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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;

Expand All @@ -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)
Expand Down Expand Up @@ -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':
Expand Down

0 comments on commit 27faeaf

Please sign in to comment.