Add test for adding and removing tracks in a loop (#59)

Author: mickel8

guides/mastering_transceivers.md

@@ -29,8 +29,8 @@ There're also a couple of other notes worth mentioning before moving forward.
 When adding a transceiver, it is created with `sendrecv` direction by default.
 When applying a remote offer that contains new m-lines, a new transceiver is created with the `recvonly` direction,
 even when the offerer only wants to receive media.
-This direction can later be changed with `addTrack`, which sends media data on the first available transceiver, 
-provided this transceiver wasn't initially created by `addTransceiver`.
+The `direction` can later be changed by `addTrack` or `removeTrack`. 
+Specifically, `addTrack` sends media on the first available transceiver, provided this transceiver wasn't initially created by `addTransceiver`.
 See [Stealing Transceiver](#stealing-transceiver).
 * `currentDirection` is a direction negotiated between the local and remote side, 
 and it changes when applying local or remote SDP.
@@ -438,8 +438,8 @@ the peer connection will attempt to find a transceiver it can use to associate w
 This is provided that the transceiver was created with `addTrack` and not with `addTransceiver`. 
 But why is this so? 
 The assumption is that when the user calls `addTrack` (and thereby creates a transceiver under the hood), 
-they might not pay attention to how this track is sent to the other side. 
-However, this is not the case when a user explicitly creates a transceiver with `addTransceiver`.
+they don't pay attention to how this track is sent to the other side. 
+However, this is not the case when the user explicitly creates a transceiver with `addTransceiver`.
 
 <!-- tabs-open -->
 

lib/ex_webrtc/peer_connection.ex

@@ -217,8 +217,6 @@ defmodule ExWebRTC.PeerConnection do
 
   @impl true
   def handle_call({:create_offer, options}, _from, state) do
-    # TODO: handle subsequent offers
-
     if Keyword.get(options, :ice_restart, false) do
       :ok = state.ice_transport.restart(state.ice_pid)
     end

test/ex_webrtc/renegotiation_test.exs

@@ -3,7 +3,7 @@ defmodule ExWebRTC.RenegotiationTest do
 
   import ExWebRTC.Support.TestUtils
 
-  alias ExWebRTC.{PeerConnection, RTPTransceiver}
+  alias ExWebRTC.{MediaStreamTrack, PeerConnection, RTPTransceiver}
 
   test "stop two and add one with different kind" do
     # 1. add audio and video transceiver
@@ -152,6 +152,80 @@ defmodule ExWebRTC.RenegotiationTest do
     assert tr3.id != pc2_tr1.id
   end
 
+  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. 
+    # pc1 adds audio and video tracks
+    # pc2 adds audio and video tracks
+    # pc1 adds screenshare track
+    # pc1 removes screenshare track
+    # pc2 adds screenshare track
+    # pc2 removes screenshare track
+
+    {:ok, pc1} = PeerConnection.start_link()
+    {:ok, pc2} = PeerConnection.start_link()
+
+    pc1_audio_track = MediaStreamTrack.new(:audio)
+    pc1_video_track = MediaStreamTrack.new(:video)
+
+    pc2_audio_track = MediaStreamTrack.new(:audio)
+    pc2_video_track = MediaStreamTrack.new(:video)
+
+    {:ok, _} = PeerConnection.add_track(pc1, pc1_audio_track)
+    {:ok, _} = PeerConnection.add_track(pc1, pc1_video_track)
+
+    :ok = negotiate(pc1, pc2)
+
+    {:ok, _} = PeerConnection.add_track(pc2, pc2_audio_track)
+    {:ok, _} = PeerConnection.add_track(pc2, pc2_video_track)
+
+    :ok = negotiate(pc2, pc1)
+
+    assert [%{kind: :audio}, %{kind: :video}] = PeerConnection.get_transceivers(pc1)
+
+    assert [%{kind: :audio}, %{kind: :video}] = PeerConnection.get_transceivers(pc2)
+
+    for _i <- 0..5 do
+      add_and_remove_screenshare(pc1, pc2)
+
+      assert [
+               %{kind: :audio, direction: :sendrecv, current_direction: :sendrecv},
+               %{kind: :video, direction: :sendrecv, current_direction: :sendrecv},
+               %{kind: :video, direction: :recvonly, current_direction: :inactive}
+             ] =
+               PeerConnection.get_transceivers(pc1)
+
+      assert [
+               %{kind: :audio, direction: :sendrecv, current_direction: :sendrecv},
+               %{kind: :video, direction: :sendrecv, current_direction: :sendrecv},
+               %{kind: :video, direction: :recvonly, current_direction: :inactive}
+             ] =
+               PeerConnection.get_transceivers(pc2)
+    end
+  end
+
+  defp add_and_remove_screenshare(pc1, pc2) do
+    pc1_screenshare_track = MediaStreamTrack.new(:video)
+    pc2_screenshare_track = MediaStreamTrack.new(:video)
+
+    {:ok, pc1_screenshare_sender} = PeerConnection.add_track(pc1, pc1_screenshare_track)
+
+    :ok = negotiate(pc1, pc2)
+
+    :ok = PeerConnection.remove_track(pc1, pc1_screenshare_sender.id)
+
+    :ok = negotiate(pc1, pc2)
+
+    {:ok, pc2_screenshare_sender} = PeerConnection.add_track(pc2, pc2_screenshare_track)
+
+    :ok = negotiate(pc2, pc1)
+
+    :ok = PeerConnection.remove_track(pc2, pc2_screenshare_sender.id)
+
+    :ok = negotiate(pc2, pc1)
+  end
+
   defp continue_negotiation(pc1, pc2, offer) do
     :ok = PeerConnection.set_local_description(pc1, offer)
     :ok = PeerConnection.set_remote_description(pc2, offer)