Skip to content

Commit 7cd2666

Browse files
mickel8mickel8
committed
Reject SDP offer when there are no matching codecs
1 parent 579773b commit 7cd2666

File tree

4 files changed

+117
-47
lines changed

4 files changed

+117
-47
lines changed

‎lib/ex_webrtc/peer_connection.ex‎

Lines changed: 23 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -553,14 +553,10 @@ defmodule ExWebRTC.PeerConnection do
553553
:ok <- SDPUtils.ensure_rtcp_mux(sdp),
554554
{:ok, {ice_ufrag, ice_pwd}} <- SDPUtils.get_ice_credentials(sdp),
555555
{:ok, {:fingerprint, {:sha256, peer_fingerprint}}} <- SDPUtils.get_cert_fingerprint(sdp),
556-
{:ok, dtls_role} <- SDPUtils.get_dtls_role(sdp) do
557-
config = Configuration.update(state.config, sdp)
558-
state = %{state | config: config}
559-
560-
transceivers =
561-
state
562-
|> update_transceivers(sdp)
563-
|> update_transceiver_directions(sdp, :remote, type)
556+
{:ok, dtls_role} <- SDPUtils.get_dtls_role(sdp),
557+
state <- %{state | config: Configuration.update(state.config, sdp)},
558+
{:ok, transceivers} <- update_transceivers(state, sdp) do
559+
transceivers = update_transceiver_directions(transceivers, sdp, :remote, type)
564560

565561
# TODO: this can result in ICE restart (when it should, e.g. when this is answer)
566562
:ok = state.ice_transport.set_remote_credentials(state.ice_pid, ice_ufrag, ice_pwd)
@@ -683,21 +679,32 @@ defmodule ExWebRTC.PeerConnection do
683679
end
684680

