Fix video codec probing

CHANGELOG.md

@@ -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
 
 
@@ -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

Cargo.toml

@@ -104,7 +104,7 @@ wee_alloc = { version = "0.4", optional = true }
         "RtcPeerConnectionIceEvent", "RtcPeerConnectionIceErrorEvent",
         "RtcPeerConnectionState",
         "RtcRtpCapabilities",
-        "RtcRtpCodecParameters",
+        "RtcRtpCodecCapability", "RtcRtpCodecParameters",
         "RtcRtpEncodingParameters",
         "RtcRtpParameters",
         "RtcRtpReceiver", "RtcRtpSender",

src/peer/media/mod.rs

@@ -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

src/platform/dart/codec_capability.rs

@@ -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`].

src/platform/wasm/codec_capability.rs

@@ -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,
@@ -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.
@@ -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
     }
 }

tests/peer/media/mod.rs

@@ -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
@@ -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]
@@ -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![