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

Add/refactor set_local_description and create_answer #7

Merged
merged 4 commits into from
Oct 31, 2023
Merged
Show file tree
Hide file tree
Changes from 3 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
91 changes: 76 additions & 15 deletions lib/ex_webrtc/peer_connection.ex
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ defmodule ExWebRTC.PeerConnection do
IceCandidate,
MediaStreamTrack,
RTPTransceiver,
SessionDescription,
SDPUtils,
SessionDescription,
Utils
}

Expand Down Expand Up @@ -192,26 +192,34 @@ defmodule ExWebRTC.PeerConnection do
@impl true
def handle_call({:create_answer, _options}, _from, state)
when state.signaling_state in [:have_remote_offer, :have_local_pranswer] do
{:offer, remote_offer} = state.pending_remote_desc

{:ok, ice_ufrag, ice_pwd} = ICEAgent.get_local_credentials(state.ice_agent)
{:ok, dtls_fingerprint} = ExDTLS.get_cert_fingerprint(state.dtls_client)

answer = %ExSDP{ExSDP.new() | timing: %ExSDP.Timing{start_time: 0, stop_time: 0}}

answer =
case ExSDP.get_attribute(remote_offer, :ice_options) do
{:ice_options, "trickle"} = attr -> ExSDP.add_attribute(answer, attr)
_other -> answer
end
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just FYI we don't support anything other than trickle ice so this attribute has to always be in offer/answer


config =
[
ice_ufrag: ice_ufrag,
ice_pwd: ice_pwd,
ice_options: "trickle",
fingerprint: {:sha256, Utils.hex_dump(dtls_fingerprint)},
# TODO offer will always contain actpass
# and answer should contain active
# see RFC 8829 sec. 5.3.1
setup: :passive
setup: :active
]

# TODO: rejected media sections
mlines =
Enum.map(state.transceivers, fn transceiver ->
RTPTransceiver.to_mline(transceiver, config)
Enum.map(remote_offer.media, fn mline ->
{:mid, mid} = ExSDP.Media.get_attribute(mline, :mid)
{_ix, transceiver} = RTPTransceiver.find_by_mid(state.transceivers, mid)
SDPUtils.get_answer_mline(mline, transceiver, config)
end)

mids =
Expand Down Expand Up @@ -381,7 +389,7 @@ defmodule ExWebRTC.PeerConnection do
%{dtls_buffered_packets: packets} = state
) do
# we got DTLS packets from the other side but
# we haven't established ICE connection yet so
# we haven't established ICE connection yet so
# packets to retransmit have to be the same as dtls_buffered_packets
{:noreply, state}
end
Expand All @@ -392,17 +400,33 @@ defmodule ExWebRTC.PeerConnection do
{:noreply, state}
end

defp apply_local_description(_type, _sdp, state) do
# TODO: implement
{:ok, state}
defp apply_local_description(type, sdp, state) do
new_transceivers = update_local_transceivers(type, state.transceivers, sdp)
state = set_description(:local, type, sdp, state)

{:ok, %{state | transceivers: new_transceivers}}
end

defp update_local_transceivers(:offer, transceivers, sdp) do
sdp.media
|> Enum.zip(transceivers)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am really not sure about zipping those two lists. We can discuss this offline but consider the following scenario:

  1. remote sends to us two tracks
  2. at some point, remote stops one track - the first one in SDP. In such the case we should probably delete our first transceiver as it will be marked as stopped. We will have two mlines (the first one inactive, the second one recvonly from local perspective) and one transceiver.
  3. If at this point, we decide to add a track on local side, we will add a new transceiver that will correspond to the first mline (which will be recycled). So we will end up with two transceivers and two mlines but the first transceiver will correspond to the second mline and the second transceiver will correspond to the first mline.

Does it make sense?

That's why I didn't decide to zip those two lists in RTPTransceiver.update_or_create.

We can also merge this as is and go back to the problem once it appears i.e. when we support track removal or stopping transceivers.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applying the local offer is the moment when mids are assigned to the transceivers, so if we want to prevent the issues that you have mentioned (so basically order of transceivers != order of mlines) we would need to map mline <-> transceiver in some other way (keep the mline index in transceiver state?).

For now I would probably leave it as it is, as that a bigger issue.

Also there's question: what if I create an offer, remove a transceiver and then apply the offer? Should it fail? Will need to take care of such cases in the near future probably.

|> 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)
end

defp apply_remote_description(_type, sdp, state) do
defp update_local_transceivers(:answer, transceivers, _sdp) do
transceivers
end

defp apply_remote_description(type, sdp, state) do
# TODO apply steps listed in RFC 8829 5.10
with :ok <- SDPUtils.ensure_mid(sdp),
:ok <- SDPUtils.ensure_bundle(sdp),
{:ok, {ice_ufrag, ice_pwd}} <- SDPUtils.get_ice_credentials(sdp),
{:ok, new_transceivers} <- update_transceivers(state.transceivers, sdp) do
{:ok, new_transceivers} <- update_remote_transceivers(state.transceivers, sdp) do
:ok = ICEAgent.set_remote_credentials(state.ice_agent, ice_ufrag, ice_pwd)
:ok = ICEAgent.gather_candidates(state.ice_agent)

Expand All @@ -419,13 +443,15 @@ defmodule ExWebRTC.PeerConnection do
notify(state.owner, {:track, track})
end

{:ok, %{state | current_remote_desc: sdp, transceivers: new_transceivers}}
state = set_description(:remote, type, sdp, state)