685681
defp update_transceivers(state, sdp) do
686-
Enum.reduce(sdp.media, state.transceivers, fn mline, transceivers ->
682+
Enum.reduce_while(sdp.media, {:ok, state.transceivers}, fn mline, {:ok, transceivers} ->
687683
{:mid, mid} = ExSDP.get_attribute(mline, :mid)
688684

689-
# TODO: consider recycled transceivers
685+
# TODO: consider recycled transceivers
690686
case find_transceiver(transceivers, mid) do
691687
{idx, %RTPTransceiver{} = tr} ->
692-
new_tr = RTPTransceiver.update(tr, mline, state.config)
693-
notify_if_new_track(mline, new_tr, state.owner)
688+
case RTPTransceiver.update(tr, mline, state.config) do
689+
{:ok, new_tr} ->
690+
notify_if_new_track(mline, new_tr, state.owner)
691+
transceivers = List.replace_at(transceivers, idx, new_tr)
692+
{:cont, {:ok, transceivers}}
694693

695-
List.replace_at(transceivers, idx, new_tr)
694+
{:error, _reason} = error ->
695+
{:halt, error}
696+
end
696697

697698
nil ->
698-
new_tr = RTPTransceiver.from_mline(mline, state.config)
699-
notify_if_new_track(mline, new_tr, state.owner)
700-
transceivers ++ [new_tr]
699+
case RTPTransceiver.from_mline(mline, state.config) do
700+
{:ok, new_tr} ->
701+
notify_if_new_track(mline, new_tr, state.owner)
702+
transceivers = transceivers ++ [new_tr]
703+
{:cont, {:ok, transceivers}}
704+
705+
{:error, _reason} = error ->
706+
{:halt, error}
707+
end
701708
end
702709
end)
703710
end

‎lib/ex_webrtc/rtp_transceiver.ex‎

Lines changed: 43 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -71,47 +71,61 @@ defmodule ExWebRTC.RTPTransceiver do
7171
end
7272

7373
@doc false
74-
@spec from_mline(ExSDP.Media.t(), Configuration.t()) :: t()
74+
@spec from_mline(ExSDP.Media.t(), Configuration.t()) ::
75+
{:ok, t()} | {:error, :no_matching_codec}
7576
def from_mline(mline, config) do
76-
codecs = get_codecs(mline, config)
77-
rtp_hdr_exts = get_rtp_hdr_extensions(mline, config)
78-
{:mid, mid} = ExSDP.get_attribute(mline, :mid)
79-
80-
track = MediaStreamTrack.new(mline.type)
81-
82-
%__MODULE__{
83-
mid: mid,
84-
direction: :recvonly,
85-
current_direction: nil,
86-
kind: mline.type,
87-
codecs: codecs,
88-
rtp_hdr_exts: rtp_hdr_exts,
89-
receiver: %RTPReceiver{track: track},
90-
sender: RTPSender.new(nil, List.first(codecs), rtp_hdr_exts, mid, nil)
91-
}
77+
case get_codecs(mline, config) do
78+
[] ->
79+
{:error, :no_matching_codec}
80+
81+
codecs ->
82+
rtp_hdr_exts = get_rtp_hdr_extensions(mline, config)
83+
{:mid, mid} = ExSDP.get_attribute(mline, :mid)
84+
85+
track = MediaStreamTrack.new(mline.type)
86+
87+
{:ok,
88+
%__MODULE__{
89+
mid: mid,
90+
direction: :recvonly,
91+
current_direction: nil,
92+
kind: mline.type,
93+
codecs: codecs,
94+
rtp_hdr_exts: rtp_hdr_exts,
95+
receiver: %RTPReceiver{track: track},
96+
sender: RTPSender.new(nil, List.first(codecs), rtp_hdr_exts, mid, nil)
97+
}}
98+
end
9299
end
93100

94101
@doc false
95-
@spec update(t(), ExSDP.Media.t(), Configuration.t()) :: t()
102+
@spec update(t(), ExSDP.Media.t(), Configuration.t()) ::
103+
{:ok, t()} | {:error, :no_matching_codec}
96104
def update(transceiver, mline, config) do
97-
codecs = get_codecs(mline, config)
98-
rtp_hdr_exts = get_rtp_hdr_extensions(mline, config)
99-
sender = RTPSender.update(transceiver.sender, List.first(codecs), rtp_hdr_exts)
100-
101-
%__MODULE__{
102-
transceiver
103-
| codecs: codecs,
104-
rtp_hdr_exts: rtp_hdr_exts,
105-
sender: sender
106-
}
105+
case get_codecs(mline, config) do
106+
[] ->
107+
{:error, :no_matching_codec}
108+
109+
codecs ->
110+
rtp_hdr_exts = get_rtp_hdr_extensions(mline, config)
111+
sender = RTPSender.update(transceiver.sender, List.first(codecs), rtp_hdr_exts)
112+
113+
{:ok,
114+
%__MODULE__{
115+
transceiver
116+
| codecs: codecs,
117+
rtp_hdr_exts: rtp_hdr_exts,
118+
sender: sender
119+
}}
120+
end
107121
end
108122

109123
@doc false
110124
@spec to_answer_mline(t(), ExSDP.Media.t(), Keyword.t()) :: ExSDP.Media.t()
111125
def to_answer_mline(transceiver, mline, opts) do
112126
if transceiver.codecs == [] do
113127
# reject mline and skip further processing
114-
# see RFC 8299 sec. 5.3.1 and RFC 3264 sec. 6
128+
# see RFC 8829 sec. 5.3.1 and RFC 3264 sec. 6
115129
%ExSDP.Media{mline | port: 0}
116130
else
117131
offered_direction = ExSDP.get_attribute(mline, :direction)

‎test/ex_webrtc/peer_connection/configuration_test.exs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ defmodule ExWebRTC.PeerConnection.ConfigurationTest do
55

66
alias ExSDP.Attribute.{Extmap, FMTP}
77

8+
@tag :debug
89
test "codecs and rtp hdr extensions" do
910
audio_level_rtp_hdr_ext = %Extmap{
1011
id: 1,

‎test/ex_webrtc/peer_connection_test.exs‎

Lines changed: 50 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ defmodule ExWebRTC.PeerConnectionTest do
22
use ExUnit.Case, async: true
33

44
alias ExWebRTC.{
5+
RTPCodecParameters,
56
RTPTransceiver,
67
RTPSender,
78
MediaStreamTrack,
@@ -16,7 +17,7 @@ defmodule ExWebRTC.PeerConnectionTest do
1617
|> ExDTLS.get_cert_fingerprint()
1718
|> Utils.hex_dump()
1819

19-
@audio_mline ExSDP.Media.new("audio", 9, "UDP/TLS/RTP/SAVPF", [108])
20+
@audio_mline ExSDP.Media.new("audio", 9, "UDP/TLS/RTP/SAVPF", [111])
2021
|> ExSDP.add_attribute(:rtcp_mux)
2122
|> ExSDP.add_attributes(
2223
setup: :active,
@@ -25,6 +26,15 @@ defmodule ExWebRTC.PeerConnectionTest do
2526
ice_pwd: "somepwd",
2627
fingerprint: {:sha256, @fingerprint}
2728
)
29+
|> ExSDP.add_attributes([
30+
%ExSDP.Attribute.RTPMapping{
31+
clock_rate: 48_000,
32+
encoding: "opus",
33+
payload_type: 111,
34+
params: 2
35+
},
36+
%ExSDP.Attribute.FMTP{pt: 111, minptime: 10, useinbandfec: true}
37+
])
2838

2939
@video_mline ExSDP.Media.new("video", 9, "UDP/TLS/RTP/SAVPF", [96])
3040
|> ExSDP.add_attribute(:rtcp_mux)
@@ -35,6 +45,13 @@ defmodule ExWebRTC.PeerConnectionTest do
3545
ice_pwd: "somepwd",
3646
fingerprint: {:sha256, @fingerprint}
3747
)
48+
|> ExSDP.add_attributes([
49+
%ExSDP.Attribute.RTPMapping{
50+
clock_rate: 90_000,
51+
encoding: "VP8",
52+
payload_type: 96
53+
}
54+
])
3855

3956
test "negotiation needed" do
4057
# is fired on add_transceiver
@@ -535,7 +552,7 @@ defmodule ExWebRTC.PeerConnectionTest do
535552

536553
sdp = ExSDP.parse!(offer.sdp)
537554

538-
# munge Extmap and RTPMappingsso so that we use different ids and pts
555+
# munge Extmap and RTPMappings so that we use different ids and pts
539556
[mline] = sdp.media
540557

541558
extmaps =
@@ -599,4 +616,35 @@ defmodule ExWebRTC.PeerConnectionTest do
599616
assert false == Process.alive?(pc)
600617
Enum.each(links, fn link -> assert false == Process.alive?(link) end)
601618
end
619+
620+
@tag :debug
621+
test "no matching codec" do
622+
{:ok, pc} =
623+
PeerConnection.start_link(
624+
video_codecs: [
625+
%RTPCodecParameters{
626+
payload_type: 96,
627+
mime_type: "video/VP8",
628+
clock_rate: 90_000
629+
}
630+
]
631+
)
632+
633+
{:ok, pc2} =
634+
PeerConnection.start_link(
635+
video_codecs: [
636+
%RTPCodecParameters{
637+
payload_type: 45,
638+
mime_type: "video/AV1",
639+
clock_rate: 90_000
640+
}
641+
]
642+
)
643+
644+
{:ok, _tr} = PeerConnection.add_transceiver(pc, :video)
645+
{:ok, offer} = PeerConnection.create_offer(pc)
646+
:ok = PeerConnection.set_local_description(pc, offer)
647+
648+
assert {:error, :no_matching_codec} = PeerConnection.set_remote_description(pc2, offer)
649+
end
602650
end

0 commit comments

Comments
 (0)