Skip to content

Commit f99d4ed

Browse files
committed
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.
1 parent 540af5b commit f99d4ed

File tree

2 files changed

+58
-8
lines changed

2 files changed

+58
-8
lines changed

peerconnection.go

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1118,10 +1118,18 @@ func (pc *PeerConnection) SetRemoteDescription(desc SessionDescription) error {
11181118
case direction == RTPTransceiverDirectionRecvonly:
11191119
if t.Direction() == RTPTransceiverDirectionSendrecv {
11201120
t.setDirection(RTPTransceiverDirectionSendonly)
1121+
} else if t.Direction() == RTPTransceiverDirectionRecvonly {
1122+
t.setDirection(RTPTransceiverDirectionInactive)
11211123
}
11221124
case direction == RTPTransceiverDirectionSendrecv:
11231125
if t.Direction() == RTPTransceiverDirectionSendonly {
11241126
t.setDirection(RTPTransceiverDirectionSendrecv)
1127+
} else if t.Direction() == RTPTransceiverDirectionInactive {
1128+
t.setDirection(RTPTransceiverDirectionRecvonly)
1129+
}
1130+
case direction == RTPTransceiverDirectionSendonly:
1131+
if t.Direction() == RTPTransceiverDirectionInactive {
1132+
t.setDirection(RTPTransceiverDirectionRecvonly)
11251133
}
11261134
}
11271135

@@ -1288,7 +1296,7 @@ func setRTPTransceiverCurrentDirection(answer *SessionDescription, currentTransc
12881296
// If a transceiver is created by applying a remote description that has recvonly transceiver,
12891297
// it will have no sender. In this case, the transceiver's current direction is set to inactive so
12901298
// that the transceiver can be reused by next AddTrack.
1291-
if direction == RTPTransceiverDirectionSendonly && t.Sender() == nil {
1299+
if !weOffer && direction == RTPTransceiverDirectionSendonly && t.Sender() == nil {
12921300
direction = RTPTransceiverDirectionInactive
12931301
}
12941302

@@ -1340,28 +1348,28 @@ func (pc *PeerConnection) configureRTPReceivers(isRenegotiation bool, remoteDesc
13401348

13411349
mid := t.Mid()
13421350
receiverNeedsStopped := false
1343-
func() {
1344-
for _, t := range tracks {
1351+
for _, track := range tracks {
1352+
func(t *TrackRemote) {
13451353
t.mu.Lock()
13461354
defer t.mu.Unlock()
13471355

13481356
if t.rid != "" {
13491357
if details := trackDetailsForRID(incomingTracks, mid, t.rid); details != nil {
13501358
t.id = details.id
13511359
t.streamID = details.streamID
1352-
continue
1360+
return
13531361
}
13541362
} else if t.ssrc != 0 {
13551363
if details := trackDetailsForSSRC(incomingTracks, t.ssrc); details != nil {
13561364
t.id = details.id
13571365
t.streamID = details.streamID
1358-
continue
1366+
return
13591367
}
13601368
}
13611369

13621370
receiverNeedsStopped = true
1363-
}
1364-
}()
1371+
}(track)
1372+
}
13651373

13661374
if !receiverNeedsStopped {
13671375
continue

peerconnection_renegotiation_test.go

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1167,12 +1167,15 @@ func TestPeerConnection_Regegotiation_ReuseTransceiver(t *testing.T) {
11671167
t.Fatal(err)
11681168
}
11691169

1170-
vp8Track, err := NewTrackLocalStaticSample(RTPCodecCapability{MimeType: MimeTypeVP8}, "foo", "bar")
1170+
vp8Track, err := NewTrackLocalStaticRTP(RTPCodecCapability{MimeType: MimeTypeVP8}, "foo", "bar")
11711171
assert.NoError(t, err)
11721172
sender, err := pcOffer.AddTrack(vp8Track)
11731173
assert.NoError(t, err)
11741174
assert.NoError(t, signalPair(pcOffer, pcAnswer))
11751175

1176+
peerConnectionConnected := untilConnectionState(PeerConnectionStateConnected, pcOffer, pcAnswer)
1177+
peerConnectionConnected.Wait()
1178+
11761179
assert.Equal(t, len(pcOffer.GetTransceivers()), 1)
11771180
assert.Equal(t, pcOffer.GetTransceivers()[0].getCurrentDirection(), RTPTransceiverDirectionSendonly)
11781181
assert.NoError(t, pcOffer.RemoveTrack(sender))
@@ -1192,6 +1195,45 @@ func TestPeerConnection_Regegotiation_ReuseTransceiver(t *testing.T) {
11921195
assert.NoError(t, err)
11931196
assert.Equal(t, len(pcOffer.GetTransceivers()), 2)
11941197
assert.True(t, sender.rtpTransceiver == pcOffer.GetTransceivers()[0])
1198+
assert.NoError(t, signalPair(pcOffer, pcAnswer))
1199+
1200+
tracksCh := make(chan *TrackRemote, 2)
1201+
pcAnswer.OnTrack(func(tr *TrackRemote, _ *RTPReceiver) {
1202+
tracksCh <- tr
1203+
})
1204+
1205+
ssrcReuse := sender.GetParameters().Encodings[0].SSRC
1206+
for i := 0; i < 10; i++ {
1207+
assert.NoError(t, vp8Track.WriteRTP(&rtp.Packet{Header: rtp.Header{Version: 2}, Payload: []byte{0, 1, 2, 3, 4, 5}}))
1208+
time.Sleep(20 * time.Millisecond)
1209+
}
1210+
1211+
// shold not reuse tranceiver between two CreateOffer
1212+
offer, err := pcOffer.CreateOffer(nil)
1213+
assert.NoError(t, err)
1214+
assert.NoError(t, pcOffer.RemoveTrack(sender))
1215+
assert.NoError(t, pcOffer.SetLocalDescription(offer))
1216+
assert.NoError(t, pcAnswer.SetRemoteDescription(offer))
1217+
answer, err := pcAnswer.CreateAnswer(nil)
1218+
assert.NoError(t, pcAnswer.SetLocalDescription(answer))
1219+
assert.NoError(t, err)
1220+
assert.NoError(t, pcOffer.SetRemoteDescription(answer))
1221+
sender3, err := pcOffer.AddTrack(vp8Track)
1222+
ssrcNotReuse := sender3.GetParameters().Encodings[0].SSRC
1223+
assert.NoError(t, err)
1224+
assert.Equal(t, len(pcOffer.GetTransceivers()), 3)
1225+
assert.NoError(t, signalPair(pcOffer, pcAnswer))
1226+
assert.True(t, sender3.rtpTransceiver == pcOffer.GetTransceivers()[2])
1227+
1228+
for i := 0; i < 10; i++ {
1229+
assert.NoError(t, vp8Track.WriteRTP(&rtp.Packet{Header: rtp.Header{Version: 2}, Payload: []byte{0, 1, 2, 3, 4, 5}}))
1230+
time.Sleep(20 * time.Millisecond)
1231+
}
1232+
1233+
tr1 := <-tracksCh
1234+
tr2 := <-tracksCh
1235+
assert.Equal(t, tr1.SSRC(), ssrcReuse)
1236+
assert.Equal(t, tr2.SSRC(), ssrcNotReuse)
11951237

11961238
closePairNow(t, pcOffer, pcAnswer)
11971239
}

0 commit comments

Comments
 (0)