]> git.ipfire.org Git - thirdparty/shairport-sync.git/commitdiff
Move to using pthread_cancel in place of pthread_kill -- due to a problem in CYGWIN...
authorMike Brady <mikebrady@eircom.net>
Thu, 17 May 2018 09:16:44 +0000 (10:16 +0100)
committerMike Brady <mikebrady@eircom.net>
Thu, 17 May 2018 09:16:44 +0000 (10:16 +0100)
mdns.c
player.c
player.h
rtp.c
rtsp.c

diff --git a/mdns.c b/mdns.c
index e45dacc511cae4537184a99c6d0afc17ecd4c6ea..f6a191a3d65b273e6774bb23bf8557933380248b 100644 (file)
--- a/mdns.c
+++ b/mdns.c
@@ -113,7 +113,7 @@ void *mdns_dacp_monitor(char *dacp_id) {
       debug(1, "Error starting a DACP monitor.");
     }
   } else
-    debug(1, "Can't start a DACP monitor -- none registered.");
+    debug(3, "Can't start a DACP monitor -- none registered.");
   }
   return reply;
 }
@@ -122,7 +122,7 @@ void mdns_dacp_dont_monitor(void *userdata) {
   if ((config.mdns) && (config.mdns->mdns_dacp_dont_monitor)) {
     config.mdns->mdns_dacp_dont_monitor(userdata);
   } else
-    debug(1, "Can't stop a DACP monitor -- none registered.");
+    debug(3, "Can't stop a DACP monitor -- none registered.");
 }
 void mdns_ls_backends(void) {
   mdns_backend **b = NULL;
index f1e1e6bda433aeb577314c13c6430f405daac57e..41199da9755e0290072304a8da42ee5852e770de 100644 (file)
--- a/player.c
+++ b/player.c
@@ -1449,7 +1449,6 @@ static void *player_thread_func(void *arg) {
 
   // it's safe now
 
-  conn->please_stop = 0;
   conn->packet_count = 0;
   conn->previous_random_number = 0;
   conn->input_bytes_per_frame = 4;
@@ -2305,7 +2304,7 @@ static void *player_thread_func(void *arg) {
     }
   }
 
-  debug(3, "Play thread main loop exited.");
+  debug(3, "Connection %d: player thread main loop exit.", conn->connection_number);
 
   if (config.statistics_requested) {
     int rawSeconds = (int)difftime(time(NULL), playstart);
@@ -2327,23 +2326,16 @@ static void *player_thread_func(void *arg) {
   if (config.output->stop)
     config.output->stop();
 
-  debug(2, "Shutting down audio, control and timing threads");
-  conn->please_stop = 1;
-  pthread_kill(rtp_audio_thread, SIGUSR1);
-  pthread_kill(rtp_control_thread, SIGUSR1);
-  pthread_kill(rtp_timing_thread, SIGUSR1);
+  debug(3, "Shutting down timing, control and audio threads");
+  pthread_cancel(rtp_timing_thread);
   pthread_join(rtp_timing_thread, NULL);
-  debug(2, "timing thread joined");
-  pthread_join(rtp_audio_thread, NULL);
-  debug(2, "audio thread joined");
+  pthread_cancel(rtp_control_thread);
   pthread_join(rtp_control_thread, NULL);
-  debug(2, "control thread joined");
+  pthread_cancel(rtp_audio_thread);
+  pthread_join(rtp_audio_thread, NULL);
   clear_reference_timestamp(conn);
   conn->rtp_running = 0;
 
-  // usleep(100000); // allow this time to (?) allow the alsa subsystem to finish cleaning up after
-  // itself. 50 ms seems too short
-
   debug(3, "Freeing audio buffers and decoders.");
 
   free_audio_buffers(conn);
@@ -2362,7 +2354,7 @@ static void *player_thread_func(void *arg) {
   if (rc)
     debug(1, "Error destroying vol_mutex variable.");
 
-  debug(2, "Player thread exit on RTSP conversation thread %d.", conn->connection_number);
+  debug(2, "Connection %d: player thread terminated.", conn->connection_number);
   if (conn->dacp_id) {
     free(conn->dacp_id);
     conn->dacp_id = NULL;
@@ -2373,7 +2365,7 @@ static void *player_thread_func(void *arg) {
     free(tbuf);
   if (sbuf)
     free(sbuf);
-  return 0;
+  pthread_exit(NULL);
 }
 
 // takes the volume as specified by the airplay protocol
index d941c65841b613aa794195935f19bd332f87b7a0..eb8046e60ff36323b1acb028db6b71f40f2911b3 100644 (file)
--- a/player.h
+++ b/player.h
@@ -93,7 +93,6 @@ typedef struct {
   int max_frame_size_change;
   int64_t previous_random_number;
   alac_file *decoder_info;
-  uint32_t please_stop;
   uint64_t packet_count;
   int connection_state_to_output;
   int player_thread_please_stop;
@@ -180,7 +179,6 @@ typedef struct {
 
   uint64_t local_to_remote_time_difference; // used to switch between local and remote clocks
 
-  int timing_sender_stop; // for asking the timing-sending thread to stop
   int last_stuff_request;
 
   int64_t play_segment_reference_frame;
diff --git a/rtp.c b/rtp.c
index 4189d1650678c47a8ddc4a55a446c483e7af15b0..a900c7a4c5b117911c3943f9bf10ff3e88e3c03f 100644 (file)
--- a/rtp.c
+++ b/rtp.c
@@ -48,8 +48,6 @@
 uint64_t local_to_remote_time_jitters;
 uint64_t local_to_remote_time_jitters_count;
 
-void memory_barrier();
-
 void rtp_initialise(rtsp_conn_info *conn) {
   conn->rtp_time_of_last_resend_request_error_fp = 0;
   conn->rtp_running = 0;
@@ -67,10 +65,13 @@ void rtp_terminate(rtsp_conn_info *conn) {
     debug(1, "Error destroying reference_time_mutex variable.");
 }
 
-void *rtp_audio_receiver(void *arg) {
-  debug(3, "Audio receiver -- Server RTP thread starting.");
+void rtp_audio_receiver_cleanup_handler(void *arg) {
+  rtsp_conn_info *conn = (rtsp_conn_info *)arg;
+  close(conn->audio_socket);
+}
 
-  // we inherit the signal mask (SIGUSR1)
+void *rtp_audio_receiver(void *arg) {
+  pthread_cleanup_push(rtp_audio_receiver_cleanup_handler, arg);
   rtsp_conn_info *conn = (rtsp_conn_info *)arg;
 
   int32_t last_seqno = -1;
@@ -87,17 +88,8 @@ void *rtp_audio_receiver(void *arg) {
   float stat_M2 = 0.0;
 
   ssize_t nread;
-  while (conn->please_stop == 0) {
-    fd_set readfds;
-    FD_ZERO(&readfds);
-    FD_SET(conn->audio_socket, &readfds);
-    do {
-      memory_barrier();
-    } while (conn->please_stop == 0 &&
-             pselect(conn->audio_socket + 1, &readfds, NULL, NULL, NULL, &pselect_sigset) <= 0);
-    if (conn->please_stop != 0) {
-      break;
-    }
+  while (1) {
+
     nread = recv(conn->audio_socket, packet, sizeof(packet), 0);
 
     uint64_t local_time_now_fp = get_absolute_time_in_fp();
@@ -181,17 +173,24 @@ void *rtp_audio_receiver(void *arg) {
     }
   }
 
+  /*
   debug(3, "Audio receiver -- Server RTP thread interrupted. terminating.");
   close(conn->audio_socket);
+  */
+  
+  debug(1, "Audio receiver thread \"normal\" exit -- this can't happen. Hah!");
+  pthread_cleanup_pop(0); // don't execute anything here.
+  debug(2, "Audio receiver thread exit.");
+  pthread_exit(NULL);
+}
 
-  return NULL;
+void rtp_control_handler_cleanup_handler(void *arg) {
+  rtsp_conn_info *conn = (rtsp_conn_info *)arg;
+  close(conn->control_socket);
 }
 
 void *rtp_control_receiver(void *arg) {
-  // we inherit the signal mask (SIGUSR1)
-
-  debug(3, "Control receiver -- Server RTP thread starting.");
-
+  pthread_cleanup_push(rtp_control_handler_cleanup_handler, arg);
   rtsp_conn_info *conn = (rtsp_conn_info *)arg;
 
   conn->reference_timestamp = 0; // nothing valid received yet
@@ -200,18 +199,7 @@ void *rtp_control_receiver(void *arg) {
   uint64_t remote_time_of_sync;
   int64_t sync_rtp_timestamp;
   ssize_t nread;
-  while (conn->please_stop == 0) {
-    fd_set readfds;
-    FD_ZERO(&readfds);
-    FD_SET(conn->control_socket, &readfds);
-    do {
-      memory_barrier();
-    } while (conn->please_stop == 0 &&
-             pselect(conn->control_socket + 1, &readfds, NULL, NULL, NULL, &pselect_sigset) <= 0);
-    if (conn->please_stop != 0) {
-      break;
-    }
-
+  while (1) {
     nread = recv(conn->control_socket, packet, sizeof(packet), 0);
     // local_time_now = get_absolute_time_in_fp();
     //        clock_gettime(CLOCK_MONOTONIC,&tn);
@@ -223,50 +211,50 @@ void *rtp_control_receiver(void *arg) {
           (drand48() > config.diagnostic_drop_packet_fraction)) {
 
         ssize_t plen = nread;
-        if (packet[1] == 0xd4) {                       // sync data
-                                                       /*
-                                                             // the following stanza is for debugging only -- normally commented out.
-                                                             {
-                                                               char obf[4096];
-                                                               char *obfp = obf;
-                                                               int obfc;
-                                                               for (obfc = 0; obfc < plen; obfc++) {
-                                                                 snprintf(obfp, 3, "%02X", packet[obfc]);
-                                                                 obfp += 2;
-                                                               };
-                                                               *obfp = 0;
-                                             
-                                             
-                                                               // get raw timestamp information
-                                                               // I think that a good way to understand these timestamps is that
-                                                               // (1) the rtlt below is the timestamp of the frame that should be playing at the
-                                                          client-time specified in the packet if there was no delay
-                                                               // and (2) that the rt below is the timestamp of the frame that should be playing
-                                                          at
-                                                          the client-time specified in the packet on this device taking account of the delay
-                                                               // Thus, (3) the latency can be calculated by subtracting the second from the
-                                                          first.
-                                                               // There must be more to it -- there something missing.
-                                             
-                                                               // In addition, it seems that if the value of the short represented by the second
-                                                          pair
-                                                          of bytes in the packe is 7
-                                                               // then an extra time lag is expected to be added, presumably by the AirPort
-                                                          Express.
-                                                          Best guess is that this delay is 11,025 frames.
-                                             
-                                                               // uint32_t rtlt = nctohl(&packet[4]); // raw timestamp less latency
-                                                               // uint32_t rt = nctohl(&packet[16]);  // raw timestamp
-                                             
-                                                               // uint32_t fl = nctohs(&packet[2]); //
-                                             
-                                                               // debug(1,"Sync Packet of %d bytes received: \"%s\", flags: %d, timestamps %u and
-                                                          %u,
-                                                          giving a latency of %d frames.",plen,obf,fl,rt,rtlt,rt-rtlt);
-                                                               // debug(1,"Monotonic timestamps are: %" PRId64 " and %" PRId64 "
-                                                          respectively.",monotonic_timestamp(rt, conn),monotonic_timestamp(rtlt, conn));
-                                                             }
-                                                       */
+        if (packet[1] == 0xd4) {                     // sync data
+      /*
+           // the following stanza is for debugging only -- normally commented out.
+           {
+             char obf[4096];
+             char *obfp = obf;
+             int obfc;
+             for (obfc = 0; obfc < plen; obfc++) {
+               snprintf(obfp, 3, "%02X", packet[obfc]);
+               obfp += 2;
+             };
+             *obfp = 0;
+
+
+             // get raw timestamp information
+             // I think that a good way to understand these timestamps is that
+             // (1) the rtlt below is the timestamp of the frame that should be playing at the
+             // client-time specified in the packet if there was no delay
+             // and (2) that the rt below is the timestamp of the frame that should be playing
+             // at the client-time specified in the packet on this device taking account of
+             // the delay
+             // Thus, (3) the latency can be calculated by subtracting the second from the
+             // first.
+             // There must be more to it -- there something missing.
+
+             // In addition, it seems that if the value of the short represented by the second
+             // pair of bytes in the packe is 7
+             // then an extra time lag is expected to be added, presumably by
+             // the AirPort Express.
+             
+             // Best guess is that this delay is 11,025 frames.
+
+             // uint32_t rtlt = nctohl(&packet[4]); // raw timestamp less latency
+             // uint32_t rt = nctohl(&packet[16]);  // raw timestamp
+
+             // uint32_t fl = nctohs(&packet[2]); //
+
+             // debug(1,"Sync Packet of %d bytes received: \"%s\", flags: %d, timestamps %u and
+        %u,
+        giving a latency of %d frames.",plen,obf,fl,rt,rtlt,rt-rtlt);
+             // debug(1,"Monotonic timestamps are: %" PRId64 " and %" PRId64 "
+        respectively.",monotonic_timestamp(rt, conn),monotonic_timestamp(rtlt, conn));
+           }
+      */
           if (conn->local_to_remote_time_difference) { // need a time packet to be interchanged
                                                        // first...
 
@@ -406,15 +394,13 @@ void *rtp_control_receiver(void *arg) {
       debug(1, "Control Receiver -- error receiving a packet.");
     }
   }
-
-  debug(3, "Control RTP thread interrupted. terminating.");
-  close(conn->control_socket);
-
-  return NULL;
+  debug(1, "Control RTP thread \"normal\" exit -- this can't happen. Hah!");
+  pthread_cleanup_pop(0); // don't execute anything here.
+  debug(2, "Control RTP thread exit.");
+  pthread_exit(NULL);
 }
 
 void *rtp_timing_sender(void *arg) {
-  debug(3, "Timing sender thread starting.");
   rtsp_conn_info *conn = (rtsp_conn_info *)arg;
   struct timing_request {
     char leader;
@@ -434,9 +420,7 @@ void *rtp_timing_sender(void *arg) {
   req.seqno = htons(7);
 
   conn->time_ping_count = 0;
-
-  // we inherit the signal mask (SIGUSR1)
-  while (conn->timing_sender_stop == 0) {
+  while (1) {
     // debug(1,"Send a timing request");
 
     if (!conn->rtp_running)
@@ -456,17 +440,6 @@ void *rtp_timing_sender(void *arg) {
       msgsize = sizeof(struct sockaddr_in6);
     }
 #endif
-    fd_set writefds;
-    FD_ZERO(&writefds);
-    FD_SET(conn->timing_socket, &writefds);
-    do {
-      memory_barrier();
-    } while (conn->timing_sender_stop == 0 &&
-             pselect(conn->timing_socket + 1, NULL, &writefds, NULL, NULL, &pselect_sigset) <= 0);
-    if (conn->timing_sender_stop != 0) {
-      break;
-    }
-
     if ((config.diagnostic_drop_packet_fraction == 0.0) ||
         (drand48() > config.diagnostic_drop_packet_fraction)) {
       if (sendto(conn->timing_socket, &req, sizeof(req), 0,
@@ -481,35 +454,30 @@ void *rtp_timing_sender(void *arg) {
 
     request_number++;
 
-    // this is to deal with the possibility of missing a timing_sender_stop signal.
-    // if the signal came in just before the usleep, then it wouldn't cause the sleep to end.
-    // so, we will wait a maximum time of the wait_interval
-
-    int wait_time;
-    int wait_interval = 20000; // 20 milliseconds
-
     if (request_number <= 4)
-      wait_time = 500000;
+      usleep(500000); //these are thread cancellation points
     else
-      wait_time = 3000000;
-    while ((wait_time > 0) && (conn->timing_sender_stop == 0)) {
-      usleep(wait_interval);
-      wait_time -= wait_interval;
-    }
+      usleep(3000000);
   }
   debug(3, "rtp_timing_sender thread interrupted. terminating.");
-  return NULL;
+  pthread_exit(NULL);
+}
+
+pthread_t timer_requester;
+
+void rtp_timing_receiver_cleanup_handler(void *arg) {
+  rtsp_conn_info *conn = (rtsp_conn_info *)arg;
+  pthread_cancel(timer_requester);
+  pthread_join(timer_requester, NULL);
+  close(conn->timing_socket);
 }
 
 void *rtp_timing_receiver(void *arg) {
-  debug(3, "Timing receiver -- Server RTP thread starting.");
-  // we inherit the signal mask (SIGUSR1)
+  pthread_cleanup_push(rtp_timing_receiver_cleanup_handler, arg);
   rtsp_conn_info *conn = (rtsp_conn_info *)arg;
 
   uint8_t packet[2048];
   ssize_t nread;
-  conn->timing_sender_stop = 0;
-  pthread_t timer_requester;
   pthread_create(&timer_requester, NULL, &rtp_timing_sender, arg);
   //    struct timespec att;
   uint64_t distant_receive_time, distant_transmit_time, arrival_time, return_time;
@@ -521,17 +489,7 @@ void *rtp_timing_receiver(void *arg) {
   uint64_t first_local_to_remote_time_difference = 0;
   // uint64_t first_local_to_remote_time_difference_time;
   // uint64_t l2rtd = 0;
-  while (conn->please_stop == 0) {
-    fd_set readfds;
-    FD_ZERO(&readfds);
-    FD_SET(conn->timing_socket, &readfds);
-    do {
-      memory_barrier();
-    } while (conn->please_stop == 0 &&
-             pselect(conn->timing_socket + 1, &readfds, NULL, NULL, NULL, &pselect_sigset) <= 0);
-    if (conn->please_stop != 0) {
-      break;
-    }
+  while (1) {
     nread = recv(conn->timing_socket, packet, sizeof(packet), 0);
 
     if (nread >= 0) {
@@ -738,17 +696,10 @@ void *rtp_timing_receiver(void *arg) {
     }
   }
 
-  debug(3, "Timing thread interrupted. terminating.");
-  conn->timing_sender_stop = 1;
-  void *retval;
-  pthread_kill(timer_requester, SIGUSR1);
-  debug(3, "Wait for timer requester to exit.");
-  pthread_join(timer_requester, &retval);
-  debug(3, "Closed and terminated timer requester thread.");
-  debug(3, "Timing RTP thread terminated.");
-  close(conn->timing_socket);
-
-  return NULL;
+  debug(1, "Timing Receiver RTP thread \"normal\" exit -- this can't happen. Hah!");
+  pthread_cleanup_pop(0); // don't execute anything here.
+  debug(2, "Timing Receiver RTP thread exit.");
+  pthread_exit(NULL);
 }
 
 static uint16_t bind_port(int ip_family, const char *self_ip_address, uint32_t scope_id,
diff --git a/rtsp.c b/rtsp.c
index d62f61fc2800ff09a460d19d50484935dabd38ae..8366069ccb9a3fb54d086b940fefc30469074777 100644 (file)
--- a/rtsp.c
+++ b/rtsp.c
@@ -1972,8 +1972,7 @@ static void *rtsp_conversation_thread_func(void *pconn) {
     playing_conn = NULL;
     pthread_mutex_unlock(&play_lock);
   }
-  debug(2, "RTSP conversation thread %d terminated.", conn->connection_number);
-  //  please_shutdown = 0;
+  debug(2, "Connection %d: RTSP thread terminated.", conn->connection_number);
   conn->running = 0;
 
   // release the player_thread_lock
@@ -1982,7 +1981,7 @@ static void *rtsp_conversation_thread_func(void *pconn) {
     debug(1, "Error %d destroying player_thread_lock for conversation thread %d.", rwld,
           conn->connection_number);
 
-  return NULL;
+  pthread_exit(NULL);
 }
 
 /*