webrtcbin: Store pending mid to make create-offer idempotent

If the mid is not stored in the transceiver, but it is stored in
last_offer, then a further create-offer call will just ignore that
transceiver.

Also include unit test for ensure it doesn't regress.

Part-of: <https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/2439>
This commit is contained in:
Olivier Crête 2022-05-16 17:17:13 -04:00
parent 3c9e4f4886
commit 3503599e0a
4 changed files with 101 additions and 19 deletions

View file

@ -3213,7 +3213,7 @@ sdp_media_from_transceiver (GstWebRTCBin * webrtc, GstSDPMedia * media,
* multiple dtls fingerprints https://tools.ietf.org/html/draft-ietf-mmusic-4572-update-05
*/
GstSDPMessage *last_offer = _get_latest_self_generated_sdp (webrtc);
gchar *direction, *ufrag, *pwd, *mid;
gchar *direction, *ufrag, *pwd, *mid = NULL;
gboolean bundle_only;
guint rtp_session_idx;
GstCaps *caps;
@ -3422,11 +3422,13 @@ sdp_media_from_transceiver (GstWebRTCBin * webrtc, GstSDPMedia * media,
return FALSE;
}
mid = g_strdup (trans->mid);
} else {
g_hash_table_insert (all_mids, g_strdup (mid), NULL);
}
if (mid == NULL) {
const GstStructure *s = gst_caps_get_structure (caps, 0);
mid = g_strdup (gst_structure_get_string (s, "a-mid"));
if (mid) {
if (g_hash_table_contains (all_mids, (gpointer) mid)) {
g_set_error (error, GST_WEBRTC_ERROR,
@ -3436,8 +3438,28 @@ sdp_media_from_transceiver (GstWebRTCBin * webrtc, GstSDPMedia * media,
media_idx);
return FALSE;
}
g_free (WEBRTC_TRANSCEIVER (trans)->pending_mid);
WEBRTC_TRANSCEIVER (trans)->pending_mid = g_strdup (mid);
g_hash_table_insert (all_mids, g_strdup (mid), NULL);
}
}
if (mid == NULL) {
mid = g_strdup (WEBRTC_TRANSCEIVER (trans)->pending_mid);
if (mid) {
/* If it's already used, just ignore the pending one and generate
* a new one */
if (g_hash_table_contains (all_mids, (gpointer) mid)) {
g_clear_pointer (&mid, free);
g_clear_pointer (&WEBRTC_TRANSCEIVER (trans)->pending_mid, free);
} else {
gst_sdp_media_add_attribute (media, "mid", mid);
g_hash_table_insert (all_mids, g_strdup (mid), NULL);
}
}
}
if (mid == NULL) {
/* Make sure to avoid mid collisions */
while (TRUE) {
mid = g_strdup_printf ("%s%u", gst_sdp_media_get_media (media),
@ -3447,11 +3469,11 @@ sdp_media_from_transceiver (GstWebRTCBin * webrtc, GstSDPMedia * media,
} else {
gst_sdp_media_add_attribute (media, "mid", mid);
g_hash_table_insert (all_mids, g_strdup (mid), NULL);
WEBRTC_TRANSCEIVER (trans)->pending_mid = g_strdup (mid);
break;
}
}
}
}
/* TODO:
* - add a=candidate lines for gathered candidates
@ -3706,14 +3728,22 @@ _create_offer_task (GstWebRTCBin * webrtc, const GstStructure * options,
|| g_strcmp0 (gst_sdp_media_get_media (last_media), "video") == 0) {
const gchar *last_mid;
int j;
last_mid = gst_sdp_media_get_attribute_val (last_media, "mid");
for (j = 0; j < webrtc->priv->transceivers->len; j++) {
trans = g_ptr_array_index (webrtc->priv->transceivers, j);
WebRTCTransceiver *wtrans;
const gchar *mid;
if (trans->mid && g_strcmp0 (trans->mid, last_mid) == 0) {
WebRTCTransceiver *wtrans = WEBRTC_TRANSCEIVER (trans);
const char *mid;
trans = g_ptr_array_index (webrtc->priv->transceivers, j);
wtrans = WEBRTC_TRANSCEIVER (trans);
if (trans->mid)
mid = trans->mid;
else
mid = wtrans->pending_mid;
if (mid && g_strcmp0 (mid, last_mid) == 0) {
GstSDPMedia media;
memset (&media, 0, sizeof (media));
@ -3800,6 +3830,11 @@ _create_offer_task (GstWebRTCBin * webrtc, const GstStructure * options,
}
g_hash_table_insert (all_mids, g_strdup (trans->mid), NULL);
} else if (WEBRTC_TRANSCEIVER (trans)->pending_mid &&
!g_hash_table_contains (all_mids,
WEBRTC_TRANSCEIVER (trans)->pending_mid)) {
g_hash_table_insert (all_mids,
g_strdup (WEBRTC_TRANSCEIVER (trans)->pending_mid), NULL);
}
}

View file

@ -160,6 +160,8 @@ webrtc_transceiver_finalize (GObject * object)
gst_caps_replace (&trans->last_retrieved_caps, NULL);
gst_caps_replace (&trans->last_send_configured_caps, NULL);
g_free (trans->pending_mid);
gst_event_replace (&trans->tos_event, NULL);
G_OBJECT_CLASS (parent_class)->finalize (object);

View file

@ -54,6 +54,8 @@ struct _WebRTCTransceiver
*/
GstCaps *last_send_configured_caps;
gchar *pending_mid;
gboolean mline_locked;
GstElement *ulpfecdec;

View file

@ -5426,6 +5426,48 @@ GST_START_TEST (test_invalid_add_media_in_answer)
GST_END_TEST;
GST_START_TEST (test_data_channel_recreate_offer)
{
GstHarness *h;
GstWebRTCDataChannel *channel;
GstPromise *promise;
const GstStructure *s;
GstPromiseResult res;
GstPad *pad;
h = gst_harness_new_with_padnames ("webrtcbin", "sink_0", NULL);
add_audio_test_src_harness (h, 0xDEADBEEF);
g_signal_emit_by_name (h->element, "create-data-channel", "label", NULL,
&channel);
fail_unless (GST_IS_WEBRTC_DATA_CHANNEL (channel));
pad = gst_element_get_static_pad (h->element, "sink_0");
fail_unless (pad != NULL);
promise = gst_promise_new ();
g_signal_emit_by_name (h->element, "create-offer", NULL, promise);
res = gst_promise_wait (promise);
fail_unless_equals_int (res, GST_PROMISE_RESULT_REPLIED);
s = gst_promise_get_reply (promise);
fail_unless (s != NULL);
gst_promise_unref (promise);
promise = gst_promise_new ();
g_signal_emit_by_name (h->element, "create-offer", NULL, promise);
res = gst_promise_wait (promise);
fail_unless_equals_int (res, GST_PROMISE_RESULT_REPLIED);
s = gst_promise_get_reply (promise);
fail_unless (s != NULL);
gst_promise_unref (promise);
gst_object_unref (pad);
gst_object_unref (channel);
gst_harness_teardown (h);
}
GST_END_TEST;
static Suite *
webrtcbin_suite (void)
{
@ -5502,6 +5544,7 @@ webrtcbin_suite (void)
tcase_add_test (tc, test_renego_stream_add_data_channel);
tcase_add_test (tc, test_renego_data_channel_add_stream);
tcase_add_test (tc, test_renego_stream_data_channel_add_stream);
tcase_add_test (tc, test_data_channel_recreate_offer);
} else {
GST_WARNING ("Some required elements were not found. "
"All datachannel tests are disabled. sctpenc %p, sctpdec %p", sctpenc,