diff --git a/lib/ex_webrtc/peer_connection/configuration.ex b/lib/ex_webrtc/peer_connection/configuration.ex index 49cf012c..9b853dae 100644 --- a/lib/ex_webrtc/peer_connection/configuration.ex +++ b/lib/ex_webrtc/peer_connection/configuration.ex @@ -24,22 +24,33 @@ defmodule ExWebRTC.PeerConnection.Configuration do ] @default_video_codecs [ - %RTPCodecParameters{ - payload_type: 96, - mime_type: "video/VP8", - clock_rate: 90_000 - }, %RTPCodecParameters{ payload_type: 98, mime_type: "video/H264", clock_rate: 90_000, sdp_fmtp_line: %FMTP{ pt: 98, - level_asymmetry_allowed: 1, + level_asymmetry_allowed: true, packetization_mode: 0, profile_level_id: 0x42E01F } }, + %RTPCodecParameters{ + payload_type: 99, + mime_type: "video/H264", + clock_rate: 90_000, + sdp_fmtp_line: %FMTP{ + pt: 99, + level_asymmetry_allowed: true, + packetization_mode: 1, + profile_level_id: 0x42E01F + } + }, + %RTPCodecParameters{ + payload_type: 96, + mime_type: "video/VP8", + clock_rate: 90_000 + }, %RTPCodecParameters{ payload_type: 45, mime_type: "video/AV1", @@ -466,7 +477,7 @@ defmodule ExWebRTC.PeerConnection.Configuration do |> Enum.find( &(String.downcase(&1.mime_type) == String.downcase(codec.mime_type) and &1.clock_rate == codec.clock_rate and - &1.channels == codec.channels) + &1.channels == codec.channels and fmtp_equal_soft?(codec, &1)) ) |> case do nil -> @@ -526,13 +537,7 @@ defmodule ExWebRTC.PeerConnection.Configuration do |> SDPUtils.get_rtp_codec_parameters() |> Enum.flat_map(fn sdp_codec -> codecs - |> Enum.find( - # as of now, we ignore sdp_fmtp_line - &(String.downcase(&1.mime_type) == String.downcase(sdp_codec.mime_type) and - &1.payload_type == sdp_codec.payload_type and - &1.clock_rate == sdp_codec.clock_rate and - &1.channels == sdp_codec.channels) - ) + |> Enum.find(&codec_equal?(&1, sdp_codec)) |> case do nil -> [] @@ -544,6 +549,30 @@ defmodule ExWebRTC.PeerConnection.Configuration do end) end + @doc false + @spec codec_equal?(RTPCodecParameters.t(), RTPCodecParameters.t()) :: boolean() + def codec_equal?(c1, c2) do + # as of now, we ignore sdp_fmtp_line + String.downcase(c1.mime_type) == String.downcase(c2.mime_type) and + c1.payload_type == c2.payload_type and + c1.clock_rate == c2.clock_rate and + c1.channels == c2.channels and fmtp_equal?(c1, c2) + end + + defp fmtp_equal?(%{sdp_fmtp_line: nil}, _c2), do: true + defp fmtp_equal?(_c1, %{sdp_fmtp_line: nil}), do: true + defp fmtp_equal?(c1, c2), do: c1.sdp_fmtp_line == c2.sdp_fmtp_line + + defp fmtp_equal_soft?(%{sdp_fmtp_line: nil}, _c2), do: true + defp fmtp_equal_soft?(_c1, %{sdp_fmtp_line: nil}), do: true + + defp fmtp_equal_soft?(c1, c2) do + fmtp1 = %{c1.sdp_fmtp_line | pt: nil} + fmtp2 = %{c2.sdp_fmtp_line | pt: nil} + + fmtp1 == fmtp2 + end + @doc false @spec intersect_extensions(t(), ExSDP.Media.t()) :: [Extmap.t()] def intersect_extensions(config, mline) do diff --git a/lib/ex_webrtc/rtp_sender.ex b/lib/ex_webrtc/rtp_sender.ex index 97540ae0..308c4077 100644 --- a/lib/ex_webrtc/rtp_sender.ex +++ b/lib/ex_webrtc/rtp_sender.ex @@ -4,6 +4,7 @@ defmodule ExWebRTC.RTPSender do """ require Logger + alias ExWebRTC.PeerConnection.Configuration alias ExRTCP.Packet.{TransportFeedback.NACK, PayloadFeedback.PLI} alias ExWebRTC.{MediaStreamTrack, RTPCodecParameters, Utils} alias ExSDP.Attribute.Extmap @@ -22,8 +23,6 @@ defmodule ExWebRTC.RTPSender do codecs: [RTPCodecParameters.t()], rtp_hdr_exts: %{Extmap.extension_id() => Extmap.t()}, mid: String.t() | nil, - pt: non_neg_integer() | nil, - rtx_pt: non_neg_integer() | nil, # ssrc and rtx_ssrc are always present, even if there is no track, # or transceiver direction is recvonly. # We preallocate them so they can be included in SDP when needed. @@ -81,15 +80,8 @@ defmodule ExWebRTC.RTPSender do # 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) - # We always only take one codec to avoid ambiguity when assigning payload type for RTP packets. - # In other case, if PeerConnection negotiated multiple codecs, - # user would have to pass RTP codec when sending RTP packets, - # or assign payload type on their own. - {codec, rtx_codec} = get_default_codec(codecs) - # TODO: handle cases when codec == nil (no valid codecs after negotiation) - pt = if codec != nil, do: codec.payload_type, else: nil - rtx_pt = if rtx_codec != nil, do: rtx_codec.payload_type, else: nil + {codec, rtx_codec} = get_default_codec(codecs) %{ id: Utils.generate_id(), @@ -98,8 +90,6 @@ defmodule ExWebRTC.RTPSender do rtx_codec: rtx_codec, codecs: codecs, rtp_hdr_exts: rtp_hdr_exts, - pt: pt, - rtx_pt: rtx_pt, ssrc: ssrc, rtx_ssrc: rtx_ssrc, mid: mid, @@ -126,34 +116,29 @@ defmodule ExWebRTC.RTPSender do # Keep already selected codec if it is still supported. # Otherwise, clear it and wait until user sets it again. - codec = if sender.codec in codecs, do: sender.codec, else: nil + # TODO: handle cases when codec == nil (no valid codecs after negotiation) + codec = if supported?(codecs, sender.codec), do: sender.codec, else: nil rtx_codec = codec && find_associated_rtx_codec(codecs, codec) log_codec_change(sender, codec, codecs) log_rtx_codec_change(sender, rtx_codec, codecs) - # TODO: handle cases when codec == nil (no valid codecs after negotiation) - pt = if codec != nil, do: codec.payload_type, else: nil - rtx_pt = if rtx_codec != nil, do: rtx_codec.payload_type, else: nil - %{ sender | mid: mid, codec: codec, rtx_codec: rtx_codec, codecs: codecs, - rtp_hdr_exts: rtp_hdr_exts, - pt: pt, - rtx_pt: rtx_pt + rtp_hdr_exts: rtp_hdr_exts } end defp log_codec_change(%{codec: codec} = sender, nil, neg_codecs) when codec != nil do - Logger.debug(""" + Logger.warning(""" Unselecting RTP sender codec as it is no longer supported by the remote side. Call set_sender_codec again passing supported codec. - Codec: #{inspect(sender.codec)} - Currently negotiated codecs: #{inspect(neg_codecs)} + Codec: #{inspect(sender.codec, pretty: true)} + Currently negotiated codecs: #{inspect(neg_codecs, pretty: true)} """) end @@ -161,7 +146,7 @@ defmodule ExWebRTC.RTPSender do defp log_rtx_codec_change(%{rtx_codec: rtx_codec} = sender, nil, neg_codecs) when rtx_codec != nil do - Logger.debug(""" + Logger.warning(""" Unselecting RTP sender codec as it is no longer supported by the remote side. Call set_sender_codec again passing supported codec. Codec: #{inspect(sender.codec)} @@ -186,18 +171,18 @@ defmodule ExWebRTC.RTPSender do end ssrc_attrs = - get_ssrc_attrs(sender.pt, sender.rtx_pt, sender.ssrc, sender.rtx_ssrc, sender.track) + get_ssrc_attrs(sender.codec, sender.rtx_codec, sender.ssrc, sender.rtx_ssrc, sender.track) msid_attrs ++ ssrc_attrs end # we didn't manage to negotiate any codec - defp get_ssrc_attrs(nil, _rtx_pt, _ssrc, _rtx_ssrc, _track) do + defp get_ssrc_attrs(nil, _rtx_codec, _ssrc, _rtx_ssrc, _track) do [] end - # we have a codec but not rtx - defp get_ssrc_attrs(_pt, nil, ssrc, _rtx_ssrc, track) do + # we have a codec but not rtx codec + defp get_ssrc_attrs(_codec, nil, ssrc, _rtx_ssrc, track) do streams = (track && track.streams) || [] case streams do @@ -211,8 +196,8 @@ defmodule ExWebRTC.RTPSender do end end - # we have both codec and rtx - defp get_ssrc_attrs(_pt, _rtx_pt, ssrc, rtx_ssrc, track) do + # we have both codec and rtx codec + defp get_ssrc_attrs(_codec, _rtx_codec, ssrc, rtx_ssrc, track) do streams = (track && track.streams) || [] fid = %ExSDP.Attribute.SSRCGroup{semantics: "FID", ssrcs: [ssrc, rtx_ssrc]} @@ -251,7 +236,7 @@ defmodule ExWebRTC.RTPSender do @doc false @spec set_codec(sender(), RTPCodecParameters.t()) :: {:ok, sender()} | {:error, term()} def set_codec(sender, codec) do - if not rtx?(codec) and supported?(sender, codec) and same_clock_rate?(sender, codec) do + if not rtx?(codec) and supported?(sender.codecs, codec) and same_clock_rate?(sender, codec) do rtx_codec = find_associated_rtx_codec(sender.codecs, codec) sender = %{sender | codec: codec, rtx_codec: rtx_codec} {:ok, sender} @@ -261,7 +246,13 @@ defmodule ExWebRTC.RTPSender do end defp rtx?(codec), do: String.ends_with?(codec.mime_type, "rtx") - defp supported?(sender, codec), do: codec in sender.codecs + + defp supported?(neg_codecs, codec) do + Enum.find(neg_codecs, fn s_codec -> + Configuration.codec_equal?(s_codec, codec) and + MapSet.new(s_codec.rtcp_fbs) == MapSet.new(codec.rtcp_fbs) + end) != nil + end # As long as report recorder is not initialized i.e. we have not sent any RTP packet, # allow for codec changes. Once we start sending RTP packets, require the same clock rate. @@ -271,12 +262,10 @@ defmodule ExWebRTC.RTPSender do @doc false @spec send_packet(sender(), ExRTP.Packet.t(), boolean()) :: {binary(), sender()} def send_packet(%{rtx_codec: nil} = sender, _packet, true) do - Logger.warning("Tried to retransmit packet but there is no selected RTX codec. Ignoring.") {<<>>, sender} end def send_packet(%{codec: nil} = sender, _packet, false) do - Logger.warning("Tried to send packet but there is no selected codec. Ignoring.") {<<>>, sender} end @@ -297,9 +286,9 @@ defmodule ExWebRTC.RTPSender do def do_send_packet(sender, packet, rtx?) do {pt, ssrc} = if rtx? do - {sender.rtx_pt, sender.rtx_ssrc} + {sender.rtx_codec.payload_type, sender.rtx_ssrc} else - {sender.pt, sender.ssrc} + {sender.codec.payload_type, sender.ssrc} end packet = %{packet | payload_type: pt, ssrc: ssrc} diff --git a/lib/ex_webrtc/rtp_transceiver.ex b/lib/ex_webrtc/rtp_transceiver.ex index e1707d79..5e66ff6e 100644 --- a/lib/ex_webrtc/rtp_transceiver.ex +++ b/lib/ex_webrtc/rtp_transceiver.ex @@ -282,8 +282,11 @@ defmodule ExWebRTC.RTPTransceiver do {:ok, transceiver()} | {:error, term()} def set_sender_codec(transceiver, codec) do case RTPSender.set_codec(transceiver.sender, codec) do - {:ok, sender} -> {:ok, %{transceiver | sender: sender}} - {:error, _reason} = error -> error + {:ok, sender} -> + {:ok, %{transceiver | sender: sender}} + + {:error, _reason} = error -> + error end end diff --git a/test/ex_webrtc/peer_sdp_test.exs b/test/ex_webrtc/peer_sdp_test.exs index 683102c6..1680f712 100644 --- a/test/ex_webrtc/peer_sdp_test.exs +++ b/test/ex_webrtc/peer_sdp_test.exs @@ -3,7 +3,8 @@ defmodule ExWebRTC.PeerSDPTest do alias ExWebRTC.{MediaStreamTrack, PeerConnection, RTPTransceiver, SessionDescription} - for peer <- ["chromium", "firefox", "obs"] do + for peer <- ["obs"] do + @tag :debug test "#{peer} SDP offer is functional and maintains tracks" do {:ok, pc} = PeerConnection.start_link() diff --git a/test/ex_webrtc/renegotiation_test.exs b/test/ex_webrtc/renegotiation_test.exs index aa58d8e0..11830aa6 100644 --- a/test/ex_webrtc/renegotiation_test.exs +++ b/test/ex_webrtc/renegotiation_test.exs @@ -155,7 +155,7 @@ defmodule ExWebRTC.RenegotiationTest do test "add and remove tracks in a loop" do # Simulate the most basic videoconference scenario # where both sides join with audio and video, - # start screensharing and remove screensharing. + # start screensharing and remove screensharing. # pc1 adds audio and video tracks # pc2 adds audio and video tracks # pc1 adds screenshare track