Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
3 changes: 3 additions & 0 deletions lib/ex_webrtc/peer_connection.ex
Original file line number Diff line number Diff line change
Expand Up @@ -844,7 +844,7 @@
|> ExSDP.add_attributes([
%ExSDP.Attribute.Group{semantics: "BUNDLE", mids: mids},
# always allow for mixing one- and two-byte RTP header extensions
# TODO ensure this was also offered

Check warning on line 847 in lib/ex_webrtc/peer_connection.ex

View workflow job for this annotation

GitHub Actions / CI on OTP 27 / Elixir 1.17

Found a TODO tag in a comment: # TODO ensure this was also offered
"extmap-allow-mixed",
{"msid-semantic", "WMS *"}
])
Expand Down Expand Up @@ -2417,6 +2417,9 @@
# https://www.w3.org/TR/webrtc/#dfn-check-if-negotiation-is-needed
defp tr_negotiation_needed?([], _), do: false

defp tr_negotiation_needed?([tr | _transceivers], _state) when tr.stopping and not tr.stopped,
Copy link
Member Author

Choose a reason for hiding this comment

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

W3C, section 4.7.3

Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add a comment referencing this section?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's right above already, so I think we're good

do: true

defp tr_negotiation_needed?([tr | _transceivers], _state) when tr.mid == nil, do: true

defp tr_negotiation_needed?([tr | transceivers], state) do
Expand Down
6 changes: 5 additions & 1 deletion lib/ex_webrtc/rtp_transceiver.ex
Original file line number Diff line number Diff line change
Expand Up @@ -434,7 +434,11 @@ defmodule ExWebRTC.RTPTransceiver do
mline = to_mline(transceiver, opts)
%ExSDP.Media{mline | port: 0}

transceiver.stopping == true or transceiver.stopped == true ->
transceiver.stopping == true ->
opts = Keyword.put(opts, :direction, :inactive)
to_mline(transceiver, opts)

transceiver.stopped == true ->
opts = Keyword.put(opts, :direction, :inactive)
mline = to_mline(transceiver, opts)
%ExSDP.Media{mline | port: 0}
Expand Down
53 changes: 53 additions & 0 deletions test/ex_webrtc/peer_connection_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -651,6 +651,59 @@ defmodule ExWebRTC.PeerConnectionTest do
assert sdp.media == []
end

test "after remote offer" do
{:ok, pc1} = PeerConnection.start_link()
{:ok, pc2} = PeerConnection.start_link()

PeerConnection.add_transceiver(pc1, :video)
{:ok, offer} = PeerConnection.create_offer(pc1)
:ok = PeerConnection.set_local_description(pc1, offer)

:ok = PeerConnection.set_remote_description(pc2, offer)
[tr] = PeerConnection.get_transceivers(pc2)
:ok = PeerConnection.stop_transceiver(pc2, tr.id)

assert [
%RTPTransceiver{
current_direction: nil,
direction: :inactive,
stopping: true,
stopped: false
}
] = PeerConnection.get_transceivers(pc2)

{:ok, answer} = PeerConnection.create_answer(pc2)
[video] = ExSDP.parse!(answer.sdp).media
assert video.port == 9

:ok = PeerConnection.set_local_description(pc2, answer)
:ok = PeerConnection.set_remote_description(pc1, answer)

assert_receive {:ex_webrtc, ^pc2, :negotiation_needed}

# Ensure transceiver is removed after renegotiation

{:ok, offer2} = PeerConnection.create_offer(pc2)
:ok = PeerConnection.set_local_description(pc2, offer2)
[new_video] = ExSDP.parse!(offer2.sdp).media
assert new_video.port == 0

assert [
%RTPTransceiver{
current_direction: :inactive,
direction: :inactive,
stopping: true,
stopped: false
}
] = PeerConnection.get_transceivers(pc2)

:ok = PeerConnection.set_remote_description(pc1, offer2)
{:ok, answer2} = PeerConnection.create_answer(pc1)

:ok = PeerConnection.set_remote_description(pc2, answer2)
assert [] = PeerConnection.get_transceivers(pc2)
end

test "with renegotiation" do
{:ok, pc1} = PeerConnection.start_link()
{:ok, pc2} = PeerConnection.start_link()
Expand Down
Loading