From 8f3fac2c81e4d89fc726a3aaf66da06065b18c17 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Wala?= <84684231+LVala@users.noreply.github.com> Date: Thu, 2 Nov 2023 09:03:51 +0100 Subject: [PATCH] Assign mids in `create_offer` (#8) --- lib/ex_webrtc/peer_connection.ex | 60 ++++++++++++++------------------ lib/ex_webrtc/rtp_transceiver.ex | 39 --------------------- lib/ex_webrtc/sdp_utils.ex | 39 +++++++++++++++++++++ 3 files changed, 65 insertions(+), 73 deletions(-) diff --git a/lib/ex_webrtc/peer_connection.ex b/lib/ex_webrtc/peer_connection.ex index c1d3e845..e12aeac4 100644 --- a/lib/ex_webrtc/peer_connection.ex +++ b/lib/ex_webrtc/peer_connection.ex @@ -127,17 +127,19 @@ defmodule ExWebRTC.PeerConnection do end @impl true - def handle_call({:create_offer, options}, _from, state) - when state.signaling_state in [:stable, :have_local_offer, :have_remote_pranswer] do + def handle_call({:create_offer, _options}, _from, %{signaling_state: ss} = state) + when ss not in [:stable, :have_local_offer] do + {:reply, {:error, :invalid_state}, state} + end + + @impl true + def handle_call({:create_offer, options}, _from, state) do # TODO: handle subsequent offers if Keyword.get(options, :ice_restart, false) do - ICEAgent.restart(state.ice_agent) + :ok = ICEAgent.restart(state.ice_agent) end - # we need to asign unique mid values for the transceivers - # in this case internal counter is used - next_mid = find_next_mid(state) transceivers = assign_mids(state.transceivers, next_mid) @@ -146,6 +148,7 @@ defmodule ExWebRTC.PeerConnection do offer = %ExSDP{ExSDP.new() | timing: %ExSDP.Timing{start_time: 0, stop_time: 0}} + # we support trickle ICE only |> ExSDP.add_attribute({:ice_options, "trickle"}) config = @@ -160,7 +163,7 @@ defmodule ExWebRTC.PeerConnection do mlines = Enum.map(transceivers, fn transceiver -> - RTPTransceiver.to_mline(transceiver, config) + SDPUtils.to_offer_mline(transceiver, config) end) mids = @@ -179,19 +182,21 @@ defmodule ExWebRTC.PeerConnection do sdp = to_string(offer) desc = %SessionDescription{type: :offer, sdp: sdp} - state = %{state | last_offer: sdp} + # NOTICE: we assign mids in create_offer (not in apply_local_description) + # this is fine as long as we not allow for SDP munging + state = %{state | last_offer: sdp, transceivers: transceivers} {:reply, {:ok, desc}, state} end @impl true - def handle_call({:create_offer, _options}, _from, state) do + def handle_call({:create_answer, _options}, _from, %{signaling_state: ss} = state) + when ss not in [:have_remote_offer, :have_local_pranswer] do {:reply, {:error, :invalid_state}, state} end @impl true - def handle_call({:create_answer, _options}, _from, state) - when state.signaling_state in [:have_remote_offer, :have_local_pranswer] do + def handle_call({:create_answer, _options}, _from, state) do {:offer, remote_offer} = state.pending_remote_desc {:ok, ice_ufrag, ice_pwd} = ICEAgent.get_local_credentials(state.ice_agent) @@ -242,11 +247,6 @@ defmodule ExWebRTC.PeerConnection do {:reply, {:ok, desc}, state} end - @impl true - def handle_call({:create_answer, _options}, _from, state) do - {:reply, {:error, :invalid_state}, state} - end - @impl true def handle_call({:set_local_description, desc}, _from, state) do %SessionDescription{type: type, sdp: sdp} = desc @@ -404,14 +404,9 @@ defmodule ExWebRTC.PeerConnection do {:ok, %{state | transceivers: new_transceivers}} end - defp update_local_transceivers(:offer, transceivers, sdp) do - sdp.media - |> Enum.zip(transceivers) - |> Enum.map(fn {mline, transceiver} -> - {:mid, mid} = ExSDP.Media.get_attribute(mline, :mid) - # TODO: check if mid from mline == mid from transceiver - %{transceiver | mid: mid} - end) + defp update_local_transceivers(:offer, transceivers, _sdp) do + # TODO: at the moment, we assign mids in create_offer + transceivers end defp update_local_transceivers(:answer, transceivers, _sdp) do @@ -461,22 +456,19 @@ defmodule ExWebRTC.PeerConnection do end) end - defp assign_mids(transceivers, next_mid, acc \\ []) - defp assign_mids([], _next_mid, acc), do: Enum.reverse(acc) - - defp assign_mids([transceiver | rest], next_mid, acc) when is_nil(transceiver.mid) do - transceiver = %RTPTransceiver{transceiver | mid: Integer.to_string(next_mid)} - assign_mids(rest, next_mid + 1, [transceiver | acc]) - end + defp assign_mids(transceivers, next_mid) do + {new_transceivers, _next_mid} = + Enum.map_reduce(transceivers, next_mid, fn + %{mid: nil} = t, nm -> {%{t | mid: to_string(nm)}, nm + 1} + other, nm -> {other, nm} + end) - defp assign_mids([transceiver | rest], next_mid, acc) do - assign_mids(rest, next_mid, [transceiver | acc]) + new_transceivers end defp find_next_mid(state) do # next mid must be unique, it's acomplished by looking for values # greater than any mid in remote description or our own transceivers - # should we keep track of next_mid in the state? crd_mids = if is_nil(state.current_remote_desc) do [] diff --git a/lib/ex_webrtc/rtp_transceiver.ex b/lib/ex_webrtc/rtp_transceiver.ex index 4346ad3d..e18d5d02 100644 --- a/lib/ex_webrtc/rtp_transceiver.ex +++ b/lib/ex_webrtc/rtp_transceiver.ex @@ -55,45 +55,6 @@ defmodule ExWebRTC.RTPTransceiver do end end - def to_mline(transceiver, config) do - pt = Enum.map(transceiver.codecs, fn codec -> codec.payload_type end) - - media_formats = - Enum.flat_map(transceiver.codecs, fn codec -> - [_type, encoding] = String.split(codec.mime_type, "/") - - rtp_mapping = %ExSDP.Attribute.RTPMapping{ - clock_rate: codec.clock_rate, - encoding: encoding, - params: codec.channels, - payload_type: codec.payload_type - } - - [rtp_mapping, codec.sdp_fmtp_line, codec.rtcp_fbs] - end) - - attributes = - if(Keyword.get(config, :rtcp, false), do: [{"rtcp", "9 IN IP4 0.0.0.0"}], else: []) ++ - [ - transceiver.direction, - {:mid, transceiver.mid}, - {:ice_ufrag, Keyword.fetch!(config, :ice_ufrag)}, - {:ice_pwd, Keyword.fetch!(config, :ice_pwd)}, - {:ice_options, Keyword.fetch!(config, :ice_options)}, - {:fingerprint, Keyword.fetch!(config, :fingerprint)}, - {:setup, Keyword.fetch!(config, :setup)}, - :rtcp_mux - ] - - %ExSDP.Media{ - ExSDP.Media.new(transceiver.kind, 9, "UDP/TLS/RTP/SAVPF", pt) - | # mline must be followed by a cline, which must contain - # the default value "IN IP4 0.0.0.0" (as there are no candidates yet) - connection_data: [%ExSDP.ConnectionData{address: {0, 0, 0, 0}}] - } - |> ExSDP.Media.add_attributes(attributes ++ media_formats) - end - defp update(transceiver, mline) do codecs = get_codecs(mline) hdr_exts = ExSDP.Media.get_attributes(mline, ExSDP.Attribute.Extmap) diff --git a/lib/ex_webrtc/sdp_utils.ex b/lib/ex_webrtc/sdp_utils.ex index 99314fa9..8f37cb50 100644 --- a/lib/ex_webrtc/sdp_utils.ex +++ b/lib/ex_webrtc/sdp_utils.ex @@ -46,6 +46,45 @@ defmodule ExWebRTC.SDPUtils do |> ExSDP.Media.add_attributes(attributes ++ media_formats) end + def to_offer_mline(transceiver, config) do + pt = Enum.map(transceiver.codecs, fn codec -> codec.payload_type end) + + media_formats = + Enum.flat_map(transceiver.codecs, fn codec -> + [_type, encoding] = String.split(codec.mime_type, "/") + + rtp_mapping = %ExSDP.Attribute.RTPMapping{ + clock_rate: codec.clock_rate, + encoding: encoding, + params: codec.channels, + payload_type: codec.payload_type + } + + [rtp_mapping, codec.sdp_fmtp_line, codec.rtcp_fbs] + end) + + attributes = + if(Keyword.get(config, :rtcp, false), do: [{"rtcp", "9 IN IP4 0.0.0.0"}], else: []) ++ + [ + transceiver.direction, + {:mid, transceiver.mid}, + {:ice_ufrag, Keyword.fetch!(config, :ice_ufrag)}, + {:ice_pwd, Keyword.fetch!(config, :ice_pwd)}, + {:ice_options, Keyword.fetch!(config, :ice_options)}, + {:fingerprint, Keyword.fetch!(config, :fingerprint)}, + {:setup, Keyword.fetch!(config, :setup)}, + :rtcp_mux + ] + + %ExSDP.Media{ + ExSDP.Media.new(transceiver.kind, 9, "UDP/TLS/RTP/SAVPF", pt) + | # mline must be followed by a cline, which must contain + # the default value "IN IP4 0.0.0.0" (as there are no candidates yet) + connection_data: [%ExSDP.ConnectionData{address: {0, 0, 0, 0}}] + } + |> ExSDP.Media.add_attributes(attributes ++ media_formats) + end + @spec get_media_direction(ExSDP.Media.t()) :: :sendrecv | :sendonly | :recvonly | :inactive | nil def get_media_direction(media) do