Ensure FMTP compatibility

Author: mickel8

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 ->
@@ -515,7 +526,7 @@ defmodule ExWebRTC.PeerConnection.Configuration do
   def intersect_codecs(config, mline) do
     # we assume that this function is called after
     # the config was updated based on the remote SDP
-    # so the payload types should match
+    # so the payload types (in codec_equal?) should match
     codecs =
       case mline.type do
         :audio -> config.audio_codecs
@@ -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,29 @@ defmodule ExWebRTC.PeerConnection.Configuration do
     end)
   end
 
+  @doc false
+  @spec codec_equal?(RTPCodecParameters.t(), RTPCodecParameters.t()) :: boolean()
+  def codec_equal?(c1, c2) do
+    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

lib/ex_webrtc/rtp_sender.ex

@@ -5,7 +5,7 @@ defmodule ExWebRTC.RTPSender do
   require Logger
 
   alias ExRTCP.Packet.{TransportFeedback.NACK, PayloadFeedback.PLI}
-  alias ExWebRTC.{MediaStreamTrack, RTPCodecParameters, Utils}
+  alias ExWebRTC.{MediaStreamTrack, RTPCodecParameters, Utils, PeerConnection.Configuration}
   alias ExSDP.Attribute.Extmap
   alias __MODULE__.{NACKResponder, ReportRecorder}
 
@@ -22,8 +22,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 +79,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 +89,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,30 +115,25 @@ 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)}
@@ -161,7 +145,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 +170,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 +195,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 +235,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 +245,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 +261,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 +285,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}

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