]> git.ipfire.org Git - thirdparty/shairport-sync.git/commitdiff
Tighten up integer promotion checks to calculate large delays properly. Fix input...
authorMike Brady <mikebrady@eircom.net>
Tue, 23 Oct 2018 10:24:40 +0000 (10:24 +0000)
committerMike Brady <mikebrady@eircom.net>
Tue, 23 Oct 2018 10:26:08 +0000 (10:26 +0000)
audio_alsa.c
player.c
player.h
rtp.c

index 056fe7a7ec7b8f5b592388a61768102d64c6fb3a..f0fae65a78c1e96d4516cdd8063cbe8363098e12 100644 (file)
@@ -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);
index f69a563c793d55f6d4f2caf0c7152710c9442c8f..c107bc34ab8fb0623f989d470f2eb13c3916421c 100644 (file)
--- a/player.c
+++ b/player.c
 // 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
index a7531b906eb3ddefd121b4d30da88de53c1b4b43..2f1a3284eebb3b10683abe23031ee568cb714767 100644 (file)
--- 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 eb86da480288466e23e216726599337751f3b8ec..35b2a2bc4d72c882095bad05c869bfdcd3041431 100644 (file)
--- 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 {