From 9ebea33bb4524a2d8286ec28dea3d15099996b8f Mon Sep 17 00:00:00 2001 From: Mike Brady Date: Sat, 5 May 2018 12:08:45 +0100 Subject: [PATCH 1/8] Update development version of TROUBLESHOOTING to match that of 3.2RC6. --- configure.ac | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index ce66e32d..1b953518 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.2d45], [mikebrady@eircom.net]) +AC_INIT([shairport-sync], [3.2d46], [mikebrady@eircom.net]) AM_INIT_AUTOMAKE AC_CONFIG_SRCDIR([shairport.c]) AC_CONFIG_HEADERS([config.h]) From bee073e3cfd4b7e646a9a5b2e0b4343a26acdb0c Mon Sep 17 00:00:00 2001 From: Mike Brady Date: Sat, 5 May 2018 12:11:28 +0100 Subject: [PATCH 2/8] Update RELEASENOTES.md --- RELEASENOTES.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/RELEASENOTES.md b/RELEASENOTES.md index 400dc504..ecc4fbb0 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -1,3 +1,8 @@ +Version 3.2d46 +==== + +Update [TROUBLESHOOTING.md](https://github.com/mikebrady/shairport-sync/blob/development/TROUBLESHOOTING.md) to correspond to 3.2RC6. + Version 3.2d45 ==== **Enhancement** From a5f7cd89cfbbaabbaa1e909eaef19590cafd0e05 Mon Sep 17 00:00:00 2001 From: Mike Brady Date: Sat, 5 May 2018 19:24:57 +0100 Subject: [PATCH 3/8] Quieten a debug message. --- rtsp.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/rtsp.c b/rtsp.c index 516e9968..0fb11ce9 100644 --- a/rtsp.c +++ b/rtsp.c @@ -1593,15 +1593,15 @@ static void handle_announce(rtsp_conn_info *conn, rtsp_message *req, rtsp_messag if (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) + // 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); // unsigned integer -- up to 2^32-1 - debug(1, "AirPlay version %d detected.", conn->AirPlayVersion); + conn->AirPlayVersion = atoi(p + 1); + debug(2, "AirPlay version %d detected.", conn->AirPlayVersion); } } From 709ffa591551a072164a72e2b6316101d3398e83 Mon Sep 17 00:00:00 2001 From: Mike Brady Date: Sun, 6 May 2018 17:25:43 +0100 Subject: [PATCH 4/8] Replace snd_pcm_drop with snd_pcm_drain. --- audio_alsa.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/audio_alsa.c b/audio_alsa.c index defd798e..3310eeaa 100644 --- a/audio_alsa.c +++ b/audio_alsa.c @@ -892,8 +892,11 @@ static void flush(void) { int derr; do_mute(1); if (alsa_handle) { - if ((derr = snd_pcm_drop(alsa_handle))) - debug(1, "Error %d (\"%s\") draining the output device.", derr, snd_strerror(derr)); + + debug(1,"Start drain."); + if ((derr = snd_pcm_drain(alsa_handle))) + debug(1, "Error %d (\"%s\") draining output device.", derr, snd_strerror(derr)); + debug(1,"Finish drain."); if ((derr = snd_pcm_hw_free(alsa_handle))) debug(1, "Error %d (\"%s\") freeing the output device hardware.", derr, snd_strerror(derr)); From 03210b38298fb8de5ca3f55b3343b5a18cc334ff Mon Sep 17 00:00:00 2001 From: Mike Brady Date: Sun, 6 May 2018 17:27:01 +0100 Subject: [PATCH 5/8] Replace snd_pcm_drop with snd_pcm_drain -- issue 694 --- configure.ac | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index 1b953518..b76c1b0b 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.2d46], [mikebrady@eircom.net]) +AC_INIT([shairport-sync], [3.2d47i694,1], [mikebrady@eircom.net]) AM_INIT_AUTOMAKE AC_CONFIG_SRCDIR([shairport.c]) AC_CONFIG_HEADERS([config.h]) From 6d66b2ed338741ee482ec5524ad02a81f8192433 Mon Sep 17 00:00:00 2001 From: Mike Brady Date: Mon, 7 May 2018 22:00:01 +0100 Subject: [PATCH 6/8] Stop erroneously assuming that the buffer passed to the player is not changed -- sometimes it is, it seems. Tidy up some typing. --- audio.h | 2 +- audio_alsa.c | 13 ++++++----- player.c | 61 +++++++++++++++++++++++++++++++--------------------- rtp.c | 12 ++++++----- 4 files changed, 51 insertions(+), 37 deletions(-) diff --git a/audio.h b/audio.h index e0defc55..dc7eb873 100644 --- a/audio.h +++ b/audio.h @@ -22,7 +22,7 @@ typedef struct { void (*start)(int sample_rate, int sample_format); // block of samples - void (*play)(short buf[], int samples); + void (*play)(void *buf, int samples); void (*stop)(void); // may be null if not implemented diff --git a/audio_alsa.c b/audio_alsa.c index 3310eeaa..d6bf73dd 100644 --- a/audio_alsa.c +++ b/audio_alsa.c @@ -40,7 +40,7 @@ static void help(void); static int init(int argc, char **argv); static void deinit(void); static void start(int i_sample_rate, int i_sample_format); -static void play(short buf[], int samples); +static void play(void *buf, int samples); static void stop(void); static void flush(void); int delay(long *the_delay); @@ -836,7 +836,7 @@ int delay(long *the_delay) { } } -static void play(short buf[], int samples) { +static void play(void *buf, int samples) { // debug(3,"audio_alsa play called."); int ret = 0; if (alsa_handle == NULL) { @@ -867,7 +867,7 @@ static void play(short buf[], int samples) { if (samples == 0) debug(1, "empty buffer being passed to pcm_writei -- skipping it"); if ((samples != 0) && (buf != NULL)) { - err = alsa_pcm_write(alsa_handle, (char *)buf, samples); + err = alsa_pcm_write(alsa_handle, buf, samples); if (err < 0) { debug(1, "Error %d writing %d samples in play(): \"%s\".", err, samples, snd_strerror(err)); @@ -893,10 +893,8 @@ static void flush(void) { do_mute(1); if (alsa_handle) { - debug(1,"Start drain."); - if ((derr = snd_pcm_drain(alsa_handle))) - debug(1, "Error %d (\"%s\") draining output device.", derr, snd_strerror(derr)); - debug(1,"Finish drain."); + if ((derr = snd_pcm_drop(alsa_handle))) + debug(1, "Error %d (\"%s\") dropping output device.", derr, snd_strerror(derr)); if ((derr = snd_pcm_hw_free(alsa_handle))) debug(1, "Error %d (\"%s\") freeing the output device hardware.", derr, snd_strerror(derr)); @@ -904,6 +902,7 @@ static void flush(void) { // flush also closes the device if ((derr = snd_pcm_close(alsa_handle))) debug(1, "Error %d (\"%s\") closing the output device.", derr, snd_strerror(derr)); + alsa_handle = NULL; } pthread_mutex_unlock(&alsa_mutex); diff --git a/player.c b/player.c index d3dcb0db..70d6f7a9 100644 --- a/player.c +++ b/player.c @@ -452,8 +452,7 @@ static void terminate_decoders(rtsp_conn_info *conn) { static void init_buffer(rtsp_conn_info *conn) { int i; for (i = 0; i < BUFFER_FRAMES; i++) - conn->audio_buffer[i].data = malloc( - conn->input_bytes_per_frame * (conn->max_frames_per_packet + conn->max_frame_size_change)); + conn->audio_buffer[i].data = malloc(conn->input_bytes_per_frame * conn->max_frames_per_packet); ab_resync(conn); } @@ -1036,13 +1035,13 @@ static abuf_t *buffer_get_frame(rtsp_conn_info *conn) { // ab_write),ab_read,ab_write); conn->ab_buffering = 0; } - signed short *silence; + void *silence; // if (fs==0) // debug(2,"Zero length silence buffer needed with gross_frame_gap of %lld and // dac_delay of %lld.",gross_frame_gap,dac_delay); // the fs (number of frames of silence to play) can be zero in the DAC doesn't // start - // ouotputting frames for a while -- it could get loaded up but not start + // outputting frames for a while -- it could get loaded up but not start // responding // for many milliseconds. if (fs > 0) { @@ -1066,7 +1065,7 @@ static abuf_t *buffer_get_frame(rtsp_conn_info *conn) { // debug(1,"Back end has no delay function."); // send the appropriate prefiller here... - signed short *silence; + void *silence; if (lead_time != 0) { int64_t frame_gap = (lead_time * config.output_rate) >> 32; // debug(1,"%d frames needed.",frame_gap); @@ -1079,7 +1078,7 @@ static abuf_t *buffer_get_frame(rtsp_conn_info *conn) { if (silence == NULL) debug(1, "Failed to allocate %d frame silence buffer.", fs); else { - // debug(1, "Outputting %d frames of silence.", fs); + // debug(1, "No delay function -- outputting %d frames of silence.", fs); memset(silence, 0, conn->output_bytes_per_frame * fs); config.output->play(silence, fs); free(silence); @@ -1546,7 +1545,7 @@ static void *player_thread_func(void *arg) { static char rnstate[256]; initstate(time(NULL), rnstate, 256); - signed short *inbuf, *tbuf, *silence; + signed short *inbuf, *tbuf; int32_t *sbuf; @@ -1597,13 +1596,12 @@ static void *player_thread_func(void *arg) { die("Failed to allocate memory for the transition buffer."); sbuf = 0; // initialise this, because soxr stuffing might be chosen later - // if (config.packet_stuffing == ST_soxr) { // needed for stuffing + sbuf = malloc(sizeof(int32_t) * 2 * (conn->max_frames_per_packet * conn->output_sample_ratio + conn->max_frame_size_change)); if (sbuf == NULL) debug(1, "Failed to allocate memory for the sbuf buffer."); - // } - // We might need an output buffer and a buffer of silence. + // The size of these dependents on the number of frames, the size of each frame and the maximum // size change outbuf = malloc( @@ -1611,12 +1609,6 @@ static void *player_thread_func(void *arg) { (conn->max_frames_per_packet * conn->output_sample_ratio + conn->max_frame_size_change)); if (outbuf == NULL) die("Failed to allocate memory for an output buffer."); - silence = malloc(conn->output_bytes_per_frame * conn->max_frames_per_packet * - conn->output_sample_ratio); - if (silence == NULL) - die("Failed to allocate memory for a silence buffer."); - memset(silence, 0, - conn->output_bytes_per_frame * conn->max_frames_per_packet * conn->output_sample_ratio); conn->first_packet_timestamp = 0; conn->missing_packets = conn->late_packets = conn->too_late_packets = conn->resend_requests = 0; conn->flush_rtp_timestamp = @@ -1695,10 +1687,22 @@ static void *player_thread_func(void *arg) { conn->play_number_after_flush++; if (inframe->timestamp == 0) { debug(3, "Player has supplied a silent frame, (possibly frame %u) for play number %d.", - SUCCESSOR(conn->last_seqno_read),play_number); + SUCCESSOR(conn->last_seqno_read), play_number); conn->last_seqno_read = (SUCCESSOR(conn->last_seqno_read) & 0xffff); // manage the packet out of sequence minder - config.output->play(silence, conn->max_frames_per_packet * conn->output_sample_ratio); + + void *silence = malloc(conn->output_bytes_per_frame * conn->max_frames_per_packet * + conn->output_sample_ratio); + if (silence == NULL) { + debug(1, "Failed to allocate memory for a silent frame silence buffer."); + } else { + // the player may change the contents of the buffer, so it has to be zeroed each time; + // might as well malloc and freee it locally + memset(silence, 0, conn->output_bytes_per_frame * conn->max_frames_per_packet * + conn->output_sample_ratio); + config.output->play(silence, conn->max_frames_per_packet * conn->output_sample_ratio); + free(silence); + } } else if (conn->play_number_after_flush < 10) { /* int64_t difference = 0; @@ -1708,7 +1712,18 @@ static void *player_thread_func(void *arg) { debug(1, "Play number %d, monotonic timestamp %llx, difference %lld.",conn->play_number_after_flush,inframe->timestamp,difference); */ - config.output->play(silence, conn->max_frames_per_packet * conn->output_sample_ratio); + void *silence = malloc(conn->output_bytes_per_frame * conn->max_frames_per_packet * + conn->output_sample_ratio); + if (silence == NULL) { + debug(1, "Failed to allocate memory for a flush silence buffer."); + } else { + // the player may change the contents of the buffer, so it has to be zeroed each time; + // might as well malloc and freee it locally + memset(silence, 0, conn->output_bytes_per_frame * conn->max_frames_per_packet * + conn->output_sample_ratio); + config.output->play(silence, conn->max_frames_per_packet * conn->output_sample_ratio); + free(silence); + } } else if (frames_to_drop) { if (frames_to_drop > 3 * config.output_rate) warn("Very large number of frames to drop: %" PRId64 ".", frames_to_drop); @@ -1948,7 +1963,7 @@ static void *player_thread_func(void *arg) { die("Failed to allocate memory for a long_silence buffer of %d frames.", silence_length); memset(long_silence, 0, conn->output_bytes_per_frame * silence_length); - config.output->play((short *)long_silence, silence_length); + config.output->play(long_silence, silence_length); free(long_silence); } } else { @@ -2085,7 +2100,7 @@ static void *player_thread_func(void *arg) { if (play_samples == 0) debug(1, "play_samples==0 skipping it (1)."); else - config.output->play((short *)outbuf, play_samples); // remove the (short*)! + config.output->play(outbuf, play_samples); } // check for loss of sync @@ -2119,7 +2134,7 @@ static void *player_thread_func(void *arg) { if (outbuf == NULL) debug(1, "NULL outbuf to play -- skipping it."); else - config.output->play((short *)outbuf, play_samples); // remove the (short*)! + config.output->play(outbuf, play_samples); // remove the (short*)! } // mark the frame as finished @@ -2314,8 +2329,6 @@ static void *player_thread_func(void *arg) { } if (outbuf) free(outbuf); - if (silence) - free(silence); if (tbuf) free(tbuf); if (sbuf) diff --git a/rtp.c b/rtp.c index 18bcc198..40c8bc8b 100644 --- a/rtp.c +++ b/rtp.c @@ -302,13 +302,15 @@ void *rtp_control_receiver(void *arg) { uint16_t flags = nctohs(&packet[2]); int64_t la = sync_rtp_timestamp - rtp_timestamp_less_latency; - debug(3, "Latency derived just from the sync packet is %" PRId64 " frames.", la); + // debug(3, "Latency derived just from the sync packet is %" PRId64 " frames.", la); if ((flags == 7) || ((conn->AirPlayVersion > 0) && (conn->AirPlayVersion <= 353))) { la += config.fixedLatencyOffset; - debug(3, "A fixed latency offset of %d frames has been added, giving a latency of " - "%" PRId64 - " frames with flags: %d and AirPlay version %d (triggers if 353 or less).", - config.fixedLatencyOffset, la, flags, conn->AirPlayVersion); + // debug(3, "A fixed latency offset of %d frames has been added, giving a latency of + // " + // "%" PRId64 + // " frames with flags: %d and AirPlay version %d (triggers if 353 or + // less).", + // config.fixedLatencyOffset, la, flags, conn->AirPlayVersion); } if ((conn->maximum_latency) && (conn->maximum_latency < la)) la = conn->maximum_latency; From 927191b16228a377707b1098b735edc0eca0cd82 Mon Sep 17 00:00:00 2001 From: Mike Brady Date: Mon, 7 May 2018 22:00:59 +0100 Subject: [PATCH 7/8] Stop erroneously assuming that the buffer passed to the player is not changed -- sometimes it is, it seems. Tidy up some typing. Revert to snd_pcm_drop from snd_pcm_drain. --- configure.ac | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index b76c1b0b..f664dc95 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.2d47i694,1], [mikebrady@eircom.net]) +AC_INIT([shairport-sync], [3.2d47i694,2], [mikebrady@eircom.net]) AM_INIT_AUTOMAKE AC_CONFIG_SRCDIR([shairport.c]) AC_CONFIG_HEADERS([config.h]) From 1ddaa14224b1fca512aa73586906392c7ad0c673 Mon Sep 17 00:00:00 2001 From: Mike Brady Date: Tue, 8 May 2018 20:26:26 +0100 Subject: [PATCH 8/8] Merge resolution to issue 694 (buzz during track change) into development branch --- configure.ac | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index 1b953518..66816c8a 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.2d46], [mikebrady@eircom.net]) +AC_INIT([shairport-sync], [3.2d47], [mikebrady@eircom.net]) AM_INIT_AUTOMAKE AC_CONFIG_SRCDIR([shairport.c]) AC_CONFIG_HEADERS([config.h])