From bc4bdaf0b845132f4b568718b4e044a58ee4f2dc Mon Sep 17 00:00:00 2001 From: cnderrauber Date: Fri, 6 Sep 2024 22:25:01 +0800 Subject: [PATCH] Don't reuse transceiver in one round negotiation Should not reuse transceiver (remove & add track) in one round negotiation, it cause the transceiver changes ssrc/id without transit to inactive and the remote peer connection can't fire track close and OnTrack event. --- peerconnection.go | 22 +++++++++----- peerconnection_renegotiation_test.go | 44 +++++++++++++++++++++++++++- 2 files changed, 58 insertions(+), 8 deletions(-) diff --git a/peerconnection.go b/peerconnection.go index 316e8afdceb..aae87ef0082 100644 --- a/peerconnection.go +++ b/peerconnection.go @@ -1118,10 +1118,18 @@ func (pc *PeerConnection) SetRemoteDescription(desc SessionDescription) error { case direction == RTPTransceiverDirectionRecvonly: if t.Direction() == RTPTransceiverDirectionSendrecv { t.setDirection(RTPTransceiverDirectionSendonly) + } else if t.Direction() == RTPTransceiverDirectionRecvonly { + t.setDirection(RTPTransceiverDirectionInactive) } case direction == RTPTransceiverDirectionSendrecv: if t.Direction() == RTPTransceiverDirectionSendonly { t.setDirection(RTPTransceiverDirectionSendrecv) + } else if t.Direction() == RTPTransceiverDirectionInactive { + t.setDirection(RTPTransceiverDirectionRecvonly) + } + case direction == RTPTransceiverDirectionSendonly: + if t.Direction() == RTPTransceiverDirectionInactive { + t.setDirection(RTPTransceiverDirectionRecvonly) } } @@ -1288,7 +1296,7 @@ func setRTPTransceiverCurrentDirection(answer *SessionDescription, currentTransc // If a transceiver is created by applying a remote description that has recvonly transceiver, // it will have no sender. In this case, the transceiver's current direction is set to inactive so // that the transceiver can be reused by next AddTrack. - if direction == RTPTransceiverDirectionSendonly && t.Sender() == nil { + if !weOffer && direction == RTPTransceiverDirectionSendonly && t.Sender() == nil { direction = RTPTransceiverDirectionInactive } @@ -1340,8 +1348,8 @@ func (pc *PeerConnection) configureRTPReceivers(isRenegotiation bool, remoteDesc mid := t.Mid() receiverNeedsStopped := false - func() { - for _, t := range tracks { + for _, track := range tracks { + func(t *TrackRemote) { t.mu.Lock() defer t.mu.Unlock() @@ -1349,19 +1357,19 @@ func (pc *PeerConnection) configureRTPReceivers(isRenegotiation bool, remoteDesc if details := trackDetailsForRID(incomingTracks, mid, t.rid); details != nil { t.id = details.id t.streamID = details.streamID - continue + return } } else if t.ssrc != 0 { if details := trackDetailsForSSRC(incomingTracks, t.ssrc); details != nil { t.id = details.id t.streamID = details.streamID - continue + return } } receiverNeedsStopped = true - } - }() + }(track) + } if !receiverNeedsStopped { continue diff --git a/peerconnection_renegotiation_test.go b/peerconnection_renegotiation_test.go index 6f4465475f2..8d2e5b7bdfc 100644 --- a/peerconnection_renegotiation_test.go +++ b/peerconnection_renegotiation_test.go @@ -1160,12 +1160,15 @@ func TestPeerConnection_Regegotiation_ReuseTransceiver(t *testing.T) { t.Fatal(err) } - vp8Track, err := NewTrackLocalStaticSample(RTPCodecCapability{MimeType: MimeTypeVP8}, "foo", "bar") + vp8Track, err := NewTrackLocalStaticRTP(RTPCodecCapability{MimeType: MimeTypeVP8}, "foo", "bar") assert.NoError(t, err) sender, err := pcOffer.AddTrack(vp8Track) assert.NoError(t, err) assert.NoError(t, signalPair(pcOffer, pcAnswer)) + peerConnectionConnected := untilConnectionState(PeerConnectionStateConnected, pcOffer, pcAnswer) + peerConnectionConnected.Wait() + assert.Equal(t, len(pcOffer.GetTransceivers()), 1) assert.Equal(t, pcOffer.GetTransceivers()[0].getCurrentDirection(), RTPTransceiverDirectionSendonly) assert.NoError(t, pcOffer.RemoveTrack(sender)) @@ -1185,6 +1188,45 @@ func TestPeerConnection_Regegotiation_ReuseTransceiver(t *testing.T) { assert.NoError(t, err) assert.Equal(t, len(pcOffer.GetTransceivers()), 2) assert.True(t, sender.rtpTransceiver == pcOffer.GetTransceivers()[0]) + assert.NoError(t, signalPair(pcOffer, pcAnswer)) + + tracksCh := make(chan *TrackRemote, 2) + pcAnswer.OnTrack(func(tr *TrackRemote, _ *RTPReceiver) { + tracksCh <- tr + }) + + ssrcReuse := sender.GetParameters().Encodings[0].SSRC + for i := 0; i < 10; i++ { + assert.NoError(t, vp8Track.WriteRTP(&rtp.Packet{Header: rtp.Header{Version: 2}, Payload: []byte{0, 1, 2, 3, 4, 5}})) + time.Sleep(20 * time.Millisecond) + } + + // shold not reuse tranceiver between two CreateOffer + offer, err := pcOffer.CreateOffer(nil) + assert.NoError(t, err) + assert.NoError(t, pcOffer.RemoveTrack(sender)) + assert.NoError(t, pcOffer.SetLocalDescription(offer)) + assert.NoError(t, pcAnswer.SetRemoteDescription(offer)) + answer, err := pcAnswer.CreateAnswer(nil) + assert.NoError(t, pcAnswer.SetLocalDescription(answer)) + assert.NoError(t, err) + assert.NoError(t, pcOffer.SetRemoteDescription(answer)) + sender3, err := pcOffer.AddTrack(vp8Track) + ssrcNotReuse := sender3.GetParameters().Encodings[0].SSRC + assert.NoError(t, err) + assert.Equal(t, len(pcOffer.GetTransceivers()), 3) + assert.NoError(t, signalPair(pcOffer, pcAnswer)) + assert.True(t, sender3.rtpTransceiver == pcOffer.GetTransceivers()[2]) + + for i := 0; i < 10; i++ { + assert.NoError(t, vp8Track.WriteRTP(&rtp.Packet{Header: rtp.Header{Version: 2}, Payload: []byte{0, 1, 2, 3, 4, 5}})) + time.Sleep(20 * time.Millisecond) + } + + tr1 := <-tracksCh + tr2 := <-tracksCh + assert.Equal(t, tr1.SSRC(), ssrcReuse) + assert.Equal(t, tr2.SSRC(), ssrcNotReuse) closePairNow(t, pcOffer, pcAnswer) }