From 906b90287c6751391e305f39c76e0361b306f451 Mon Sep 17 00:00:00 2001 From: Philippe Normand Date: Fri, 17 Feb 2023 16:20:37 +0000 Subject: [PATCH] webrtcbin: Relay add-ice-candidate errors from Ice implementation to Application The `add_candidate` vfunc of the GstWebRTCICE interface gained a GstPromise argument, which is an ABI break. We're not aware of any external user of this interface yet so we think it's OK. This change is useful in cases where the application needs to bubble up errors from the underlying ICE agent, for instance when the agent was given an invalid ICE candidate. Part-of: --- .../webrtc/sendrecv/gst/custom_agent.c | 4 +- .../gst-plugins-bad/ext/webrtc/gstwebrtcbin.c | 26 ++++++-- .../gst-plugins-bad/gst-libs/gst/webrtc/ice.c | 6 +- .../gst-plugins-bad/gst-libs/gst/webrtc/ice.h | 6 +- .../gst-libs/gst/webrtc/nice/nice.c | 60 ++++++++++++++++--- 5 files changed, 84 insertions(+), 18 deletions(-) diff --git a/subprojects/gst-examples/webrtc/sendrecv/gst/custom_agent.c b/subprojects/gst-examples/webrtc/sendrecv/gst/custom_agent.c index 1b7bacefe5..b059669d4b 100644 --- a/subprojects/gst-examples/webrtc/sendrecv/gst/custom_agent.c +++ b/subprojects/gst-examples/webrtc/sendrecv/gst/custom_agent.c @@ -28,10 +28,10 @@ customice_agent_find_transport (GstWebRTCICE * ice, void customice_agent_add_candidate (GstWebRTCICE * ice, - GstWebRTCICEStream * stream, const gchar * candidate) + GstWebRTCICEStream * stream, const gchar * candidate, GstPromise * promise) { GstWebRTCICE *c_ice = GST_WEBRTC_ICE (CUSTOMICE_AGENT (ice)->nice_agent); - gst_webrtc_ice_add_candidate (c_ice, stream, candidate); + gst_webrtc_ice_add_candidate (c_ice, stream, candidate, promise); } gboolean diff --git a/subprojects/gst-plugins-bad/ext/webrtc/gstwebrtcbin.c b/subprojects/gst-plugins-bad/ext/webrtc/gstwebrtcbin.c index fba63d1807..c79261a11c 100644 --- a/subprojects/gst-plugins-bad/ext/webrtc/gstwebrtcbin.c +++ b/subprojects/gst-plugins-bad/ext/webrtc/gstwebrtcbin.c @@ -5303,12 +5303,15 @@ typedef struct { guint mlineindex; gchar *candidate; + GstPromise *promise; } IceCandidateItem; static void _clear_ice_candidate_item (IceCandidateItem * item) { g_free (item->candidate); + if (item->promise) + gst_promise_unref (item->promise); } static void @@ -5320,12 +5323,23 @@ _add_ice_candidate (GstWebRTCBin * webrtc, IceCandidateItem * item, stream = _find_ice_stream_for_session (webrtc, item->mlineindex); if (stream == NULL) { if (drop_invalid) { - GST_WARNING_OBJECT (webrtc, "Unknown mline %u, dropping", - item->mlineindex); + if (item->promise) { + GError *error = + g_error_new (GST_WEBRTC_ERROR, GST_WEBRTC_ERROR_INTERNAL_FAILURE, + "Unknown mline %u, dropping", item->mlineindex); + GstStructure *s = gst_structure_new ("application/x-gst-promise", + "error", G_TYPE_ERROR, error, NULL); + gst_promise_reply (item->promise, s); + g_clear_error (&error); + } else { + GST_WARNING_OBJECT (webrtc, "Unknown mline %u, dropping", + item->mlineindex); + } } else { IceCandidateItem new; new.mlineindex = item->mlineindex; new.candidate = g_strdup (item->candidate); + new.promise = NULL; GST_INFO_OBJECT (webrtc, "Unknown mline %u, deferring", item->mlineindex); ICE_LOCK (webrtc); @@ -5338,7 +5352,8 @@ _add_ice_candidate (GstWebRTCBin * webrtc, IceCandidateItem * item, GST_LOG_OBJECT (webrtc, "adding ICE candidate with mline:%u, %s", item->mlineindex, item->candidate); - gst_webrtc_ice_add_candidate (webrtc->priv->ice, stream, item->candidate); + gst_webrtc_ice_add_candidate (webrtc->priv->ice, stream, item->candidate, + item->promise); } static void @@ -5364,7 +5379,7 @@ _add_ice_candidates_from_sdp (GstWebRTCBin * webrtc, gint mlineindex, candidate = g_strdup_printf ("a=candidate:%s", attr->value); GST_LOG_OBJECT (webrtc, "adding ICE candidate with mline:%u, %s", mlineindex, candidate); - gst_webrtc_ice_add_candidate (webrtc->priv->ice, stream, candidate); + gst_webrtc_ice_add_candidate (webrtc->priv->ice, stream, candidate, NULL); g_free (candidate); } } @@ -6768,6 +6783,7 @@ _add_ice_candidate_task (GstWebRTCBin * webrtc, IceCandidateItem * item) IceCandidateItem new; new.mlineindex = item->mlineindex; new.candidate = g_steal_pointer (&item->candidate); + new.promise = NULL; ICE_LOCK (webrtc); g_array_append_val (webrtc->priv->pending_remote_ice_candidates, new); @@ -6794,6 +6810,7 @@ gst_webrtc_bin_add_ice_candidate (GstWebRTCBin * webrtc, guint mline, item = g_new0 (IceCandidateItem, 1); item->mlineindex = mline; + item->promise = promise ? gst_promise_ref (promise) : NULL; if (attr && attr[0] != 0) { if (!g_ascii_strncasecmp (attr, "a=candidate:", 12)) item->candidate = g_strdup (attr); @@ -6885,6 +6902,7 @@ _on_local_ice_candidate_cb (GstWebRTCICE * ice, guint session_id, item.mlineindex = session_id; item.candidate = g_strdup (candidate); + item.promise = NULL; ICE_LOCK (webrtc); g_array_append_val (webrtc->priv->pending_local_ice_candidates, item); diff --git a/subprojects/gst-plugins-bad/gst-libs/gst/webrtc/ice.c b/subprojects/gst-plugins-bad/gst-libs/gst/webrtc/ice.c index 2328d0b82d..d160216826 100644 --- a/subprojects/gst-plugins-bad/gst-libs/gst/webrtc/ice.c +++ b/subprojects/gst-plugins-bad/gst-libs/gst/webrtc/ice.c @@ -101,17 +101,19 @@ gst_webrtc_ice_find_transport (GstWebRTCICE * ice, * @ice: The #GstWebRTCICE * @stream: The #GstWebRTCICEStream * @candidate: The ICE candidate + * @promise: (allow none): A #GstPromise for task notifications (Since: 1.24) * * Since: 1.22 */ void gst_webrtc_ice_add_candidate (GstWebRTCICE * ice, - GstWebRTCICEStream * stream, const gchar * candidate) + GstWebRTCICEStream * stream, const gchar * candidate, GstPromise * promise) { g_return_if_fail (GST_IS_WEBRTC_ICE (ice)); g_assert (GST_WEBRTC_ICE_GET_CLASS (ice)->add_candidate); - GST_WEBRTC_ICE_GET_CLASS (ice)->add_candidate (ice, stream, candidate); + GST_WEBRTC_ICE_GET_CLASS (ice)->add_candidate (ice, stream, candidate, + promise); } /** diff --git a/subprojects/gst-plugins-bad/gst-libs/gst/webrtc/ice.h b/subprojects/gst-plugins-bad/gst-libs/gst/webrtc/ice.h index f67889b1f4..e1422f37a2 100644 --- a/subprojects/gst-plugins-bad/gst-libs/gst/webrtc/ice.h +++ b/subprojects/gst-plugins-bad/gst-libs/gst/webrtc/ice.h @@ -84,7 +84,8 @@ struct _GstWebRTCICEClass { GstWebRTCICEStream * stream); void (*add_candidate) (GstWebRTCICE * ice, GstWebRTCICEStream * stream, - const gchar * candidate); + const gchar * candidate, + GstPromise * promise); gboolean (*set_local_credentials) (GstWebRTCICE * ice, GstWebRTCICEStream * stream, const gchar * ufrag, @@ -169,7 +170,8 @@ gboolean gst_webrtc_ice_gather_candidates (GstWebRTCIC GST_WEBRTC_API void gst_webrtc_ice_add_candidate (GstWebRTCICE * ice, GstWebRTCICEStream * stream, - const gchar * candidate); + const gchar * candidate, + GstPromise * promise); GST_WEBRTC_API gboolean gst_webrtc_ice_set_local_credentials (GstWebRTCICE * ice, diff --git a/subprojects/gst-plugins-bad/gst-libs/gst/webrtc/nice/nice.c b/subprojects/gst-plugins-bad/gst-libs/gst/webrtc/nice/nice.c index d7feae1949..ed9cb4d2e4 100644 --- a/subprojects/gst-plugins-bad/gst-libs/gst/webrtc/nice/nice.c +++ b/subprojects/gst-plugins-bad/gst-libs/gst/webrtc/nice/nice.c @@ -699,6 +699,7 @@ struct resolve_candidate_data guint nice_stream_id; char *prefix; char *postfix; + GstPromise *promise; }; static void @@ -706,6 +707,8 @@ free_resolve_candidate_data (struct resolve_candidate_data *rc) { g_free (rc->prefix); g_free (rc->postfix); + if (rc->promise) + gst_promise_unref (rc->promise); g_free (rc); } @@ -743,8 +746,14 @@ on_candidate_resolved (GstWebRTCICE * ice, GAsyncResult * res, GstWebRTCNice *nice = GST_WEBRTC_NICE (ice); if (!(addresses = resolve_host_finish (nice, res, &error))) { - GST_WARNING_OBJECT (ice, "Could not resolve candidate address: %s", - error->message); + if (rc->promise) { + GstStructure *s = gst_structure_new ("application/x-gst-promise", "error", + G_TYPE_ERROR, error, NULL); + gst_promise_reply (rc->promise, s); + } else { + GST_WARNING_OBJECT (ice, "Could not resolve candidate address: %s", + error->message); + } g_clear_error (&error); return; } @@ -764,7 +773,18 @@ on_candidate_resolved (GstWebRTCICE * ice, GAsyncResult * res, rc->nice_stream_id, new_candidate); g_free (new_candidate); if (!cand) { - GST_WARNING_OBJECT (ice, "Could not parse candidate \'%s\'", new_candidate); + if (rc->promise) { + GError *error = + g_error_new (GST_WEBRTC_ERROR, GST_WEBRTC_ERROR_INTERNAL_FAILURE, + "Could not parse candidate \'%s\'", new_candidate); + GstStructure *s = gst_structure_new ("application/x-gst-promise", "error", + G_TYPE_ERROR, error, NULL); + gst_promise_reply (rc->promise, s); + g_clear_error (&error); + } else { + GST_WARNING_OBJECT (ice, "Could not parse candidate \'%s\'", + new_candidate); + } return; } @@ -777,7 +797,7 @@ on_candidate_resolved (GstWebRTCICE * ice, GAsyncResult * res, /* candidate must start with "a=candidate:" or be NULL*/ static void gst_webrtc_nice_add_candidate (GstWebRTCICE * ice, GstWebRTCICEStream * stream, - const gchar * candidate) + const gchar * candidate, GstPromise * promise) { struct NiceStreamItem *item; NiceCandidate *cand; @@ -801,14 +821,37 @@ gst_webrtc_nice_add_candidate (GstWebRTCICE * ice, GstWebRTCICEStream * stream, struct resolve_candidate_data *rc; if (!get_candidate_address (candidate, &prefix, &address, &postfix)) { - GST_WARNING_OBJECT (nice, "Failed to retrieve address from candidate %s", - candidate); + if (promise) { + GError *error = + g_error_new (GST_WEBRTC_ERROR, GST_WEBRTC_ERROR_INTERNAL_FAILURE, + "Failed to retrieve address from candidate %s", + candidate); + GstStructure *s = gst_structure_new ("application/x-gst-promise", + "error", G_TYPE_ERROR, error, NULL); + gst_promise_reply (promise, s); + g_clear_error (&error); + } else { + GST_WARNING_OBJECT (nice, + "Failed to retrieve address from candidate %s", candidate); + } goto done; } if (!g_str_has_suffix (address, ".local")) { - GST_WARNING_OBJECT (nice, "candidate address \'%s\' does not end " - "with \'.local\'", address); + if (promise) { + GError *error = + g_error_new (GST_WEBRTC_ERROR, GST_WEBRTC_ERROR_INTERNAL_FAILURE, + "candidate address \'%s\' does not end " "with \'.local\'", + address); + GstStructure *s = gst_structure_new ("application/x-gst-promise", + "error", G_TYPE_ERROR, error, NULL); + gst_promise_reply (promise, s); + g_clear_error (&error); + } else { + GST_WARNING_OBJECT (nice, + "candidate address \'%s\' does not end " + "with \'.local\'", address); + } goto done; } @@ -816,6 +859,7 @@ gst_webrtc_nice_add_candidate (GstWebRTCICE * ice, GstWebRTCICEStream * stream, rc->nice_stream_id = item->nice_stream_id; rc->prefix = prefix; rc->postfix = postfix; + rc->promise = promise ? gst_promise_ref (promise) : NULL; resolve_host_async (nice, address, (GAsyncReadyCallback) on_candidate_resolved, rc, (GDestroyNotify) free_resolve_candidate_data);