From 3557183a37def98177c67e6fc754d21a5bd4fb00 Mon Sep 17 00:00:00 2001 From: Jasper Hugo Date: Mon, 7 Mar 2022 15:48:45 +0700 Subject: [PATCH] TWCC fixes, improved logging * Added `payload` field to recv caps, fixing TWCC! * Re-enabled TWCC for audio * Added context to TLS error logging * Added missing rtcp-fb for RTX --- lib-gst-meet/src/colibri.rs | 2 +- lib-gst-meet/src/jingle.rs | 58 ++++++++++++++++++++++++++--- lib-gst-meet/src/tls.rs | 8 ++-- lib-gst-meet/src/xmpp/connection.rs | 2 +- 4 files changed, 59 insertions(+), 11 deletions(-) diff --git a/lib-gst-meet/src/colibri.rs b/lib-gst-meet/src/colibri.rs index 6599460..9cd9b63 100644 --- a/lib-gst-meet/src/colibri.rs +++ b/lib-gst-meet/src/colibri.rs @@ -50,7 +50,7 @@ impl ColibriChannel { match tokio_tungstenite::connect_async_tls_with_config( request, None, - Some(wss_connector(tls_insecure)?), + Some(wss_connector(tls_insecure).context("failed to build TLS connector")?), ) .await { diff --git a/lib-gst-meet/src/jingle.rs b/lib-gst-meet/src/jingle.rs index d850a94..ca18948 100644 --- a/lib-gst-meet/src/jingle.rs +++ b/lib-gst-meet/src/jingle.rs @@ -3,10 +3,11 @@ use std::{collections::HashMap, fmt, net::SocketAddr}; use anyhow::{anyhow, bail, Context, Result}; use futures::stream::StreamExt; use glib::{ObjectExt, ToValue}; -use gstreamer::prelude::{ElementExt, GObjectExtManualGst, GstBinExt, PadExt}; +use gstreamer::{Element, prelude::{ElementExt, ElementExtManual, GObjectExtManualGst, GstBinExt, GstObjectExt, PadExt}}; use gstreamer_rtp::{prelude::RTPHeaderExtensionExt, RTPHeaderExtension}; #[cfg(feature = "log-rtp")] use gstreamer_rtp::RTPBuffer; +use itertools::Itertools; use jitsi_xmpp_parsers::{ jingle::{Action, Content, Description, Jingle, Transport}, jingle_dtls_srtp::Fingerprint, @@ -128,6 +129,7 @@ impl Codec { struct ParsedRtpDescription { codecs: Vec, audio_hdrext_ssrc_audio_level: Option, + audio_hdrext_transport_cc: Option, video_hdrext_transport_cc: Option, } @@ -191,6 +193,7 @@ impl JingleSession { let mut vp8 = None; let mut vp9 = None; let mut audio_hdrext_ssrc_audio_level = None; + let mut audio_hdrext_transport_cc = None; let mut video_hdrext_transport_cc = None; if description.media == "audio" { @@ -209,6 +212,9 @@ impl JingleSession { if hdrext.uri == RTP_HDREXT_SSRC_AUDIO_LEVEL { audio_hdrext_ssrc_audio_level = Some(hdrext.id); } + else if hdrext.uri == RTP_HDREXT_TRANSPORT_CC { + audio_hdrext_transport_cc = Some(hdrext.id); + } } } else if description.media == "video" { @@ -320,6 +326,7 @@ impl JingleSession { Ok(Some(ParsedRtpDescription { codecs, audio_hdrext_ssrc_audio_level, + audio_hdrext_transport_cc, video_hdrext_transport_cc, })) } @@ -461,6 +468,7 @@ impl JingleSession { let mut ice_transport = None; let mut codecs = vec![]; let mut audio_hdrext_ssrc_audio_level = None; + let mut audio_hdrext_transport_cc = None; let mut video_hdrext_transport_cc = None; let mut remote_ssrc_map = HashMap::new(); @@ -473,6 +481,8 @@ impl JingleSession { codecs.extend(description.codecs); audio_hdrext_ssrc_audio_level = audio_hdrext_ssrc_audio_level.or(description.audio_hdrext_ssrc_audio_level); + audio_hdrext_transport_cc = + audio_hdrext_transport_cc.or(description.audio_hdrext_transport_cc); video_hdrext_transport_cc = video_hdrext_transport_cc.or(description.video_hdrext_transport_cc); } @@ -572,7 +582,8 @@ impl JingleSession { let f = || { debug!("rtpbin request-pt-map {:?}", values); let pt = values[2].get::()? as u8; - let mut caps = gstreamer::Caps::builder("application/x-rtp"); + let mut caps = gstreamer::Caps::builder("application/x-rtp") + .field("payload", pt as i32); for codec in codecs.iter() { if codec.is(pt) { if codec.is_audio() { @@ -583,6 +594,9 @@ impl JingleSession { if let Some(hdrext) = audio_hdrext_ssrc_audio_level { caps = caps.field(&format!("extmap-{}", hdrext), RTP_HDREXT_SSRC_AUDIO_LEVEL); } + if let Some(hdrext) = audio_hdrext_transport_cc { + caps = caps.field(&format!("extmap-{}", hdrext), RTP_HDREXT_TRANSPORT_CC); + } } else { // A video codec, as the only audio codec we support is Opus. @@ -591,7 +605,7 @@ impl JingleSession { .field("clock-rate", 90000) .field("encoding-name", codec.encoding_name()); if let Some(hdrext) = video_hdrext_transport_cc { - caps = caps.field(&format!("extmap-{}", hdrext), &RTP_HDREXT_TRANSPORT_CC); + caps = caps.field(&format!("extmap-{}", hdrext), RTP_HDREXT_TRANSPORT_CC); } } return Ok::<_, anyhow::Error>(Some(caps.build())); @@ -836,8 +850,11 @@ impl JingleSession { rtpbin .link_pads(Some(&pad_name), &source_element, None) .context(format!("failed to link rtpbin.{} to depayloader", pad_name))?; + debug!("linked rtpbin.{} to depayloader", pad_name); + debug!("rtpbin pads:\n{}", dump_pads(&rtpbin)); + let src_pad = source_element .static_pad("src") .context("depayloader has no src pad")?; @@ -1006,6 +1023,8 @@ impl JingleSession { #[cfg(feature = "log-rtp")] if conference.config.log_rtp { + debug!("setting up RTP/RTCP packet logging"); + let make_rtp_logger = |direction: &'static str| { move |values: &[glib::Value]| -> Option { let f = || { @@ -1013,9 +1032,11 @@ impl JingleSession { let rtp_buffer = RTPBuffer::from_buffer_readable(&buffer)?; debug!( ssrc=rtp_buffer.ssrc(), + pt=rtp_buffer.payload_type(), seq=rtp_buffer.seq(), ts=rtp_buffer.timestamp(), marker=rtp_buffer.is_marker(), + extension=rtp_buffer.is_extension(), payload_size=rtp_buffer.payload_size(), "RTP {}", direction, @@ -1037,7 +1058,7 @@ impl JingleSession { buffer.copy_to_slice(0, &mut buf[..buffer.size()]).map_err(|_| anyhow!("invalid RTCP packet size"))?; let decoded = rtcp::packet::unmarshal(&mut &buf[..buffer.size()])?; debug!( - "RTCP {} size={}\n{:?}", + "RTCP {} size={}\n{:#?}", direction, buffer.size(), decoded, @@ -1056,7 +1077,7 @@ impl JingleSession { rtcp_recv_identity.connect("handoff", false, make_rtcp_logger("RECV")); rtcp_send_identity.connect("handoff", false, make_rtcp_logger("SEND")); } - + debug!("link dtlssrtpdec -> rtpbin"); dtlssrtpdec.link_pads(Some("rtp_src"), &rtp_recv_identity, None)?; rtp_recv_identity.link_pads(None, &rtpbin, Some("recv_rtp_sink_0"))?; @@ -1069,6 +1090,8 @@ impl JingleSession { rtpbin.link_pads(Some("send_rtcp_src_0"), &rtcp_send_identity, None)?; rtcp_send_identity.link_pads(None, &dtlssrtpenc, Some("rtcp_sink_0"))?; + debug!("rtpbin pads:\n{}", dump_pads(&rtpbin)); + debug!("linking ice src -> dtlssrtpdec"); nicesrc.link(&dtlssrtpdec)?; @@ -1152,6 +1175,12 @@ impl JingleSession { name: "apt".to_owned(), value: codec.pt.to_string(), }]; + rtx_pt.rtcp_fbs = codec + .rtcp_fbs + .clone() + .into_iter() + .filter(|fb| fb.type_ != "transport-cc") + .collect(); pts.push(rtx_pt); } } @@ -1215,6 +1244,11 @@ impl JingleSession { RTP_HDREXT_SSRC_AUDIO_LEVEL.to_owned(), )); } + if let Some(hdrext) = audio_hdrext_transport_cc { + description + .hdrexts + .push(RtpHdrext::new(hdrext, RTP_HDREXT_TRANSPORT_CC.to_owned())); + } } else if initiate_content.name.0 == "video" { if let Some(hdrext) = video_hdrext_transport_cc { @@ -1332,3 +1366,17 @@ impl JingleSession { Ok(()) } } + +fn dump_pads(element: &Element) -> String { + element + .pads() + .into_iter() + .map(|pad| format!( + " {}, peer={}.{}, caps=\"{}\"", + pad.name(), + pad.peer().and_then(|peer| peer.parent_element()).map(|element| element.name().to_string()).unwrap_or_default(), + pad.peer().map(|peer| peer.name().to_string()).unwrap_or_default(), + pad.caps().map(|caps| caps.to_string()).unwrap_or_default(), + )) + .join("\n") +} \ No newline at end of file diff --git a/lib-gst-meet/src/tls.rs b/lib-gst-meet/src/tls.rs index 32221aa..bf6cc59 100644 --- a/lib-gst-meet/src/tls.rs +++ b/lib-gst-meet/src/tls.rs @@ -6,14 +6,14 @@ use std::sync::Arc; #[cfg(not(feature = "tls-insecure"))] use anyhow::bail; -use anyhow::Result; +use anyhow::{Context, Result}; use tokio_tungstenite::Connector; #[cfg(feature = "tls-rustls-native-roots")] pub(crate) fn wss_connector(insecure: bool) -> Result { let mut roots = rustls::RootCertStore::empty(); - for cert in rustls_native_certs::load_native_certs()? { - roots.add(&rustls::Certificate(cert.0))?; + for cert in rustls_native_certs::load_native_certs().context("failed to load native root certs")? { + roots.add(&rustls::Certificate(cert.0)).context("failed to add native root certs")?; } let mut config = rustls::ClientConfig::builder() @@ -80,7 +80,7 @@ pub(crate) fn wss_connector(insecure: bool) -> Result