From de071aeffc16d0d94b5c70741d55dfd9ff54edfb Mon Sep 17 00:00:00 2001 From: Mike Brady Date: Thu, 18 Oct 2018 17:44:50 +0100 Subject: [PATCH 01/16] Try to fix modulo calculations --- common.h | 4 +- player.c | 192 +++++++++++++++++++++++++------------------------------ player.h | 22 +++---- rtp.c | 40 ++++++------ rtp.h | 2 +- 5 files changed, 119 insertions(+), 141 deletions(-) diff --git a/common.h b/common.h index c5d03aa3..0042d2c4 100644 --- a/common.h +++ b/common.h @@ -134,8 +134,8 @@ typedef struct { char *mdns_name; mdns_backend *mdns; int buffer_start_fill; - int64_t userSuppliedLatency; // overrides all other latencies -- use with caution - int64_t fixedLatencyOffset; // add this to all automatic latencies supplied to get the actual + uint32_t userSuppliedLatency; // overrides all other latencies -- use with caution + uint32_t fixedLatencyOffset; // add this to all automatic latencies supplied to get the actual // total latency // the total latency will be limited to the min and max-latency values, if supplied int daemonise; diff --git a/player.c b/player.c index 457cd702..235b03ab 100644 --- a/player.c +++ b/player.c @@ -99,60 +99,15 @@ // static abuf_t audio_buffer[BUFFER_FRAMES]; #define BUFIDX(seqno) ((seq_t)(seqno) % BUFFER_FRAMES) -void do_flush(int64_t timestamp, rtsp_conn_info *conn); - -// make timestamps and seqnos definitely monotonic - -// add an epoch to the timestamp. The monotonic timestamp guaranteed to start between 2^32 and 2^33 -// frames and continue up to 2^63-1 frames -// if should never get into the negative range -// which is about 2*10^8 * 1,000 seconds at 384,000 frames per second -- about 2 trillion seconds or -// over 50,000 years. -// also, it won't reach zero until then, if ever, so we can safely say that a null monotonic -// timestamp can mean something special -int64_t monotonic_timestamp(uint32_t timestamp, rtsp_conn_info *conn) { - int64_t previous_value; - int64_t return_value; - if (conn->timestamp_epoch == 0) { - if (timestamp > conn->maximum_timestamp_interval) - conn->timestamp_epoch = 1; - else - conn->timestamp_epoch = 2; - previous_value = conn->timestamp_epoch; - previous_value <<= 32; - previous_value += timestamp; - } else { - previous_value = conn->timestamp_epoch; - previous_value <<= 32; - previous_value += conn->last_timestamp; - if (timestamp < conn->last_timestamp) { - // the incoming timestamp is less than the last one. - // if the difference is more than a minute, assume it's really from the next epoch - if ((conn->last_timestamp - timestamp) > conn->maximum_timestamp_interval) - conn->timestamp_epoch++; - } else { - // the incoming timestamp is greater than the last one. - // if the difference is more than a minute, assume it's really from the previous epoch - if ((timestamp - conn->last_timestamp) > conn->maximum_timestamp_interval) - conn->timestamp_epoch--; - } - } - return_value = conn->timestamp_epoch; - return_value <<= 32; - return_value += timestamp; - if (previous_value > return_value) { - if ((previous_value - return_value) > conn->maximum_timestamp_interval) - debug(2, "interval between successive rtptimes greater than allowed!"); - } else { - if ((return_value - previous_value) > conn->maximum_timestamp_interval) - debug(2, "interval between successive rtptimes greater than allowed!"); - } - if (return_value < 0) - debug(1, "monotonic rtptime is negative!"); - conn->last_timestamp = timestamp; - return return_value; +uint32_t rtp_frame_offset(uint32_t base, uint32_t frame_in_question) { + if (base <= frame_in_question) + return frame_in_question - base; + else + return UINT32_MAX - base + frame_in_question + 1; } +void do_flush(uint32_t timestamp, rtsp_conn_info *conn); + static void ab_resync(rtsp_conn_info *conn) { int i; for (i = 0; i < BUFFER_FRAMES; i++) { @@ -165,11 +120,36 @@ static void ab_resync(rtsp_conn_info *conn) { conn->ab_buffering = 1; } -// the sequence number is a 16-bit unsigned number which wraps pretty often -// to work out if one seqno is 'after' another therefore depends whether wrap has occurred -// this function works out the actual ordinate of the seqno, i.e. the distance up from -// the zeroth element, at ab_read, taking due account of wrap. +// given starting and ending points as unsigned 32-bit integers running modulo 2^32, returns the position of x in the interval in *pos +// returns true if x is actually within the buffer +int position_in_modulo_uint32_t_buffer(uint32_t x, uint32_t start, uint32_t end, uint32_t *pos ) { + int response = 0; // not in the buffer + if (start <= end ) { + if (x < start) { + if (pos) + *pos = UINT32_MAX - start + 1 + x; + } else { + if (pos) + *pos = x - start; + if (x < end) + response = 1; + } + } else if ((x >= start) && (x <= UINT32_MAX)) { + response = 1; + if (pos) + *pos = x - start; + } else { + if (pos) + *pos = UINT32_MAX - start + 1 + x; + if (x < end) { + response = 1; + } + } + return response; +} + +// this is used. static inline seq_t SUCCESSOR(seq_t x) { uint32_t p = x & 0xffff; p += 1; @@ -177,6 +157,7 @@ static inline seq_t SUCCESSOR(seq_t x) { return p; } +// this is not used static inline seq_t PREDECESSOR(seq_t x) { uint32_t p = (x & 0xffff) + 0x10000; p -= 1; @@ -184,6 +165,8 @@ static inline seq_t PREDECESSOR(seq_t x) { return p; } +// used in seq_diff and seq_order + // anything with ORDINATE in it must be proctected by the ab_mutex static inline int32_t ORDINATE(seq_t x, seq_t base) { int32_t p = x; // int32_t from seq_t, i.e. uint16_t, so okay @@ -458,14 +441,8 @@ static void free_audio_buffers(rtsp_conn_info *conn) { free(conn->audio_buffer[i].data); } -void player_put_packet(seq_t seqno, uint32_t actual_timestamp, int64_t timestamp, uint8_t *data, +void player_put_packet(seq_t seqno, uint32_t actual_timestamp, uint8_t *data, int len, rtsp_conn_info *conn) { - // all timestamps are done at the output rate - // the "actual_timestamp" is the one that comes in the packet, and is carried over for - // debugging - // and checking only. - - int64_t ltimestamp = timestamp * conn->output_sample_ratio; // ignore a request to flush that has been made before the first packet... if (conn->packet_count == 0) { @@ -483,22 +460,26 @@ void player_put_packet(seq_t seqno, uint32_t actual_timestamp, int64_t timestamp // if (flush_rtp_timestamp != 0) // debug(1,"Flush_rtp_timestamp is %u",flush_rtp_timestamp); - - if ((conn->flush_rtp_timestamp != 0) && (ltimestamp <= conn->flush_rtp_timestamp)) { - debug(3, "Dropping flushed packet in player_put_packet, seqno %u, timestamp %" PRId64 - ", flushing to " - "timestamp: %" PRId64 ".", - seqno, ltimestamp, conn->flush_rtp_timestamp); - conn->initial_reference_time = 0; - conn->initial_reference_timestamp = 0; + + // now, if a flush_rtp_timestamp has been defined and the incoming timestamp is "before" it, drop it… + + if ((conn->flush_rtp_timestamp != 0) && (rtp_frame_offset(actual_timestamp,conn->flush_rtp_timestamp) < 8820)) {// if it's less than 0.2 seconds + debug(2, "Dropping flushed packet in player_put_packet, seqno %u, timestamp %" PRIu32 + ", flushing to " + "timestamp: %" PRIu32 ".", + seqno, actual_timestamp, conn->flush_rtp_timestamp); + conn->initial_reference_time = 0; + conn->initial_reference_timestamp = 0; } else { - if ((conn->flush_rtp_timestamp != 0x0) && - (ltimestamp > conn->flush_rtp_timestamp)) // if we have gone past the flush boundary time - conn->flush_rtp_timestamp = 0x0; + if ((conn->flush_rtp_timestamp != 0) && (rtp_frame_offset(conn->flush_rtp_timestamp,actual_timestamp) > 8820) && (rtp_frame_offset(conn->flush_rtp_timestamp,actual_timestamp) < 44100)) { + debug(2,"Dropping flush request"); + conn->flush_rtp_timestamp = 0; + } abuf_t *abuf = 0; if (!conn->ab_synced) { + // if this is the first packet… debug(3, "syncing to seqno %u.", seqno); conn->ab_write = seqno; conn->ab_read = seqno; @@ -519,21 +500,21 @@ void player_put_packet(seq_t seqno, uint32_t actual_timestamp, int64_t timestamp conn->resend_interval = resend_interval; } - if (conn->ab_write == seqno) { // expected packet + if (conn->ab_write == seqno) { // if this is the expected packet (which could be the first packet…) uint64_t reception_time = get_absolute_time_in_fp(); if (conn->input_frame_rate_starting_point_is_valid == 0) { if ((conn->packet_count_since_flush >= 500) && (conn->packet_count_since_flush <= 510)) { conn->frames_inward_measurement_start_time = reception_time; - conn->frames_inward_frames_received_at_measurement_start_time = timestamp; + conn->frames_inward_frames_received_at_measurement_start_time = actual_timestamp; conn->input_frame_rate_starting_point_is_valid = 1; // valid now } } conn->frames_inward_measurement_time = reception_time; - conn->frames_inward_frames_received_at_measurement_time = timestamp; + conn->frames_inward_frames_received_at_measurement_time = actual_timestamp; abuf = conn->audio_buffer + BUFIDX(seqno); - conn->ab_write = SUCCESSOR(seqno); + conn->ab_write = SUCCESSOR(seqno); // move the write pointer to the next free space } else if (seq_order(conn->ab_write, seqno, conn->ab_read)) { // newer than expected // if (ORDINATE(seqno)>(BUFFER_FRAMES*7)/8) // debug(1,"An interval of %u frames has opened, with ab_read: %u, ab_write: %u and @@ -547,7 +528,7 @@ void player_put_packet(seq_t seqno, uint32_t actual_timestamp, int64_t timestamp abuf = conn->audio_buffer + BUFIDX(seq_sum(conn->ab_write, i)); abuf->ready = 0; // to be sure, to be sure abuf->resend_level = 0; - abuf->timestamp = 0; + // abuf->timestamp = 0; abuf->given_timestamp = 0; abuf->sequence_number = 0; } @@ -580,14 +561,14 @@ void player_put_packet(seq_t seqno, uint32_t actual_timestamp, int64_t timestamp if (alac_decode(abuf->data, &datalen, data, len, conn) == 0) { abuf->ready = 1; abuf->length = datalen; - abuf->timestamp = ltimestamp; + // abuf->timestamp = ltimestamp; abuf->given_timestamp = actual_timestamp; abuf->sequence_number = seqno; } else { debug(1, "Bad audio packet detected and discarded."); abuf->ready = 0; abuf->resend_level = 0; - abuf->timestamp = 0; + // abuf->timestamp = 0; abuf->given_timestamp = 0; abuf->sequence_number = 0; } @@ -621,12 +602,6 @@ void player_put_packet(seq_t seqno, uint32_t actual_timestamp, int64_t timestamp j)) { // prevent multiple requests from the same level of lookback check_buf->resend_level = j; if (config.disable_resend_requests == 0) { - if (((int)(resend_interval * pow(j + 1, step_exponent)) + k) >= - seq_diff(conn->ab_read, conn->ab_write, conn->ab_read)) - debug(3, "Last-ditch (#%d) resend request for packet %u in range %u to %u. " - "Looking back %d packets.", - j, next, conn->ab_read, conn->ab_write, back_step + k); - debug_mutex_unlock(&conn->ab_mutex, 3); rtp_request_resend(next, 1, conn); conn->resend_requests++; debug_mutex_lock(&conn->ab_mutex, 20000, 1); @@ -877,20 +852,23 @@ static abuf_t *buffer_get_frame(rtsp_conn_info *conn) { debug(1, "Inconsistent sequence numbers detected"); } } - - if ((conn->flush_rtp_timestamp != 0) && - (curframe->timestamp <= conn->flush_rtp_timestamp)) { - debug(2, "Dropping flushed packet in buffer_get_frame seqno %u, timestamp %" PRId64 ".", - curframe->sequence_number, curframe->timestamp); + + if ((conn->flush_rtp_timestamp != 0) && (rtp_frame_offset(curframe->given_timestamp,conn->flush_rtp_timestamp) < 8820)) {// if it's less than 0.2 seconds + debug(2, "Dropping flushed packet in buffer_get_frame, seqno %u, timestamp %" PRIu32 + ", flushing to " + "timestamp: %" PRIu32 ".", + curframe->sequence_number, curframe->given_timestamp, conn->flush_rtp_timestamp); curframe->ready = 0; curframe->resend_level = 0; - flush_limit++; + flush_limit += curframe->length; conn->ab_read = SUCCESSOR(conn->ab_read); conn->initial_reference_time = 0; conn->initial_reference_timestamp = 0; } - if (curframe->timestamp > conn->flush_rtp_timestamp) - conn->flush_rtp_timestamp = 0; + if ((conn->flush_rtp_timestamp != 0) && (rtp_frame_offset(conn->flush_rtp_timestamp,curframe->given_timestamp) > 8820) && (rtp_frame_offset(conn->flush_rtp_timestamp,curframe->given_timestamp) < 44100)) { + debug(2,"Dropping flush request"); + conn->flush_rtp_timestamp = 0; + } } } while ((conn->flush_rtp_timestamp != 0) && (flush_limit <= 8820) && (curframe->ready == 0)); @@ -921,7 +899,7 @@ static abuf_t *buffer_get_frame(rtsp_conn_info *conn) { if (have_timestamp_timing_information(conn)) { // if we have a reference time // debug(1,"First frame seen with timestamp..."); conn->first_packet_timestamp = - curframe->timestamp; // we will keep buffering until we are + curframe->given_timestamp; // we will keep buffering until we are // supposed to start playing this have_sent_prefiller_silence = 0; conn->packet_stream_established = 1; @@ -1174,7 +1152,7 @@ static abuf_t *buffer_get_frame(rtsp_conn_info *conn) { // Note: the last three items are expressed in frames and must be converted to time. int do_wait = 0; // don't wait unless we can really prove we must - if ((conn->ab_synced) && (curframe) && (curframe->ready) && (curframe->timestamp)) { + if ((conn->ab_synced) && (curframe) && (curframe->ready) && (curframe->given_timestamp)) { do_wait = 1; // if the current frame exists and is ready, then wait unless it's time to let it go... @@ -1184,7 +1162,7 @@ static abuf_t *buffer_get_frame(rtsp_conn_info *conn) { uint64_t time_to_play; frame_to_local_time( - curframe->timestamp + conn->latency * conn->output_sample_ratio + + (curframe->given_timestamp + conn->latency) * conn->output_sample_ratio + (int64_t)(config.audio_backend_latency_offset * config.output_rate) - config.audio_backend_buffer_desired_length * config.output_rate, &time_to_play, conn); @@ -1242,7 +1220,7 @@ static abuf_t *buffer_get_frame(rtsp_conn_info *conn) { if (!curframe->ready) { // debug(1, "Supplying a silent frame for frame %u", read); conn->missing_packets++; - curframe->timestamp = 0; // indicate a silent frame should be substituted + curframe->given_timestamp = 0; // indicate a silent frame should be substituted } curframe->ready = 0; curframe->resend_level = 0; @@ -1791,7 +1769,7 @@ void *player_thread_func(void *arg) { // if (play_number % 100 == 0) // debug(3, "Play frame %d.", play_number); conn->play_number_after_flush++; - if (inframe->timestamp == 0) { + if (inframe->given_timestamp == 0) { debug(3, "Player has supplied a silent frame, (possibly frame %u) for play number %d.", SUCCESSOR(conn->last_seqno_read), play_number); conn->last_seqno_read = (SUCCESSOR(conn->last_seqno_read) & @@ -1925,14 +1903,16 @@ void *player_thread_func(void *arg) { // now, go back as far as the total latency less, say, 100 ms, and check the presence of // frames from then onwards - int64_t reference_timestamp; + uint32_t reference_timestamp; uint64_t reference_timestamp_time, remote_reference_timestamp_time; get_reference_timestamp_stuff(&reference_timestamp, &reference_timestamp_time, &remote_reference_timestamp_time, conn); // types okay reference_timestamp *= conn->output_sample_ratio; int64_t rt, nt; rt = reference_timestamp; // uint32_t to int64_t - nt = inframe->timestamp; // uint32_t to int64_t + nt = inframe->given_timestamp; // uint32_t to int64_t + rt *= conn->output_sample_ratio; + nt *= conn->output_sample_ratio; uint64_t local_time_now = get_absolute_time_in_fp(); // types okay // struct timespec tn; @@ -2056,7 +2036,7 @@ void *player_thread_func(void *arg) { if (abs_sync_error < 0) abs_sync_error = -abs_sync_error; - if ((config.no_sync == 0) && (inframe->timestamp != 0) && + if ((config.no_sync == 0) && (inframe->given_timestamp != 0) && (config.resyncthreshold > 0.0) && (abs_sync_error > config.resyncthreshold * config.output_rate)) { if (abs_sync_error > 3 * config.output_rate) { @@ -2282,7 +2262,7 @@ void *player_thread_func(void *arg) { } // mark the frame as finished - inframe->timestamp = 0; + inframe->given_timestamp = 0; inframe->sequence_number = 0; // debug(1,"Sync error %lld frames. Amount to stuff %d." ,sync_error,amount_to_stuff); @@ -2726,7 +2706,7 @@ void player_volume(double airplay_volume, rtsp_conn_info *conn) { player_volume_without_notification(airplay_volume, conn); } -void do_flush(int64_t timestamp, rtsp_conn_info *conn) { +void do_flush(uint32_t timestamp, rtsp_conn_info *conn) { debug(3, "Flush requested up to %u. It seems as if 0 is special.", timestamp); debug_mutex_lock(&conn->flush_mutex, 1000, 1); @@ -2755,7 +2735,7 @@ void do_flush(int64_t timestamp, rtsp_conn_info *conn) { debug(3, "Flush request made."); } -void player_flush(int64_t timestamp, rtsp_conn_info *conn) { +void player_flush(uint32_t timestamp, rtsp_conn_info *conn) { debug(3, "player_flush"); do_flush(timestamp, conn); } diff --git a/player.h b/player.h index fe7077b1..6c10d34d 100644 --- a/player.h +++ b/player.h @@ -40,7 +40,7 @@ typedef uint16_t seq_t; typedef struct audio_buffer_entry { // decoded audio packets int ready; int resend_level; - int64_t timestamp; + // int64_t timestamp; seq_t sequence_number; uint32_t given_timestamp; // for debugging and checking signed short *data; @@ -71,10 +71,10 @@ typedef struct { int connection_number; // for debug ID purposes, nothing else... int resend_interval; // this is really just for debugging int AirPlayVersion; // zero if not an AirPlay session. Used to help calculate latency - int64_t latency; // the actual latency used for this play session - int64_t minimum_latency; // set if an a=min-latency: line appears in the ANNOUNCE message; zero + uint32_t latency; // the actual latency used for this play session + uint32_t minimum_latency; // set if an a=min-latency: line appears in the ANNOUNCE message; zero // otherwise - int64_t maximum_latency; // set if an a=max-latency: line appears in the ANNOUNCE message; zero + uint32_t maximum_latency; // set if an a=max-latency: line appears in the ANNOUNCE message; zero // otherwise int fd; @@ -103,10 +103,10 @@ typedef struct { int input_frame_rate_starting_point_is_valid; uint64_t frames_inward_measurement_start_time; - uint64_t frames_inward_frames_received_at_measurement_start_time; + uint32_t frames_inward_frames_received_at_measurement_start_time; uint64_t frames_inward_measurement_time; - uint64_t frames_inward_frames_received_at_measurement_time; + uint32_t frames_inward_frames_received_at_measurement_time; // other stuff... pthread_t *player_thread; @@ -136,7 +136,7 @@ typedef struct { int ab_buffering, ab_synced; int64_t first_packet_timestamp; int flush_requested; - int64_t flush_rtp_timestamp; + uint32_t flush_rtp_timestamp; uint64_t time_of_last_audio_packet; seq_t ab_read, ab_write; @@ -187,7 +187,7 @@ typedef struct { // this is what connects an rtp timestamp to the remote time - int64_t reference_timestamp; + uint32_t reference_timestamp; uint64_t remote_reference_timestamp_time; int packet_stream_established; // true if a stream of packets is flowing, made true by a first @@ -195,7 +195,7 @@ typedef struct { // used as the initials values for calculating the rate at which the source thinks it's sending // frames - int64_t initial_reference_timestamp; + uint32_t initial_reference_timestamp; uint64_t initial_reference_time; double remote_frame_rate; @@ -247,8 +247,8 @@ int player_stop(rtsp_conn_info *conn); void player_volume(double f, rtsp_conn_info *conn); void player_volume_without_notification(double f, rtsp_conn_info *conn); -void player_flush(int64_t timestamp, rtsp_conn_info *conn); -void player_put_packet(seq_t seqno, uint32_t actual_timestamp, int64_t timestamp, uint8_t *data, +void player_flush(uint32_t timestamp, rtsp_conn_info *conn); +void player_put_packet(seq_t seqno, uint32_t actual_timestamp, uint8_t *data, int len, rtsp_conn_info *conn); int64_t monotonic_timestamp(uint32_t timestamp, rtsp_conn_info *conn); // add an epoch to the timestamp. The monotonic diff --git a/rtp.c b/rtp.c index b8c8ee52..4cd96aba 100644 --- a/rtp.c +++ b/rtp.c @@ -179,7 +179,6 @@ void *rtp_audio_receiver(void *arg) { } uint32_t actual_timestamp = ntohl(*(uint32_t *)(pktp + 4)); - int64_t timestamp = monotonic_timestamp(actual_timestamp, conn); // if (packet[1]&0x10) // debug(1,"Audio packet Extension bit set."); @@ -191,7 +190,7 @@ void *rtp_audio_receiver(void *arg) { if (plen >= 16) { if ((config.diagnostic_drop_packet_fraction == 0.0) || (drand48() > config.diagnostic_drop_packet_fraction)) - player_put_packet(seqno, actual_timestamp, timestamp, pktp, plen, conn); + player_put_packet(seqno, actual_timestamp, pktp, plen, conn); else debug(3, "Dropping audio packet %u to simulate a bad connection.", seqno); continue; @@ -238,7 +237,7 @@ void *rtp_control_receiver(void *arg) { uint8_t packet[2048], *pktp; // struct timespec tn; uint64_t remote_time_of_sync; - int64_t sync_rtp_timestamp; + uint32_t sync_rtp_timestamp; ssize_t nread; while (1) { nread = recv(conn->control_socket, packet, sizeof(packet), 0); @@ -304,14 +303,14 @@ void *rtp_control_receiver(void *arg) { // debug(1,"Remote Sync Time: %0llx.",remote_time_of_sync); - sync_rtp_timestamp = monotonic_timestamp(nctohl(&packet[16]), conn); - int64_t rtp_timestamp_less_latency = monotonic_timestamp(nctohl(&packet[4]), conn); + sync_rtp_timestamp = nctohl(&packet[16]); + uint32_t rtp_timestamp_less_latency = nctohl(&packet[4]); // debug(1,"Sync timestamp is %u.",ntohl(*((uint32_t *)&packet[16]))); if (config.userSuppliedLatency) { if (config.userSuppliedLatency != conn->latency) { - debug(1, "Using the user-supplied latency: %" PRId64 ".", + debug(1, "Using the user-supplied latency: %" PRIu32 ".", config.userSuppliedLatency); } conn->latency = config.userSuppliedLatency; @@ -330,8 +329,8 @@ void *rtp_control_receiver(void *arg) { // Sigh, it would be nice to have a published protocol... 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); + uint32_t la = sync_rtp_timestamp - rtp_timestamp_less_latency; // note, this might loop around in modulo. Not sure if you'll get an error! + // debug(3, "Latency derived just from the sync packet is %" PRIu32 " frames.", la); if ((flags == 7) || ((conn->AirPlayVersion > 0) && (conn->AirPlayVersion <= 353)) || ((conn->AirPlayVersion > 0) && (conn->AirPlayVersion >= 371))) { la += config.fixedLatencyOffset; @@ -347,20 +346,20 @@ void *rtp_control_receiver(void *arg) { if ((conn->minimum_latency) && (conn->minimum_latency > la)) la = conn->minimum_latency; - const int max_frames = ((3 * BUFFER_FRAMES * 352) / 4) - 11025; + const uint32_t max_frames = ((3 * BUFFER_FRAMES * 352) / 4) - 11025; - if ((la < 0) || (la > max_frames)) { - warn("An out-of-range latency request of %" PRId64 - " frames was ignored. Must be %d frames or less (44,100 frames per second). " - "Latency remains at %" PRId64 " frames.", + if (la > max_frames) { + warn("An out-of-range latency request of %" PRIu32 + " frames was ignored. Must be %" PRIu32 " frames or less (44,100 frames per second). " + "Latency remains at %" PRIu32 " frames.", la, max_frames, conn->latency); } else { if (la != conn->latency) { conn->latency = la; - debug(3, "New latency detected: %" PRId64 ", sync latency: %" PRId64 - ", minimum latency: %" PRId64 ", maximum " - "latency: %" PRId64 ", fixed offset: %" PRId64 ".", + debug(3, "New latency detected: %" PRIu32 ", sync latency: %" PRIu32 + ", minimum latency: %" PRIu32 ", maximum " + "latency: %" PRIu32 ", fixed offset: %" PRIu32 ".", la, sync_rtp_timestamp - rtp_timestamp_less_latency, conn->minimum_latency, conn->maximum_latency, config.fixedLatencyOffset); } @@ -380,7 +379,7 @@ void *rtp_control_receiver(void *arg) { if (remote_frame_time_interval) { conn->remote_frame_rate = (1.0 * (conn->reference_timestamp - conn->initial_reference_timestamp)) / - remote_frame_time_interval; // an IEEE double calculation with two 64-bit + remote_frame_time_interval; // an IEEE double calculation with a 32-bit numerator and 64-bit denominator // integers conn->remote_frame_rate = conn->remote_frame_rate * (uint64_t)0x100000000; // this should just change the @@ -395,7 +394,7 @@ void *rtp_control_receiver(void *arg) { // this is for debugging uint64_t old_remote_reference_time = conn->remote_reference_timestamp_time; - int64_t old_reference_timestamp = conn->reference_timestamp; + uint32_t old_reference_timestamp = conn->reference_timestamp; // int64_t old_latency_delayed_timestamp = conn->latency_delayed_timestamp; conn->remote_reference_timestamp_time = remote_time_of_sync; @@ -444,14 +443,13 @@ void *rtp_control_receiver(void *arg) { debug(3, "Control Receiver -- Retransmitted Audio Data Packet %u received.", seqno); uint32_t actual_timestamp = ntohl(*(uint32_t *)(pktp + 4)); - int64_t timestamp = monotonic_timestamp(actual_timestamp, conn); pktp += 12; plen -= 12; // check if packet contains enough content to be reasonable if (plen >= 16) { - player_put_packet(seqno, actual_timestamp, timestamp, pktp, plen, conn); + player_put_packet(seqno, actual_timestamp, pktp, plen, conn); continue; } else { debug(3, "Too-short retransmitted audio packet received in control port, ignored."); @@ -1004,7 +1002,7 @@ void rtp_setup(SOCKADDR *local, SOCKADDR *remote, uint16_t cport, uint16_t tport } } -void get_reference_timestamp_stuff(int64_t *timestamp, uint64_t *timestamp_time, +void get_reference_timestamp_stuff(uint32_t *timestamp, uint64_t *timestamp_time, uint64_t *remote_timestamp_time, rtsp_conn_info *conn) { // types okay debug_mutex_lock(&conn->reference_time_mutex, 1000, 1); diff --git a/rtp.h b/rtp.h index 13bf2b3f..9bf5c696 100644 --- a/rtp.h +++ b/rtp.h @@ -17,7 +17,7 @@ void rtp_setup(SOCKADDR *local, SOCKADDR *remote, uint16_t controlport, uint16_t void rtp_request_resend(seq_t first, uint32_t count, rtsp_conn_info *conn); void rtp_request_client_pause(rtsp_conn_info *conn); // ask the client to pause -void get_reference_timestamp_stuff(int64_t *timestamp, uint64_t *timestamp_time, +void get_reference_timestamp_stuff(uint32_t *timestamp, uint64_t *timestamp_time, uint64_t *remote_timestamp_time, rtsp_conn_info *conn); void clear_reference_timestamp(rtsp_conn_info *conn); From b53fc39dd2e29dc18cb44ba3d4510c13c1a3c893 Mon Sep 17 00:00:00 2001 From: Mike Brady Date: Fri, 19 Oct 2018 14:04:30 +0100 Subject: [PATCH 02/16] Replace a mutex unlock deleted by accident. --- player.c | 1 + 1 file changed, 1 insertion(+) diff --git a/player.c b/player.c index 235b03ab..a96fc845 100644 --- a/player.c +++ b/player.c @@ -602,6 +602,7 @@ void player_put_packet(seq_t seqno, uint32_t actual_timestamp, uint8_t *data, j)) { // prevent multiple requests from the same level of lookback check_buf->resend_level = j; if (config.disable_resend_requests == 0) { + debug_mutex_unlock(&conn->ab_mutex, 3); rtp_request_resend(next, 1, conn); conn->resend_requests++; debug_mutex_lock(&conn->ab_mutex, 20000, 1); From 6de64d5954e4223615784584b5906c4bd83e5da3 Mon Sep 17 00:00:00 2001 From: Mike Brady Date: Fri, 19 Oct 2018 14:05:59 +0100 Subject: [PATCH 03/16] Replace the entire timestamp processing software -- preliminary attempt --- configure.ac | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index cfda987c..c66718a8 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.3d16], [mikebrady@eircom.net]) +AC_INIT([shairport-sync], [3.3d17], [mikebrady@eircom.net]) AM_INIT_AUTOMAKE AC_CONFIG_SRCDIR([shairport.c]) AC_CONFIG_HEADERS([config.h]) From d2dba7cdba6ddbe433563db6c9aa2bd493f7ad4b Mon Sep 17 00:00:00 2001 From: Mike Brady Date: Fri, 19 Oct 2018 22:14:23 +0100 Subject: [PATCH 04/16] Seems to work for higher rates --- player.c | 38 +++++++++++++++++++------------------- player.h | 2 ++ rtp.c | 38 +++++++++++++++++--------------------- rtp.h | 6 +++--- 4 files changed, 41 insertions(+), 43 deletions(-) diff --git a/player.c b/player.c index a96fc845..7c055a5c 100644 --- a/player.c +++ b/player.c @@ -99,11 +99,11 @@ // static abuf_t audio_buffer[BUFFER_FRAMES]; #define BUFIDX(seqno) ((seq_t)(seqno) % BUFFER_FRAMES) -uint32_t rtp_frame_offset(uint32_t base, uint32_t frame_in_question) { - if (base <= frame_in_question) - return frame_in_question - base; +uint32_t rtp_frame_offset(uint32_t from, uint32_t to) { + if (from <= to) + return to - from; else - return UINT32_MAX - base + frame_in_question + 1; + return UINT32_MAX - from + to + 1; } void do_flush(uint32_t timestamp, rtsp_conn_info *conn); @@ -944,8 +944,8 @@ static abuf_t *buffer_get_frame(rtsp_conn_info *conn) { uint64_t should_be_time; frame_to_local_time( - conn->first_packet_timestamp + conn->latency * conn->output_sample_ratio + - (int64_t)(config.audio_backend_latency_offset * config.output_rate), + conn->first_packet_timestamp + conn->latency + + (uint32_t)(config.audio_backend_latency_offset * conn->input_rate), // this will go modulo 2^32 &should_be_time, conn); conn->first_packet_time_to_play = should_be_time; @@ -965,8 +965,8 @@ static abuf_t *buffer_get_frame(rtsp_conn_info *conn) { uint64_t should_be_time; frame_to_local_time( - conn->first_packet_timestamp + conn->latency * conn->output_sample_ratio + - (int64_t)(config.audio_backend_latency_offset * config.output_rate), + conn->first_packet_timestamp + conn->latency + + (uint32_t)(config.audio_backend_latency_offset * conn->input_rate), // this should go modulo 2^32 &should_be_time, conn); conn->first_packet_time_to_play = should_be_time; @@ -1163,9 +1163,9 @@ static abuf_t *buffer_get_frame(rtsp_conn_info *conn) { uint64_t time_to_play; frame_to_local_time( - (curframe->given_timestamp + conn->latency) * conn->output_sample_ratio + - (int64_t)(config.audio_backend_latency_offset * config.output_rate) - - config.audio_backend_buffer_desired_length * config.output_rate, + curframe->given_timestamp + conn->latency + + (uint32_t)(config.audio_backend_latency_offset * conn->input_rate) - + (uint32_t)(config.audio_backend_buffer_desired_length * conn->input_rate), // this will go modulo 2^32 &time_to_play, conn); if (local_time_now >= time_to_play) { @@ -1908,7 +1908,6 @@ void *player_thread_func(void *arg) { uint64_t reference_timestamp_time, remote_reference_timestamp_time; get_reference_timestamp_stuff(&reference_timestamp, &reference_timestamp_time, &remote_reference_timestamp_time, conn); // types okay - reference_timestamp *= conn->output_sample_ratio; int64_t rt, nt; rt = reference_timestamp; // uint32_t to int64_t nt = inframe->given_timestamp; // uint32_t to int64_t @@ -2000,8 +1999,9 @@ void *player_thread_func(void *arg) { if (resp >= 0) { - int64_t should_be_frame; - local_time_to_frame(local_time_now, &should_be_frame, conn); + uint32_t should_be_frame_32; + local_time_to_frame(local_time_now, &should_be_frame_32, conn); + int64_t should_be_frame = ((int64_t)should_be_frame_32) * conn->output_sample_ratio; // int64_t absolute_difference_in_frames = td_in_frames + rt - should_be_frame; // if (absolute_difference_in_frames < 0) @@ -2041,11 +2041,11 @@ void *player_thread_func(void *arg) { (config.resyncthreshold > 0.0) && (abs_sync_error > config.resyncthreshold * config.output_rate)) { if (abs_sync_error > 3 * config.output_rate) { - warn("Very large sync error: %" PRId64 " frames, with delay: %" PRId64 - ", td_in_frames: %" PRId64 ", rt: %" PRId64 ", nt: %" PRId64 - ", current_delay: %" PRId64 ", seqno: %u, given timestamp: %" PRIu32 ".", - sync_error, delay, td_in_frames, rt, nt, current_delay, - inframe->sequence_number, inframe->given_timestamp); + + warn("Very large sync error: %" PRId64 " frames, with should_be_frame: %" PRId64 + ", nt: %" PRId64 + ", current_delay: %" PRId64 ", given timestamp %" PRIX32 ", reference timestamp %" PRIX32 ", should_be_frame %" PRIX32 ".", + sync_error, should_be_frame, nt, current_delay, inframe->given_timestamp, reference_timestamp, should_be_frame_32); } sync_error_out_of_bounds++; } else { diff --git a/player.h b/player.h index 6c10d34d..43d93c05 100644 --- a/player.h +++ b/player.h @@ -242,6 +242,8 @@ typedef struct { void *dapo_private_storage; // this is used for compatibility, if dacp stuff isn't enabled. } rtsp_conn_info; +uint32_t rtp_frame_offset(uint32_t from, uint32_t to); + int player_play(rtsp_conn_info *conn); int player_stop(rtsp_conn_info *conn); diff --git a/rtp.c b/rtp.c index 4cd96aba..599d5a48 100644 --- a/rtp.c +++ b/rtp.c @@ -1034,13 +1034,14 @@ int have_timestamp_timing_information(rtsp_conn_info *conn) { // right... const int use_nominal_rate = 0; // specify whether to use the nominal input rate, usually 44100 fps -int sanitised_source_rate_information(int64_t *frames, uint64_t *time, rtsp_conn_info *conn) { +int sanitised_source_rate_information(uint32_t *frames, uint64_t *time, rtsp_conn_info *conn) { int result = 1; *frames = conn->input_rate; *time = (uint64_t)(0x100000000); // one second in fp form if ((conn->packet_stream_established) && (conn->initial_reference_time) && (conn->initial_reference_timestamp)) { - int64_t local_frames = conn->reference_timestamp - conn->initial_reference_timestamp; +// uint32_t local_frames = conn->reference_timestamp - conn->initial_reference_timestamp; + uint32_t local_frames = rtp_frame_offset(conn->initial_reference_timestamp, conn->reference_timestamp); uint64_t local_time = conn->remote_reference_timestamp_time - conn->initial_reference_time; if ((local_frames == 0) || (local_time == 0) || (use_nominal_rate)) { result = 1; @@ -1060,39 +1061,34 @@ int sanitised_source_rate_information(int64_t *frames, uint64_t *time, rtsp_conn return result; } -// we assume here that the timestamp is a timestamp calculated at the output rate, which could be an -// integer multiple of the input rate +// the timestamp is a timestamp calculated at the input rate // the reference timestamps are denominated in terms of the input rate -int frame_to_local_time(int64_t timestamp, uint64_t *time, rtsp_conn_info *conn) { +int frame_to_local_time(uint32_t timestamp, uint64_t *time, rtsp_conn_info *conn) { debug_mutex_lock(&conn->reference_time_mutex, 1000, 1); int result = 0; uint64_t time_difference; - int64_t frame_difference; + uint32_t frame_difference; result = sanitised_source_rate_information(&frame_difference, &time_difference, conn); - int64_t timestamp_interval = - timestamp - - conn->reference_timestamp * - conn->output_sample_ratio; // we could be dealing with multiples of 44100 (nominally) - // debug(1, "Timestamp interval: %" PRId64 " frames with reference timestamp %" PRId64 - // ".",timestamp_interval,conn->reference_timestamp); uint64_t timestamp_interval_time; uint64_t remote_time_of_timestamp; - if (timestamp_interval >= 0) { + uint32_t timestamp_interval = rtp_frame_offset(conn->reference_timestamp,timestamp); + if (timestamp_interval <= 44100*10) { // i.e. timestamp was really after the reference timestamp timestamp_interval_time = (timestamp_interval * time_difference) / - (frame_difference * conn->output_sample_ratio); // this is the nominal time, based on the + (frame_difference); // this is the nominal time, based on the // fps specified between current and // previous sync frame. remote_time_of_timestamp = conn->remote_reference_timestamp_time + timestamp_interval_time; // based on the reference timestamp time // plus the time interval calculated based // on the specified fps. - } else { + } else { // i.e. timestamp was actually after the reference timestamp + timestamp_interval = rtp_frame_offset(timestamp,conn->reference_timestamp); // fix the calculation timestamp_interval_time = - ((-timestamp_interval) * time_difference) / - (frame_difference * conn->output_sample_ratio); // this is the nominal time, based on the + (timestamp_interval * time_difference) / + (frame_difference); // this is the nominal time, based on the // fps specified between current and // previous sync frame. remote_time_of_timestamp = conn->remote_reference_timestamp_time - @@ -1105,12 +1101,12 @@ int frame_to_local_time(int64_t timestamp, uint64_t *time, rtsp_conn_info *conn) return result; } -int local_time_to_frame(uint64_t time, int64_t *frame, rtsp_conn_info *conn) { +int local_time_to_frame(uint64_t time, uint32_t *frame, rtsp_conn_info *conn) { debug_mutex_lock(&conn->reference_time_mutex, 1000, 1); int result = 0; uint64_t time_difference; - int64_t frame_difference; + uint32_t frame_difference; result = sanitised_source_rate_information(&frame_difference, &time_difference, conn); // first, get from [local] time to remote time. @@ -1129,10 +1125,10 @@ int local_time_to_frame(uint64_t time, int64_t *frame, rtsp_conn_info *conn) { int64_t frame_interval = (time_interval * frame_difference) / time_difference; if (remote_time >= conn->remote_reference_timestamp_time) { // debug(1,"Frame interval is %" PRId64 " frames.",frame_interval); - *frame = (conn->reference_timestamp + frame_interval) * conn->output_sample_ratio; + *frame = (conn->reference_timestamp + frame_interval); } else { // debug(1,"Frame interval is %" PRId64 " frames.",-frame_interval); - *frame = (conn->reference_timestamp - frame_interval) * conn->output_sample_ratio; + *frame = (conn->reference_timestamp - frame_interval); } debug_mutex_unlock(&conn->reference_time_mutex, 3); return result; diff --git a/rtp.h b/rtp.h index 9bf5c696..171bd651 100644 --- a/rtp.h +++ b/rtp.h @@ -25,9 +25,9 @@ int have_timestamp_timing_information(rtsp_conn_info *conn); int get_frame_play_time(int64_t timestamp, int sample_ratio, uint64_t *time_to_play); -int frame_to_local_time(int64_t timestamp, uint64_t *time, rtsp_conn_info *conn); -int local_time_to_frame(uint64_t time, int64_t *frame, rtsp_conn_info *conn); +int frame_to_local_time(uint32_t timestamp, uint64_t *time, rtsp_conn_info *conn); +int local_time_to_frame(uint64_t time, uint32_t *frame, rtsp_conn_info *conn); -int sanitised_source_rate_information(int64_t *frames, uint64_t *time, rtsp_conn_info *conn); +int sanitised_source_rate_information(uint32_t *frames, uint64_t *time, rtsp_conn_info *conn); #endif // _RTP_H From a5f165d982774665bec5988085579c65c2b03f3d Mon Sep 17 00:00:00 2001 From: Mike Brady Date: Sun, 21 Oct 2018 16:31:55 +0100 Subject: [PATCH 05/16] Use a parameter rather than a hard-wired input rate. --- rtp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rtp.c b/rtp.c index 599d5a48..e2c8f944 100644 --- a/rtp.c +++ b/rtp.c @@ -1074,7 +1074,7 @@ int frame_to_local_time(uint32_t timestamp, uint64_t *time, rtsp_conn_info *conn uint64_t timestamp_interval_time; uint64_t remote_time_of_timestamp; uint32_t timestamp_interval = rtp_frame_offset(conn->reference_timestamp,timestamp); - if (timestamp_interval <= 44100*10) { // i.e. timestamp was really after the reference timestamp + if (timestamp_interval <= conn->input_rate*10) { // i.e. timestamp was really after the reference timestamp timestamp_interval_time = (timestamp_interval * time_difference) / (frame_difference); // this is the nominal time, based on the From e5259e8764cc35b6f7ea510a8a2627eb7b87305c Mon Sep 17 00:00:00 2001 From: Mike Brady Date: Sun, 21 Oct 2018 17:00:56 +0100 Subject: [PATCH 06/16] Remove a few compiler warnings. --- player.c | 2 +- player.h | 2 +- rtsp.c | 32 ++++++++++++++++---------------- 3 files changed, 18 insertions(+), 18 deletions(-) diff --git a/player.c b/player.c index 7c055a5c..ac799409 100644 --- a/player.c +++ b/player.c @@ -1610,7 +1610,7 @@ void *player_thread_func(void *arg) { signed short *inbuf; int inbuflength; - int output_bit_depth = 16; // default; + unsigned int output_bit_depth = 16; // default; switch (config.output_format) { case SPS_FORMAT_S8: diff --git a/player.h b/player.h index 43d93c05..5c0adc24 100644 --- a/player.h +++ b/player.h @@ -111,7 +111,7 @@ typedef struct { // other stuff... pthread_t *player_thread; abuf_t audio_buffer[BUFFER_FRAMES]; - int max_frames_per_packet, input_num_channels, input_bit_depth, input_rate; + unsigned int max_frames_per_packet, input_num_channels, input_bit_depth, input_rate; int input_bytes_per_frame, output_bytes_per_frame, output_sample_ratio; int max_frame_size_change; int64_t previous_random_number; diff --git a/rtsp.c b/rtsp.c index 32759d43..834e56e8 100644 --- a/rtsp.c +++ b/rtsp.c @@ -285,7 +285,7 @@ void cancel_all_RTSP_threads(void) { } } -static void cleanup_threads(void) { +void cleanup_threads(void) { void *retval; int i; // debug(2, "culling threads."); @@ -348,7 +348,7 @@ static char *nextline(char *in, int inbuf) { return out; } -static void msg_retain(rtsp_message *msg) { +void msg_retain(rtsp_message *msg) { if (msg) { int rc = pthread_mutex_lock(&reference_counter_lock); if (rc) @@ -362,7 +362,7 @@ static void msg_retain(rtsp_message *msg) { } } -static rtsp_message *msg_init(void) { +rtsp_message *msg_init(void) { rtsp_message *msg = malloc(sizeof(rtsp_message)); if (msg) { memset(msg, 0, sizeof(rtsp_message)); @@ -373,7 +373,7 @@ static rtsp_message *msg_init(void) { return msg; } -static int msg_add_header(rtsp_message *msg, char *name, char *value) { +int msg_add_header(rtsp_message *msg, char *name, char *value) { if (msg->nheaders >= sizeof(msg->name) / sizeof(char *)) { warn("too many headers?!"); return 1; @@ -386,7 +386,7 @@ static int msg_add_header(rtsp_message *msg, char *name, char *value) { return 0; } -static char *msg_get_header(rtsp_message *msg, char *name) { +char *msg_get_header(rtsp_message *msg, char *name) { unsigned int i; for (i = 0; i < msg->nheaders; i++) if (!strcasecmp(msg->name[i], name)) @@ -394,7 +394,7 @@ static char *msg_get_header(rtsp_message *msg, char *name) { return NULL; } -static void debug_print_msg_headers(int level, rtsp_message *msg) { +void debug_print_msg_headers(int level, rtsp_message *msg) { unsigned int i; for (i = 0; i < msg->nheaders; i++) { debug(level, " Type: \"%s\", content: \"%s\"", msg->name[i], msg->value[i]); @@ -424,7 +424,7 @@ static void debug_print_msg_content(int level, rtsp_message *msg) { } */ -static void msg_free(rtsp_message *msg) { +void msg_free(rtsp_message *msg) { if (msg) { int rc = pthread_mutex_lock(&reference_counter_lock); @@ -452,7 +452,7 @@ static void msg_free(rtsp_message *msg) { } } -static int msg_handle_line(rtsp_message **pmsg, char *line) { +int msg_handle_line(rtsp_message **pmsg, char *line) { rtsp_message *msg = *pmsg; if (!msg) { @@ -507,7 +507,7 @@ fail: return 0; } -static enum rtsp_read_request_response rtsp_read_request(rtsp_conn_info *conn, +enum rtsp_read_request_response rtsp_read_request(rtsp_conn_info *conn, rtsp_message **the_packet) { enum rtsp_read_request_response reply = rtsp_read_request_response_ok; ssize_t buflen = 4096; @@ -649,7 +649,7 @@ shutdown: return reply; } -static void msg_write_response(int fd, rtsp_message *resp) { +void msg_write_response(int fd, rtsp_message *resp) { char pkt[2048]; int pktfree = sizeof(pkt); char *p = pkt; @@ -697,7 +697,7 @@ static void msg_write_response(int fd, rtsp_message *resp) { debug(1, "Error writing an RTSP packet -- requested bytes not fully written."); } -static void handle_record(rtsp_conn_info *conn, rtsp_message *req, rtsp_message *resp) { +void handle_record(rtsp_conn_info *conn, rtsp_message *req, rtsp_message *resp) { debug(2, "Connection %d: RECORD", conn->connection_number); if (conn->player_thread) @@ -737,7 +737,7 @@ static void handle_record(rtsp_conn_info *conn, rtsp_message *req, rtsp_message } } -static void handle_options(rtsp_conn_info *conn, __attribute__((unused)) rtsp_message *req, +void handle_options(rtsp_conn_info *conn, __attribute__((unused)) rtsp_message *req, rtsp_message *resp) { debug(3, "Connection %d: OPTIONS", conn->connection_number); resp->respcode = 200; @@ -746,7 +746,7 @@ static void handle_options(rtsp_conn_info *conn, __attribute__((unused)) rtsp_me "OPTIONS, GET_PARAMETER, SET_PARAMETER"); } -static void handle_teardown(rtsp_conn_info *conn, __attribute__((unused)) rtsp_message *req, +void handle_teardown(rtsp_conn_info *conn, __attribute__((unused)) rtsp_message *req, rtsp_message *resp) { debug(2, "Connection %d: TEARDOWN", conn->connection_number); // if (!rtsp_playing()) @@ -763,7 +763,7 @@ static void handle_teardown(rtsp_conn_info *conn, __attribute__((unused)) rtsp_m conn->connection_number); } -static void handle_flush(rtsp_conn_info *conn, rtsp_message *req, rtsp_message *resp) { +void handle_flush(rtsp_conn_info *conn, rtsp_message *req, rtsp_message *resp) { debug(3, "Connection %d: FLUSH", conn->connection_number); // if (!rtsp_playing()) // debug(1, "This RTSP conversation thread (%d) doesn't think it's playing, but " @@ -793,7 +793,7 @@ static void handle_flush(rtsp_conn_info *conn, rtsp_message *req, rtsp_message * resp->respcode = 200; } -static void handle_setup(rtsp_conn_info *conn, rtsp_message *req, rtsp_message *resp) { +void handle_setup(rtsp_conn_info *conn, rtsp_message *req, rtsp_message *resp) { debug(3, "Connection %d: SETUP", conn->connection_number); uint16_t cport, tport; @@ -903,7 +903,7 @@ static void handle_ignore(rtsp_conn_info *conn, rtsp_message *req, rtsp_message } */ -static void handle_set_parameter_parameter(rtsp_conn_info *conn, rtsp_message *req, +void handle_set_parameter_parameter(rtsp_conn_info *conn, rtsp_message *req, __attribute__((unused)) rtsp_message *resp) { char *cp = req->content; int cp_left = req->contentlength; From e0a4e5cde80abeebaa399367df8a7c07ee607b7c Mon Sep 17 00:00:00 2001 From: Mike Brady Date: Sun, 21 Oct 2018 17:10:23 +0100 Subject: [PATCH 07/16] Format tidy-up --- audio_alsa.c | 6 +- audio_pa.c | 8 ++- common.h | 6 +- player.c | 200 ++++++++++++++++++++++++++++----------------------- player.h | 14 ++-- rtp.c | 47 ++++++------ rtsp.c | 9 ++- 7 files changed, 160 insertions(+), 130 deletions(-) diff --git a/audio_alsa.c b/audio_alsa.c index 5dc4d32f..056fe7a7 100644 --- a/audio_alsa.c +++ b/audio_alsa.c @@ -894,15 +894,15 @@ int delay(long *the_delay) { } } } else { - reply = -EIO; // shomething is wrong + reply = -EIO; // shomething is wrong frame_index = 0; // we'll be starting over... measurement_data_is_valid = 0; if (dac_state == SND_PCM_STATE_PREPARED) { - debug(1,"delay not available -- state is SND_PCM_STATE_PREPARED"); + debug(1, "delay not available -- state is SND_PCM_STATE_PREPARED"); } else { if (dac_state == SND_PCM_STATE_XRUN) { - debug(1,"delay not available -- state is SND_PCM_STATE_XRUN"); + debug(1, "delay not available -- state is SND_PCM_STATE_XRUN"); } else { debug(1, "Error -- ALSA delay(): bad state: %d.", dac_state); diff --git a/audio_pa.c b/audio_pa.c index aec4d992..ca79b4dc 100644 --- a/audio_pa.c +++ b/audio_pa.c @@ -187,12 +187,14 @@ static void start(__attribute__((unused)) int sample_rate, if (config.pa_sink) { // Connect stream to the sink specified in the config - connect_result = pa_stream_connect_playback(stream, config.pa_sink, &buffer_attr, stream_flags, NULL, NULL); + connect_result = + pa_stream_connect_playback(stream, config.pa_sink, &buffer_attr, stream_flags, NULL, NULL); } else { // Connect stream to the default audio output sink - connect_result = pa_stream_connect_playback(stream, NULL, &buffer_attr, stream_flags, NULL, NULL); + connect_result = + pa_stream_connect_playback(stream, NULL, &buffer_attr, stream_flags, NULL, NULL); } - + if (connect_result != 0) die("could not connect to the pulseaudio playback stream -- the error message is \"%s\".", pa_strerror(pa_context_errno(context))); diff --git a/common.h b/common.h index 0042d2c4..c9674c41 100644 --- a/common.h +++ b/common.h @@ -84,9 +84,9 @@ typedef struct { #ifdef CONFIG_PA char *pa_application_name; // the name under which Shairport Sync shows up as an "Application" in // the Sound Preferences in most desktop Linuxes. - // Defaults to "Shairport Sync". Shairport Sync must be playing to see it. + // Defaults to "Shairport Sync". Shairport Sync must be playing to see it. - char *pa_sink; // the name (or id) of the sink that Shairport Sync will play on. + char *pa_sink; // the name (or id) of the sink that Shairport Sync will play on. #endif #ifdef CONFIG_METADATA int metadata_enabled; @@ -136,7 +136,7 @@ typedef struct { int buffer_start_fill; uint32_t userSuppliedLatency; // overrides all other latencies -- use with caution uint32_t fixedLatencyOffset; // add this to all automatic latencies supplied to get the actual - // total latency + // total latency // the total latency will be limited to the min and max-latency values, if supplied int daemonise; int daemonise_store_pid; // don't try to save a PID file diff --git a/player.c b/player.c index ac799409..f51d8aef 100644 --- a/player.c +++ b/player.c @@ -100,10 +100,10 @@ #define BUFIDX(seqno) ((seq_t)(seqno) % BUFFER_FRAMES) uint32_t rtp_frame_offset(uint32_t from, uint32_t to) { - if (from <= to) - return to - from; - else - return UINT32_MAX - from + to + 1; + if (from <= to) + return to - from; + else + return UINT32_MAX - from + to + 1; } void do_flush(uint32_t timestamp, rtsp_conn_info *conn); @@ -120,33 +120,34 @@ static void ab_resync(rtsp_conn_info *conn) { conn->ab_buffering = 1; } -// given starting and ending points as unsigned 32-bit integers running modulo 2^32, returns the position of x in the interval in *pos +// given starting and ending points as unsigned 32-bit integers running modulo 2^32, returns the +// position of x in the interval in *pos // returns true if x is actually within the buffer -int position_in_modulo_uint32_t_buffer(uint32_t x, uint32_t start, uint32_t end, uint32_t *pos ) { - int response = 0; // not in the buffer - if (start <= end ) { - if (x < start) { - if (pos) - *pos = UINT32_MAX - start + 1 + x; - } else { - if (pos) - *pos = x - start; - if (x < end) - response = 1; - } - } else if ((x >= start) && (x <= UINT32_MAX)) { - response = 1; - if (pos) - *pos = x - start; - } else { - if (pos) - *pos = UINT32_MAX - start + 1 + x; - if (x < end) { - response = 1; - } - } - return response; +int position_in_modulo_uint32_t_buffer(uint32_t x, uint32_t start, uint32_t end, uint32_t *pos) { + int response = 0; // not in the buffer + if (start <= end) { + if (x < start) { + if (pos) + *pos = UINT32_MAX - start + 1 + x; + } else { + if (pos) + *pos = x - start; + if (x < end) + response = 1; + } + } else if ((x >= start) && (x <= UINT32_MAX)) { + response = 1; + if (pos) + *pos = x - start; + } else { + if (pos) + *pos = UINT32_MAX - start + 1 + x; + if (x < end) { + response = 1; + } + } + return response; } // this is used. @@ -441,8 +442,8 @@ static void free_audio_buffers(rtsp_conn_info *conn) { free(conn->audio_buffer[i].data); } -void player_put_packet(seq_t seqno, uint32_t actual_timestamp, uint8_t *data, - int len, rtsp_conn_info *conn) { +void player_put_packet(seq_t seqno, uint32_t actual_timestamp, uint8_t *data, int len, + rtsp_conn_info *conn) { // ignore a request to flush that has been made before the first packet... if (conn->packet_count == 0) { @@ -460,26 +461,31 @@ void player_put_packet(seq_t seqno, uint32_t actual_timestamp, uint8_t *data, // if (flush_rtp_timestamp != 0) // debug(1,"Flush_rtp_timestamp is %u",flush_rtp_timestamp); - - // now, if a flush_rtp_timestamp has been defined and the incoming timestamp is "before" it, drop it… - - if ((conn->flush_rtp_timestamp != 0) && (rtp_frame_offset(actual_timestamp,conn->flush_rtp_timestamp) < 8820)) {// if it's less than 0.2 seconds - debug(2, "Dropping flushed packet in player_put_packet, seqno %u, timestamp %" PRIu32 - ", flushing to " - "timestamp: %" PRIu32 ".", - seqno, actual_timestamp, conn->flush_rtp_timestamp); - conn->initial_reference_time = 0; - conn->initial_reference_timestamp = 0; + + // now, if a flush_rtp_timestamp has been defined and the incoming timestamp is "before" it, + // drop it… + + if ((conn->flush_rtp_timestamp != 0) && + (rtp_frame_offset(actual_timestamp, conn->flush_rtp_timestamp) < + 8820)) { // if it's less than 0.2 seconds + debug(2, "Dropping flushed packet in player_put_packet, seqno %u, timestamp %" PRIu32 + ", flushing to " + "timestamp: %" PRIu32 ".", + seqno, actual_timestamp, conn->flush_rtp_timestamp); + conn->initial_reference_time = 0; + conn->initial_reference_timestamp = 0; } else { - if ((conn->flush_rtp_timestamp != 0) && (rtp_frame_offset(conn->flush_rtp_timestamp,actual_timestamp) > 8820) && (rtp_frame_offset(conn->flush_rtp_timestamp,actual_timestamp) < 44100)) { - debug(2,"Dropping flush request"); + if ((conn->flush_rtp_timestamp != 0) && + (rtp_frame_offset(conn->flush_rtp_timestamp, actual_timestamp) > 8820) && + (rtp_frame_offset(conn->flush_rtp_timestamp, actual_timestamp) < 44100)) { + debug(2, "Dropping flush request"); conn->flush_rtp_timestamp = 0; } abuf_t *abuf = 0; if (!conn->ab_synced) { - // if this is the first packet… + // if this is the first packet… debug(3, "syncing to seqno %u.", seqno); conn->ab_write = seqno; conn->ab_read = seqno; @@ -500,7 +506,8 @@ void player_put_packet(seq_t seqno, uint32_t actual_timestamp, uint8_t *data, conn->resend_interval = resend_interval; } - if (conn->ab_write == seqno) { // if this is the expected packet (which could be the first packet…) + if (conn->ab_write == + seqno) { // if this is the expected packet (which could be the first packet…) uint64_t reception_time = get_absolute_time_in_fp(); if (conn->input_frame_rate_starting_point_is_valid == 0) { if ((conn->packet_count_since_flush >= 500) && (conn->packet_count_since_flush <= 510)) { @@ -602,7 +609,7 @@ void player_put_packet(seq_t seqno, uint32_t actual_timestamp, uint8_t *data, j)) { // prevent multiple requests from the same level of lookback check_buf->resend_level = j; if (config.disable_resend_requests == 0) { - debug_mutex_unlock(&conn->ab_mutex, 3); + debug_mutex_unlock(&conn->ab_mutex, 3); rtp_request_resend(next, 1, conn); conn->resend_requests++; debug_mutex_lock(&conn->ab_mutex, 20000, 1); @@ -853,12 +860,14 @@ static abuf_t *buffer_get_frame(rtsp_conn_info *conn) { debug(1, "Inconsistent sequence numbers detected"); } } - - if ((conn->flush_rtp_timestamp != 0) && (rtp_frame_offset(curframe->given_timestamp,conn->flush_rtp_timestamp) < 8820)) {// if it's less than 0.2 seconds - debug(2, "Dropping flushed packet in buffer_get_frame, seqno %u, timestamp %" PRIu32 - ", flushing to " - "timestamp: %" PRIu32 ".", - curframe->sequence_number, curframe->given_timestamp, conn->flush_rtp_timestamp); + + if ((conn->flush_rtp_timestamp != 0) && + (rtp_frame_offset(curframe->given_timestamp, conn->flush_rtp_timestamp) < + 8820)) { // if it's less than 0.2 seconds + debug(2, "Dropping flushed packet in buffer_get_frame, seqno %u, timestamp %" PRIu32 + ", flushing to " + "timestamp: %" PRIu32 ".", + curframe->sequence_number, curframe->given_timestamp, conn->flush_rtp_timestamp); curframe->ready = 0; curframe->resend_level = 0; flush_limit += curframe->length; @@ -866,10 +875,12 @@ static abuf_t *buffer_get_frame(rtsp_conn_info *conn) { conn->initial_reference_time = 0; conn->initial_reference_timestamp = 0; } - if ((conn->flush_rtp_timestamp != 0) && (rtp_frame_offset(conn->flush_rtp_timestamp,curframe->given_timestamp) > 8820) && (rtp_frame_offset(conn->flush_rtp_timestamp,curframe->given_timestamp) < 44100)) { - debug(2,"Dropping flush request"); - conn->flush_rtp_timestamp = 0; - } + if ((conn->flush_rtp_timestamp != 0) && + (rtp_frame_offset(conn->flush_rtp_timestamp, curframe->given_timestamp) > 8820) && + (rtp_frame_offset(conn->flush_rtp_timestamp, curframe->given_timestamp) < 44100)) { + debug(2, "Dropping flush request"); + conn->flush_rtp_timestamp = 0; + } } } while ((conn->flush_rtp_timestamp != 0) && (flush_limit <= 8820) && (curframe->ready == 0)); @@ -901,7 +912,7 @@ static abuf_t *buffer_get_frame(rtsp_conn_info *conn) { // debug(1,"First frame seen with timestamp..."); conn->first_packet_timestamp = curframe->given_timestamp; // we will keep buffering until we are - // supposed to start playing this + // supposed to start playing this have_sent_prefiller_silence = 0; conn->packet_stream_established = 1; @@ -943,10 +954,11 @@ static abuf_t *buffer_get_frame(rtsp_conn_info *conn) { // frame should be played" uint64_t should_be_time; - frame_to_local_time( - conn->first_packet_timestamp + conn->latency + - (uint32_t)(config.audio_backend_latency_offset * conn->input_rate), // this will go modulo 2^32 - &should_be_time, conn); + frame_to_local_time(conn->first_packet_timestamp + conn->latency + + (uint32_t)(config.audio_backend_latency_offset * + conn->input_rate), // this will go modulo 2^32 + &should_be_time, + conn); conn->first_packet_time_to_play = should_be_time; @@ -964,10 +976,11 @@ static abuf_t *buffer_get_frame(rtsp_conn_info *conn) { // recalculate conn->first_packet_time_to_play -- the latency might change uint64_t should_be_time; - frame_to_local_time( - conn->first_packet_timestamp + conn->latency + - (uint32_t)(config.audio_backend_latency_offset * conn->input_rate), // this should go modulo 2^32 - &should_be_time, conn); + frame_to_local_time(conn->first_packet_timestamp + conn->latency + + (uint32_t)(config.audio_backend_latency_offset * + conn->input_rate), // this should go modulo 2^32 + &should_be_time, + conn); conn->first_packet_time_to_play = should_be_time; @@ -979,9 +992,9 @@ static abuf_t *buffer_get_frame(rtsp_conn_info *conn) { int64_t max_dac_delay = config.output_rate / 10; // so the lead-in time must be greater // than this, say 0.2 sec, to allow for - // dynamic adjustment + // dynamic adjustment int64_t filler_size = max_dac_delay; - + if (local_time_now >= conn->first_packet_time_to_play) { // debug(1,"Gone past starting time"); have_sent_prefiller_silence = 1; @@ -1006,34 +1019,38 @@ static abuf_t *buffer_get_frame(rtsp_conn_info *conn) { int64_t lead_time = conn->first_packet_time_to_play - local_time_now; // an audio_backend_silent_lead_in_time of less than zero means start filling ASAP int64_t lead_in_time = -1; - if (config.audio_backend_silent_lead_in_time>=0) + if (config.audio_backend_silent_lead_in_time >= 0) lead_in_time = - (int64_t)(config.audio_backend_silent_lead_in_time * (int64_t)0x100000000); + (int64_t)(config.audio_backend_silent_lead_in_time * (int64_t)0x100000000); // debug(1,"Lead time is %llx at fpttp // %llx.",lead_time,conn->first_packet_time_to_play); if ((lead_in_time < 0) || (lead_time <= lead_in_time)) { - // debug(1,"Lead time is %" PRIx64 ", lead-in time is %" PRIx64 " at fpttp %llx.",lead_time,conn->first_packet_time_to_play); + // debug(1,"Lead time is %" PRIx64 ", lead-in time is %" PRIx64 " at fpttp + // %llx.",lead_time,conn->first_packet_time_to_play); // debug(1,"Checking"); if (config.output->delay) { // conn->first_packet_time_to_play is definitely later than local_time_now int resp = 0; dac_delay = 0; - if (have_sent_prefiller_silence != 0) + if (have_sent_prefiller_silence != 0) resp = config.output->delay(&dac_delay); if (resp == 0) { int64_t gross_frame_gap = ((conn->first_packet_time_to_play - local_time_now) * config.output_rate) >> 32; int64_t exact_frame_gap = gross_frame_gap - dac_delay; - // debug(1,"Exact and gross frame gaps are %" PRId64 " and %" PRId64 " frames, and the dac delay is %ld.", exact_frame_gap, gross_frame_gap, dac_delay); + // debug(1,"Exact and gross frame gaps are %" PRId64 " and %" PRId64 " frames, + // and the dac delay is %ld.", exact_frame_gap, gross_frame_gap, dac_delay); if (exact_frame_gap < 0) { // we've gone past the time... // debug(1,"Run past time."); - - // this might happen if a big clock adjustment was made at just the wrong time. - - debug(1,"Run a bit past the exact start time by %" PRId64 " frames.",-exact_frame_gap); + + // this might happen if a big clock adjustment was made at just the wrong + // time. + + debug(1, "Run a bit past the exact start time by %" PRId64 " frames.", + -exact_frame_gap); if (config.output->flush) config.output->flush(); ab_resync(conn); @@ -1044,7 +1061,9 @@ static abuf_t *buffer_get_frame(rtsp_conn_info *conn) { if (fs > (max_dac_delay - dac_delay)) fs = max_dac_delay - dac_delay; if (fs < 0) { - // this could happen if the dac delay mysteriously grows between samples, which could happen in a transition between having no interpolation and having interpolated buffer numbers. + // this could happen if the dac delay mysteriously grows between samples, + // which could happen in a transition between having no interpolation and + // having interpolated buffer numbers. debug(2, "frame size (fs) < 0 with max_dac_delay of %lld and dac_delay of %ld", max_dac_delay, dac_delay); @@ -1062,7 +1081,8 @@ static abuf_t *buffer_get_frame(rtsp_conn_info *conn) { } void *silence; // if (fs==0) - // debug(2,"Zero length silence buffer needed with gross_frame_gap of %lld and + // 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 @@ -1162,11 +1182,12 @@ static abuf_t *buffer_get_frame(rtsp_conn_info *conn) { if (have_timestamp_timing_information(conn)) { // if we have a reference time uint64_t time_to_play; - frame_to_local_time( - curframe->given_timestamp + conn->latency + - (uint32_t)(config.audio_backend_latency_offset * conn->input_rate) - - (uint32_t)(config.audio_backend_buffer_desired_length * conn->input_rate), // this will go modulo 2^32 - &time_to_play, conn); + frame_to_local_time(curframe->given_timestamp + conn->latency + + (uint32_t)(config.audio_backend_latency_offset * conn->input_rate) - + (uint32_t)(config.audio_backend_buffer_desired_length * + conn->input_rate), // this will go modulo 2^32 + &time_to_play, + conn); if (local_time_now >= time_to_play) { do_wait = 0; @@ -1909,8 +1930,8 @@ void *player_thread_func(void *arg) { get_reference_timestamp_stuff(&reference_timestamp, &reference_timestamp_time, &remote_reference_timestamp_time, conn); // types okay int64_t rt, nt; - rt = reference_timestamp; // uint32_t to int64_t - nt = inframe->given_timestamp; // uint32_t to int64_t + rt = reference_timestamp; // uint32_t to int64_t + nt = inframe->given_timestamp; // uint32_t to int64_t rt *= conn->output_sample_ratio; nt *= conn->output_sample_ratio; @@ -2041,11 +2062,12 @@ void *player_thread_func(void *arg) { (config.resyncthreshold > 0.0) && (abs_sync_error > config.resyncthreshold * config.output_rate)) { if (abs_sync_error > 3 * config.output_rate) { - + warn("Very large sync error: %" PRId64 " frames, with should_be_frame: %" PRId64 - ", nt: %" PRId64 - ", current_delay: %" PRId64 ", given timestamp %" PRIX32 ", reference timestamp %" PRIX32 ", should_be_frame %" PRIX32 ".", - sync_error, should_be_frame, nt, current_delay, inframe->given_timestamp, reference_timestamp, should_be_frame_32); + ", nt: %" PRId64 ", current_delay: %" PRId64 ", given timestamp %" PRIX32 + ", reference timestamp %" PRIX32 ", should_be_frame %" PRIX32 ".", + sync_error, should_be_frame, nt, current_delay, inframe->given_timestamp, + reference_timestamp, should_be_frame_32); } sync_error_out_of_bounds++; } else { diff --git a/player.h b/player.h index 5c0adc24..a7531b90 100644 --- a/player.h +++ b/player.h @@ -68,14 +68,14 @@ typedef struct { } stream_cfg; typedef struct { - int connection_number; // for debug ID purposes, nothing else... - int resend_interval; // this is really just for debugging - int AirPlayVersion; // zero if not an AirPlay session. Used to help calculate latency + int connection_number; // for debug ID purposes, nothing else... + int resend_interval; // this is really just for debugging + int AirPlayVersion; // zero if not an AirPlay session. Used to help calculate latency uint32_t latency; // the actual latency used for this play session uint32_t minimum_latency; // set if an a=min-latency: line appears in the ANNOUNCE message; zero - // otherwise + // otherwise uint32_t maximum_latency; // set if an a=max-latency: line appears in the ANNOUNCE message; zero - // otherwise + // otherwise int fd; int authorized; // set if a password is required and has been supplied @@ -250,8 +250,8 @@ int player_stop(rtsp_conn_info *conn); void player_volume(double f, rtsp_conn_info *conn); void player_volume_without_notification(double f, rtsp_conn_info *conn); void player_flush(uint32_t timestamp, rtsp_conn_info *conn); -void player_put_packet(seq_t seqno, uint32_t actual_timestamp, uint8_t *data, - int len, rtsp_conn_info *conn); +void player_put_packet(seq_t seqno, uint32_t actual_timestamp, uint8_t *data, int len, + rtsp_conn_info *conn); int64_t monotonic_timestamp(uint32_t timestamp, rtsp_conn_info *conn); // add an epoch to the timestamp. The monotonic // timestamp guaranteed to start between 2^32 2^33 diff --git a/rtp.c b/rtp.c index e2c8f944..eb86da48 100644 --- a/rtp.c +++ b/rtp.c @@ -329,10 +329,14 @@ void *rtp_control_receiver(void *arg) { // Sigh, it would be nice to have a published protocol... uint16_t flags = nctohs(&packet[2]); - uint32_t la = sync_rtp_timestamp - rtp_timestamp_less_latency; // note, this might loop around in modulo. Not sure if you'll get an error! + uint32_t la = sync_rtp_timestamp - rtp_timestamp_less_latency; // note, this might + // loop around in + // modulo. Not sure if + // you'll get an error! // debug(3, "Latency derived just from the sync packet is %" PRIu32 " frames.", la); - - if ((flags == 7) || ((conn->AirPlayVersion > 0) && (conn->AirPlayVersion <= 353)) || ((conn->AirPlayVersion > 0) && (conn->AirPlayVersion >= 371))) { + + if ((flags == 7) || ((conn->AirPlayVersion > 0) && (conn->AirPlayVersion <= 353)) || + ((conn->AirPlayVersion > 0) && (conn->AirPlayVersion >= 371))) { la += config.fixedLatencyOffset; // debug(3, "A fixed latency offset of %d frames has been added, giving a latency of // " @@ -350,7 +354,8 @@ void *rtp_control_receiver(void *arg) { if (la > max_frames) { warn("An out-of-range latency request of %" PRIu32 - " frames was ignored. Must be %" PRIu32 " frames or less (44,100 frames per second). " + " frames was ignored. Must be %" PRIu32 + " frames or less (44,100 frames per second). " "Latency remains at %" PRIu32 " frames.", la, max_frames, conn->latency); } else { @@ -379,7 +384,8 @@ void *rtp_control_receiver(void *arg) { if (remote_frame_time_interval) { conn->remote_frame_rate = (1.0 * (conn->reference_timestamp - conn->initial_reference_timestamp)) / - remote_frame_time_interval; // an IEEE double calculation with a 32-bit numerator and 64-bit denominator + remote_frame_time_interval; // an IEEE double calculation with a 32-bit + // numerator and 64-bit denominator // integers conn->remote_frame_rate = conn->remote_frame_rate * (uint64_t)0x100000000; // this should just change the @@ -1040,8 +1046,9 @@ int sanitised_source_rate_information(uint32_t *frames, uint64_t *time, rtsp_con *time = (uint64_t)(0x100000000); // one second in fp form if ((conn->packet_stream_established) && (conn->initial_reference_time) && (conn->initial_reference_timestamp)) { -// uint32_t local_frames = conn->reference_timestamp - conn->initial_reference_timestamp; - uint32_t local_frames = rtp_frame_offset(conn->initial_reference_timestamp, conn->reference_timestamp); + // uint32_t local_frames = conn->reference_timestamp - conn->initial_reference_timestamp; + uint32_t local_frames = + rtp_frame_offset(conn->initial_reference_timestamp, conn->reference_timestamp); uint64_t local_time = conn->remote_reference_timestamp_time - conn->initial_reference_time; if ((local_frames == 0) || (local_time == 0) || (use_nominal_rate)) { result = 1; @@ -1073,24 +1080,24 @@ int frame_to_local_time(uint32_t timestamp, uint64_t *time, rtsp_conn_info *conn uint64_t timestamp_interval_time; uint64_t remote_time_of_timestamp; - uint32_t timestamp_interval = rtp_frame_offset(conn->reference_timestamp,timestamp); - if (timestamp_interval <= conn->input_rate*10) { // i.e. timestamp was really after the reference timestamp - timestamp_interval_time = - (timestamp_interval * time_difference) / - (frame_difference); // this is the nominal time, based on the - // fps specified between current and - // previous sync frame. + uint32_t timestamp_interval = rtp_frame_offset(conn->reference_timestamp, timestamp); + if (timestamp_interval <= + conn->input_rate * 10) { // i.e. timestamp was really after the reference timestamp + timestamp_interval_time = (timestamp_interval * time_difference) / + (frame_difference); // this is the nominal time, based on the + // fps specified between current and + // previous sync frame. remote_time_of_timestamp = conn->remote_reference_timestamp_time + timestamp_interval_time; // based on the reference timestamp time // plus the time interval calculated based // on the specified fps. } else { // i.e. timestamp was actually after the reference timestamp - timestamp_interval = rtp_frame_offset(timestamp,conn->reference_timestamp); // fix the calculation - timestamp_interval_time = - (timestamp_interval * time_difference) / - (frame_difference); // this is the nominal time, based on the - // fps specified between current and - // previous sync frame. + timestamp_interval = + rtp_frame_offset(timestamp, conn->reference_timestamp); // fix the calculation + timestamp_interval_time = (timestamp_interval * time_difference) / + (frame_difference); // this is the nominal time, based on the + // fps specified between current and + // previous sync frame. remote_time_of_timestamp = conn->remote_reference_timestamp_time - timestamp_interval_time; // based on the reference timestamp time // plus the time interval calculated based diff --git a/rtsp.c b/rtsp.c index 834e56e8..c1ba268f 100644 --- a/rtsp.c +++ b/rtsp.c @@ -507,8 +507,7 @@ fail: return 0; } -enum rtsp_read_request_response rtsp_read_request(rtsp_conn_info *conn, - rtsp_message **the_packet) { +enum rtsp_read_request_response rtsp_read_request(rtsp_conn_info *conn, rtsp_message **the_packet) { enum rtsp_read_request_response reply = rtsp_read_request_response_ok; ssize_t buflen = 4096; char *buf = malloc(buflen + 1); @@ -738,7 +737,7 @@ void handle_record(rtsp_conn_info *conn, rtsp_message *req, rtsp_message *resp) } void handle_options(rtsp_conn_info *conn, __attribute__((unused)) rtsp_message *req, - rtsp_message *resp) { + rtsp_message *resp) { debug(3, "Connection %d: OPTIONS", conn->connection_number); resp->respcode = 200; msg_add_header(resp, "Public", "ANNOUNCE, SETUP, RECORD, " @@ -747,7 +746,7 @@ void handle_options(rtsp_conn_info *conn, __attribute__((unused)) rtsp_message * } void handle_teardown(rtsp_conn_info *conn, __attribute__((unused)) rtsp_message *req, - rtsp_message *resp) { + rtsp_message *resp) { debug(2, "Connection %d: TEARDOWN", conn->connection_number); // if (!rtsp_playing()) // debug(1, "This RTSP connection thread (%d) doesn't think it's playing, but " @@ -904,7 +903,7 @@ static void handle_ignore(rtsp_conn_info *conn, rtsp_message *req, rtsp_message */ void handle_set_parameter_parameter(rtsp_conn_info *conn, rtsp_message *req, - __attribute__((unused)) rtsp_message *resp) { + __attribute__((unused)) rtsp_message *resp) { char *cp = req->content; int cp_left = req->contentlength; char *next; From 7c35f647467c758ce2d67ee1d85ccacf5ed53f00 Mon Sep 17 00:00:00 2001 From: Mike Brady Date: Sun, 21 Oct 2018 18:32:18 +0100 Subject: [PATCH 08/16] Fix a reporting error, allow a larger time window for legitimate flush requests. --- player.c | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/player.c b/player.c index f51d8aef..a374c0c9 100644 --- a/player.c +++ b/player.c @@ -465,9 +465,9 @@ void player_put_packet(seq_t seqno, uint32_t actual_timestamp, uint8_t *data, in // now, if a flush_rtp_timestamp has been defined and the incoming timestamp is "before" it, // drop it… - if ((conn->flush_rtp_timestamp != 0) && + if ((conn->flush_rtp_timestamp != 0) && (actual_timestamp != conn->flush_rtp_timestamp) && (rtp_frame_offset(actual_timestamp, conn->flush_rtp_timestamp) < - 8820)) { // if it's less than 0.2 seconds + conn->input_rate * 10)) { // if it's less than 10 seconds debug(2, "Dropping flushed packet in player_put_packet, seqno %u, timestamp %" PRIu32 ", flushing to " "timestamp: %" PRIu32 ".", @@ -476,9 +476,10 @@ void player_put_packet(seq_t seqno, uint32_t actual_timestamp, uint8_t *data, in conn->initial_reference_timestamp = 0; } else { if ((conn->flush_rtp_timestamp != 0) && - (rtp_frame_offset(conn->flush_rtp_timestamp, actual_timestamp) > 8820) && - (rtp_frame_offset(conn->flush_rtp_timestamp, actual_timestamp) < 44100)) { - debug(2, "Dropping flush request"); + (rtp_frame_offset(conn->flush_rtp_timestamp, actual_timestamp) > conn->input_rate/5) && + (rtp_frame_offset(conn->flush_rtp_timestamp, actual_timestamp) < conn->input_rate)) { + // between 0.2 and 1 second + debug(2, "Dropping flush request in player_put_packet"); conn->flush_rtp_timestamp = 0; } @@ -501,7 +502,7 @@ void player_put_packet(seq_t seqno, uint32_t actual_timestamp, uint8_t *data, in resend_interval = latency_based_resend_interval; if (conn->resend_interval != resend_interval) { - debug(2, "Resend interval for latency of %" PRId64 " frames is %d frames.", conn->latency, + debug(2, "Resend interval for latency of %u frames is %d frames.", conn->latency, resend_interval); conn->resend_interval = resend_interval; } @@ -861,9 +862,9 @@ static abuf_t *buffer_get_frame(rtsp_conn_info *conn) { } } - if ((conn->flush_rtp_timestamp != 0) && + if ((conn->flush_rtp_timestamp != 0) && (curframe->given_timestamp != conn->flush_rtp_timestamp) && (rtp_frame_offset(curframe->given_timestamp, conn->flush_rtp_timestamp) < - 8820)) { // if it's less than 0.2 seconds + conn->input_rate * 10)) { // if it's less than ten seconds debug(2, "Dropping flushed packet in buffer_get_frame, seqno %u, timestamp %" PRIu32 ", flushing to " "timestamp: %" PRIu32 ".", @@ -876,9 +877,9 @@ static abuf_t *buffer_get_frame(rtsp_conn_info *conn) { conn->initial_reference_timestamp = 0; } if ((conn->flush_rtp_timestamp != 0) && - (rtp_frame_offset(conn->flush_rtp_timestamp, curframe->given_timestamp) > 8820) && - (rtp_frame_offset(conn->flush_rtp_timestamp, curframe->given_timestamp) < 44100)) { - debug(2, "Dropping flush request"); + (rtp_frame_offset(conn->flush_rtp_timestamp, curframe->given_timestamp) > conn->input_rate / 5) && + (rtp_frame_offset(conn->flush_rtp_timestamp, curframe->given_timestamp) < conn->input_rate * 10 )) { + debug(2, "Dropping flush request in buffer_get_frame"); conn->flush_rtp_timestamp = 0; } } From 63cae88aca91a9fd794d03f092d2c0cd6d240d96 Mon Sep 17 00:00:00 2001 From: Mike Brady Date: Sun, 21 Oct 2018 18:33:47 +0100 Subject: [PATCH 09/16] Further improve the modulo 32 rtpstamp-based missing frames erro handling. --- configure.ac | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index c66718a8..55f63090 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.3d17], [mikebrady@eircom.net]) +AC_INIT([shairport-sync], [3.3d18], [mikebrady@eircom.net]) AM_INIT_AUTOMAKE AC_CONFIG_SRCDIR([shairport.c]) AC_CONFIG_HEADERS([config.h]) From 8914b68b42d1a2a281fc3b0932a343a195156316 Mon Sep 17 00:00:00 2001 From: Mike Brady Date: Sun, 21 Oct 2018 21:24:38 +0100 Subject: [PATCH 10/16] Calculate silence memory requirements properly. Remove some alarming messages. --- player.c | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/player.c b/player.c index a374c0c9..3898c62d 100644 --- a/player.c +++ b/player.c @@ -1832,12 +1832,14 @@ void *player_thread_func(void *arg) { free(silence); } } else if (frames_to_drop) { + /* if (frames_to_drop > 3 * config.output_rate) { warn("Shome mhistake shurely: very large number of frames to drop: %" PRId64 " -- setting it to %" PRId64 ".", frames_to_drop, 3 * config.output_rate); frames_to_drop = 3 * config.output_rate; } + */ debug(3, "%" PRId64 " frames to drop.", frames_to_drop); frames_to_drop -= inframe->length; if (frames_to_drop < 0) @@ -2062,6 +2064,7 @@ void *player_thread_func(void *arg) { if ((config.no_sync == 0) && (inframe->given_timestamp != 0) && (config.resyncthreshold > 0.0) && (abs_sync_error > config.resyncthreshold * config.output_rate)) { + /* if (abs_sync_error > 3 * config.output_rate) { warn("Very large sync error: %" PRId64 " frames, with should_be_frame: %" PRId64 @@ -2070,6 +2073,7 @@ void *player_thread_func(void *arg) { sync_error, should_be_frame, nt, current_delay, inframe->given_timestamp, reference_timestamp, should_be_frame_32); } + */ sync_error_out_of_bounds++; } else { sync_error_out_of_bounds = 0; @@ -2081,25 +2085,25 @@ void *player_thread_func(void *arg) { // sync_error_out_of_bounds, sync_error); sync_error_out_of_bounds = 0; - int filler_length = config.resyncthreshold * config.output_rate; // number of samples + int64_t filler_length = (int64_t)config.resyncthreshold * config.output_rate; // number of samples if ((sync_error > 0) && (sync_error > filler_length)) { - // debug(1, "Large positive sync error: %lld.", sync_error); + debug(1, "Large positive sync error: %" PRId64 ".", sync_error); frames_to_drop = sync_error / conn->output_sample_ratio; } else if ((sync_error < 0) && ((-sync_error) > filler_length)) { - // debug(1, "Large negative sync error: %lld. Inserting silence.", sync_error); - int silence_length = -sync_error; + debug(1, "Large negative sync error: %" PRId64 ". Inserting silence.", sync_error); + int64_t silence_length = -sync_error; if (silence_length > (filler_length * 5)) silence_length = filler_length * 5; - - char *long_silence = malloc(conn->output_bytes_per_frame * silence_length); + size_t silence_length_sized = silence_length; + char *long_silence = malloc(conn->output_bytes_per_frame * silence_length_sized); if (long_silence) { - memset(long_silence, 0, conn->output_bytes_per_frame * silence_length); - config.output->play(long_silence, silence_length); + memset(long_silence, 0, conn->output_bytes_per_frame * silence_length_sized); + config.output->play(long_silence, silence_length_sized); free(long_silence); } else { warn("Failed to allocate memory for a long_silence buffer of %d frames for a " - "sync error of %" PRId64 " frames.", - silence_length, sync_error); + "sync error of %d frames.", + silence_length_sized, sync_error); } } } else { From ab139ffc8c0c8521058583b717568f6c15030c6e Mon Sep 17 00:00:00 2001 From: Mike Brady Date: Sun, 21 Oct 2018 22:52:06 +0100 Subject: [PATCH 11/16] Fix calculation of filler length. --- player.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/player.c b/player.c index 3898c62d..2291182b 100644 --- a/player.c +++ b/player.c @@ -2085,7 +2085,7 @@ void *player_thread_func(void *arg) { // sync_error_out_of_bounds, sync_error); sync_error_out_of_bounds = 0; - int64_t filler_length = (int64_t)config.resyncthreshold * config.output_rate; // number of samples + int64_t filler_length = (int64_t)(config.resyncthreshold * config.output_rate); // number of samples if ((sync_error > 0) && (sync_error > filler_length)) { debug(1, "Large positive sync error: %" PRId64 ".", sync_error); frames_to_drop = sync_error / conn->output_sample_ratio; @@ -2098,6 +2098,7 @@ void *player_thread_func(void *arg) { char *long_silence = malloc(conn->output_bytes_per_frame * silence_length_sized); if (long_silence) { memset(long_silence, 0, conn->output_bytes_per_frame * silence_length_sized); + debug(1,"Play a silence of %d frames.",silence_length_sized); config.output->play(long_silence, silence_length_sized); free(long_silence); } else { From 77176ec4e0b03bbd7ff3fad4c55d5a6c59ec5a1a Mon Sep 17 00:00:00 2001 From: Mike Brady Date: Mon, 22 Oct 2018 20:53:44 +0000 Subject: [PATCH 12/16] See where a weird large negative delay is being calculated. --- player.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/player.c b/player.c index 2291182b..f69a563c 100644 --- a/player.c +++ b/player.c @@ -1935,8 +1935,8 @@ void *player_thread_func(void *arg) { int64_t rt, nt; rt = reference_timestamp; // uint32_t to int64_t nt = inframe->given_timestamp; // uint32_t to int64_t - rt *= conn->output_sample_ratio; - nt *= conn->output_sample_ratio; + rt = rt*conn->output_sample_ratio; + nt = nt*conn->output_sample_ratio; uint64_t local_time_now = get_absolute_time_in_fp(); // types okay // struct timespec tn; @@ -2050,7 +2050,7 @@ void *player_thread_func(void *arg) { // if negative, the packet will be early -- the delay is less than expected. sync_error = - delay - (conn->latency * conn->output_sample_ratio + + delay - ((int64_t)conn->latency * conn->output_sample_ratio + (int64_t)(config.audio_backend_latency_offset * config.output_rate)); // int64_t from int64_t - int32_t, so okay @@ -2090,7 +2090,8 @@ void *player_thread_func(void *arg) { debug(1, "Large positive sync error: %" PRId64 ".", sync_error); frames_to_drop = sync_error / conn->output_sample_ratio; } else if ((sync_error < 0) && ((-sync_error) > filler_length)) { - debug(1, "Large negative sync error: %" PRId64 ". Inserting silence.", sync_error); + debug(1, "Large negative sync error: %" PRId64 " with should_be_frame_32 of %" PRIu32 + ", nt of %" PRId64 " and current_delay of %" PRId64 ".", sync_error, should_be_frame_32, nt, current_delay); int64_t silence_length = -sync_error; if (silence_length > (filler_length * 5)) silence_length = filler_length * 5; From 3cb359c747e9ea606e133fac919fe44f7e0896a8 Mon Sep 17 00:00:00 2001 From: Mike Brady Date: Tue, 23 Oct 2018 10:24:40 +0000 Subject: [PATCH 13/16] Tighten up integer promotion checks to calculate large delays properly. Fix input rate calculation restarting. --- audio_alsa.c | 6 +++--- player.c | 50 ++++++++++++++++++++++++++++++-------------------- player.h | 6 ++++-- rtp.c | 45 +++++++++++++++++++++++++-------------------- 4 files changed, 62 insertions(+), 45 deletions(-) diff --git a/audio_alsa.c b/audio_alsa.c index 056fe7a7..f0fae65a 100644 --- a/audio_alsa.c +++ b/audio_alsa.c @@ -899,13 +899,13 @@ int delay(long *the_delay) { measurement_data_is_valid = 0; if (dac_state == SND_PCM_STATE_PREPARED) { - debug(1, "delay not available -- state is SND_PCM_STATE_PREPARED"); + debug(2, "delay not available -- state is SND_PCM_STATE_PREPARED"); } else { if (dac_state == SND_PCM_STATE_XRUN) { - debug(1, "delay not available -- state is SND_PCM_STATE_XRUN"); + debug(2, "delay not available -- state is SND_PCM_STATE_XRUN"); } else { - debug(1, "Error -- ALSA delay(): bad state: %d.", dac_state); + debug(2, "Error -- ALSA delay(): bad state: %d.", dac_state); } if ((derr = snd_pcm_prepare(alsa_handle))) { snd_pcm_recover(alsa_handle, derr, 1); diff --git a/player.c b/player.c index f69a563c..c107bc34 100644 --- a/player.c +++ b/player.c @@ -99,13 +99,20 @@ // static abuf_t audio_buffer[BUFFER_FRAMES]; #define BUFIDX(seqno) ((seq_t)(seqno) % BUFFER_FRAMES) -uint32_t rtp_frame_offset(uint32_t from, uint32_t to) { +uint32_t modulo_32_offset(uint32_t from, uint32_t to) { if (from <= to) return to - from; else return UINT32_MAX - from + to + 1; } +uint64_t modulo_64_offset(uint64_t from, uint64_t to) { + if (from <= to) + return to - from; + else + return UINT64_MAX - from + to + 1; +} + void do_flush(uint32_t timestamp, rtsp_conn_info *conn); static void ab_resync(rtsp_conn_info *conn) { @@ -223,6 +230,15 @@ static inline int seq32_order(uint32_t a, uint32_t b) { return (C & 0x80000000) == 0; } +void reset_input_flow_metrics(rtsp_conn_info *conn) { + conn->play_number_after_flush = 0; + conn->packet_count_since_flush = 0; + conn->packet_stream_established = 0; + conn->input_frame_rate_starting_point_is_valid = 0; + conn->initial_reference_time = 0; + conn->initial_reference_timestamp = 0; +} + static int alac_decode(short *dest, int *destlen, uint8_t *buf, int len, rtsp_conn_info *conn) { // parameters: where the decoded stuff goes, its length in samples, // the incoming packet, the length of the incoming packet in bytes @@ -466,7 +482,7 @@ void player_put_packet(seq_t seqno, uint32_t actual_timestamp, uint8_t *data, in // drop it… if ((conn->flush_rtp_timestamp != 0) && (actual_timestamp != conn->flush_rtp_timestamp) && - (rtp_frame_offset(actual_timestamp, conn->flush_rtp_timestamp) < + (modulo_32_offset(actual_timestamp, conn->flush_rtp_timestamp) < conn->input_rate * 10)) { // if it's less than 10 seconds debug(2, "Dropping flushed packet in player_put_packet, seqno %u, timestamp %" PRIu32 ", flushing to " @@ -476,8 +492,8 @@ void player_put_packet(seq_t seqno, uint32_t actual_timestamp, uint8_t *data, in conn->initial_reference_timestamp = 0; } else { if ((conn->flush_rtp_timestamp != 0) && - (rtp_frame_offset(conn->flush_rtp_timestamp, actual_timestamp) > conn->input_rate/5) && - (rtp_frame_offset(conn->flush_rtp_timestamp, actual_timestamp) < conn->input_rate)) { + (modulo_32_offset(conn->flush_rtp_timestamp, actual_timestamp) > conn->input_rate/5) && + (modulo_32_offset(conn->flush_rtp_timestamp, actual_timestamp) < conn->input_rate)) { // between 0.2 and 1 second debug(2, "Dropping flush request in player_put_packet"); conn->flush_rtp_timestamp = 0; @@ -863,7 +879,7 @@ static abuf_t *buffer_get_frame(rtsp_conn_info *conn) { } if ((conn->flush_rtp_timestamp != 0) && (curframe->given_timestamp != conn->flush_rtp_timestamp) && - (rtp_frame_offset(curframe->given_timestamp, conn->flush_rtp_timestamp) < + (modulo_32_offset(curframe->given_timestamp, conn->flush_rtp_timestamp) < conn->input_rate * 10)) { // if it's less than ten seconds debug(2, "Dropping flushed packet in buffer_get_frame, seqno %u, timestamp %" PRIu32 ", flushing to " @@ -877,8 +893,8 @@ static abuf_t *buffer_get_frame(rtsp_conn_info *conn) { conn->initial_reference_timestamp = 0; } if ((conn->flush_rtp_timestamp != 0) && - (rtp_frame_offset(conn->flush_rtp_timestamp, curframe->given_timestamp) > conn->input_rate / 5) && - (rtp_frame_offset(conn->flush_rtp_timestamp, curframe->given_timestamp) < conn->input_rate * 10 )) { + (modulo_32_offset(conn->flush_rtp_timestamp, curframe->given_timestamp) > conn->input_rate / 5) && + (modulo_32_offset(conn->flush_rtp_timestamp, curframe->given_timestamp) < conn->input_rate * 10 )) { debug(2, "Dropping flush request in buffer_get_frame"); conn->flush_rtp_timestamp = 0; } @@ -1200,9 +1216,7 @@ static abuf_t *buffer_get_frame(rtsp_conn_info *conn) { if (notified_buffer_empty == 0) { debug(3, "Buffers exhausted."); notified_buffer_empty = 1; - conn->initial_reference_time = 0; - conn->initial_reference_timestamp = 0; - conn->input_frame_rate_starting_point_is_valid = 0; + reset_input_flow_metrics(conn); } do_wait = 1; } @@ -2009,7 +2023,7 @@ void *player_thread_func(void *arg) { current_delay = l_delay; if (resp == 0) { // no error if (current_delay < 0) { - debug(1, "Underrun of %lld frames reported, but ignored.", current_delay); + debug(2, "Underrun of %lld frames reported, but ignored.", current_delay); current_delay = 0; // could get a negative value if there was underrun, but ignore it. } @@ -2087,10 +2101,11 @@ void *player_thread_func(void *arg) { int64_t filler_length = (int64_t)(config.resyncthreshold * config.output_rate); // number of samples if ((sync_error > 0) && (sync_error > filler_length)) { - debug(1, "Large positive sync error: %" PRId64 ".", sync_error); + debug(2, "Large positive sync error: %" PRId64 ".", sync_error); frames_to_drop = sync_error / conn->output_sample_ratio; + reset_input_flow_metrics(conn); } else if ((sync_error < 0) && ((-sync_error) > filler_length)) { - debug(1, "Large negative sync error: %" PRId64 " with should_be_frame_32 of %" PRIu32 + debug(2, "Large negative sync error: %" PRId64 " with should_be_frame_32 of %" PRIu32 ", nt of %" PRId64 " and current_delay of %" PRId64 ".", sync_error, should_be_frame_32, nt, current_delay); int64_t silence_length = -sync_error; if (silence_length > (filler_length * 5)) @@ -2107,6 +2122,7 @@ void *player_thread_func(void *arg) { "sync error of %d frames.", silence_length_sized, sync_error); } + reset_input_flow_metrics(conn); } } else { @@ -2744,13 +2760,7 @@ void do_flush(uint32_t timestamp, rtsp_conn_info *conn) { // if (timestamp!=0) conn->flush_rtp_timestamp = timestamp; // flush all packets up to (and including?) this // conn->play_segment_reference_frame = 0; - conn->play_number_after_flush = 0; - conn->packet_count_since_flush = 0; - conn->packet_stream_established = 0; - conn->input_frame_rate_starting_point_is_valid = 0; - conn->initial_reference_time = 0; - conn->initial_reference_timestamp = 0; - + reset_input_flow_metrics(conn); debug_mutex_unlock(&conn->flush_mutex, 3); #ifdef CONFIG_METADATA diff --git a/player.h b/player.h index a7531b90..2f1a3284 100644 --- a/player.h +++ b/player.h @@ -218,7 +218,8 @@ typedef struct { // slightly above or below. int local_to_remote_time_gradient_sample_count; // the number of samples used to calculate the // gradient - uint64_t local_to_remote_time_difference; // used to switch between local and remote clocks + // add the following to the local time to get the remote time modulo 2^64 + uint64_t local_to_remote_time_difference; // used to switch between local and remote clocks uint64_t local_to_remote_time_difference_measurement_time; // when the above was calculated int last_stuff_request; @@ -242,7 +243,8 @@ typedef struct { void *dapo_private_storage; // this is used for compatibility, if dacp stuff isn't enabled. } rtsp_conn_info; -uint32_t rtp_frame_offset(uint32_t from, uint32_t to); +uint32_t modulo_32_offset(uint32_t from, uint32_t to); +uint64_t modulo_64_offset(uint64_t from, uint64_t to); int player_play(rtsp_conn_info *conn); int player_stop(rtsp_conn_info *conn); diff --git a/rtp.c b/rtp.c index eb86da48..35b2a2bc 100644 --- a/rtp.c +++ b/rtp.c @@ -1042,21 +1042,23 @@ const int use_nominal_rate = 0; // specify whether to use the nominal input rate int sanitised_source_rate_information(uint32_t *frames, uint64_t *time, rtsp_conn_info *conn) { int result = 1; - *frames = conn->input_rate; - *time = (uint64_t)(0x100000000); // one second in fp form + uint32_t fs = conn->input_rate; + *frames = fs; + uint64_t one_fp = (uint64_t)(0x100000000); // one second in fp form + *time = one_fp; if ((conn->packet_stream_established) && (conn->initial_reference_time) && (conn->initial_reference_timestamp)) { // uint32_t local_frames = conn->reference_timestamp - conn->initial_reference_timestamp; uint32_t local_frames = - rtp_frame_offset(conn->initial_reference_timestamp, conn->reference_timestamp); + modulo_32_offset(conn->initial_reference_timestamp, conn->reference_timestamp); uint64_t local_time = conn->remote_reference_timestamp_time - conn->initial_reference_time; if ((local_frames == 0) || (local_time == 0) || (use_nominal_rate)) { result = 1; } else { - double calculated_frame_rate = ((1.0 * local_frames) / local_time) * (uint64_t)0x100000000; - if (((calculated_frame_rate / conn->input_rate) > 1.001) || - ((calculated_frame_rate / conn->input_rate) < 0.999)) { - // debug(1, "input frame rate out of bounds at %.2f fps.", calculated_frame_rate); + double calculated_frame_rate = ((1.0 * local_frames) / local_time) * one_fp; + if (((calculated_frame_rate / conn->input_rate) > 1.002) || + ((calculated_frame_rate / conn->input_rate) < 0.998)) { + debug(1, "input frame rate out of bounds at %.2f fps.", calculated_frame_rate); result = 1; } else { *frames = local_frames; @@ -1080,24 +1082,24 @@ int frame_to_local_time(uint32_t timestamp, uint64_t *time, rtsp_conn_info *conn uint64_t timestamp_interval_time; uint64_t remote_time_of_timestamp; - uint32_t timestamp_interval = rtp_frame_offset(conn->reference_timestamp, timestamp); + uint32_t timestamp_interval = modulo_32_offset(conn->reference_timestamp, timestamp); if (timestamp_interval <= - conn->input_rate * 10) { // i.e. timestamp was really after the reference timestamp + conn->input_rate * 3600) { // i.e. timestamp was really after the reference timestamp timestamp_interval_time = (timestamp_interval * time_difference) / - (frame_difference); // this is the nominal time, based on the - // fps specified between current and - // previous sync frame. + frame_difference; // this is the nominal time, based on the + // fps specified between current and + // previous sync frame. remote_time_of_timestamp = conn->remote_reference_timestamp_time + timestamp_interval_time; // based on the reference timestamp time // plus the time interval calculated based // on the specified fps. - } else { // i.e. timestamp was actually after the reference timestamp + } else { // i.e. timestamp was actually before the reference timestamp timestamp_interval = - rtp_frame_offset(timestamp, conn->reference_timestamp); // fix the calculation + modulo_32_offset(timestamp, conn->reference_timestamp); // fix the calculation timestamp_interval_time = (timestamp_interval * time_difference) / - (frame_difference); // this is the nominal time, based on the - // fps specified between current and - // previous sync frame. + frame_difference; // this is the nominal time, based on the + // fps specified between current and + // previous sync frame. remote_time_of_timestamp = conn->remote_reference_timestamp_time - timestamp_interval_time; // based on the reference timestamp time // plus the time interval calculated based @@ -1122,15 +1124,18 @@ int local_time_to_frame(uint64_t time, uint32_t *frame, rtsp_conn_info *conn) { uint64_t time_interval; // here, we calculate the time interval, in terms of remote time - if (remote_time >= conn->remote_reference_timestamp_time) + uint64_t offset = modulo_64_offset(conn->remote_reference_timestamp_time, remote_time); + int reference_time_was_earlier = (offset <= (uint64_t)0x100000000 * 3600); + if (reference_time_was_earlier) // if we haven't had a reference within the last hour, it'll be + // taken as afterwards time_interval = remote_time - conn->remote_reference_timestamp_time; else time_interval = conn->remote_reference_timestamp_time - remote_time; // now, convert the remote time interval into frames using the frame rate we have observed or // which has been nominated - int64_t frame_interval = (time_interval * frame_difference) / time_difference; - if (remote_time >= conn->remote_reference_timestamp_time) { + uint32_t frame_interval = (time_interval * frame_difference) / time_difference; + if (reference_time_was_earlier) { // debug(1,"Frame interval is %" PRId64 " frames.",frame_interval); *frame = (conn->reference_timestamp + frame_interval); } else { From 06716b70a9d7d073a1b1c02f1567be370096affb Mon Sep 17 00:00:00 2001 From: Mike Brady Date: Tue, 23 Oct 2018 10:29:09 +0000 Subject: [PATCH 14/16] Improve handling of network delays and outages -- more robust error handling. --- configure.ac | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index 55f63090..2968d410 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.3d18], [mikebrady@eircom.net]) +AC_INIT([shairport-sync], [3.3d19], [mikebrady@eircom.net]) AM_INIT_AUTOMAKE AC_CONFIG_SRCDIR([shairport.c]) AC_CONFIG_HEADERS([config.h]) From 3ae8c4e9bc08f0324576c148a48c670370773cc5 Mon Sep 17 00:00:00 2001 From: Mike Brady Date: Tue, 23 Oct 2018 12:15:00 +0100 Subject: [PATCH 15/16] Quieten an unnecessary debug message. --- player.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/player.c b/player.c index c107bc34..a4b14bbb 100644 --- a/player.c +++ b/player.c @@ -2114,7 +2114,7 @@ void *player_thread_func(void *arg) { char *long_silence = malloc(conn->output_bytes_per_frame * silence_length_sized); if (long_silence) { memset(long_silence, 0, conn->output_bytes_per_frame * silence_length_sized); - debug(1,"Play a silence of %d frames.",silence_length_sized); + debug(2,"Play a silence of %d frames.",silence_length_sized); config.output->play(long_silence, silence_length_sized); free(long_silence); } else { From 4faa654ba995b2ef2adbfe0664bd074024f6ebaf Mon Sep 17 00:00:00 2001 From: Mike Brady Date: Sun, 28 Oct 2018 14:20:22 +0000 Subject: [PATCH 16/16] Remove a signed/unsigned comparison warning --- player.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/player.c b/player.c index a4b14bbb..436eeb0c 100644 --- a/player.c +++ b/player.c @@ -1136,7 +1136,7 @@ static abuf_t *buffer_get_frame(rtsp_conn_info *conn) { int64_t frame_gap = (lead_time * config.output_rate) >> 32; // debug(1,"%d frames needed.",frame_gap); while (frame_gap > 0) { - size_t fs = config.output_rate / 10; + ssize_t fs = config.output_rate / 10; if (fs > frame_gap) fs = frame_gap;