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

Fix no supported codecs scenario #61

Merged
merged 2 commits into from
Jan 18, 2024
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
21 changes: 14 additions & 7 deletions lib/ex_webrtc/peer_connection.ex
Original file line number Diff line number Diff line change
Expand Up @@ -1005,6 +1005,13 @@ defmodule ExWebRTC.PeerConnection do
end

tr = %RTPTransceiver{tr | current_direction: direction, fired_direction: direction}

# This is not defined in the W3C but see https://github.com/w3c/webrtc-pc/issues/2927
tr =
if SDPUtils.rejected?(mline),
do: RTPTransceiver.stop(tr, on_track_ended(owner, tr.receiver.track.id)),
else: tr

transceivers = List.replace_at(transceivers, idx, tr)
process_mlines_local(mlines, transceivers, :answer, owner)
end
Expand All @@ -1021,7 +1028,7 @@ defmodule ExWebRTC.PeerConnection do
{:mid, mid} = ExSDP.get_attribute(mline, :mid)

direction =
if SDPUtils.is_rejected(mline),
if SDPUtils.rejected?(mline),
do: :inactive,
else: SDPUtils.get_media_direction(mline) |> reverse_direction()

Expand All @@ -1037,7 +1044,7 @@ defmodule ExWebRTC.PeerConnection do
tr = if sdp_type == :answer, do: %RTPTransceiver{tr | current_direction: direction}, else: tr

tr =
if SDPUtils.is_rejected(mline),
if SDPUtils.rejected?(mline),
do: RTPTransceiver.stop(tr, on_track_ended(owner, tr.receiver.track.id)),
else: tr

Expand Down Expand Up @@ -1153,7 +1160,7 @@ defmodule ExWebRTC.PeerConnection do
do: state

defp update_negotiation_needed(state) do
negotiation_needed = is_negotiation_needed(state.transceivers, state)
negotiation_needed = negotiation_needed?(state.transceivers, state)

cond do
negotiation_needed == true and state.negotiation_needed == true ->
Expand All @@ -1175,11 +1182,11 @@ defmodule ExWebRTC.PeerConnection do
# We don't support MSIDs and stopping transceivers so
# we only check 5.2 and 5.3 from 4.7.3#check-if-negotiation-is-needed
# https://www.w3.org/TR/webrtc/#dfn-check-if-negotiation-is-needed
defp is_negotiation_needed([], _), do: false
defp negotiation_needed?([], _), do: false

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

defp is_negotiation_needed([tr | transceivers], state) do
defp negotiation_needed?([tr | transceivers], state) do
{local_desc_type, local_desc} = state.current_local_desc
{_, remote_desc} = state.current_remote_desc

Expand All @@ -1204,7 +1211,7 @@ defmodule ExWebRTC.PeerConnection do
true

true ->
is_negotiation_needed(transceivers, state)
negotiation_needed?(transceivers, state)
end
end

Expand Down
14 changes: 7 additions & 7 deletions lib/ex_webrtc/peer_connection/configuration.ex
Original file line number Diff line number Diff line change
Expand Up @@ -157,20 +157,20 @@ defmodule ExWebRTC.PeerConnection.Configuration do
end

@doc false
@spec is_supported_codec(t(), RTPCodecParameters.t()) :: boolean()
def is_supported_codec(config, codec) do
@spec supported_codec?(t(), RTPCodecParameters.t()) :: boolean()
def supported_codec?(config, codec) do
# This function doesn't check if rtcp-fb is supported.
# Instead, `is_supported_rtcp_fb` has to be used to filter out
# Instead, `supported_rtcp_fb?` has to be used to filter out
# rtcp-fb that are not supported.
Enum.any?(config.audio_codecs ++ config.video_codecs, fn supported_codec ->
%{supported_codec | rtcp_fbs: codec.rtcp_fbs} == codec
end)
end

@doc false
@spec is_supported_rtp_hdr_extension(t(), Extmap.t(), :audio | :video) ::
@spec supported_rtp_hdr_extension?(t(), Extmap.t(), :audio | :video) ::
boolean()
def is_supported_rtp_hdr_extension(config, rtp_hdr_extension, media_type) do
def supported_rtp_hdr_extension?(config, rtp_hdr_extension, media_type) do
supported_uris =
case media_type do
:audio -> Map.keys(config.audio_rtp_hdr_exts)
Expand All @@ -181,8 +181,8 @@ defmodule ExWebRTC.PeerConnection.Configuration do
end

@doc false
@spec is_supported_rtcp_fb(t(), RTCPFeedback.t()) :: boolean()
def is_supported_rtcp_fb(_config, _rtcp_fb), do: false
@spec supported_rtcp_fb?(t(), RTCPFeedback.t()) :: boolean()
def supported_rtcp_fb?(_config, _rtcp_fb), do: false

