Audio transport cleanup

Fix some race conditions, implement reconnect, etc.

diff --git a/chime/chime-call-audio.c b/chime/chime-call-audio.c
index 5f3855f5352fc3737a08af7bc56323f3f9263433..599d01eb979fc5272936ea8bdb316d08e61bf29f 100644 (file)
--- a/chime/chime-call-audio.c
+++ b/chime/chime-call-audio.c
@@ -95,8 +95,11 @@ static gboolean audio_receive_rt_msg(ChimeCallAudio *audio, gconstpointer pkt, g
 
                const gchar *profile_id = g_hash_table_lookup(audio->profiles,
                                                              GUINT_TO_POINTER(msg->profiles[i]->stream_id));
-               if (!profile_id)
+               if (!profile_id) {
+                       chime_debug("no profile for stream id %d\n",
+                              msg->profiles[i]->stream_id);
                        continue;
+               }
 
                int vol;
                if (msg->profiles[i]->has_muted && msg->profiles[i]->muted)
@@ -120,6 +123,18 @@ static gboolean audio_receive_rt_msg(ChimeCallAudio *audio, gconstpointer pkt, g
        return TRUE;
 }
 
+static gboolean audio_reconnect(gpointer _audio)
+{
+       ChimeCallAudio *audio = _audio;
+
+       audio->timeout_source = 0;
+
+       chime_call_transport_disconnect(audio, TRUE);
+       chime_call_transport_connect(audio, audio->silent);
+
+       return G_SOURCE_REMOVE;
+}
+
 static gboolean timed_send_rt_packet(ChimeCallAudio *audio);
 static void do_send_rt_packet(ChimeCallAudio *audio, GstBuffer *buffer)
 {
@@ -128,6 +143,10 @@ static void do_send_rt_packet(ChimeCallAudio *audio, GstBuffer *buffer)
 
        g_mutex_lock(&audio->rt_lock);
        gint64 now = g_get_monotonic_time();
+       if (!audio->timeout_source && audio->last_rx + 10000000 < now) {
+               chime_debug("RX timeout, reconnect audio\n");
+               audio->timeout_source = g_timeout_add(0, audio_reconnect, audio);
+       }
        audio->audio_msg.seq = (audio->audio_msg.seq + 1) & 0xffff;
 
        if (audio->last_server_time_offset) {
@@ -194,7 +213,8 @@ static void do_send_rt_packet(ChimeCallAudio *audio, GstBuffer *buffer)
 
 static gboolean timed_send_rt_packet(ChimeCallAudio *audio)
 {
-       do_send_rt_packet(audio, NULL);
+       if (audio->state >= CHIME_AUDIO_STATE_AUDIOLESS)
+               do_send_rt_packet(audio, NULL);
        return TRUE;
 }
 
@@ -209,7 +229,8 @@ static gboolean audio_receive_auth_msg(ChimeCallAudio *audio, gconstpointer pkt,
                do_send_rt_packet(audio, NULL);
                chime_call_audio_set_state(audio, audio->silent ? CHIME_AUDIO_STATE_AUDIOLESS :
                                           (audio->local_mute ? CHIME_AUDIO_STATE_AUDIO_MUTED : CHIME_AUDIO_STATE_AUDIO));
-               if (audio->silent || audio->local_mute)
+               if ((audio->silent || audio->local_mute) &&
+                   !audio->send_rt_source)
                        audio->send_rt_source = g_timeout_add(100, (GSourceFunc)timed_send_rt_packet, audio);
        }
 
@@ -264,6 +285,21 @@ static void free_msgbuf(struct message_buf *m)
        g_free(m);
 }
 
+void chime_call_audio_cleanup_datamsgs(ChimeCallAudio *audio)
+{
+       if (audio->data_ack_source) {
+               g_source_remove(audio->data_ack_source);
+               audio->data_ack_source = 0;
+       }
+
+       g_slist_free_full(audio->data_messages, (GDestroyNotify) free_msgbuf);
+       audio->data_messages = NULL;
+
+       audio->data_next_seq = 0;
+       audio->data_ack_mask = 0;
+       audio->data_next_logical_msg = 0;
+}
+
 static void do_send_ack(ChimeCallAudio *audio)
 {
        DataMessage msg = DATA_MESSAGE__INIT;
@@ -391,7 +427,6 @@ static gboolean audio_receive_data_msg(ChimeCallAudio *audio, gconstpointer pkt,
        if (!audio->data_ack_source)
                audio->data_ack_source = g_idle_add(idle_send_ack, audio);
 
-
        /* Now process the incoming data packet. First, drop packets
           that look like replays and are too old. */
        if (msg->msg_id < audio->data_next_logical_msg)
@@ -437,6 +472,8 @@ gboolean audio_receive_packet(ChimeCallAudio *audio, gconstpointer pkt, gsize le
        if (len != ntohs(hdr->len))
                return FALSE;
 
+       audio->last_rx = g_get_monotonic_time();
+
        /* Point to the payload, without (void *) arithmetic */
        pkt = hdr + 1;
        len -= 4;
@@ -466,15 +503,10 @@ void chime_call_audio_close(ChimeCallAudio *audio, gboolean hangup)
        if (audio->audio_sink)
                gst_app_sink_set_callbacks(audio->audio_sink, &no_appsink_callbacks, NULL, NULL);
 
-       if (audio->data_ack_source)
-               g_source_remove(audio->data_ack_source);
-       if (audio->send_rt_source)
-               g_source_remove(audio->send_rt_source);
-
-       g_hash_table_destroy(audio->profiles);
-       g_slist_free_full(audio->data_messages, (GDestroyNotify) free_msgbuf);
        chime_call_transport_disconnect(audio, hangup);
        chime_call_audio_set_state(audio, CHIME_AUDIO_STATE_HANGUP);
+
+       g_hash_table_destroy(audio->profiles);
        g_free(audio);
 }
 
@@ -545,11 +577,14 @@ void chime_call_audio_install_gst_app_callbacks(ChimeCallAudio *audio, GstAppSrc
 ChimeCallAudio *chime_call_audio_open(ChimeConnection *cxn, ChimeCall *call, gboolean silent)
 {
        ChimeCallAudio *audio = g_new0(ChimeCallAudio, 1);
+
        audio->call = call;
        audio->profiles = g_hash_table_new_full(g_direct_hash, g_direct_equal, NULL, g_free);
        g_mutex_init(&audio->transport_lock);
        g_mutex_init(&audio->rt_lock);
 
+       audio->session_id = ((guint64)g_random_int() << 32) | g_random_int();
+
        rtmessage__init(&audio->rt_msg);
        audio_message__init(&audio->audio_msg);
        client_status_message__init(&audio->client_status_msg);
@@ -560,7 +595,6 @@ ChimeCallAudio *chime_call_audio_open(ChimeConnection *cxn, ChimeCall *call, gbo
        audio->audio_msg.sample_time = g_random_int();
 
        chime_call_transport_connect(audio, silent);
-       chime_call_audio_set_state(audio, CHIME_AUDIO_STATE_CONNECTING);
 
        return audio;
 }
@@ -570,17 +604,8 @@ void chime_call_audio_reopen(ChimeCallAudio *audio, gboolean silent)
 {
        chime_call_audio_local_mute(audio, silent);
        if (silent != audio->silent) {
-               if (audio->send_rt_source) {
-                       g_source_remove(audio->send_rt_source);
-                       audio->send_rt_source = 0;
-               }
-               if (audio->data_ack_source) {
-                       g_source_remove(audio->data_ack_source);
-                       audio->data_ack_source = 0;
-               }
                chime_call_transport_disconnect(audio, TRUE);
                chime_call_transport_connect(audio, silent);
-               chime_call_audio_set_state(audio, CHIME_AUDIO_STATE_CONNECTING);
        }
 }
 

diff --git a/chime/chime-call-audio.h b/chime/chime-call-audio.h
index 5c7ad51e29a92e1231dc4d577c58e2919df66fb3..663f8a9665cbc7f8fc0bcce97df616ea489ce3f2 100644 (file)
--- a/chime/chime-call-audio.h
+++ b/chime/chime-call-audio.h
@@ -40,7 +40,10 @@ struct _ChimeCallAudio {
        gboolean silent; /* No audio; only participant data */
        GMutex transport_lock;
        SoupWebsocketConnection *ws;
+       guint64 session_id;
 
+       time_t last_rx;
+       guint timeout_source;
        gboolean dtls_handshaked;
        GSocket *dtls_sock;
        GSource *dtls_source;
@@ -99,3 +102,4 @@ void chime_call_transport_send_packet(ChimeCallAudio *audio, enum xrp_pkt_type t
 gboolean audio_receive_packet(ChimeCallAudio *audio, gconstpointer pkt, gsize len);
 
 void chime_call_audio_install_gst_app_callbacks(ChimeCallAudio *audio, GstAppSrc *appsrc, GstAppSink *appsink);
+void chime_call_audio_cleanup_datamsgs(ChimeCallAudio *audio);

diff --git a/chime/chime-call-transport.c b/chime/chime-call-transport.c
index 6341ae2117a37a6380c935383b754c6a4adae743..6e4bbbc923c7e699a4935d1577335824c5835e88 100644 (file)
--- a/chime/chime-call-transport.c
+++ b/chime/chime-call-transport.c
@@ -52,7 +52,10 @@ static void hexdump(const void *buf, int len)
 
 static void on_audiows_closed(SoupWebsocketConnection *ws, gpointer _audio)
 {
-       /* XXX: Reconnect it */
+       ChimeCallAudio *audio = _audio;
+
+       chime_call_transport_disconnect(audio, FALSE);
+       chime_call_transport_connect(audio, audio->silent);
 }
 
 static void on_audiows_message(SoupWebsocketConnection *ws, gint type,
@@ -91,6 +94,9 @@ static void audio_send_auth_packet(ChimeCallAudio *audio)
        msg.profile_id = 0;
        msg.has_profile_id = TRUE;
 
+       msg.session_id = audio->session_id;
+       msg.has_session_id = TRUE;
+
        msg.profile_uuid = (char *)chime_connection_get_profile_id(cxn);
 
        /* XX: What if it *just* expired? We'll need to renew it and try again? */
@@ -152,6 +158,7 @@ static void audio_ws_connect_cb(GObject *obj, GAsyncResult *res, gpointer _audio
        GError *error = NULL;
        SoupWebsocketConnection *ws = chime_connection_websocket_connect_finish(cxn, res, &error);
        if (!ws) {
+               /* If it was cancelled, 'audio' may have been freed. */
                if (!g_error_matches(error, G_IO_ERROR, G_IO_ERROR_CANCELLED)) {
                        chime_debug("audio ws error %s\n", error->message);
                        audio->state = CHIME_AUDIO_STATE_FAILED;
@@ -306,11 +313,11 @@ read_datagram_based_cb (GDatagramBased *datagram_based,
 static gboolean
 read_timeout_cb (gpointer user_data)
 {
-  gboolean *timed_out = user_data;
+       gboolean *timed_out = user_data;
 
-  *timed_out = TRUE;
+       *timed_out = TRUE;
 
-  return G_SOURCE_REMOVE;
+       return G_SOURCE_REMOVE;
 }
 
 static int
@@ -369,14 +376,22 @@ g_tls_connection_gnutls_pull_timeout_func (gnutls_transport_ptr_t transport_data
        return 0;
 }
 
+static gboolean dtls_timeout(ChimeCallAudio *audio);
 
 static gboolean dtls_src_cb(GDatagramBased *dgram, GIOCondition condition, ChimeCallAudio *audio)
 {
        if (!audio->dtls_handshaked) {
                int ret = gnutls_handshake(audio->dtls_sess);
 
-               if (ret == GNUTLS_E_AGAIN)
+               if (ret == GNUTLS_E_AGAIN) {
+                       if (audio->timeout_source)
+                               g_source_remove(audio->timeout_source);
+
+                       int timeo = gnutls_dtls_get_timeout(audio->dtls_sess);
+                       audio->timeout_source = g_timeout_add(timeo, (GSourceFunc)dtls_timeout, audio);
+
                        return G_SOURCE_CONTINUE;
+               }
 
                if (ret) {
                        gnutls_deinit(audio->dtls_sess);
@@ -385,12 +400,17 @@ static gboolean dtls_src_cb(GDatagramBased *dgram, GIOCondition condition, Chime
                        audio->dtls_source = NULL;
                        g_object_unref(audio->dtls_sock);
                        audio->dtls_sock = NULL;
+                       if (audio->timeout_source)
+                               g_source_remove(audio->timeout_source);
+                       audio->timeout_source = 0;
 
                        chime_call_transport_connect_ws(audio);
                        return G_SOURCE_REMOVE;
                }
 
                chime_debug("DTLS established\n");
+               g_source_remove(audio->timeout_source);
+               audio->timeout_source = 0;
                audio->dtls_handshaked = TRUE;
                audio_send_auth_packet(audio);
                /* Fall through and receive data, not that it should be there */
@@ -398,12 +418,26 @@ static gboolean dtls_src_cb(GDatagramBased *dgram, GIOCondition condition, Chime
 
        unsigned char pkt[CHIME_DTLS_MTU];
        ssize_t len = gnutls_record_recv(audio->dtls_sess, pkt, sizeof(pkt));
-       if (len > 0)
+       if (len > 0) {
+               if (getenv("CHIME_AUDIO_DEBUG")) {
+                       printf("incoming:\n");
+                       hexdump(pkt, len);
+               }
                audio_receive_packet(audio, pkt, len);
+       }
 
        return G_SOURCE_CONTINUE;
 }
 
+static gboolean dtls_timeout(ChimeCallAudio *audio)
+{
+       audio->timeout_source = 0;
+
+       dtls_src_cb(NULL, 0, audio);
+
+       return G_SOURCE_REMOVE;
+}
+
 static void connect_dtls(ChimeCallAudio *audio, GSocket *s)
 {
        /* Not that "connected" means anything except that we think we can route to it. */
@@ -418,6 +452,8 @@ static void connect_dtls(ChimeCallAudio *audio, GSocket *s)
        gnutls_set_default_priority(audio->dtls_sess);
        gnutls_session_set_ptr(audio->dtls_sess, audio);
 
+       /* We can't rely on the length argument to gnutls_server_name_set().
+          https://bugs.launchpad.net/ubuntu/+bug/1762710 */
        gchar *hostname = g_strdup(chime_call_get_media_host(audio->call));
        if (!hostname)
                goto err;
@@ -429,6 +465,7 @@ static void connect_dtls(ChimeCallAudio *audio, GSocket *s)
        *colon = 0;
        gnutls_server_name_set(audio->dtls_sess, GNUTLS_NAME_DNS, hostname, colon - hostname);
        g_free(hostname);
+
        if (!audio->dtls_cred) {
                gnutls_certificate_allocate_credentials(&audio->dtls_cred);
                gnutls_certificate_set_x509_system_trust(audio->dtls_cred);
@@ -443,14 +480,29 @@ static void connect_dtls(ChimeCallAudio *audio, GSocket *s)
                                                    g_tls_connection_gnutls_pull_timeout_func);
        gnutls_transport_set_vec_push_function (audio->dtls_sess,
                                                g_tls_connection_gnutls_vec_push_func);
+       gnutls_dtls_set_timeouts(audio->dtls_sess, 250, 2500);
        gnutls_dtls_set_mtu(audio->dtls_sess, CHIME_DTLS_MTU);
-       gnutls_handshake(audio->dtls_sess);
+
+       if (gnutls_handshake(audio->dtls_sess) != GNUTLS_E_AGAIN) {
+               chime_debug("Initial DTLS handshake failed\n");
+
+               gnutls_deinit(audio->dtls_sess);
+               audio->dtls_sess = NULL;
+
+               if (audio->dtls_source) {
+                       g_source_destroy(audio->dtls_source);
+                       audio->dtls_source = NULL;
+               }
+               goto err;
+       }
+
+       int timeo = gnutls_dtls_get_timeout(audio->dtls_sess);
+       audio->timeout_source = g_timeout_add(timeo, (GSourceFunc)dtls_timeout, audio);
 
        return;
 
  err:
-       g_object_unref(s);
-       audio->dtls_sock = NULL;
+       g_clear_object(&audio->dtls_sock);
        chime_call_transport_connect_ws(audio);
 }
 
@@ -462,6 +514,7 @@ static void audio_dtls_one(GObject *obj, GAsyncResult *res, gpointer user_data)
 
        GSocketAddress *addr = g_socket_address_enumerator_next_finish(enumerator, res, &error);
        if (!addr) {
+               /* If it was cancelled, 'audio' may have been freed. */
                if (!g_error_matches(error, G_IO_ERROR, G_IO_ERROR_CANCELLED))
                        chime_call_transport_connect_ws(audio);
                g_clear_error(&error);
@@ -505,11 +558,12 @@ static void audio_dtls_one(GObject *obj, GAsyncResult *res, gpointer user_data)
 
 void chime_call_transport_connect(ChimeCallAudio *audio, gboolean silent)
 {
-
        audio->silent = silent;
        audio->cancel = g_cancellable_new();
        audio->dtls_handshaked = FALSE;
 
+       chime_call_audio_set_state(audio, CHIME_AUDIO_STATE_CONNECTING);
+
        GSocketConnectable *addr = g_network_address_parse(chime_call_get_media_host(audio->call),
                                                           0, NULL);
        if (!addr) {
@@ -533,7 +587,16 @@ static void on_final_audiows_close(SoupWebsocketConnection *ws, gpointer _unused
 
 void chime_call_transport_disconnect(ChimeCallAudio *audio, gboolean hangup)
 {
-       if (hangup)
+       if (audio->send_rt_source) {
+               g_source_remove(audio->send_rt_source);
+               audio->send_rt_source = 0;
+       }
+
+       g_hash_table_remove_all(audio->profiles);
+
+       chime_call_audio_cleanup_datamsgs(audio);
+
+       if (hangup && audio->state >= CHIME_AUDIO_STATE_AUDIOLESS)
                audio_send_hangup_packet(audio);
 
        g_mutex_lock(&audio->transport_lock);
@@ -544,9 +607,9 @@ void chime_call_transport_disconnect(ChimeCallAudio *audio, gboolean hangup)
                audio->cancel = NULL;
        }
        if (audio->ws) {
-               soup_websocket_connection_close(audio->ws, 0, NULL);
                g_signal_handlers_disconnect_matched(G_OBJECT(audio->ws), G_SIGNAL_MATCH_DATA, 0, 0, NULL, NULL, audio);
                g_signal_connect(G_OBJECT(audio->ws), "closed", G_CALLBACK(on_final_audiows_close), NULL);
+               soup_websocket_connection_close(audio->ws, 0, NULL);
                audio->ws = NULL;
        } else if (audio->dtls_sess) {
                gnutls_deinit(audio->dtls_sess);
@@ -559,10 +622,18 @@ void chime_call_transport_disconnect(ChimeCallAudio *audio, gboolean hangup)
                g_clear_object(&audio->dtls_sock);
        }
 
-       g_mutex_unlock(&audio->transport_lock);
-}
+       if (audio->timeout_source) {
+               g_source_remove(audio->timeout_source);
+               audio->timeout_source = 0;
+       }
 
+       if (hangup && audio->dtls_cred) {
+               gnutls_certificate_free_credentials(audio->dtls_cred);
+               audio->dtls_cred = NULL;
+       }
 
+       g_mutex_unlock(&audio->transport_lock);
+}
 
 void chime_call_transport_send_packet(ChimeCallAudio *audio, enum xrp_pkt_type type, const ProtobufCMessage *message)
 {

diff --git a/chime/chime-call.h b/chime/chime-call.h
index 697c4807293911016c24a5643a4a4dda83547d46..488bbc6c8961f2f3e27a552b37efb95c0d42f4ff 100644 (file)
--- a/chime/chime-call.h
+++ b/chime/chime-call.h
@@ -100,11 +100,10 @@ void chime_call_emit_participants(ChimeCall *call);
 typedef enum {
        CHIME_AUDIO_STATE_CONNECTING = 0,
        CHIME_AUDIO_STATE_FAILED,
+       CHIME_AUDIO_STATE_HANGUP,
        CHIME_AUDIO_STATE_AUDIOLESS,
        CHIME_AUDIO_STATE_AUDIO,
        CHIME_AUDIO_STATE_AUDIO_MUTED,
-       CHIME_AUDIO_STATE_HANGUP,
-       CHIME_AUDIO_STATE_DISCONNECTED,
 } ChimeAudioState;
 
 void chime_call_install_gst_app_callbacks(ChimeCall *call, GstAppSrc *appsrc, GstAppSink *appsink);