Improve the README and test regarding OnNegotiationNeeded event.

Author: tyohan

README.md

@@ -69,9 +69,14 @@ When a client wants to publish a track to the SFU, usually the flow is like this
 See the [client_test.go](./client_test.go) for more details on how to publish and subscribe to tracks.
 
 
-### Renegotiation
-Usually, renegotiation will be needed when a new track is added or removed. The renegotiation can be initiated by the SFU if a new client joins the room and publishes a new track to the room. To broadcast the track to all clients, the SFU must renegotiate with each client to inform them about the new tracks. 
-
+### Client Renegotiation
+Usually, renegotiation will be needed when a new track is added or removed. The renegotiation can be initiated by the SFU if a new client joins the room and publishes a new track to the room. To broadcast the track to all clients, the SFU must renegotiate with each client to inform them about the new tracks. The renegotiation is can be handled in peerConnection.OnNegotiationNeeded event. This event will be triggered when the client needs to renegotiate with the SFU.
+
+> [!IMPORTANT]
+> When adding a track and using the `OnNegotiationNeeded` event, issues may arise if you previously added a single `recvOnly` transceiver and then add more than one `sendonly` or `sendrecv` transceiver. The `OnNegotiationNeeded` event may be triggered multiple times. To prevent this, follow these rules:
+> - If you add a `recvonly` transceiver and want to add more tracks, use `PeerConnection.AddTrack()` so the send direction of the existing transceiver will be used.
+> - Always balance the number of transceivers by adding the same number of additional tracks as the `recvonly` transceivers previously added. For example, if you added 2 `recvonly` transceivers, you can add 2 tracks with `PeerConnection.AddTrack()`. Adding more than 2 tracks will cause `OnNegotiationNeeded` to be called multiple times. It is common to add `recvonly` transceivers first for audio and video, then add more tracks later when the client wants to share the screen or other media.
+  
 #### Renegotiation from the SFU
 To wait for the renegotiation process from SFU to make sure you receive a new track once published in a room, you can do this:
 1. When creating a new client after calling `currentRoom.addClient()` method, you can listen to the `client.OnRenegotiation` event. The event will be triggered when the SFU is trying to renegotiate with the client.

negotiation_test.go

@@ -71,7 +71,7 @@ connected:
 	assert.Equal(t, ClientStateActive, client.state.Load())
 }
 
-func TestOnNegotiationNeededCalledOnce(t *testing.T) {
+func TestOnNegotiationNeededCalledTwice(t *testing.T) {
 	ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
 	defer cancel()
 
@@ -158,7 +158,7 @@ func TestOnNegotiationNeededCalledOnce(t *testing.T) {
 
 	client.OnAllowedRemoteRenegotiation(func() {
 		TestLogger.Infof("allowed remote renegotiation")
-		negotiate(pc, client, TestLogger, true)
+
 	})
 
 	// Override OnNegotiationNeeded to count calls and still perform negotiation
@@ -185,17 +185,58 @@ func TestOnNegotiationNeededCalledOnce(t *testing.T) {
 		}
 	})
 
-	// Add tracks to trigger negotiation
+	// Add track to trigger negotiation
+	pc.AddTransceiverFromKind(webrtc.RTPCodecTypeVideo, webrtc.RTPTransceiverInit{
+		Direction: webrtc.RTPTransceiverDirectionRecvonly,
+	})
+	pc.AddTransceiverFromKind(webrtc.RTPCodecTypeAudio, webrtc.RTPTransceiverInit{
+		Direction: webrtc.RTPTransceiverDirectionRecvonly,
+	})
+
+	// Wait until the first negotiation is done
+	timeout := time.After(15 * time.Second)
+	for {
+		select {
+		case <-time.After(200 * time.Millisecond):
+			if negotiationCount > 0 {
+				t.Logf("Negotiation needed called %d times", negotiationCount)
+				goto negotiationDone
+			}
+		case <-timeout:
+			t.Fatal("Timeout waiting for negotiation needed to be called")
+		case <-ctx.Done():
+			t.Fatal("Test context cancelled")
+		}
+	}
+
+negotiationDone:
+
+	// Add more tracks to re-trigger negotiation
 	tracks, _ := GetStaticTracks(clientContext, clientContext, "test-peer", false)
 	SetPeerConnectionTracks(clientContext, pc, tracks)
 
-	// Wait a bit for any potential additional negotiation calls
-	time.Sleep(3 * time.Second)
+	// Wait for the second negotiation to be done
+	timeout = time.After(15 * time.Second)
+	for {
+		select {
+		case <-time.After(200 * time.Millisecond):
+			if negotiationCount > 1 {
+				t.Logf("Negotiation needed called %d times", negotiationCount)
+				goto finalCheck
+			}
+		case <-timeout:
+			t.Fatal("Timeout waiting for second negotiation needed to be called")
+		case <-ctx.Done():
+			t.Fatal("Test context cancelled")
+		}
+	}
+
+finalCheck:
 
 	// Verify OnNegotiationNeeded was called exactly once
 	mu.Lock()
 	finalCount := negotiationCount
 	mu.Unlock()
 
-	assert.Equal(t, 1, finalCount, "OnNegotiationNeeded should be called exactly once, but was called %d times", finalCount)
+	assert.Equal(t, 2, finalCount, "OnNegotiationNeeded should be called exactly twice, but was called %d times", finalCount)
 }

testhelper.go

@@ -265,9 +265,7 @@ func GetStaticAudioTrack(ctx, iceConnectedCtx context.Context, trackID, streamID
 
 func SetPeerConnectionTracks(ctx context.Context, peerConnection *webrtc.PeerConnection, tracks []*webrtc.TrackLocalStaticSample) {
 	for _, track := range tracks {
-		rtpTranscv, trackErr := peerConnection.AddTransceiverFromTrack(track, webrtc.RTPTransceiverInit{
-			Direction: webrtc.RTPTransceiverDirectionSendonly,
-		})
+		sender, trackErr := peerConnection.AddTrack(track)
 		if trackErr != nil {
 			panic(trackErr)
 		}
@@ -280,8 +278,8 @@ func SetPeerConnectionTracks(ctx context.Context, peerConnection *webrtc.PeerCon
 			ctxx, cancel := context.WithCancel(ctx)
 
 			defer func() {
-				if rtpTranscv != nil {
-					_ = rtpTranscv.Stop()
+				if sender != nil {
+					_ = sender.Stop()
 				}
 			}()
 
@@ -292,7 +290,7 @@ func SetPeerConnectionTracks(ctx context.Context, peerConnection *webrtc.PeerCon
 				case <-ctxx.Done():
 					return
 				default:
-					if _, _, rtcpErr := rtpTranscv.Sender().Read(rtcpBuf); rtcpErr != nil {
+					if _, _, rtcpErr := sender.Read(rtcpBuf); rtcpErr != nil {
 						return
 					}