{:ok, %{state | transceivers: new_transceivers}}
else
error -> error
end
end

defp update_transceivers(transceivers, sdp) do
defp update_remote_transceivers(transceivers, sdp) do
Enum.reduce_while(sdp.media, {:ok, transceivers}, fn mline, {:ok, transceivers} ->
case ExSDP.Media.get_attribute(mline, :mid) do
{:mid, mid} ->
Expand Down Expand Up @@ -504,5 +530,40 @@ defmodule ExWebRTC.PeerConnection do
defp maybe_next_state(:have_remote_pranswer, :remote, :answer), do: {:ok, :stable}
defp maybe_next_state(:have_remote_pranswer, _, _), do: {:error, :invalid_transition}

defp set_description(:local, :answer, sdp, state) do
# NOTICE: internaly, we don't create SessionDescription
# as it would require serialization of sdp
%{
state
| current_local_desc: {:answer, sdp},
current_remote_desc: state.pending_remote_desc,
pending_local_desc: nil,
pending_remote_desc: nil,
# W3C 4.4.1.5 (.4.7.5.2) suggests setting these to "", not nil
last_offer: nil,
last_answer: nil
}
end

defp set_description(:local, other, sdp, state) when other in [:offer, :pranswer] do
%{state | pending_local_desc: {other, sdp}}
end

defp set_description(:remote, :answer, sdp, state) do
%{
state
| current_remote_desc: {:answer, sdp},
current_local_desc: state.pending_local_desc,
pending_remote_desc: nil,
pending_local_desc: nil,
last_offer: nil,
last_answer: nil
}
end

defp set_description(:remote, other, sdp, state) when other in [:offer, :pranswer] do
%{state | pending_remote_desc: {other, sdp}}
end

defp notify(pid, msg), do: send(pid, {:ex_webrtc, self(), msg})
end
55 changes: 55 additions & 0 deletions lib/ex_webrtc/sdp_utils.ex
Original file line number Diff line number Diff line change
@@ -1,6 +1,51 @@
defmodule ExWebRTC.SDPUtils do
@moduledoc false

alias ExWebRTC.RTPTransceiver

@spec get_answer_mline(ExSDP.Media.t(), RTPTransceiver.t(), Keyword.t()) :: ExSDP.Media.t()
def get_answer_mline(mline, transceiver, config) do
# TODO: we need to filter the media formats according to our capabilities
media_formats =
Enum.filter(mline.attributes, fn
%ExSDP.Attribute.RTPMapping{} -> true
%ExSDP.Attribute.FMTP{} -> true
_other -> false
end)

payload_types =
Enum.flat_map(media_formats, fn
%ExSDP.Attribute.RTPMapping{payload_type: pt} -> [pt]
_other -> []
end)

offered_direction = ExSDP.Media.get_attribute(mline, :direction)
direction = get_direction(offered_direction, transceiver.direction)

attributes =
[
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)},
# TODO: probably should fail if the offer doesn't contain rtcp-mux
# as we don't support non-muxed streams
:rtcp_mux
]

# TODO: validation of some the stuff in remote SDP
%ExSDP.Media{
ExSDP.Media.new(mline.type, 9, mline.protocol, payload_types)
| # 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
Expand Down Expand Up @@ -145,4 +190,14 @@ defmodule ExWebRTC.SDPUtils do
_ -> {:error, :conflicting_ice_credentials}
end
end

# RFC 3264 (6.1) + RFC 8829 (5.3.1)
# AFAIK one of the cases should always match
# bc we won't assign/create an inactive transceiver to i.e. sendonly mline
# also neither of the arguments should ever be :stopped
defp get_direction(_, :inactive), do: :inactive
defp get_direction(:sendonly, t) when t in [:sendrecv, :recvonly], do: :recvonly
defp get_direction(:recvonly, t) when t in [:sendrecv, :sendonly], do: :sendonly
defp get_direction(o, other) when o in [:sendrecv, nil], do: other
defp get_direction(:inactive, _), do: :inactive
end
2 changes: 1 addition & 1 deletion lib/ex_webrtc/session_description.ex
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ defmodule ExWebRTC.SessionDescription do
defstruct @enforce_keys

@spec from_json(%{String.t() => String.t()}) :: {:ok, t()} | :error
def from_init(%{"type" => type})
def from_json(%{"type" => type})
when type not in ["answer", "offer", "pranswer", "rollback"],
do: :error

Expand Down
14 changes: 14 additions & 0 deletions test/peer_connection_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,20 @@ defmodule ExWebRTC.PeerConnectionTest do
refute_receive {:ex_webrtc, ^pc, {:track, %MediaStreamTrack{}}}
end

test "offer/answer exchange" do
{:ok, pc1} = PeerConnection.start_link()
{:ok, _} = PeerConnection.add_transceiver(pc1, :audio)
{:ok, offer} = PeerConnection.create_offer(pc1)
:ok = PeerConnection.set_local_description(pc1, offer)

{:ok, pc2} = PeerConnection.start_link()
:ok = PeerConnection.set_remote_description(pc2, offer)
{:ok, answer} = PeerConnection.create_answer(pc2)
:ok = PeerConnection.set_local_description(pc2, answer)

:ok = PeerConnection.set_remote_description(pc1, answer)
end

describe "set_remote_description/2" do
test "MID" do
{:ok, pc} = PeerConnection.start_link()
Expand Down
Loading