Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix video codec probing #180

Merged
merged 8 commits into from
Aug 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,16 @@ All user visible changes to this project will be documented in this file. This p

[Diff](https://github.com/instrumentisto/medea-jason/compare/medea-jason-0.5.0...medea-jason-0.5.1)

### Fixed

- [VP9] being forced to use `profile-id=2` in [SFU] mode ([#180]).

### Upgraded

- Dependencies:
- [`derive-more`] to `1.0` ([#181]).

[#180]: /../../pull/180
[#181]: /../../pull/181


Expand Down Expand Up @@ -376,6 +381,7 @@ All user visible changes to this project will be documented in this file. This p



[`derive_more`]: https://docs.rs/derive_more
[Semantic Versioning 2.0.0]: https://semver.org
[SFU]: https://webrtcglossary.com/sfu
[Semantic Versioning 2.0.0]: https://semver.org
[VP9]: https://bloggeek.me/webrtcglossary/vp9/
[`derive_more`]: https://docs.rs/derive_more
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ wee_alloc = { version = "0.4", optional = true }
"RtcPeerConnectionIceEvent", "RtcPeerConnectionIceErrorEvent",
"RtcPeerConnectionState",
"RtcRtpCapabilities",
"RtcRtpCodecParameters",
"RtcRtpCodecCapability", "RtcRtpCodecParameters",
"RtcRtpEncodingParameters",
"RtcRtpParameters",
"RtcRtpReceiver", "RtcRtpSender",
Expand Down
68 changes: 32 additions & 36 deletions src/peer/media/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -251,49 +251,45 @@ pub enum GetMidsError {
/// Returns the required [`CodecCapability`]s and [`ScalabilityMode`] for a
/// [`platform::Transceiver`] based on the provided [`SvcSettings`].
pub async fn probe_video_codecs(
svc: &Vec<SvcSettings>,
svc_settings: &Vec<SvcSettings>,
) -> (Vec<CodecCapability>, Option<ScalabilityMode>) {
/// List of required codecs for every [`MediaKind::Video`] of a
/// [`platform::Transceiver`].
const REQUIRED_CODECS: [&str; 3] =
["video/rtx", "video/red", "video/ulpfec"];

CodecCapability::get_sender_codec_capabilities(MediaKind::Video)
.await
.map_or_else(
|_| (vec![], None),
|codecs| {
let mut target_scalability_mode = None;
let mut target_codecs = Vec::new();
let mut codecs: HashMap<String, CodecCapability> = codecs
.into_iter()
.filter_map(|codec| Some((codec.mime_type().ok()?, codec)))
.collect();

for svc_setting in svc {
if let Some(codec_cap) =
codecs.remove(svc_setting.codec.mime_type())
{
target_codecs.push(codec_cap);
target_scalability_mode =
Some(svc_setting.scalability_mode);
break;
}
}
if !target_codecs.is_empty() {
codecs
.into_iter()
.filter_map(|(mime, codec)| {
REQUIRED_CODECS
.contains(&mime.as_str())
.then_some(codec)
})
.for_each(|cap| target_codecs.push(cap));
}
let mut target_scalability_mode = None;
let mut target_codecs = Vec::new();

(target_codecs, target_scalability_mode)
},
)
let Ok(codecs) =
CodecCapability::get_sender_codec_capabilities(MediaKind::Video).await
else {
return (target_codecs, target_scalability_mode);
};

let mut codecs: HashMap<String, Vec<_>> =
codecs.into_iter().fold(HashMap::new(), |mut map, c| {
map.entry(c.mime_type()).or_default().push(c);
map
});

for svc in svc_settings {
if let Some(mut codec_cap) = codecs.remove(svc.codec.mime_type()) {
target_codecs.append(&mut codec_cap);
target_scalability_mode = Some(svc.scalability_mode);
break;
}
}
if !target_codecs.is_empty() {
#[allow(clippy::iter_over_hash_type)] // order doesn't matter here
for (mime, mut c) in codecs {
if REQUIRED_CODECS.contains(&mime.as_str()) {
target_codecs.append(&mut c);
}
}
}

(target_codecs, target_scalability_mode)
}

/// Returns [`SendEncodingParameters`] for a [`platform::Transceiver`] based on
Expand Down
11 changes: 3 additions & 8 deletions src/platform/dart/codec_capability.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,17 +86,12 @@ impl CodecCapability {

/// Returns [MIME media type][2] of this [`CodecCapability`].
///
/// # Errors
///
/// Never errors, but [`Result`] is needed for consistency with WASM
/// implementation.
///
/// [2]: https://w3.org/TR/webrtc#dom-rtcrtpcodeccapability-mimetype
#[allow(clippy::unwrap_in_result)] // intentional
pub fn mime_type(&self) -> Result<String, Error> {
#[must_use]
pub fn mime_type(&self) -> String {
let mime_type =
unsafe { codec_capability::mime_type(self.0.get()) }.unwrap();
Ok(unsafe { dart_string_into_rust(mime_type) })
unsafe { dart_string_into_rust(mime_type) }
}

/// Returns the underlying [`Dart_Handle`] of this [`CodecCapability`].
Expand Down
72 changes: 50 additions & 22 deletions src/platform/wasm/codec_capability.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,7 @@
//! [RTCRtpCodecCapability]: https://w3.org/TR/webrtc#dom-rtcrtpcodeccapability

use js_sys::{Array, JsString, Reflect};
use wasm_bindgen::JsValue;
use web_sys::RtcRtpSender;
use web_sys::{RtcRtpCodecCapability, RtcRtpSender};

use crate::{
media::MediaKind, platform::codec_capability::CodecCapabilityError as Error,
Expand All @@ -14,7 +13,15 @@ use crate::{
///
/// [RTCRtpCodecCapability]: https://w3.org/TR/webrtc#dom-rtcrtpcodeccapability
#[derive(Clone, Debug)]
pub struct CodecCapability(JsValue);
pub struct CodecCapability {
/// Actual JS-side [`RtcRtpCodecCapability`].
codec_cap: RtcRtpCodecCapability,

/// [MIME media type/subtype][2] of the codec.
///
/// [2]: https://w3.org/TR/webrtc#dom-rtcrtpcodeccapability-mimetype
mime_type: String,
}

impl CodecCapability {
/// Returns available [RTCRtpSender]'s [`CodecCapability`]s.
Expand All @@ -29,33 +36,54 @@ impl CodecCapability {
pub async fn get_sender_codec_capabilities(
kind: MediaKind,
) -> Result<Vec<Self>, Error> {
let codecs = RtcRtpSender::get_capabilities(&kind.to_string())
.and_then(|capabs| {
Reflect::get(&capabs, &JsString::from("codecs")).ok()
})
.ok_or(Error::FailedToGetCapabilities)?;
let mut result = Vec::new();

let Some(caps) = RtcRtpSender::get_capabilities(&kind.to_string())
else {
return Err(Error::FailedToGetCapabilities);
};

// TODO: Get rid of reflection in #183 which updates `web-sys` to
// 0.3.70.
let Ok(codecs) = Reflect::get(&caps, &JsString::from("codecs")) else {
return Err(Error::FailedToGetCapabilities);
};

for codec in Array::from(&codecs).values() {
let Ok(codec) = codec else {
continue;
};

Ok(Array::from(&codecs).iter().map(Self).collect())
let codec_cap = RtcRtpCodecCapability::from(codec);
let Some(mime_type) =
Reflect::get(&codec_cap, &JsString::from("mimeType"))
.ok()
.and_then(|v| v.as_string())
else {
return Err(Error::FailedToGetCapabilities);
};

result.push(Self {
codec_cap,
mime_type,
});
}

Ok(result)
}

/// Returns [MIME media type][2] of this [`CodecCapability`].
///
/// # Errors
///
/// With [`Error::FailedToGetMimeType`] if fails to retrieve codec's
/// [MIME media type][2].
///
/// [2]: https://w3.org/TR/webrtc#dom-rtcrtpcodeccapability-mimetype
pub fn mime_type(&self) -> Result<String, Error> {
Reflect::get(&self.0, &JsString::from("mimeType"))
.ok()
.and_then(|a| a.as_string())
.ok_or(Error::FailedToGetMimeType)
#[must_use]
pub fn mime_type(&self) -> String {
self.mime_type.clone()
}

/// Returns the underlying [`JsValue`] of this [`CodecCapability`].
/// Returns the underlying [`RtcRtpCodecCapability`] of this
/// [`CodecCapability`].
#[must_use]
pub const fn handle(&self) -> &JsValue {
&self.0
pub const fn handle(&self) -> &RtcRtpCodecCapability {
&self.codec_cap
}
}
59 changes: 41 additions & 18 deletions tests/peer/media/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -460,7 +460,7 @@ mod codec_probing {

fn target_codecs_mime_types(codecs: &[CodecCapability]) -> Vec<String> {
let mut mime_types: Vec<_> =
codecs.iter().map(|c| c.mime_type().unwrap()).collect();
codecs.iter().map(|c| c.mime_type()).collect();
mime_types.sort();
mime_types.dedup();
mime_types
Expand All @@ -483,23 +483,45 @@ mod codec_probing {

#[wasm_bindgen_test]
async fn probes_first_codec_when_many() {
let (target_codecs, svc) = probe_video_codecs(&vec![
SvcSettings {
codec: Codec::VP9,
scalability_mode: ScalabilityMode::L1T2,
},
SvcSettings {
codec: Codec::VP8,
scalability_mode: ScalabilityMode::L1T1,
},
])
.await;
{
let (target_codecs, svc) = probe_video_codecs(&vec![
SvcSettings {
codec: Codec::VP9,
scalability_mode: ScalabilityMode::L1T2,
},
SvcSettings {
codec: Codec::VP8,
scalability_mode: ScalabilityMode::L1T1,
},
])
.await;

assert_eq!(svc, Some(ScalabilityMode::L1T2));
assert_eq!(
target_codecs_mime_types(&target_codecs),
vec!["video/VP9", "video/red", "video/rtx", "video/ulpfec"]
);
assert_eq!(svc, Some(ScalabilityMode::L1T2));
assert_eq!(
target_codecs_mime_types(&target_codecs),
vec!["video/VP9", "video/red", "video/rtx", "video/ulpfec"]
);
}

{
let (target_codecs, svc) = probe_video_codecs(&vec![
SvcSettings {
codec: Codec::VP8,
scalability_mode: ScalabilityMode::L1T1,
},
SvcSettings {
codec: Codec::VP9,
scalability_mode: ScalabilityMode::L1T2,
},
])
.await;

assert_eq!(svc, Some(ScalabilityMode::L1T1));
assert_eq!(
target_codecs_mime_types(&target_codecs),
vec!["video/VP8", "video/red", "video/rtx", "video/ulpfec"]
);
}
}

#[wasm_bindgen_test]
Expand All @@ -523,10 +545,11 @@ mod codec_probing {
.unwrap();

// Filter codecs which are not fully supported by all browsers.
let codecs_caps: Vec<_> = target_codecs_mime_types(&caps)
let mut codecs_caps: Vec<_> = target_codecs_mime_types(&caps)
.into_iter()
.filter(|c| !NOT_FULLY_SUPPORTED_CODECS.contains(&c.as_str()))
.collect();
codecs_caps.sort();
assert_eq!(
codecs_caps,
vec![
Expand Down
Loading