webrtcsink: don't panic with bitrate handling unsupported encoders

When an encoder was not supported by the `VideoEncoder` `bitrate` accessors, an
`unimplemented` panic would occur which would poison `state` & `settings`
`Mutex`s resulting in other threads panicking, notably entering `end_session()`,
which lead to many failures in `BinImplExt::parent_remove_element()` until a
segmentation fault ended the process. This was observed using `vaapivp9enc`.

This commit logs a warning if an encoder isn't supported by the `bitrate`
accessors and silently by-passes `bitrate`-related operations when unsupported.

Part-of: <https://gitlab.freedesktop.org/gstreamer/gst-plugins-rs/-/merge_requests/1534>
This commit is contained in:
François Laignel 2024-04-09 17:50:12 +02:00 committed by GStreamer Marge Bot
parent 5d939498f1
commit 2ad452ee89
3 changed files with 67 additions and 25 deletions

View file

@ -413,10 +413,11 @@ impl CongestionController {
let fec_percentage = (fec_ratio * 50f64) as u32; let fec_percentage = (fec_ratio * 50f64) as u32;
for encoder in encoders.iter_mut() { for encoder in encoders.iter_mut() {
encoder.set_bitrate(element, target_bitrate); if encoder.set_bitrate(element, target_bitrate).is_ok() {
encoder encoder
.transceiver .transceiver
.set_property("fec-percentage", fec_percentage); .set_property("fec-percentage", fec_percentage);
}
} }
} }
} }

View file

