changeset 4289:33ecebb2c02f

plugin XEP-0167: fix session level SDP attributes parsing: rel 447
author Goffi <goffi@goffi.org>
date Mon, 29 Jul 2024 03:31:02 +0200
parents f46891f2c9cb
children 4837ec911c43
files libervia/backend/plugins/plugin_xep_0167/mapping.py
diffstat 1 files changed, 126 insertions(+), 129 deletions(-) [+]
line wrap: on
line diff
--- a/libervia/backend/plugins/plugin_xep_0167/mapping.py	Mon Jul 29 03:30:58 2024 +0200
+++ b/libervia/backend/plugins/plugin_xep_0167/mapping.py	Mon Jul 29 03:31:02 2024 +0200
@@ -284,12 +284,11 @@
     @param role: Role of the entities which produces the SDP.
     @return: A dictionary containing parsed session data.
     """
-    # FIXME: to be removed once host is accessible from global var
     assert host is not None
     lines = sdp.strip().split("\r\n")
-    # session metadata
     metadata: Dict[str, Any] = {}
     call_data = {"metadata": metadata}
+    session_attributes: Dict[str, Any] = {}
 
     media_type = None
     media_data: Optional[Dict[str, Any]] = None
@@ -299,14 +298,13 @@
     ice_pwd: Optional[str] = None
     ice_ufrag: Optional[str] = None
     payload_types: Optional[Dict[int, dict]] = None
-    # default value, will be be modified by SDP
     senders: str = "both"
 
     for line in lines:
         try:
             parts = line.split()
-            prefix = parts[0][:2]  # Extract the 'a=', 'm=', etc., prefix
-            parts[0] = parts[0][2:]  # Remove the prefix from the first element
+            prefix = parts[0][:2]
+            parts[0] = parts[0][2:]
 
             if prefix == "m=":
                 media_type = parts[0]
@@ -336,134 +334,140 @@
                     "senders": senders,
                 }
 
+                # Apply session attributes to the new media
+                for attr, value in session_attributes.items():
+                    if attr in ("fingerprint", "ice-pwd", "ice-ufrag"):
+                        transport_data[attr] = value
+                    elif attr in ("sendrecv", "sendonly", "recvonly", "inactive"):
+                        application_data["senders"] = value
+                    else:
+                        application_data[attr] = value
+
             elif prefix == "a=":
                 if ":" in parts[0]:
                     attribute, parts[0] = parts[0].split(":", 1)
                 else:
                     attribute = parts[0]
 
-                if (
-                    media_type is None
-                    or application_data is None
-                    or transport_data is None
-                ) and not (
-                    attribute
-                    in (
-                        "sendrecv",
-                        "sendonly",
-                        "recvonly",
-                        "inactive",
-                        "fingerprint",
-                        "group",
-                        "ice-options",
-                        "msid-semantic",
-                        "ice-pwd",
-                        "ice-ufrag",
-                    )
-                ):
-                    log.warning(
-                        "Received attribute before media description, this is "
-                        f"invalid: {line}"
-                    )
-                    continue
+                if media_type is None:
+                    # This is a session-level attribute
+                    if attribute == "fingerprint":
+                        algorithm, fingerprint = parts[0], parts[1]
+                        session_attributes["fingerprint"] = {
+                            "hash": algorithm,
+                            "fingerprint": fingerprint,
+                        }
+                    elif attribute == "ice-pwd":
+                        session_attributes["ice-pwd"] = parts[0]
+                    elif attribute == "ice-ufrag":
+                        session_attributes["ice-ufrag"] = parts[0]
+                    elif attribute in ("sendrecv", "sendonly", "recvonly", "inactive"):
+                        if attribute == "sendrecv":
+                            value = "both"
+                        elif attribute == "sendonly":
+                            value = role
+                        elif attribute == "recvonly":
+                            value = "responder" if role == "initiator" else "initiator"
+                        else:
+                            value = "none"
+                        session_attributes[attribute] = value
+                    else:
+                        session_attributes[attribute] = parts[0] if parts else True
+                else:
+                    # This is a media-level attribute
+                    if attribute == "mid":
+                        assert media_data is not None
+                        try:
+                            media_data["id"] = parts[0]
+                        except IndexError:
+                            log.warning(f"invalid media ID: {line}")
 
-                if attribute == "mid":
-                    assert media_data is not None
-                    try:
-                        media_data["id"] = parts[0]
-                    except IndexError:
-                        log.warning(f"invalid media ID: {line}")
+                    elif attribute == "rtpmap":
+                        assert application_data is not None
+                        assert payload_types is not None
+                        pt_id = int(parts[0])
+                        codec_info = parts[1].split("/")
+                        codec = codec_info[0]
+                        clockrate = int(codec_info[1])
+                        payload_type = {
+                            "id": pt_id,
+                            "name": codec,
+                            "clockrate": clockrate,
+                        }
+                        if len(codec_info) > 2:
+                            channels = int(codec_info[2])
+                            payload_type["channels"] = channels
 
-                elif attribute == "rtpmap":
-                    assert application_data is not None
-                    assert payload_types is not None
-                    pt_id = int(parts[0])
-                    codec_info = parts[1].split("/")
-                    codec = codec_info[0]
-                    clockrate = int(codec_info[1])
-                    payload_type = {
-                        "id": pt_id,
-                        "name": codec,
-                        "clockrate": clockrate,
-                    }
-                    # Handle optional channel count
-                    if len(codec_info) > 2:
-                        channels = int(codec_info[2])
-                        payload_type["channels"] = channels
-
-                    payload_types.setdefault(pt_id, {}).update(payload_type)
+                        payload_types.setdefault(pt_id, {}).update(payload_type)
 
-                elif attribute == "fmtp":
-                    assert payload_types is not None
-                    pt_id = int(parts[0])
-                    params = parts[1].split(";")
-                    try:
-                        payload_type = payload_types[pt_id]
-                    except KeyError:
-                        raise ValueError(
-                            f"Can find content type {pt_id}, ignoring: {line}"
-                        )
+                    elif attribute == "fmtp":
+                        assert payload_types is not None
+                        pt_id = int(parts[0])
+                        params = parts[1].split(";")
+                        try:
+                            payload_type = payload_types[pt_id]
+                        except KeyError:
+                            raise ValueError(
+                                f"Can find content type {pt_id}, ignoring: {line}"
+                            )
 
-                    try:
-                        payload_type["parameters"] = {
-                            name: value
-                            for name, value in (param.split("=") for param in params)
-                        }
-                    except ValueError:
-                        payload_type.setdefault("exra-parameters", []).extend(params)
+                        try:
+                            payload_type["parameters"] = {
+                                name: value
+                                for name, value in (param.split("=") for param in params)
+                            }
+                        except ValueError:
+                            payload_type.setdefault("exra-parameters", []).extend(params)
 
-                elif attribute == "candidate":
-                    assert transport_data is not None
-                    candidate = parse_candidate(parts)
+                    elif attribute == "candidate":
+                        assert transport_data is not None
+                        candidate = parse_candidate(parts)
 
-                    transport_data.setdefault("candidates", []).append(candidate)
+                        transport_data.setdefault("candidates", []).append(candidate)
 
-                elif attribute == "fingerprint":
-                    algorithm, fingerprint = parts[0], parts[1]
-                    fingerprint_data = {"hash": algorithm, "fingerprint": fingerprint}
-                    if transport_data is not None:
-                        transport_data.setdefault("fingerprint", {}).update(
-                            fingerprint_data
-                        )
-                elif attribute == "setup":
-                    assert transport_data is not None
-                    setup = parts[0]
-                    transport_data.setdefault("fingerprint", {})["setup"] = setup
+                    elif attribute == "fingerprint":
+                        algorithm, fingerprint = parts[0], parts[1]
+                        fingerprint_data = {"hash": algorithm, "fingerprint": fingerprint}
+                        if transport_data is not None:
+                            transport_data.setdefault("fingerprint", {}).update(
+                                fingerprint_data
+                            )
+                    elif attribute == "setup":
+                        assert transport_data is not None
+                        setup = parts[0]
+                        transport_data.setdefault("fingerprint", {})["setup"] = setup
 
-                elif attribute == "b":
-                    assert application_data is not None
-                    bandwidth = int(parts[0])
-                    application_data["bandwidth"] = bandwidth
+                    elif attribute == "b":
+                        assert application_data is not None
+                        bandwidth = int(parts[0])
+                        application_data["bandwidth"] = bandwidth
 
-                elif attribute == "rtcp-mux":
-                    assert application_data is not None
-                    application_data["rtcp-mux"] = True
+                    elif attribute == "rtcp-mux":
+                        assert application_data is not None
+                        application_data["rtcp-mux"] = True
 
-                elif attribute == "ice-ufrag":
-                    if transport_data is not None:
-                        transport_data["ufrag"] = parts[0]
+                    elif attribute == "ice-ufrag":
+                        if transport_data is not None:
+                            transport_data["ufrag"] = parts[0]
 
-                elif attribute == "ice-pwd":
-                    if transport_data is not None:
-                        transport_data["pwd"] = parts[0]
+                    elif attribute == "ice-pwd":
+                        if transport_data is not None:
+                            transport_data["pwd"] = parts[0]
 
-                elif attribute in ("sendrecv", "sendonly", "recvonly", "inactive"):
-                    if attribute == "sendrecv":
-                        value = "both"
-                    elif attribute == "sendonly":
-                        value = role
-                    elif attribute == "recvonly":
-                        value = "responder" if role == "initiator" else "initiator"
-                    else:
-                        value = "none"
+                    elif attribute in ("sendrecv", "sendonly", "recvonly", "inactive"):
+                        if attribute == "sendrecv":
+                            value = "both"
+                        elif attribute == "sendonly":
+                            value = role
+                        elif attribute == "recvonly":
+                            value = "responder" if role == "initiator" else "initiator"
+                        else:
+                            value = "none"
 
-                    if application_data is None:
-                        # this is a global value, we use is as new default value
-                        senders = value
-                    else:
-                        # this is a media specific value, it will replace the one used as
-                        # default for this media
-                        application_data["senders"] = value
+                        if application_data is None:
+                            senders = value
+                        else:
+                            application_data["senders"] = value
 
                 host.trigger.point(
                     "XEP-0167_parse_sdp_a",
@@ -488,22 +492,15 @@
         log.debug(f"cleaning remaining private data {key!r}")
         del call_data[key]
 
-    # FIXME: is this really useful?
-    # ICE candidates may only be specified for the first media, this
-    # duplicate the candidate for the other in this case
     all_media = {k: v for k, v in call_data.items() if k in ("audio", "video")}
-    if len(all_media) > 1 and not all(
-        "candidates" in c["transport_data"] for c in all_media.values()
-    ):
-        first_content = next(iter(all_media.values()))
-        try:
-            ice_candidates = first_content["transport_data"]["candidates"]
-        except KeyError:
-            ice_candidates = []
-        for idx, content in enumerate(all_media.values()):
-            if idx == 0:
-                continue
-            content["transport_data"].setdefault("candidates", ice_candidates)
+    if len(all_media) > 1:
+        media_with_candidates = [
+            m for m in all_media.values() if "candidates" in m["transport_data"]
+        ]
+        if len(media_with_candidates) == 1:
+            log.warning(
+                "SDP contains ICE candidates only for one media stream. This is non-standard behavior."
+            )
 
     return call_data