]> git.ipfire.org Git - thirdparty/shairport-sync.git/commitdiff
Improve handling of player lock and simplify handling of loss of connection and impro...
authorMike Brady <mikebrady@eircom.net>
Sun, 27 Aug 2017 11:43:42 +0000 (12:43 +0100)
committerMike Brady <mikebrady@eircom.net>
Sun, 27 Aug 2017 11:43:42 +0000 (12:43 +0100)
common.c
player.c
player.h
rtp.c
rtsp.c

index 8f615be4c143aca431e50ceba8cd46e8eeb89898..08243fb3bca73d2e600882ebd545950021a6cfef 100644 (file)
--- a/common.c
+++ b/common.c
@@ -103,7 +103,7 @@ void die(const char *format, ...) {
   va_start(args, format);
   vsprintf(s, format, args);
   va_end(args);
-  daemon_log(LOG_EMERG, "%s", s);
+  daemon_log(LOG_EMERG, "fatal error: %s", s);
   shairport_shutdown();
   exit(1);
 }
index a0dee6f7d803fe44520079d7abde743b1b73b271..9605d7b793ae367de974768f8b75d4c76586edc3 100644 (file)
--- a/player.c
+++ b/player.c
@@ -707,14 +707,15 @@ static abuf_t *buffer_get_frame(rtsp_conn_info *conn) {
     // config.timeout of zero means don't check..., but iTunes may be confused by a long gap
     // followed by a resumption...
 
-    if ((conn->time_of_last_audio_packet != 0) && (conn->shutdown_requested == 0) &&
+    if ((conn->time_of_last_audio_packet != 0) && (conn->stop == 0) &&
         (config.dont_check_timeout == 0)) {
       uint64_t ct = config.timeout; // go from int to 64-bit int
+//      if (conn->packet_count>500) { //for testing -- about 4 seconds of play first
       if ((local_time_now > conn->time_of_last_audio_packet) &&
           (local_time_now - conn->time_of_last_audio_packet >= ct << 32)) {
-        debug(1, "As Yeats almost said, \"Too long a silence / can make a stone of the heart\"");
-        rtsp_request_shutdown_stream();
-        conn->shutdown_requested = 1;
+        debug(1, "As Yeats almost said, \"Too long a silence / can make a stone of the heart\" from RTSP conversation %d.",conn->connection_number);
+        conn->stop = 1;
+        pthread_kill(conn->thread, SIGUSR1);
       }
     }
     int rco = get_requested_connection_state_to_output();
@@ -1461,7 +1462,7 @@ static void *player_thread_func(void *arg) {
   conn->play_number_after_flush = 0;
   //  int last_timestamp = 0; // for debugging only
   conn->time_of_last_audio_packet = 0;
-  conn->shutdown_requested = 0;
+  // conn->shutdown_requested = 0;
   number_of_statistics = oldest_statistic = newest_statistic = 0;
   tsum_of_sync_errors = tsum_of_corrections = tsum_of_insertions_and_deletions = tsum_of_drifts = 0;
 
index b0422daa5f3c25eba45aacca1bedc16972590393..9eee0956d04f8c89dc41ca5798c9704e1050ceb5 100644 (file)
--- a/player.h
+++ b/player.h
@@ -72,7 +72,6 @@ typedef struct {
   alac_file *decoder_info;
   uint32_t please_stop;
   uint64_t packet_count;
-  int shutdown_requested;
   int connection_state_to_output;
   int player_thread_please_stop;
   int64_t first_packet_time_to_play, time_since_play_started; // nanoseconds
diff --git a/rtp.c b/rtp.c
index 4dd75026184cd97c5a703811c5eac94335be6b40..092fb123bac2390164c64fafa0004c57b8467a65 100644 (file)
--- a/rtp.c
+++ b/rtp.c
@@ -290,7 +290,7 @@ void *rtp_timing_sender(void *arg) {
     // debug(1,"Send a timing request");
 
     if (!conn->rtp_running)
-      die("rtp_timing_sender called without active stream!");
+      debug(1,"rtp_timing_sender called without active stream in RTSP conversation thread %d!",conn->connection_number);
 
     // debug(1, "Requesting ntp timestamp exchange.");
 
@@ -639,8 +639,8 @@ void rtp_setup(SOCKADDR *local, SOCKADDR *remote, int cport, int tport, uint32_t
   inet_ntop(conn->connection_ip_family, self_addr, conn->self_ip_string,
             sizeof(conn->self_ip_string));
 
-  debug(1, "Set up play connection from %s to self at %s.", conn->client_ip_string,
-        conn->self_ip_string);
+  debug(1, "Set up play connection from %s to self at %s on RTSP conversation thread %d.", conn->client_ip_string,
+        conn->self_ip_string, conn->connection_number);
 
   // set up a the record of the remote's control socket
   struct addrinfo hints;
diff --git a/rtsp.c b/rtsp.c
index 762119125c24a41ec24343397fcde8121803bfe6..4ea0b28072f688f0dc705560dd10d39044a2785d 100644 (file)
--- a/rtsp.c
+++ b/rtsp.c
@@ -243,39 +243,11 @@ int pc_queue_get_item(pc_queue *the_queue, void *the_stuff) {
 
 void ask_other_rtsp_conversation_threads_to_stop(pthread_t except_this_thread);
 
-// determine if we are the currently playing thread
-static inline int rtsp_playing(void) {
-  if (pthread_mutex_trylock(&play_lock)) {
-    // if playing_mutex is locked...
-    // return 0 if the threads are different, non-zero if the threads are the same
-    return pthread_equal(playing_conn->thread, pthread_self());
-  } else {
-    // you actually acquired the playing_mutex, implying that there is no currently playing thread
-    // so unlock it return 0, implying you are not playing
-    pthread_mutex_unlock(&play_lock);
-    return 0;
-  }
-}
-
 void rtsp_request_shutdown_stream(void) {
   debug(1, "Request to shut down all rtsp conversation threads");
   ask_other_rtsp_conversation_threads_to_stop(0); // i.e. ask all playing threads to stop
 }
 
-// static void rtsp_take_player(void) {
-//  if (rtsp_playing())
-//    return;
-
-//  if (pthread_mutex_trylock(&playing_mutex)) {
-//    debug(1, "Request to all other playing threads to stop.");
-//    ask_other_rtsp_conversation_threads_to_stop(
-//        pthread_self()); // all threads apart from self
-//    pthread_mutex_lock(&playing_mutex);
-//  }
-//  playing_thread =
-//      pthread_self(); // make us the currently-playing thread (why?)
-//}
-
 // keep track of the threads we have spawned so we can join() them
 static int nconns = 0;
 static void track_thread(rtsp_conn_info *conn) {
@@ -666,23 +638,24 @@ static void handle_options(rtsp_conn_info *conn, rtsp_message *req, rtsp_message
 
 static void handle_teardown(rtsp_conn_info *conn, rtsp_message *req, rtsp_message *resp) {
   debug(3, "Connection %d: TEARDOWN",conn->connection_number);
-  if (!rtsp_playing())
-    debug(1, "This RTSP connection thread (%d) doesn't think it's playing, but "
-             "it's sending a response to teardown anyway",conn->connection_number);
+  //if (!rtsp_playing())
+  //  debug(1, "This RTSP connection thread (%d) doesn't think it's playing, but "
+  //           "it's sending a response to teardown anyway",conn->connection_number);
   resp->respcode = 200;
   msg_add_header(resp, "Connection", "close");
   
        debug(1, "TEARDOWN: synchronously terminating the player thread of RTSP conversation thread %d (2).",conn->connection_number);
-       if (rtsp_playing()) {
-               player_stop(conn); // might be less noisy doing this first
-       }
+       //if (rtsp_playing()) {
+               player_stop(conn);
+               debug(1, "TEARDOWN: successful termination of playing thread of RTSP conversation thread %d.",conn->connection_number);
+       //}
 }
 
 static 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 "
-             "it's sending a response to flush anyway",conn->connection_number);
+//  if (!rtsp_playing())
+//    debug(1, "This RTSP conversation thread (%d) doesn't think it's playing, but "
//            "it's sending a response to flush anyway",conn->connection_number);
   char *p = NULL;
   uint32_t rtptime = 0;
   char *hdr = msg_get_header(req, "RTP-Info");
@@ -704,7 +677,7 @@ static void handle_flush(rtsp_conn_info *conn, rtsp_message *req, rtsp_message *
   else
     send_metadata('ssnc', 'flsr', NULL, 0, NULL, 0);
 #endif
-  player_flush(rtptime, conn);
+  player_flush(rtptime, conn); // will not crash even it there is no player thread.
   resp->respcode = 200;
 }
 
@@ -847,7 +820,8 @@ static void handle_setup(rtsp_conn_info *conn, rtsp_message *req, rtsp_message *
   return;
 
 error:
-  warn("Error in setup request.");
+  warn("Error in setup request -- unlocking play lock on RTSP conversation thread %d.",conn->connection_number);
+  playing_conn = NULL;
   pthread_mutex_unlock(&play_lock);
   resp->respcode = 451; // invalid arguments
 }
@@ -1401,7 +1375,7 @@ static void handle_set_parameter(rtsp_conn_info *conn, rtsp_message *req, rtsp_m
 }
 
 static void handle_announce(rtsp_conn_info *conn, rtsp_message *req, rtsp_message *resp) {
-  debug(3,"Connection %d: ANNOUNCE",conn->connection_number);
+  debug(1,"Connection %d: ANNOUNCE",conn->connection_number);
   int have_the_player = 0;
 
   // interrupt session if permitted
@@ -1439,6 +1413,7 @@ static void handle_announce(rtsp_conn_info *conn, rtsp_message *req, rtsp_messag
 
   if (have_the_player) {
     playing_conn = conn;  // the present connection is now playing
+    debug(1,"RTSP conversation thread %d has acquired play lock.",conn->connection_number);
     resp->respcode = 456; // 456 - Header Field Not Valid for Resource
     char *paesiv = NULL;
     char *prsaaeskey = NULL;
@@ -1537,6 +1512,8 @@ static void handle_announce(rtsp_conn_info *conn, rtsp_message *req, rtsp_messag
 
 out:
   if (resp->respcode != 200 && resp->respcode != 453) {
+    debug(1,"Error in handling ANNOUNCE on conversation thread %d. Unlocking the play lock.",conn->connection_number);
+    playing_conn = NULL;
     pthread_mutex_unlock(&play_lock);
   }
 }
@@ -1823,12 +1800,9 @@ static void *rtsp_conversation_thread_func(void *pconn) {
     } else {
        if ((reply==rtsp_read_request_response_immediate_shutdown_requested) ||
                (reply==rtsp_read_request_response_channel_closed)) {
-
-                               if ((rtsp_playing()) && (conn->player_thread)) {
-                                 debug(1, "Synchronously terminate playing thread of RTSP conversation thread %d.",conn->connection_number);
-                                       player_stop(conn); // might be less noisy doing this first
-                                 debug(1, "Successful termination of playing thread of RTSP conversation thread %d.",conn->connection_number);
-                               }
+        debug(1, "Synchronously terminate playing thread of RTSP conversation thread %d.",conn->connection_number);
+        player_stop(conn);
+        debug(1, "Successful termination of playing thread of RTSP conversation thread %d.",conn->connection_number);
                                debug(1, "Request termination of RTSP conversation thread %d.",conn->connection_number);
                                conn->stop = 1;
        } else {
@@ -1841,14 +1815,12 @@ static void *rtsp_conversation_thread_func(void *pconn) {
     close(conn->fd);
   if (auth_nonce)
     free(auth_nonce);
-  //    pthread_mutex_unlock(&playing_mutex);
-  // usleep(1000000);
-  //  } // else {
-  // debug(1, "This RTSP conversation thread doesn't think it's playing for a "
-  //         "close RTSP connection.");
-  // }
   rtp_terminate(conn);
-       pthread_mutex_unlock(&play_lock);
+  if (playing_conn==conn) {
+    debug(1,"Unlocking play lock on RTSP conversation thread %d.",conn->connection_number);
+    playing_conn = NULL;
+    pthread_mutex_unlock(&play_lock);
+       }
   debug(1, "RTSP conversation thread %d terminated.",conn->connection_number);
   //  please_shutdown = 0;
   conn->running = 0;