Skip to content

Commit

Permalink
Fix bug with choosing transceiver for remote m-line
Browse files Browse the repository at this point in the history
  • Loading branch information
LVala committed Feb 2, 2024
1 parent c0754f8 commit 4d1ced1
Show file tree
Hide file tree
Showing 4 changed files with 71 additions and 12 deletions.
21 changes: 17 additions & 4 deletions lib/ex_webrtc/peer_connection.ex
Original file line number Diff line number Diff line change
Expand Up @@ -469,7 +469,7 @@ defmodule ExWebRTC.PeerConnection do
{transceivers, sender} =
case free_transceiver_idx do
nil ->
options = [direction: :sendrecv, ssrc: generate_ssrc(state)]
options = [direction: :sendrecv, ssrc: generate_ssrc(state), added_by_add_track: true]
tr = RTPTransceiver.new(kind, track, state.config, options)
{state.transceivers ++ [tr], tr.sender}

Expand Down Expand Up @@ -1025,8 +1025,6 @@ defmodule ExWebRTC.PeerConnection do
defp do_process_mlines_remote([], transceivers, _sdp_type, _config, _owner), do: transceivers

defp do_process_mlines_remote([{mline, idx} | mlines], transceivers, sdp_type, config, owner) do
{:mid, mid} = ExSDP.get_attribute(mline, :mid)

direction =
if SDPUtils.rejected?(mline),
do: :inactive,
Expand All @@ -1035,7 +1033,7 @@ defmodule ExWebRTC.PeerConnection do
# Note: in theory we should update transceiver codecs
# after processing remote track but this shouldn't have any impact
{idx, tr} =
case find_transceiver(transceivers, mid) do
case find_transceiver_from_remote(transceivers, mline) do
{idx, %RTPTransceiver{} = tr} -> {idx, RTPTransceiver.update(tr, mline, config)}
nil -> {nil, RTPTransceiver.from_mline(mline, idx, config)}
end
Expand All @@ -1059,6 +1057,21 @@ defmodule ExWebRTC.PeerConnection do
end
end

defp find_transceiver_from_remote(transceivers, mline) do
{:mid, mid} = ExSDP.get_attribute(mline, :mid)

case find_transceiver(transceivers, mid) do
{idx, %RTPTransceiver{} = tr} -> {idx, tr}
nil -> find_associable_transceiver(transceivers, mline)
end
end

defp find_associable_transceiver(transceivers, mline) do
transceivers
|> Enum.with_index(fn tr, idx -> {idx, tr} end)
|> Enum.find(fn {_idx, tr} -> RTPTransceiver.associable?(tr, mline) end)
end

# see W3C WebRTC 5.1.1
defp process_remote_track(transceiver, direction, owner) do
cond do
Expand Down
7 changes: 4 additions & 3 deletions lib/ex_webrtc/rtp_sender.ex
Original file line number Diff line number Diff line change
Expand Up @@ -52,14 +52,15 @@ defmodule ExWebRTC.RTPSender do
end

@doc false
@spec update(t(), RTPCodecParameters.t(), [Extmap.t()]) :: t()
def update(sender, codec, rtp_hdr_exts) do
@spec update(t(), String.t(), RTPCodecParameters.t(), [Extmap.t()]) :: t()
def update(sender, mid, codec, rtp_hdr_exts) do
if sender.mid != nil and mid != sender.mid, do: raise(ArgumentError)
# convert to a map to be able to find extension id using extension uri
rtp_hdr_exts = Map.new(rtp_hdr_exts, fn extmap -> {extmap.uri, extmap} end)
# TODO: handle cases when codec == nil (no valid codecs after negotiation)
pt = if codec != nil, do: codec.payload_type, else: nil

%__MODULE__{sender | codec: codec, rtp_hdr_exts: rtp_hdr_exts, pt: pt}
%__MODULE__{sender | mid: mid, codec: codec, rtp_hdr_exts: rtp_hdr_exts, pt: pt}
end

# Prepares packet for sending i.e.:
Expand Down
34 changes: 29 additions & 5 deletions lib/ex_webrtc/rtp_transceiver.ex
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@ defmodule ExWebRTC.RTPTransceiver do
receiver: RTPReceiver.t(),
sender: RTPSender.t(),
stopping: boolean(),
stopped: boolean()
stopped: boolean(),
added_by_add_track: boolean()
}

@enforce_keys [:id, :direction, :kind, :sender, :receiver]
Expand All @@ -43,7 +44,8 @@ defmodule ExWebRTC.RTPTransceiver do
codecs: [],
rtp_hdr_exts: [],
stopping: false,
stopped: false
stopped: false,
added_by_add_track: false
]

@doc false
Expand Down Expand Up @@ -78,7 +80,8 @@ defmodule ExWebRTC.RTPTransceiver do
codecs: codecs,
rtp_hdr_exts: rtp_hdr_exts,
receiver: %RTPReceiver{track: track},
sender: RTPSender.new(sender_track, List.first(codecs), rtp_hdr_exts, options[:ssrc])
sender: RTPSender.new(sender_track, List.first(codecs), rtp_hdr_exts, options[:ssrc]),
added_by_add_track: Keyword.get(options, :added_by_add_track, false)
}
end

Expand All @@ -104,16 +107,37 @@ defmodule ExWebRTC.RTPTransceiver do
}
end

@doc false
@spec associable?(t(), ExSDP.Media.t()) :: boolean()
def associable?(transceiver, mline) do
%__MODULE__{
mid: mid,
kind: kind,
added_by_add_track: added_by_add_track,
stopped: stopped
} = transceiver

direction = SDPUtils.get_media_direction(mline)

is_nil(mid) and added_by_add_track and
kind == mline.type and not stopped and
direction in [:sendrecv, :recvonly]
end

@doc false
@spec update(t(), ExSDP.Media.t(), Configuration.t()) :: t()
def update(transceiver, mline, config) do
{:mid, mid} = ExSDP.get_attribute(mline, :mid)
if transceiver.mid != nil and mid != transceiver.mid, do: raise(ArgumentError)

codecs = get_codecs(mline, config)
rtp_hdr_exts = get_rtp_hdr_extensions(mline, config)
sender = RTPSender.update(transceiver.sender, List.first(codecs), rtp_hdr_exts)
sender = RTPSender.update(transceiver.sender, mid, List.first(codecs), rtp_hdr_exts)

%__MODULE__{
transceiver
| codecs: codecs,
| mid: mid,
codecs: codecs,
rtp_hdr_exts: rtp_hdr_exts,
sender: sender
}
Expand Down
21 changes: 21 additions & 0 deletions test/ex_webrtc/peer_connection_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -762,6 +762,27 @@ defmodule ExWebRTC.PeerConnectionTest do
test_send_data(pc1, pc2, track1, track2)
end

test "using one negotiation, with tracks added beforehand" do
{:ok, pc1} = PeerConnection.start_link()
{:ok, pc2} = PeerConnection.start_link()

track1 = MediaStreamTrack.new(:audio)
track2 = MediaStreamTrack.new(:audio)

{:ok, _sender} = PeerConnection.add_track(pc1, track1)
{:ok, _sender} = PeerConnection.add_track(pc2, track2)

{:ok, offer} = PeerConnection.create_offer(pc1)
:ok = PeerConnection.set_local_description(pc1, offer)
: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)

test_send_data(pc1, pc2, track1, track2)
end

test "using renegotiation" do
# setup track pc1 -> pc2
{:ok, pc1} = PeerConnection.start_link()
Expand Down

0 comments on commit 4d1ced1

Please sign in to comment.