Repository: pion/interceptor
Pull request #327 (2025-04-30 05:09:28)

Improve memory efficiency
Refactored packet processing to avoid multiple make calls and eliminate the use of append. This reduces garbage collection overhead and improves overall memory management efficiency.

#### Description

#### Reference issue
Fixes #...


codecov[bot]: ## [Codecov](https://app.codecov.io/gh/pion/interceptor/pull/327?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion) Report
Attention: Patch coverage is `68.00000%` with `8 lines` in your changes missing coverage. Please review.
> Project coverage is 69.72%. Comparing base [(`383f70e`)](https://app.codecov.io/gh/pion/interceptor/commit/383f70e0dd29d1f08db8855de55bb4bd01046ab1?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion) to head [(`3221f9b`)](https://app.codecov.io/gh/pion/interceptor/commit/3221f9bac669f77e9ccadbc5020ab4400badc0b7?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion).
> Report is 7 commits behind head on master.

| [Files with missing lines](https://app.codecov.io/gh/pion/interceptor/pull/327?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion) | Patch % | Lines |
|---|---|---|
| [pkg/nack/generator\_interceptor.go](https://app.codecov.io/gh/pion/interceptor/pull/327?src=pr&el=tree&filepath=pkg%2Fnack%2Fgenerator_interceptor.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#diff-cGtnL25hY2svZ2VuZXJhdG9yX2ludGVyY2VwdG9yLmdv) | 60.00% | [7 Missing and 1 partial :warning: ](https://app.codecov.io/gh/pion/interceptor/pull/327?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion) |

:x: Your patch check has failed because the patch coverage (68.00%) is below the target coverage (70.00%). You can increase the patch coverage or adjust the [target](https://docs.codecov.com/docs/commit-status#target) coverage.

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##           master     #327      +/-   ##
==========================================
- Coverage   71.63%   69.72%   -1.92%     
==========================================
  Files          79       79              
  Lines        4742     4756      +14     
==========================================
- Hits         3397     3316      -81     
- Misses       1205     1291      +86     
- Partials      140      149       +9     
```

| [Flag](https://app.codecov.io/gh/pion/interceptor/pull/327/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion) | Coverage Δ | |
|---|---|---|
| [go](https://app.codecov.io/gh/pion/interceptor/pull/327/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion) | `?` | |
| [wasm](https://app.codecov.io/gh/pion/interceptor/pull/327/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion) | `69.72% <68.00%> (+0.08%)` | :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.

</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/pion/interceptor/pull/327?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion).

<details><summary> :rocket: New features to boost your workflow: </summary>

- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
</details>

Patch:
diff --git a/pkg/nack/generator_interceptor.go b/pkg/nack/generator_interceptor.go
index e8d6a1a..10c8a01 100644
--- a/pkg/nack/generator_interceptor.go
+++ b/pkg/nack/generator_interceptor.go
@@ -145,6 +145,9 @@ func (n *GeneratorInterceptor) loop(rtcpWriter interceptor.RTCPWriter) {
 
 	senderSSRC := rand.Uint32() // #nosec
 
+	missingPacketSeqNums := make([]uint16, n.size)
+	filteredMissingPacket := make([]uint16, n.size)
+
 	ticker := time.NewTicker(n.interval)
 	defer ticker.Stop()
 	for {
@@ -155,7 +158,7 @@ func (n *GeneratorInterceptor) loop(rtcpWriter interceptor.RTCPWriter) {
 				defer n.receiveLogsMu.Unlock()
 
 				for ssrc, receiveLog := range n.receiveLogs {
-					missing := receiveLog.missingSeqNumbers(n.skipLastN)
+					missing := receiveLog.missingSeqNumbers(n.skipLastN, missingPacketSeqNums)
 
 					if len(missing) == 0 || n.nackCountLogs[ssrc] == nil {
 						n.nackCountLogs[ssrc] = map[uint16]uint16{}
@@ -164,22 +167,33 @@ func (n *GeneratorInterceptor) loop(rtcpWriter interceptor.RTCPWriter) {
 						continue
 					}
 
-					filteredMissing := []uint16{}
+					nack := &rtcp.TransportLayerNack{} // nolint:ineffassign,wastedassign
+
+					c := 0 // nolint:varnamelen,
 					if n.maxNacksPerPacket > 0 {
 						for _, missingSeq := range missing {
 							if n.nackCountLogs[ssrc][missingSeq] < n.maxNacksPerPacket {
-								filteredMissing = append(filteredMissing, missingSeq)
+								filteredMissingPacket[c] = missingSeq
+								c++
 							}
 							n.nackCountLogs[ssrc][missingSeq]++
 						}
-					} else {
-						filteredMissing = missing
-					}
 
-					nack := &rtcp.TransportLayerNack{
-						SenderSSRC: senderSSRC,
-						MediaSSRC:  ssrc,
-						Nacks:      rtcp.NackPairsFromSequenceNumbers(filteredMissing),
+						if c == 0 {
+							continue
+						}
+
+						nack = &rtcp.TransportLayerNack{
+							SenderSSRC: senderSSRC,
+							MediaSSRC:  ssrc,
+							Nacks:      rtcp.NackPairsFromSequenceNumbers(filteredMissingPacket[:c]),
+						}
+					} else {
+						nack = &rtcp.TransportLayerNack{
+							SenderSSRC: senderSSRC,
+							MediaSSRC:  ssrc,
+							Nacks:      rtcp.NackPairsFromSequenceNumbers(missing),
+						}
 					}
 
 					for nackSeq := range n.nackCountLogs[ssrc] {
@@ -196,10 +210,6 @@ func (n *GeneratorInterceptor) loop(rtcpWriter interceptor.RTCPWriter) {
 						}
 					}
 
-					if len(filteredMissing) == 0 {
-						continue
-					}
-
 					if _, err := rtcpWriter.Write([]rtcp.Packet{nack}, interceptor.Attributes{}); err != nil {
 						n.log.Warnf("failed sending nack: %+v", err)
 					}
diff --git a/pkg/nack/receive_log.go b/pkg/nack/receive_log.go
index 3bf5c49..c37407d 100644
--- a/pkg/nack/receive_log.go
+++ b/pkg/nack/receive_log.go
@@ -97,7 +97,7 @@ func (s *receiveLog) get(seq uint16) bool {
 	return s.getReceived(seq)
 }
 
-func (s *receiveLog) missingSeqNumbers(skipLastN uint16) []uint16 {
+func (s *receiveLog) missingSeqNumbers(skipLastN uint16, missingPacketSeqNums []uint16) []uint16 {
 	s.m.RLock()
 	defer s.m.RUnlock()
 
@@ -107,14 +107,15 @@ func (s *receiveLog) missingSeqNumbers(skipLastN uint16) []uint16 {
 		return nil
 	}
 
-	missingPacketSeqNums := make([]uint16, 0)
+	c := 0
 	for i := s.lastConsecutive + 1; i != until+1; i++ {
 		if !s.getReceived(i) {
-			missingPacketSeqNums = append(missingPacketSeqNums, i)
+			missingPacketSeqNums[c] = i
+			c++
 		}
 	}
 
-	return missingPacketSeqNums
+	return missingPacketSeqNums[:c]
 }
 
 func (s *receiveLog) setReceived(seq uint16) {


Test patch:
diff --git a/pkg/nack/generator_interceptor_test.go b/pkg/nack/generator_interceptor_test.go
index b71bfcc..7aad1fe 100644
--- a/pkg/nack/generator_interceptor_test.go
+++ b/pkg/nack/generator_interceptor_test.go
@@ -20,6 +20,7 @@ func TestGeneratorInterceptor(t *testing.T) {
 	f, err := NewGeneratorInterceptor(
 		GeneratorSize(64),
 		GeneratorSkipLastN(2),
+		GeneratorMaxNacksPerPacket(10),
 		GeneratorInterval(interval),
 		GeneratorLog(logging.NewDefaultLoggerFactory().NewLogger("test")),
 	)
diff --git a/pkg/nack/receive_log_test.go b/pkg/nack/receive_log_test.go
index 0822d7f..b23f1f4 100644
--- a/pkg/nack/receive_log_test.go
+++ b/pkg/nack/receive_log_test.go
@@ -61,7 +61,8 @@ func TestReceivedBuffer(t *testing.T) {
 			}
 			assertMissing := func(skipLastN uint16, nums []uint16) {
 				t.Helper()
-				missing := rl.missingSeqNumbers(skipLastN)
+				missingPacketSeqNums := make([]uint16, rl.size)
+				missing := rl.missingSeqNumbers(skipLastN, missingPacketSeqNums)
 				if missing == nil {
 					missing = []uint16{}
 				}
@@ -131,6 +132,24 @@ func TestReceivedBuffer(t *testing.T) {
 			assertGet(321)
 			assertMissing(0, all(330, 332))
 			assertLastConsecutive(329) // after adding a single missing packet, lastConsecutive should jump forward
+
+			add(all(330, 332)...)
+			assertMissing(0, []uint16{})
+			assertLastConsecutive(334)
+
+			// Add a packet beyond the current missing range to trigger buffer overflow behavior.
+			// Ensure that when the number of missing packets exceeds the buffer size,
+			// only the latest (rl.size - 1) entries are considered for NACKs.
+			add(466)
+			assertGet(466)
+
+			missing := all(335, 465)
+			if len(missing) > int(rl.size) {
+				assertLastConsecutive(missing[len(missing)-int(rl.size)])
+				assertMissing(0, missing[len(missing)-(int(rl.size-1)):])
+			} else {
+				assertMissing(0, all(335, 465))
+			}
 		})
 	}
 }