Repository: pion/webrtc
Pull request #1893 (2021-07-20 10:30:23)

Fixes ReplaceTrack
When ReplaceTrack was set previously to nil it would be impossible to ReplaceTrack again with a non-nil value as r.track was set to nil.


codecov[bot]: # [Codecov](https://codecov.io/gh/pion/webrtc/pull/1893?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion) Report
> Merging [#1893](https://codecov.io/gh/pion/webrtc/pull/1893?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion) (2e88fc1) into [master](https://codecov.io/gh/pion/webrtc/commit/a33d7cdc37d3c0d865cefe6a0b6885f5d6eb7620?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion) (a33d7cd) will **decrease** coverage by `4.38%`.
> The diff coverage is `n/a`.

> :exclamation: Current head 2e88fc1 differs from pull request most recent head b806c79. Consider uploading reports for the commit b806c79 to get more accurate results
[![Impacted file tree graph](https://codecov.io/gh/pion/webrtc/pull/1893/graphs/tree.svg?width=650&height=150&src=pr&token=XOOuQo07I2&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion)](https://codecov.io/gh/pion/webrtc/pull/1893?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion)

```diff
@@            Coverage Diff             @@
##           master    #1893      +/-   ##
==========================================
- Coverage   75.53%   71.14%   -4.39%     
==========================================
  Files          84       61      -23     
  Lines        6037     2277    -3760     
==========================================
- Hits         4560     1620    -2940     
+ Misses       1086      549     -537     
+ Partials      391      108     -283     
```

| Flag | Coverage Δ | |
|---|---|---|
| go | `?` | |
| wasm | `71.14% <ø> (ø)` | |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#carryforward-flags-in-the-pull-request-comment) to find out more.

| [Impacted Files](https://codecov.io/gh/pion/webrtc/pull/1893?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion) | Coverage Δ | |
|---|---|---|
| [constants.go](https://codecov.io/gh/pion/webrtc/pull/1893/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#diff-Y29uc3RhbnRzLmdv) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [icecandidatepair.go](https://codecov.io/gh/pion/webrtc/pull/1893/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#diff-aWNlY2FuZGlkYXRlcGFpci5nbw==) | `0.00% <0.00%> (-85.72%)` | :arrow_down: |
| [track\_local.go](https://codecov.io/gh/pion/webrtc/pull/1893/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#diff-dHJhY2tfbG9jYWwuZ28=) | `0.00% <0.00%> (-80.00%)` | :arrow_down: |
| [rtpcodec.go](https://codecov.io/gh/pion/webrtc/pull/1893/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#diff-cnRwY29kZWMuZ28=) | `15.00% <0.00%> (-75.00%)` | :arrow_down: |
| [stats.go](https://codecov.io/gh/pion/webrtc/pull/1893/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#diff-c3RhdHMuZ28=) | `0.00% <0.00%> (-67.86%)` | :arrow_down: |
| [internal/mux/muxfunc.go](https://codecov.io/gh/pion/webrtc/pull/1893/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#diff-aW50ZXJuYWwvbXV4L211eGZ1bmMuZ28=) | `8.33% <0.00%> (-58.34%)` | :arrow_down: |
| [icemux.go](https://codecov.io/gh/pion/webrtc/pull/1893/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#diff-aWNlbXV4Lmdv) | `0.00% <0.00%> (-55.56%)` | :arrow_down: |
| [gathering\_complete\_promise.go](https://codecov.io/gh/pion/webrtc/pull/1893/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#diff-Z2F0aGVyaW5nX2NvbXBsZXRlX3Byb21pc2UuZ28=) | `60.00% <0.00%> (-40.00%)` | :arrow_down: |
| [internal/mux/endpoint.go](https://codecov.io/gh/pion/webrtc/pull/1893/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#diff-aW50ZXJuYWwvbXV4L2VuZHBvaW50Lmdv) | `22.22% <0.00%> (-38.89%)` | :arrow_down: |
| [sdpsemantics.go](https://codecov.io/gh/pion/webrtc/pull/1893/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#diff-c2Rwc2VtYW50aWNzLmdv) | `0.00% <0.00%> (-37.50%)` | :arrow_down: |
| ... and [32 more](https://codecov.io/gh/pion/webrtc/pull/1893/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion) | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pion/webrtc/pull/1893?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/pion/webrtc/pull/1893?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion). Last update [a33d7cd...b806c79](https://codecov.io/gh/pion/webrtc/pull/1893?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion).


Patch:
diff --git a/AUTHORS.txt b/AUTHORS.txt
index c0461385..7436509c 100644
--- a/AUTHORS.txt
+++ b/AUTHORS.txt
@@ -110,6 +110,7 @@ OrlandoCo <luisorlando.co@gmail.com>
 Pascal Benoit <pascal.benoit@acemediastools.fr>
 pascal-ace <47424881+pascal-ace@users.noreply.github.com>
 Patrick Lange <mail@langep.com>
+Patryk Rogalski <digitalix4@gmail.com>
 q191201771 <191201771@qq.com>
 Quentin Renard <contact@asticode.com>
 Rafael Viscarra <rafael@viscarra.dev>
diff --git a/errors.go b/errors.go
index 31595b59..1dd5d38d 100644
--- a/errors.go
+++ b/errors.go
@@ -135,6 +135,9 @@ var (
 	// ErrUnsupportedCodec indicates the remote peer doesn't support the requested codec
 	ErrUnsupportedCodec = errors.New("unable to start track, codec is not supported by remote")
 
+	// ErrRTPSenderNewTrackHasIncorrectKind indicates that the new track is of a different kind than the previous/original
+	ErrRTPSenderNewTrackHasIncorrectKind = errors.New("new track must be of the same kind as previous")
+
 	// ErrUnbindFailed indicates that a TrackLocal was not able to be unbind
 	ErrUnbindFailed = errors.New("failed to unbind TrackLocal from PeerConnection")
 
diff --git a/rtpsender.go b/rtpsender.go
index efcedf89..b6c4bd17 100644
--- a/rtpsender.go
+++ b/rtpsender.go
@@ -145,6 +145,10 @@ func (r *RTPSender) ReplaceTrack(track TrackLocal) error {
 	r.mu.Lock()
 	defer r.mu.Unlock()
 
+	if track != nil && r.tr.kind != track.Kind() {
+		return ErrRTPSenderNewTrackHasIncorrectKind
+	}
+
 	if r.hasSent() && r.track != nil {
 		if err := r.track.Unbind(r.context); err != nil {
 			return err
@@ -158,7 +162,7 @@ func (r *RTPSender) ReplaceTrack(track TrackLocal) error {
 
 	codec, err := track.Bind(TrackLocalContext{
 		id:          r.context.id,
-		params:      r.api.mediaEngine.getRTPParametersByKind(r.track.Kind(), []RTPTransceiverDirection{RTPTransceiverDirectionSendonly}),
+		params:      r.api.mediaEngine.getRTPParametersByKind(track.Kind(), []RTPTransceiverDirection{RTPTransceiverDirectionSendonly}),
 		ssrc:        r.context.ssrc,
 		writeStream: r.context.writeStream,
 	})


Test patch:
diff --git a/rtpsender_test.go b/rtpsender_test.go
index 71c6f1fb..a187a718 100644
--- a/rtpsender_test.go
+++ b/rtpsender_test.go
@@ -167,7 +167,7 @@ func Test_RTPSender_SetReadDeadline(t *testing.T) {
 	closePairNow(t, sender, receiver)
 }
 
-func Test_RTPSender_ReplaceTrack_InvalidCodecChange(t *testing.T) {
+func Test_RTPSender_ReplaceTrack_InvalidTrackKindChange(t *testing.T) {
 	lim := test.TimeOut(time.Second * 10)
 	defer lim.Stop()
 
@@ -204,6 +204,54 @@ func Test_RTPSender_ReplaceTrack_InvalidCodecChange(t *testing.T) {
 		}
 	}()
 
+	assert.True(t, errors.Is(rtpSender.ReplaceTrack(trackB), ErrRTPSenderNewTrackHasIncorrectKind))
+
+	closePairNow(t, sender, receiver)
+}
+
+func Test_RTPSender_ReplaceTrack_InvalidCodecChange(t *testing.T) {
+	lim := test.TimeOut(time.Second * 10)
+	defer lim.Stop()
+
+	report := test.CheckRoutines(t)
+	defer report()
+
+	sender, receiver, err := newPair()
+	assert.NoError(t, err)
+
+	trackA, err := NewTrackLocalStaticSample(RTPCodecCapability{MimeType: MimeTypeVP8}, "video", "pion")
+	assert.NoError(t, err)
+
+	trackB, err := NewTrackLocalStaticSample(RTPCodecCapability{MimeType: MimeTypeVP9}, "video", "pion")
+	assert.NoError(t, err)
+
+	rtpSender, err := sender.AddTrack(trackA)
+	assert.NoError(t, err)
+
+	err = rtpSender.tr.SetCodecPreferences([]RTPCodecParameters{{
+		RTPCodecCapability: RTPCodecCapability{MimeType: MimeTypeVP8},
+		PayloadType:        96,
+	}})
+	assert.NoError(t, err)
+
+	assert.NoError(t, signalPair(sender, receiver))
+
+	seenPacket, seenPacketCancel := context.WithCancel(context.Background())
+	receiver.OnTrack(func(_ *TrackRemote, _ *RTPReceiver) {
+		seenPacketCancel()
+	})
+
+	func() {
+		for range time.Tick(time.Millisecond * 20) {
+			select {
+			case <-seenPacket.Done():
+				return
+			default:
+				assert.NoError(t, trackA.WriteSample(media.Sample{Data: []byte{0xAA}, Duration: time.Second}))
+			}
+		}
+	}()
+
 	assert.True(t, errors.Is(rtpSender.ReplaceTrack(trackB), ErrUnsupportedCodec))
 
 	closePairNow(t, sender, receiver)