@ -952,8 +952,24 @@ impl VideoEncoder {
}) })
} }
fn bitrate(&self) -> i32 { fn is_bitrate_supported(factory_name: &str) -> bool {
match self.factory_name.as_str() { matches!(
factory_name,
"vp8enc"
| "vp9enc"
| "x264enc"
| "nvh264enc"
| "vaapih264enc"
| "vaapivp8enc"
| "qsvh264enc"
| "nvv4l2h264enc"
| "nvv4l2vp8enc"
| "nvv4l2vp9enc"
)
}
fn bitrate(&self) -> Result<i32, WebRTCSinkError> {
let bitrate = match self.factory_name.as_str() {
"vp8enc" | "vp9enc" => self.element.property::<i32>("target-bitrate"), "vp8enc" | "vp9enc" => self.element.property::<i32>("target-bitrate"),
"x264enc" | "nvh264enc" | "vaapih264enc" | "vaapivp8enc" | "qsvh264enc" => { "x264enc" | "nvh264enc" | "vaapih264enc" | "vaapivp8enc" | "qsvh264enc" => {
(self.element.property::<u32>("bitrate") * 1000) as i32 (self.element.property::<u32>("bitrate") * 1000) as i32
@ -961,8 +977,10 @@ impl VideoEncoder {
"nvv4l2h264enc" | "nvv4l2vp8enc" | "nvv4l2vp9enc" => { "nvv4l2h264enc" | "nvv4l2vp8enc" | "nvv4l2vp9enc" => {
(self.element.property::<u32>("bitrate")) as i32 (self.element.property::<u32>("bitrate")) as i32
} }
factory => unimplemented!("Factory {} is currently not supported", factory), _ => return Err(WebRTCSinkError::BitrateNotSupported),
} };
Ok(bitrate)
} }
fn scale_height_round_2(&self, height: i32) -> i32 { fn scale_height_round_2(&self, height: i32) -> i32 {
@ -979,16 +997,21 @@ impl VideoEncoder {
(width + 1) & !1 (width + 1) & !1
} }
pub(crate) fn set_bitrate(&mut self, element: &super::BaseWebRTCSink, bitrate: i32) { pub(crate) fn set_bitrate(
&mut self,
element: &super::BaseWebRTCSink,
bitrate: i32,
) -> Result<(), WebRTCSinkError> {
match self.factory_name.as_str() { match self.factory_name.as_str() {
"vp8enc" | "vp9enc" => self.element.set_property("target-bitrate", bitrate), "vp8enc" | "vp9enc" => self.element.set_property("target-bitrate", bitrate),
"x264enc" | "nvh264enc" | "vaapih264enc" | "vaapivp8enc" | "qsvh264enc" => self "x264enc" | "nvh264enc" | "vaapih264enc" | "vaapivp8enc" | "qsvh264enc" => {
.element self.element
.set_property("bitrate", (bitrate / 1000) as u32), .set_property("bitrate", (bitrate / 1000) as u32);
}
"nvv4l2h264enc" | "nvv4l2vp8enc" | "nvv4l2vp9enc" => { "nvv4l2h264enc" | "nvv4l2vp8enc" | "nvv4l2vp9enc" => {
self.element.set_property("bitrate", bitrate as u32) self.element.set_property("bitrate", bitrate as u32)
} }
factory => unimplemented!("Factory {} is currently not supported", factory), _ => return Err(WebRTCSinkError::BitrateNotSupported),
} }
let current_caps = self.filter.property::<gst::Caps>("caps"); let current_caps = self.filter.property::<gst::Caps>("caps");
@ -1052,11 +1075,13 @@ impl VideoEncoder {
self.filter.set_property("caps", caps); self.filter.set_property("caps", caps);
} }
Ok(())
} }
fn gather_stats(&self) -> gst::Structure { fn gather_stats(&self) -> gst::Structure {
gst::Structure::builder("application/x-webrtcsink-video-encoder-stats") gst::Structure::builder("application/x-webrtcsink-video-encoder-stats")
.field("bitrate", self.bitrate()) .field("bitrate", self.bitrate().unwrap_or(0i32))
.field("mitigation-mode", self.mitigation_mode) .field("mitigation-mode", self.mitigation_mode)
.field("codec-name", self.codec_name.as_str()) .field("codec-name", self.codec_name.as_str())
.field( .field(
@ -1339,19 +1364,21 @@ impl Session {
WebRTCSinkCongestionControl::Disabled => { WebRTCSinkCongestionControl::Disabled => {
// If congestion control is disabled, we simply use the highest // If congestion control is disabled, we simply use the highest
// known "safe" value for the bitrate. // known "safe" value for the bitrate.
enc.set_bitrate(element, self.cc_info.max_bitrate as i32); let _ = enc.set_bitrate(element, self.cc_info.max_bitrate as i32);
enc.transceiver.set_property("fec-percentage", 50u32); enc.transceiver.set_property("fec-percentage", 50u32);
} }
WebRTCSinkCongestionControl::Homegrown => { WebRTCSinkCongestionControl::Homegrown => {
if let Some(congestion_controller) = self.congestion_controller.as_mut() { if let Some(congestion_controller) = self.congestion_controller.as_mut() {
congestion_controller.target_bitrate_on_delay += enc.bitrate(); if let Ok(bitrate) = enc.bitrate() {
congestion_controller.target_bitrate_on_loss = congestion_controller.target_bitrate_on_delay += bitrate;
congestion_controller.target_bitrate_on_delay; congestion_controller.target_bitrate_on_loss =
enc.transceiver.set_property("fec-percentage", 0u32); congestion_controller.target_bitrate_on_delay;
enc.transceiver.set_property("fec-percentage", 0u32);
}
} else { } else {
/* If congestion control is disabled, we simply use the highest /* If congestion control is disabled, we simply use the highest
* known "safe" value for the bitrate. */ * known "safe" value for the bitrate. */
enc.set_bitrate(element, self.cc_info.max_bitrate as i32); let _ = enc.set_bitrate(element, self.cc_info.max_bitrate as i32);
enc.transceiver.set_property("fec-percentage", 50u32); enc.transceiver.set_property("fec-percentage", 50u32);
} }
} }
@ -1497,6 +1524,7 @@ impl BaseWebRTCSink {
fn configure_congestion_control( fn configure_congestion_control(
&self, &self,
payloader: &gst::Element, payloader: &gst::Element,
codec: &Codec,
extension_configuration_type: ExtensionConfigurationType, extension_configuration_type: ExtensionConfigurationType,
) -> Result<(), Error> { ) -> Result<(), Error> {
if let ExtensionConfigurationType::Skip = extension_configuration_type { if let ExtensionConfigurationType::Skip = extension_configuration_type {
@ -1505,6 +1533,16 @@ impl BaseWebRTCSink {
let settings = self.settings.lock().unwrap(); let settings = self.settings.lock().unwrap();
if codec.is_video() {
if let Some(enc_name) = codec.encoder_name().as_deref() {
if !VideoEncoder::is_bitrate_supported(enc_name) {
gst::error!(CAT, imp: self, "Bitrate handling is not supported yet for {enc_name}");
return Ok(());
}
}
}
if settings.cc_info.heuristic == WebRTCSinkCongestionControl::Disabled { if settings.cc_info.heuristic == WebRTCSinkCongestionControl::Disabled {
return Ok(()); return Ok(());
} }
@ -1620,7 +1658,7 @@ impl BaseWebRTCSink {
payloader.set_property("ssrc", ssrc); payloader.set_property("ssrc", ssrc);
} }
self.configure_congestion_control(payloader, extension_configuration_type) self.configure_congestion_control(payloader, codec, extension_configuration_type)
} }
fn generate_ssrc( fn generate_ssrc(
@ -2951,10 +2989,11 @@ impl BaseWebRTCSink {
} }
for encoder in session.encoders.iter_mut() { for encoder in session.encoders.iter_mut() {
encoder.set_bitrate(element, encoders_bitrate); if encoder.set_bitrate(element, encoders_bitrate).is_ok() {
encoder encoder
.transceiver .transceiver
.set_property("fec-percentage", (fec_percentage as u32).min(100)); .set_property("fec-percentage", (fec_percentage as u32).min(100));
}
} }
} }
} }

View file

@ -87,6 +87,8 @@ pub enum WebRTCSinkError {
peer_id: String, peer_id: String,
details: String, details: String,
}, },
#[error("Bitrate handling currently not supported for requested encoder")]
BitrateNotSupported,
} }
impl Default for BaseWebRTCSink { impl Default for BaseWebRTCSink {