@doc false
@spec update(t(), ExSDP.t()) :: t()
Expand Down
17 changes: 10 additions & 7 deletions lib/ex_webrtc/rtp_transceiver.ex
Original file line number Diff line number Diff line change
Expand Up @@ -122,27 +122,30 @@ defmodule ExWebRTC.RTPTransceiver do
@doc false
@spec to_answer_mline(t(), ExSDP.Media.t(), Keyword.t()) :: ExSDP.Media.t()
def to_answer_mline(transceiver, mline, opts) do
offered_direction = SDPUtils.get_media_direction(mline)
direction = get_direction(offered_direction, transceiver.direction)
opts = Keyword.put(opts, :direction, direction)

# Reject mline. See RFC 8829 sec. 5.3.1 and RFC 3264 sec. 6.
# We could reject earlier (as RFC suggests) but we generate
# answer mline at first to have consistent fingerprint, ice_ufrag and
# ice_pwd values across mlines.
# We also set direction to inactive, even though JSEP doesn't require it.
# See see https://github.com/w3c/webrtc-pc/issues/2927
cond do
transceiver.codecs == [] ->
# there has to be at least one format so take it from the offer
codecs = SDPUtils.get_rtp_codec_parameters(mline)
transceiver = %__MODULE__{transceiver | codecs: codecs}
opts = Keyword.put(opts, :direction, :inactive)
mline = to_mline(transceiver, opts)
%ExSDP.Media{mline | port: 0}

transceiver.stopping == true or transceiver.stopped == true ->
opts = Keyword.put(opts, :direction, :inactive)
mline = to_mline(transceiver, opts)
%ExSDP.Media{mline | port: 0}

true ->
offered_direction = SDPUtils.get_media_direction(mline)
direction = get_direction(offered_direction, transceiver.direction)
opts = Keyword.put(opts, :direction, direction)
Comment on lines +146 to +148
Copy link
Member

Choose a reason for hiding this comment

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

Was this previously unhandled? How did we handle the sRD(offer) -> tr.direction=inactive -> sLD(answer) scenario then?

Copy link
Member Author

Choose a reason for hiding this comment

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

The only change is that now, when we reject an mline we also set its direction as inactive

to_mline(transceiver, opts)
end
end
Expand Down Expand Up @@ -236,11 +239,11 @@ defmodule ExWebRTC.RTPTransceiver do
defp get_codecs(mline, config) do
mline
|> SDPUtils.get_rtp_codec_parameters()
|> Stream.filter(&Configuration.is_supported_codec(config, &1))
|> Stream.filter(&Configuration.supported_codec?(config, &1))
|> Enum.map(fn codec ->
rtcp_fbs =
Enum.filter(codec.rtcp_fbs, fn rtcp_fb ->
Configuration.is_supported_rtcp_fb(config, rtcp_fb)
Configuration.supported_rtcp_fb?(config, rtcp_fb)
end)

%RTPCodecParameters{codec | rtcp_fbs: rtcp_fbs}
Expand All @@ -250,6 +253,6 @@ defmodule ExWebRTC.RTPTransceiver do
defp get_rtp_hdr_extensions(mline, config) do
mline
|> ExSDP.get_attributes(ExSDP.Attribute.Extmap)
|> Enum.filter(&Configuration.is_supported_rtp_hdr_extension(config, &1, mline.type))
|> Enum.filter(&Configuration.supported_rtp_hdr_extension?(config, &1, mline.type))
end
end
6 changes: 3 additions & 3 deletions lib/ex_webrtc/sdp_utils.ex
Original file line number Diff line number Diff line change
Expand Up @@ -268,9 +268,9 @@ defmodule ExWebRTC.SDPUtils do
|> Enum.find_index(fn {mline, idx} -> mline.port == 0 and idx not in indices end)
end

@spec is_rejected(ExSDP.Media.t()) :: boolean()
def is_rejected(%ExSDP.Media{port: 0}), do: true
def is_rejected(%ExSDP.Media{}), do: false
@spec rejected?(ExSDP.Media.t()) :: boolean()
def rejected?(%ExSDP.Media{port: 0}), do: true
def rejected?(%ExSDP.Media{}), do: false

defp do_get_ice_credentials(sdp_or_mline) do
ice_ufrag =
Expand Down
2 changes: 1 addition & 1 deletion mix.exs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ defmodule ExWebRTC.MixProject do
defp deps do
[
{:ex_sdp, "~> 0.14.0"},
{:ex_ice, "~> 0.4.0"},
{:ex_ice, "~> 0.5.0"},
{:ex_dtls, "~> 0.15.0"},
{:ex_libsrtp, "~> 0.7.1"},
{:ex_rtp, "~> 0.3.0"},
Expand Down
Loading
Loading