webrtcbin: Fix renegotiation checks

When checking for renegotiation against a local offer,
reverse the remote direction in the corresponding answer
to fix falsely not triggering on-negotiation needed when
switching (for example) from local sendrecv -> recvonly
against a peer that answered 'recvonly'.

In the other direction, when the local was the answerer,
renegotiation might trigger when it didn't need to -
whenever the local transceiver direction differs from
the intersected direction we chose. Instead what we want
is to check if the intersected direction we would now
choose differs from what was previously chosen.

This makes the behaviour in both cases match the
behaviour described in
https://www.w3.org/TR/webrtc/#dfn-check-if-negotiation-is-needed

Part-of: <https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/7303>
This commit is contained in:
Jan Schmidt 2024-08-01 13:42:52 +10:00 committed by GStreamer Marge Bot
parent 2638d8135d
commit 0f8fc27892
2 changed files with 151 additions and 32 deletions

View file

@ -1773,6 +1773,22 @@ done:
return res;
}
static GstWebRTCRTPTransceiverDirection
_reverse_direction (GstWebRTCRTPTransceiverDirection direction)
{
switch (direction) {
case GST_WEBRTC_RTP_TRANSCEIVER_DIRECTION_NONE:
case GST_WEBRTC_RTP_TRANSCEIVER_DIRECTION_INACTIVE:
case GST_WEBRTC_RTP_TRANSCEIVER_DIRECTION_SENDRECV:
return direction;
case GST_WEBRTC_RTP_TRANSCEIVER_DIRECTION_SENDONLY:
return GST_WEBRTC_RTP_TRANSCEIVER_DIRECTION_RECVONLY;
case GST_WEBRTC_RTP_TRANSCEIVER_DIRECTION_RECVONLY:
return GST_WEBRTC_RTP_TRANSCEIVER_DIRECTION_SENDONLY;
}
return GST_WEBRTC_RTP_TRANSCEIVER_DIRECTION_NONE;
}
/* http://w3c.github.io/webrtc-pc/#dfn-check-if-negotiation-is-needed */
static gboolean
_check_if_negotiation_is_needed (GstWebRTCBin * webrtc)
@ -1862,33 +1878,39 @@ _check_if_negotiation_is_needed (GstWebRTCBin * webrtc)
/* If connection's currentLocalDescription if of type "offer", and
* the direction of the associated m= section in neither the offer
* nor answer matches t's direction, return "true". */
if (local_dir != trans->direction && remote_dir != trans->direction) {
GST_LOG_OBJECT (webrtc, "transceiver direction (%s) doesn't match "
"description (local %s remote %s)",
if (local_dir != trans->direction
&& _reverse_direction (remote_dir) != trans->direction) {
GST_LOG_OBJECT (webrtc,
"transceiver direction (%s) doesn't match "
"description (local %s remote %s (reversed %s))",
gst_webrtc_rtp_transceiver_direction_to_string (trans->direction),
gst_webrtc_rtp_transceiver_direction_to_string (local_dir),
gst_webrtc_rtp_transceiver_direction_to_string (remote_dir));
gst_webrtc_rtp_transceiver_direction_to_string (remote_dir),
gst_webrtc_rtp_transceiver_direction_to_string (_reverse_direction
(remote_dir))
);
return TRUE;
}
} else if (webrtc->current_local_description->type ==
GST_WEBRTC_SDP_TYPE_ANSWER) {
GstWebRTCRTPTransceiverDirection intersect_dir;
/* If connection's currentLocalDescription if of type "answer", and
* the direction of the associated m= section in the answer does not
* match t's direction intersected with the offered direction (as
* described in [JSEP] (section 5.3.1.)), return "true". */
* the direction of the associated m= section in the answer we sent
* (local_dir) does not match t's direction intersected with the
* offer direction (as described in [JSEP] (section 5.3.1.)),
* return "true" because we want to propose a different
* direction now. */
/* remote is the offer, local is the answer */
intersect_dir = _intersect_answer_directions (remote_dir, local_dir);
if (intersect_dir != trans->direction) {
GST_LOG_OBJECT (webrtc, "transceiver direction (%s) doesn't match "
"description intersected direction %s (local %s remote %s)",
GstWebRTCRTPTransceiverDirection now_intersect_dir =
_intersect_answer_directions (remote_dir, trans->direction);
if (now_intersect_dir != local_dir) {
GST_LOG_OBJECT (webrtc,
"transceiver direction (%s) doesn't match for the "
"new description intersected direction %s (prev local %s remote %s)",
gst_webrtc_rtp_transceiver_direction_to_string (trans->direction),
gst_webrtc_rtp_transceiver_direction_to_string (local_dir),
gst_webrtc_rtp_transceiver_direction_to_string (intersect_dir),
gst_webrtc_rtp_transceiver_direction_to_string
(now_intersect_dir),
gst_webrtc_rtp_transceiver_direction_to_string (remote_dir));
return TRUE;
}
@ -6283,22 +6305,6 @@ get_last_generated_description (GstWebRTCBin * webrtc, SDPSource source,
return NULL;
}
static GstWebRTCRTPTransceiverDirection
_reverse_direction (GstWebRTCRTPTransceiverDirection direction)
{
switch (direction) {
case GST_WEBRTC_RTP_TRANSCEIVER_DIRECTION_NONE:
case GST_WEBRTC_RTP_TRANSCEIVER_DIRECTION_INACTIVE:
case GST_WEBRTC_RTP_TRANSCEIVER_DIRECTION_SENDRECV:
return direction;
case GST_WEBRTC_RTP_TRANSCEIVER_DIRECTION_SENDONLY:
return GST_WEBRTC_RTP_TRANSCEIVER_DIRECTION_RECVONLY;
case GST_WEBRTC_RTP_TRANSCEIVER_DIRECTION_RECVONLY:
return GST_WEBRTC_RTP_TRANSCEIVER_DIRECTION_SENDONLY;
}
return GST_WEBRTC_RTP_TRANSCEIVER_DIRECTION_NONE;
}
/* https://w3c.github.io/webrtc-pc/#set-description (steps in 4.6.10.) */
static gboolean
_create_and_associate_transceivers_from_sdp (GstWebRTCBin * webrtc,

View file

@ -719,6 +719,14 @@ test_webrtc_reset_negotiation (struct test_webrtc *t)
test_webrtc_signal_state (t, STATE_NEGOTIATION_NEEDED);
}
static void
test_webrtc_clear_states (struct test_webrtc *t)
{
GST_DEBUG ("clearing states");
g_array_free (t->states, TRUE);
t->states = g_array_new (FALSE, TRUE, sizeof (TestState));
}
static void
test_webrtc_free (struct test_webrtc *t)
{
@ -3789,6 +3797,110 @@ GST_START_TEST (test_renego_transceiver_set_direction)
GST_END_TEST;
GST_START_TEST (test_renego_triggering)
{
struct test_webrtc *t = create_audio_test ();
VAL_SDP_INIT (no_duplicate_payloads, on_sdp_media_no_duplicate_payloads,
NULL, NULL);
guint media_format_count[] = { 1 };
VAL_SDP_INIT (media_formats, on_sdp_media_count_formats,
media_format_count, &no_duplicate_payloads);
VAL_SDP_INIT (count, _count_num_sdp_media, GUINT_TO_POINTER (1),
&media_formats);
const gchar *expected_offer_setup[] = { "actpass" };
VAL_SDP_INIT (offer_setup, on_sdp_media_setup, expected_offer_setup, &count);
const gchar *expected_answer_setup[] = { "active" };
VAL_SDP_INIT (answer_setup, on_sdp_media_setup, expected_answer_setup,
&count);
const gchar *expected_offer_direction[] = { "sendrecv" };
VAL_SDP_INIT (offer, on_sdp_media_direction, expected_offer_direction,
&offer_setup);
const gchar *expected_answer_direction[] = { "recvonly" };
VAL_SDP_INIT (answer, on_sdp_media_direction, expected_answer_direction,
&answer_setup);
GstCaps *caps;
GArray *transceivers;
/* Ensure sendrecv stream on webrtc1 */
g_signal_emit_by_name (t->webrtc1, "get-transceivers", &transceivers);
fail_unless (transceivers != NULL);
fail_unless_equals_int (transceivers->len, 1);
GstWebRTCRTPTransceiver *trans_local =
g_array_index (transceivers, GstWebRTCRTPTransceiver *, 0);
g_object_set (trans_local, "direction",
GST_WEBRTC_RTP_TRANSCEIVER_DIRECTION_SENDRECV, NULL);
/* setup recvonly peer */
caps = gst_caps_from_string (OPUS_RTP_CAPS (96));
gst_caps_set_simple (caps, "ssrc", G_TYPE_UINT, 0xDEADBEEF, NULL);
GstWebRTCRTPTransceiver *trans_remote = NULL;
GstWebRTCRTPTransceiverDirection direction =
GST_WEBRTC_RTP_TRANSCEIVER_DIRECTION_RECVONLY;
g_signal_emit_by_name (t->webrtc2, "add-transceiver", direction, caps,
&trans_remote);
gst_caps_unref (caps);
fail_unless (trans_remote != NULL);
gst_object_unref (trans_remote);
test_validate_sdp (t, &offer, &answer);
GST_LOG
("Finished validating sendrecv <-> recvonly nego. Triggering renego with recvonly <-> recvonly peers");
/* Now change the sender to recvonly and expect to renegotiate to inactive */
test_webrtc_reset_negotiation (t);
test_webrtc_clear_states (t);
GST_LOG ("Setting local transceiver to RECVONLY");
g_object_set (trans_local, "direction",
GST_WEBRTC_RTP_TRANSCEIVER_DIRECTION_RECVONLY, NULL);
GST_LOG ("Waiting for on-negotiation-needed");
test_webrtc_wait_for_state_mask (t, 1 << STATE_NEGOTIATION_NEEDED);
const gchar *new_expected_offer_direction[] = { "recvonly" };
VAL_SDP_INIT (new_offer, on_sdp_media_direction, new_expected_offer_direction,
NULL);
const gchar *new_expected_answer_direction[] = { "inactive" };
VAL_SDP_INIT (new_answer, on_sdp_media_direction,
new_expected_answer_direction, NULL);
test_validate_sdp (t, &new_offer, &new_answer);
g_array_unref (transceivers);
/* At this point webrtc2 is the answerer. Check that it also triggers nego
* if we change the direction to sendonly */
test_webrtc_reset_negotiation (t);
test_webrtc_clear_states (t);
GST_LOG ("Setting remote transceiver to SENDONLY");
g_signal_emit_by_name (t->webrtc2, "get-transceivers", &transceivers);
fail_unless (transceivers != NULL);
fail_unless_equals_int (transceivers->len, 1);
trans_remote = g_array_index (transceivers, GstWebRTCRTPTransceiver *, 0);
g_object_set (trans_remote, "direction",
GST_WEBRTC_RTP_TRANSCEIVER_DIRECTION_SENDONLY, NULL);
g_array_unref (transceivers);
GST_LOG ("Waiting for on-negotiation-needed");
test_webrtc_wait_for_state_mask (t, 1 << STATE_NEGOTIATION_NEEDED);
test_webrtc_free (t);
}
GST_END_TEST;
static void
offer_remove_last_media (struct test_webrtc *t, GstElement * element,
GstPromise * promise, gpointer user_data)
@ -6140,6 +6252,7 @@ webrtcbin_suite (void)
tcase_add_test (tc, test_bundle_renego_add_stream);
tcase_add_test (tc, test_bundle_max_compat_max_bundle_renego_add_stream);
tcase_add_test (tc, test_renego_transceiver_set_direction);
tcase_add_test (tc, test_renego_triggering);
tcase_add_test (tc, test_renego_lose_media_fails);
tcase_add_test (tc,
test_bundle_codec_preferences_rtx_no_duplicate_payloads);