Repository: pion/interceptor
Pull request #127 (2022-05-06 07:04:53)

Make TWCC wait until first packet before reporting
TWCC currently starts sending empty reports as soon as an RTCP writer is
setup. This leads to errors before the connection is setup properly.
This change makes the interceptor wait until the first packet arrived
before sending any reports.

codecov[bot]: # [Codecov](https://codecov.io/gh/pion/interceptor/pull/127?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion) Report
> Merging [#127](https://codecov.io/gh/pion/interceptor/pull/127?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion) (729789c) into [master](https://codecov.io/gh/pion/interceptor/commit/896cf7e16effd1a92515de5bc079d7ed77025800?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion) (896cf7e) will **decrease** coverage by `1.48%`.
> The diff coverage is `100.00%`.

```diff
@@            Coverage Diff             @@
##           master     #127      +/-   ##
==========================================
- Coverage   78.65%   77.16%   -1.49%     
==========================================
  Files          51       51              
  Lines        2474     2479       +5     
==========================================
- Hits         1946     1913      -33     
- Misses        430      466      +36     
- Partials       98      100       +2     
```

| Flag | Coverage Δ | |
|---|---|---|
| go | `?` | |
| wasm | `77.16% <100.00%> (+0.04%)` | :arrow_up: |

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/interceptor/pull/127?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion) | Coverage Δ | |
|---|---|---|
| [pkg/twcc/sender\_interceptor.go](https://codecov.io/gh/pion/interceptor/pull/127/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#diff-cGtnL3R3Y2Mvc2VuZGVyX2ludGVyY2VwdG9yLmdv) | `77.06% <100.00%> (-1.79%)` | :arrow_down: |
| [interceptor.go](https://codecov.io/gh/pion/interceptor/pull/127/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#diff-aW50ZXJjZXB0b3IuZ28=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [internal/test/mock\_time.go](https://codecov.io/gh/pion/interceptor/pull/127/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#diff-aW50ZXJuYWwvdGVzdC9tb2NrX3RpbWUuZ28=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [noop.go](https://codecov.io/gh/pion/interceptor/pull/127/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#diff-bm9vcC5nbw==) | `0.00% <0.00%> (-83.34%)` | :arrow_down: |
| [attributes.go](https://codecov.io/gh/pion/interceptor/pull/127/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#diff-YXR0cmlidXRlcy5nbw==) | `78.57% <0.00%> (-14.29%)` | :arrow_down: |
| [pkg/gcc/arrival\_group\_accumulator.go](https://codecov.io/gh/pion/interceptor/pull/127/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#diff-cGtnL2djYy9hcnJpdmFsX2dyb3VwX2FjY3VtdWxhdG9yLmdv) | `90.62% <0.00%> (-9.38%)` | :arrow_down: |
| [pkg/gcc/delay\_based\_bwe.go](https://codecov.io/gh/pion/interceptor/pull/127/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#diff-cGtnL2djYy9kZWxheV9iYXNlZF9id2UuZ28=) | `83.33% <0.00%> (-3.04%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pion/interceptor/pull/127?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/interceptor/pull/127?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion). Last update [896cf7e...729789c](https://codecov.io/gh/pion/interceptor/pull/127?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/pkg/twcc/sender_interceptor.go b/pkg/twcc/sender_interceptor.go
index cf8688e..89df1ca 100644
--- a/pkg/twcc/sender_interceptor.go
+++ b/pkg/twcc/sender_interceptor.go
@@ -166,6 +166,13 @@ func (s *SenderInterceptor) isClosed() bool {
 func (s *SenderInterceptor) loop(w interceptor.RTCPWriter) {
 	defer s.wg.Done()
 
+	select {
+	case <-s.close:
+		return
+	case p := <-s.packetChan:
+		s.recorder.Record(p.ssrc, p.sequenceNumber, p.arrivalTime)
+	}
+
 	ticker := time.NewTicker(s.interval)
 	for {
 		select {


Test patch:
diff --git a/pkg/twcc/sender_interceptor_test.go b/pkg/twcc/sender_interceptor_test.go
index 4293b21..9a9c7f5 100644
--- a/pkg/twcc/sender_interceptor_test.go
+++ b/pkg/twcc/sender_interceptor_test.go
@@ -29,17 +29,12 @@ func TestSenderInterceptor(t *testing.T) {
 			assert.NoError(t, stream.Close())
 		}()
 
-		pkts := <-stream.WrittenRTCP()
-		assert.Equal(t, 1, len(pkts))
-		tlcc, ok := pkts[0].(*rtcp.TransportLayerCC)
-		assert.True(t, ok)
-		assert.Equal(t, uint16(0), tlcc.PacketStatusCount)
-		assert.Equal(t, uint8(0), tlcc.FbPktCount)
-		assert.Equal(t, uint16(0), tlcc.BaseSequenceNumber)
-		assert.Equal(t, uint32(0), tlcc.MediaSSRC)
-		assert.Equal(t, uint32(0), tlcc.ReferenceTime)
-		assert.Equal(t, 0, len(tlcc.RecvDeltas))
-		assert.Equal(t, 0, len(tlcc.PacketChunks))
+		var pkts []rtcp.Packet
+		select {
+		case pkts = <-stream.WrittenRTCP():
+		case <-time.After(300 * time.Millisecond): // wait longer than default interval
+		}
+		assert.Equal(t, 0, len(pkts))
 	})
 
 	t.Run("after RTP packets", func(t *testing.T) {