diff --git a/.golangci.yml b/.golangci.yml index a3235bec..0f420d51 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -25,17 +25,32 @@ linters-settings: - ^os.Exit$ - ^panic$ - ^print(ln)?$ + varnamelen: + max-distance: 12 + min-name-length: 2 + ignore-type-assert-ok: true + ignore-map-index-ok: true + ignore-chan-recv-ok: true + ignore-decls: + - i int + - n int + - w io.Writer + - r io.Reader + - b []byte linters: enable: - asciicheck # Simple linter to check that your code does not contain non-ASCII identifiers - bidichk # Checks for dangerous unicode character sequences - bodyclose # checks whether HTTP response body is closed successfully + - containedctx # containedctx is a linter that detects struct contained context.Context field - contextcheck # check the function whether use a non-inherited context + - cyclop # checks function and package cyclomatic complexity - decorder # check declaration order and count of types, constants, variables and functions - dogsled # Checks assignments with too many blank identifiers (e.g. x, _, _, _, := f()) - dupl # Tool for code clone detection - durationcheck # check for two durations multiplied together + - err113 # Golang linter to check the errors handling expressions - errcheck # Errcheck is a program for checking for unchecked errors in go programs. These unchecked errors can be critical bugs in some cases - errchkjson # Checks types passed to the json encoding functions. Reports unsupported types and optionally reports occations, where the check for the returned error can be omitted. - errname # Checks that sentinel errors are prefixed with the `Err` and error types are suffixed with the `Error`. @@ -46,18 +61,17 @@ linters: - forcetypeassert # finds forced type assertions - gci # Gci control golang package import order and make it always deterministic. - gochecknoglobals # Checks that no globals are present in Go code - - gochecknoinits # Checks that no init functions are present in Go code - gocognit # Computes and checks the cognitive complexity of functions - goconst # Finds repeated strings that could be replaced by a constant - gocritic # The most opinionated Go source code linter + - gocyclo # Computes and checks the cyclomatic complexity of functions + - godot # Check if comments end in a period - godox # Tool for detection of FIXME, TODO and other comment keywords - - err113 # Golang linter to check the errors handling expressions - gofmt # Gofmt checks whether code was gofmt-ed. By default this tool runs with -s option to check for code simplification - gofumpt # Gofumpt checks whether code was gofumpt-ed. - goheader # Checks is file header matches to pattern - goimports # Goimports does everything that gofmt does. Additionally it checks unused imports - gomoddirectives # Manage the use of 'replace', 'retract', and 'excludes' directives in go.mod. - - gomodguard # Allow and block list linter for direct Go module dependencies. This is different from depguard where there are different block types for example version constraints and module recommendations. - goprintffuncname # Checks that printf-like functions are named with `f` at the end - gosec # Inspects source code for security problems - gosimple # Linter for Go source code that specializes in simplifying a code @@ -65,9 +79,15 @@ linters: - grouper # An analyzer to analyze expression groups. - importas # Enforces consistent import aliases - ineffassign # Detects when assignments to existing variables are not used + - lll # Reports long lines + - maintidx # maintidx measures the maintainability index of each function. + - makezero # Finds slice declarations with non-zero initial length - misspell # Finds commonly misspelled English words in comments + - nakedret # Finds naked returns in functions greater than a specified function length + - nestif # Reports deeply nested if statements - nilerr # Finds the code that returns nil even if it checks that the error is not nil. - nilnil # Checks that there is no simultaneous return of `nil` error and an invalid value. + - nlreturn # nlreturn checks for a new line before return and branch statements to increase code clarity - noctx # noctx finds sending http request without context.Context - predeclared # find code that shadows one of Go's predeclared identifiers - revive # golint replacement, finds style mistakes @@ -75,28 +95,22 @@ linters: - stylecheck # Stylecheck is a replacement for golint - tagliatelle # Checks the struct tags. - tenv # tenv is analyzer that detects using os.Setenv instead of t.Setenv since Go1.17 - - tparallel # tparallel detects inappropriate usage of t.Parallel() method in your Go test codes + - thelper # thelper detects golang test helpers without t.Helper() call and checks the consistency of test helpers - typecheck # Like the front-end of a Go compiler, parses and type-checks Go code - unconvert # Remove unnecessary type conversions - unparam # Reports unused function parameters - unused # Checks Go code for unused constants, variables, functions and types + - varnamelen # checks that the length of a variable's name matches its scope - wastedassign # wastedassign finds wasted assignment statements - whitespace # Tool for detection of leading and trailing whitespace disable: - depguard # Go linter that checks if package imports are in a list of acceptable packages - - containedctx # containedctx is a linter that detects struct contained context.Context field - - cyclop # checks function and package cyclomatic complexity - funlen # Tool for detection of long functions - - gocyclo # Computes and checks the cyclomatic complexity of functions - - godot # Check if comments end in a period - - gomnd # An analyzer to detect magic numbers. + - gochecknoinits # Checks that no init functions are present in Go code + - gomodguard # Allow and block list linter for direct Go module dependencies. This is different from depguard where there are different block types for example version constraints and module recommendations. + - interfacebloat # A linter that checks length of interface. - ireturn # Accept Interfaces, Return Concrete Types - - lll # Reports long lines - - maintidx # maintidx measures the maintainability index of each function. - - makezero # Finds slice declarations with non-zero initial length - - nakedret # Finds naked returns in functions greater than a specified function length - - nestif # Reports deeply nested if statements - - nlreturn # nlreturn checks for a new line before return and branch statements to increase code clarity + - mnd # An analyzer to detect magic numbers - nolintlint # Reports ill-formed or insufficient nolint directives - paralleltest # paralleltest detects missing usage of t.Parallel() method in your Go test - prealloc # Finds slice declarations that could potentially be preallocated @@ -104,8 +118,7 @@ linters: - rowserrcheck # checks whether Err of rows is checked successfully - sqlclosecheck # Checks that sql.Rows and sql.Stmt are closed. - testpackage # linter that makes you use a separate _test package - - thelper # thelper detects golang test helpers without t.Helper() call and checks the consistency of test helpers - - varnamelen # checks that the length of a variable's name matches its scope + - tparallel # tparallel detects inappropriate usage of t.Parallel() method in your Go test codes - wrapcheck # Checks that errors returned from external packages are wrapped - wsl # Whitespace Linter - Forces you to use empty lines! @@ -123,3 +136,5 @@ issues: - path: cmd linters: - forbidigo + max-issues-per-linter: 0 + max-same-issues: 0 diff --git a/attributes.go b/attributes.go index 8b6d0f5c..e4257b8d 100644 --- a/attributes.go +++ b/attributes.go @@ -19,7 +19,7 @@ const ( var errInvalidType = errors.New("found value of invalid type in attributes map") -// Attributes are a generic key/value store used by interceptors +// Attributes are a generic key/value store used by interceptors. type Attributes map[interface{}]interface{} // Get returns the attribute associated with key. @@ -39,6 +39,7 @@ func (a Attributes) GetRTPHeader(raw []byte) (*rtp.Header, error) { if header, ok := val.(*rtp.Header); ok { return header, nil } + return nil, errInvalidType } header := &rtp.Header{} @@ -46,6 +47,7 @@ func (a Attributes) GetRTPHeader(raw []byte) (*rtp.Header, error) { return nil, err } a[rtpHeaderKey] = header + return header, nil } @@ -57,6 +59,7 @@ func (a Attributes) GetRTCPPackets(raw []byte) ([]rtcp.Packet, error) { if packets, ok := val.([]rtcp.Packet); ok { return packets, nil } + return nil, errInvalidType } pkts, err := rtcp.Unmarshal(raw) @@ -64,5 +67,6 @@ func (a Attributes) GetRTCPPackets(raw []byte) ([]rtcp.Packet, error) { return nil, err } a[rtcpPacketsKey] = pkts + return pkts, nil } diff --git a/chain.go b/chain.go index 267f3665..62c0aeb1 100644 --- a/chain.go +++ b/chain.go @@ -50,7 +50,8 @@ func (i *Chain) UnbindLocalStream(ctx *StreamInfo) { } } -// BindRemoteStream lets you modify any incoming RTP packets. It is called once for per RemoteStream. The returned method +// BindRemoteStream lets you modify any incoming RTP packets. +// It is called once for per RemoteStream. The returned method // will be called once per rtp packet. func (i *Chain) BindRemoteStream(ctx *StreamInfo, reader RTPReader) RTPReader { for _, interceptor := range i.interceptors { diff --git a/errors.go b/errors.go index 3dafee3e..02c79539 100644 --- a/errors.go +++ b/errors.go @@ -18,6 +18,7 @@ func flattenErrs(errs []error) error { if len(errs2) == 0 { return nil } + return multiError(errs2) } @@ -50,5 +51,6 @@ func (me multiError) Is(err error) bool { } } } + return false } diff --git a/examples/nack/main.go b/examples/nack/main.go index b29180c9..76337273 100644 --- a/examples/nack/main.go +++ b/examples/nack/main.go @@ -54,10 +54,17 @@ func receiveRoutine() { // Create the writer just for a single SSRC stream // this is a callback that is fired everytime a RTP packet is ready to be sent - streamReader := chain.BindRemoteStream(&interceptor.StreamInfo{ - SSRC: ssrc, - RTCPFeedback: []interceptor.RTCPFeedback{{Type: "nack", Parameter: ""}}, - }, interceptor.RTPReaderFunc(func(b []byte, _ interceptor.Attributes) (int, interceptor.Attributes, error) { return len(b), nil, nil })) + streamReader := chain.BindRemoteStream( + &interceptor.StreamInfo{ + SSRC: ssrc, + RTCPFeedback: []interceptor.RTCPFeedback{{Type: "nack", Parameter: ""}}, + }, + interceptor.RTPReaderFunc( + func(b []byte, _ interceptor.Attributes) (int, interceptor.Attributes, error) { + return len(b), nil, nil + }, + ), + ) for rtcpBound, buffer := false, make([]byte, mtu); ; { i, addr, err := conn.ReadFrom(buffer) @@ -88,6 +95,7 @@ func receiveRoutine() { } } +//nolint:cyclop func sendRoutine() { // Dial our UDP listener that we create in receiveRoutine serverAddr, err := net.ResolveUDPAddr("udp4", fmt.Sprintf("127.0.0.1:%d", listenPort)) @@ -116,9 +124,11 @@ func sendRoutine() { // Set the interceptor wide RTCP Reader // this is a handle to send NACKs back into the interceptor. - rtcpReader := chain.BindRTCPReader(interceptor.RTCPReaderFunc(func(in []byte, _ interceptor.Attributes) (int, interceptor.Attributes, error) { - return len(in), nil, nil - })) + rtcpReader := chain.BindRTCPReader( + interceptor.RTCPReaderFunc(func(in []byte, _ interceptor.Attributes) (int, interceptor.Attributes, error) { + return len(in), nil, nil + }), + ) // Create the writer just for a single SSRC stream // this is a callback that is fired everytime a RTP packet is ready to be sent diff --git a/interceptor.go b/interceptor.go index c6ba5324..10d3e32f 100644 --- a/interceptor.go +++ b/interceptor.go @@ -12,7 +12,7 @@ import ( "github.com/pion/rtp" ) -// Factory provides an interface for constructing interceptors +// Factory provides an interface for constructing interceptors. type Factory interface { NewInterceptor(id string) (Interceptor, error) } @@ -35,7 +35,8 @@ type Interceptor interface { // UnbindLocalStream is called when the Stream is removed. It can be used to clean up any data related to that track. UnbindLocalStream(info *StreamInfo) - // BindRemoteStream lets you modify any incoming RTP packets. It is called once for per RemoteStream. The returned method + // BindRemoteStream lets you modify any incoming RTP packets. + // It is called once for per RemoteStream. The returned method // will be called once per rtp packet. BindRemoteStream(info *StreamInfo, reader RTPReader) RTPReader @@ -69,34 +70,34 @@ type RTCPReader interface { Read([]byte, Attributes) (int, Attributes, error) } -// RTPWriterFunc is an adapter for RTPWrite interface +// RTPWriterFunc is an adapter for RTPWrite interface. type RTPWriterFunc func(header *rtp.Header, payload []byte, attributes Attributes) (int, error) -// RTPReaderFunc is an adapter for RTPReader interface +// RTPReaderFunc is an adapter for RTPReader interface. type RTPReaderFunc func([]byte, Attributes) (int, Attributes, error) -// RTCPWriterFunc is an adapter for RTCPWriter interface +// RTCPWriterFunc is an adapter for RTCPWriter interface. type RTCPWriterFunc func(pkts []rtcp.Packet, attributes Attributes) (int, error) -// RTCPReaderFunc is an adapter for RTCPReader interface +// RTCPReaderFunc is an adapter for RTCPReader interface. type RTCPReaderFunc func([]byte, Attributes) (int, Attributes, error) -// Write a rtp packet +// Write a rtp packet. func (f RTPWriterFunc) Write(header *rtp.Header, payload []byte, attributes Attributes) (int, error) { return f(header, payload, attributes) } -// Read a rtp packet +// Read a rtp packet. func (f RTPReaderFunc) Read(b []byte, a Attributes) (int, Attributes, error) { return f(b, a) } -// Write a batch of rtcp packets +// Write a batch of rtcp packets. func (f RTCPWriterFunc) Write(pkts []rtcp.Packet, attributes Attributes) (int, error) { return f(pkts, attributes) } -// Read a batch of rtcp packets +// Read a batch of rtcp packets. func (f RTCPReaderFunc) Read(b []byte, a Attributes) (int, Attributes, error) { return f(b, a) } diff --git a/internal/cc/acknowledgment.go b/internal/cc/acknowledgment.go index 77ac9b35..d239aae3 100644 --- a/internal/cc/acknowledgment.go +++ b/internal/cc/acknowledgment.go @@ -27,5 +27,6 @@ func (a Acknowledgment) String() string { s += fmt.Sprintf("\tSIZE:\t%v\n", a.Size) s += fmt.Sprintf("\tDEPARTURE:\t%v\n", int64(float64(a.Departure.UnixNano())/1e+6)) s += fmt.Sprintf("\tARRIVAL:\t%v\n", int64(float64(a.Arrival.UnixNano())/1e+6)) + return s } diff --git a/internal/cc/feedback_adapter.go b/internal/cc/feedback_adapter.go index 0ae92318..fac753cf 100644 --- a/internal/cc/feedback_adapter.go +++ b/internal/cc/feedback_adapter.go @@ -16,7 +16,7 @@ import ( ) // TwccExtensionAttributesKey identifies the TWCC value in the attribute collection -// so we don't need to reparse +// so we don't need to reparse. const TwccExtensionAttributesKey = iota var ( @@ -31,7 +31,7 @@ type FeedbackAdapter struct { history *feedbackHistory } -// NewFeedbackAdapter returns a new FeedbackAdapter +// NewFeedbackAdapter returns a new FeedbackAdapter. func NewFeedbackAdapter() *FeedbackAdapter { return &FeedbackAdapter{history: newFeedbackHistory(250)} } @@ -48,6 +48,7 @@ func (f *FeedbackAdapter) onSentRFC8888(ts time.Time, header *rtp.Header, size i Arrival: time.Time{}, ECN: 0, }) + return nil } @@ -69,11 +70,12 @@ func (f *FeedbackAdapter) onSentTWCC(ts time.Time, extID uint8, header *rtp.Head Arrival: time.Time{}, ECN: 0, }) + return nil } // OnSent records that and when an outgoing packet was sent for later mapping to -// acknowledgments +// acknowledgments. func (f *FeedbackAdapter) OnSent(ts time.Time, header *rtp.Header, size int, attributes interceptor.Attributes) error { hdrExtensionID := attributes.Get(TwccExtensionAttributesKey) id, ok := hdrExtensionID.(uint8) @@ -84,7 +86,9 @@ func (f *FeedbackAdapter) OnSent(ts time.Time, header *rtp.Header, size int, att return f.onSentRFC8888(ts, header, size) } -func (f *FeedbackAdapter) unpackRunLengthChunk(start uint16, refTime time.Time, chunk *rtcp.RunLengthChunk, deltas []*rtcp.RecvDelta) (consumedDeltas int, nextRef time.Time, acks []Acknowledgment, err error) { +func (f *FeedbackAdapter) unpackRunLengthChunk( + start uint16, refTime time.Time, chunk *rtcp.RunLengthChunk, deltas []*rtcp.RecvDelta, +) (consumedDeltas int, nextRef time.Time, acks []Acknowledgment, err error) { result := make([]Acknowledgment, chunk.RunLength) deltaIndex := 0 @@ -108,17 +112,20 @@ func (f *FeedbackAdapter) unpackRunLengthChunk(start uint16, refTime time.Time, } resultIndex++ } + return deltaIndex, refTime, result, nil } -func (f *FeedbackAdapter) unpackStatusVectorChunk(start uint16, refTime time.Time, chunk *rtcp.StatusVectorChunk, deltas []*rtcp.RecvDelta) (consumedDeltas int, nextRef time.Time, acks []Acknowledgment, err error) { +func (f *FeedbackAdapter) unpackStatusVectorChunk( + start uint16, refTime time.Time, chunk *rtcp.StatusVectorChunk, deltas []*rtcp.RecvDelta, +) (consumedDeltas int, nextRef time.Time, acks []Acknowledgment, err error) { result := make([]Acknowledgment, len(chunk.SymbolList)) deltaIndex := 0 resultIndex := 0 for i, symbol := range chunk.SymbolList { key := feedbackHistoryKey{ ssrc: 0, - sequenceNumber: start + uint16(i), + sequenceNumber: start + uint16(i), //nolint:gosec // G115 } if ack, ok := f.history.get(key); ok { if symbol != rtcp.TypeTCCPacketNotReceived { @@ -139,7 +146,9 @@ func (f *FeedbackAdapter) unpackStatusVectorChunk(start uint16, refTime time.Tim // OnTransportCCFeedback converts incoming TWCC RTCP packet feedback to // Acknowledgments. -func (f *FeedbackAdapter) OnTransportCCFeedback(_ time.Time, feedback *rtcp.TransportLayerCC) ([]Acknowledgment, error) { +func (f *FeedbackAdapter) OnTransportCCFeedback( + _ time.Time, feedback *rtcp.TransportLayerCC, +) ([]Acknowledgment, error) { f.lock.Lock() defer f.lock.Unlock() @@ -158,7 +167,7 @@ func (f *FeedbackAdapter) OnTransportCCFeedback(_ time.Time, feedback *rtcp.Tran refTime = nextRefTime result = append(result, acks...) recvDeltas = recvDeltas[n:] - index = uint16(int(index) + len(acks)) + index = uint16(int(index) + len(acks)) //nolint:gosec // G115 case *rtcp.StatusVectorChunk: n, nextRefTime, acks, err := f.unpackStatusVectorChunk(index, refTime, chunk, recvDeltas) if err != nil { @@ -167,7 +176,7 @@ func (f *FeedbackAdapter) OnTransportCCFeedback(_ time.Time, feedback *rtcp.Tran refTime = nextRefTime result = append(result, acks...) recvDeltas = recvDeltas[n:] - index = uint16(int(index) + len(acks)) + index = uint16(int(index) + len(acks)) //nolint:gosec // G115 default: return nil, errInvalidFeedback } @@ -186,7 +195,7 @@ func (f *FeedbackAdapter) OnRFC8888Feedback(_ time.Time, feedback *rtcp.CCFeedba referenceTime := ntp.ToTime(uint64(feedback.ReportTimestamp) << 16) for _, rb := range feedback.ReportBlocks { for i, mb := range rb.MetricBlocks { - sequenceNumber := rb.BeginSequence + uint16(i) + sequenceNumber := rb.BeginSequence + uint16(i) //nolint:gosec // G115 key := feedbackHistoryKey{ ssrc: rb.MediaSSRC, sequenceNumber: sequenceNumber, @@ -201,6 +210,7 @@ func (f *FeedbackAdapter) OnRFC8888Feedback(_ time.Time, feedback *rtcp.CCFeedba } } } + return result } @@ -230,6 +240,7 @@ func (f *feedbackHistory) get(key feedbackHistoryKey) (Acknowledgment, bool) { return ack, true } } + return Acknowledgment{}, false } @@ -242,6 +253,7 @@ func (f *feedbackHistory) add(ack Acknowledgment) { if ent, ok := f.items[key]; ok { f.evictList.MoveToFront(ent) ent.Value = ack + return } // Add new diff --git a/internal/cc/feedback_adapter_test.go b/internal/cc/feedback_adapter_test.go index 31afa34a..40f4953a 100644 --- a/internal/cc/feedback_adapter_test.go +++ b/internal/cc/feedback_adapter_test.go @@ -426,6 +426,8 @@ func TestUnpackStatusVectorChunk(t *testing.T) { } func getPacketWithTransportCCExt(t *testing.T, sequenceNumber uint16) *rtp.Packet { + t.Helper() + pkt := rtp.Packet{ Header: rtp.Header{}, Payload: []byte{}, @@ -436,9 +438,11 @@ func getPacketWithTransportCCExt(t *testing.T, sequenceNumber uint16) *rtp.Packe b, err := ext.Marshal() assert.NoError(t, err) assert.NoError(t, pkt.SetExtension(hdrExtID, b)) + return &pkt } +//nolint:maintidx,cyclop func TestFeedbackAdapterTWCC(t *testing.T) { t.Run("empty", func(t *testing.T) { adapter := NewFeedbackAdapter() @@ -454,7 +458,10 @@ func TestFeedbackAdapterTWCC(t *testing.T) { for i := uint16(0); i < 22; i++ { pkt := getPacketWithTransportCCExt(t, i) headers = append(headers, pkt.Header) - assert.NoError(t, adapter.OnSent(t0, &pkt.Header, 1200, interceptor.Attributes{TwccExtensionAttributesKey: hdrExtID})) + assert.NoError( + t, + adapter.OnSent(t0, &pkt.Header, 1200, interceptor.Attributes{TwccExtensionAttributesKey: hdrExtID}), + ) } results, err := adapter.OnTransportCCFeedback(t0, &rtcp.TransportLayerCC{ Header: rtcp.Header{}, @@ -621,8 +628,14 @@ func TestFeedbackAdapterTWCC(t *testing.T) { adapter := NewFeedbackAdapter() pkt65535 := getPacketWithTransportCCExt(t, 65535) pkt0 := getPacketWithTransportCCExt(t, 0) - assert.NoError(t, adapter.OnSent(t0, &pkt65535.Header, 1200, interceptor.Attributes{TwccExtensionAttributesKey: hdrExtID})) - assert.NoError(t, adapter.OnSent(t0, &pkt0.Header, 1200, interceptor.Attributes{TwccExtensionAttributesKey: hdrExtID})) + assert.NoError( + t, + adapter.OnSent(t0, &pkt65535.Header, 1200, interceptor.Attributes{TwccExtensionAttributesKey: hdrExtID}), + ) + assert.NoError( + t, + adapter.OnSent(t0, &pkt0.Header, 1200, interceptor.Attributes{TwccExtensionAttributesKey: hdrExtID}), + ) results, err := adapter.OnTransportCCFeedback(t0, &rtcp.TransportLayerCC{ Header: rtcp.Header{}, @@ -684,7 +697,10 @@ func TestFeedbackAdapterTWCC(t *testing.T) { for i := uint16(0); i < 8; i++ { pkt := getPacketWithTransportCCExt(t, i) headers = append(headers, pkt.Header) - assert.NoError(t, adapter.OnSent(t0, &pkt.Header, 1200, interceptor.Attributes{TwccExtensionAttributesKey: hdrExtID})) + assert.NoError( + t, + adapter.OnSent(t0, &pkt.Header, 1200, interceptor.Attributes{TwccExtensionAttributesKey: hdrExtID}), + ) } results, err := adapter.OnTransportCCFeedback(t0, &rtcp.TransportLayerCC{ @@ -751,7 +767,10 @@ func TestFeedbackAdapterTWCC(t *testing.T) { t0 := time.Time{} for i := uint16(0); i < 20; i++ { pkt := getPacketWithTransportCCExt(t, i) - assert.NoError(t, adapter.OnSent(t0, &pkt.Header, 1200, interceptor.Attributes{TwccExtensionAttributesKey: hdrExtID})) + assert.NoError( + t, + adapter.OnSent(t0, &pkt.Header, 1200, interceptor.Attributes{TwccExtensionAttributesKey: hdrExtID}), + ) } packets, err := adapter.OnTransportCCFeedback(t0, &rtcp.TransportLayerCC{ Header: rtcp.Header{}, @@ -792,7 +811,10 @@ func TestFeedbackAdapterTWCC(t *testing.T) { t0 := time.Time{} for i := uint16(0); i < 20; i++ { pkt := getPacketWithTransportCCExt(t, i) - assert.NoError(t, adapter.OnSent(t0, &pkt.Header, 1200, interceptor.Attributes{TwccExtensionAttributesKey: hdrExtID})) + assert.NoError( + t, + adapter.OnSent(t0, &pkt.Header, 1200, interceptor.Attributes{TwccExtensionAttributesKey: hdrExtID}), + ) } packets, err := adapter.OnTransportCCFeedback(t0, &rtcp.TransportLayerCC{ Header: rtcp.Header{}, @@ -849,7 +871,10 @@ func TestFeedbackAdapterTWCC(t *testing.T) { t0 := time.Time{} for i := uint16(0); i < 20; i++ { pkt := getPacketWithTransportCCExt(t, i) - assert.NoError(t, adapter.OnSent(t0, &pkt.Header, 1200, interceptor.Attributes{TwccExtensionAttributesKey: hdrExtID})) + assert.NoError( + t, + adapter.OnSent(t0, &pkt.Header, 1200, interceptor.Attributes{TwccExtensionAttributesKey: hdrExtID}), + ) } //nolint:dupl @@ -916,7 +941,10 @@ func TestFeedbackAdapterTWCC(t *testing.T) { t0 := time.Time{} for i := uint16(1008); i < 1030; i++ { pkt := getPacketWithTransportCCExt(t, i) - assert.NoError(t, adapter.OnSent(t0, &pkt.Header, 1200, interceptor.Attributes{TwccExtensionAttributesKey: hdrExtID})) + assert.NoError( + t, + adapter.OnSent(t0, &pkt.Header, 1200, interceptor.Attributes{TwccExtensionAttributesKey: hdrExtID}), + ) } //nolint:dupl diff --git a/internal/ntp/ntp.go b/internal/ntp/ntp.go index 6d32bb5e..266f13d8 100644 --- a/internal/ntp/ntp.go +++ b/internal/ntp/ntp.go @@ -9,7 +9,7 @@ import ( "time" ) -// ToNTP converts a time.Time oboject to an uint64 NTP timestamp +// ToNTP converts a time.Time oboject to an uint64 NTP timestamp. func ToNTP(t time.Time) uint64 { // seconds since 1st January 1900 s := (float64(t.UnixNano()) / 1000000000) + 2208988800 @@ -17,18 +17,20 @@ func ToNTP(t time.Time) uint64 { // higher 32 bits are the integer part, lower 32 bits are the fractional part integerPart := uint32(s) fractionalPart := uint32((s - float64(integerPart)) * 0xFFFFFFFF) - return uint64(integerPart)<<32 | uint64(fractionalPart) + + return uint64(integerPart)<<32 | uint64(fractionalPart) //nolint:gosec // G115 } -// ToNTP32 converts a time.Time object to a uint32 NTP timestamp +// ToNTP32 converts a time.Time object to a uint32 NTP timestamp. func ToNTP32(t time.Time) uint32 { - return uint32(ToNTP(t) >> 16) + return uint32(ToNTP(t) >> 16) //nolint:gosec // G115 } -// ToTime converts a uint64 NTP timestamps to a time.Time object +// ToTime converts a uint64 NTP timestamps to a time.Time object. func ToTime(t uint64) time.Time { seconds := (t & 0xFFFFFFFF00000000) >> 32 fractional := float64(t&0x00000000FFFFFFFF) / float64(0xFFFFFFFF) + //nolint:gosec // G115 d := time.Duration(seconds)*time.Second + time.Duration(fractional*1e9)*time.Nanosecond return time.Unix(0, 0).Add(-2208988800 * time.Second).Add(d) @@ -40,5 +42,6 @@ func ToTime(t uint64) time.Time { func ToTime32(t uint32, reference time.Time) time.Time { referenceNTP := ToNTP(reference) & 0xFFFF000000000000 tu64 := ((uint64(t) << 16) & 0x0000FFFFFFFF0000) | referenceNTP + return ToTime(tu64) } diff --git a/internal/ntp/ntp_test.go b/internal/ntp/ntp_test.go index 73658095..2f668600 100644 --- a/internal/ntp/ntp_test.go +++ b/internal/ntp/ntp_test.go @@ -24,7 +24,10 @@ func TestNTPToTimeConverstion(t *testing.T) { } { t.Run(fmt.Sprintf("TimeToNTP/%v", i), func(t *testing.T) { assert.InDelta(t, cc.ts.UnixNano(), ToTime(ToNTP(cc.ts)).UnixNano(), float64(time.Millisecond.Nanoseconds())) - assert.InDelta(t, cc.ts.UnixNano(), ToTime32(ToNTP32(cc.ts), cc.ts).UnixNano(), float64(time.Millisecond.Nanoseconds())) + assert.InDelta( + t, + cc.ts.UnixNano(), ToTime32(ToNTP32(cc.ts), cc.ts).UnixNano(), float64(time.Millisecond.Nanoseconds()), + ) }) } } @@ -68,7 +71,8 @@ func TestNTPTime32(t *testing.T) { expected: 1 << 16, }, { - input: notSoLongAgo, + input: notSoLongAgo, + //nolint:gosec // G115 expected: uint32(uint(notSoLongAgo.Sub(zero).Seconds())&0xffff) << 16, }, { diff --git a/internal/rtpbuffer/packet_factory.go b/internal/rtpbuffer/packet_factory.go index d3f5a12d..3836acc3 100644 --- a/internal/rtpbuffer/packet_factory.go +++ b/internal/rtpbuffer/packet_factory.go @@ -14,19 +14,19 @@ import ( const rtxSsrcByteLength = 2 // PacketFactory allows custom logic around the handle of RTP Packets before they added to the RTPBuffer. -// The NoOpPacketFactory doesn't copy packets, while the RetainablePacket will take a copy before adding +// The NoOpPacketFactory doesn't copy packets, while the RetainablePacket will take a copy before adding. type PacketFactory interface { NewPacket(header *rtp.Header, payload []byte, rtxSsrc uint32, rtxPayloadType uint8) (*RetainablePacket, error) } -// PacketFactoryCopy is PacketFactory that takes a copy of packets when added to the RTPBuffer +// PacketFactoryCopy is PacketFactory that takes a copy of packets when added to the RTPBuffer. type PacketFactoryCopy struct { headerPool *sync.Pool payloadPool *sync.Pool rtxSequencer rtp.Sequencer } -// NewPacketFactoryCopy constructs a PacketFactory that takes a copy of packets when added to the RTPBuffer +// NewPacketFactoryCopy constructs a PacketFactory that takes a copy of packets when added to the RTPBuffer. func NewPacketFactoryCopy() *PacketFactoryCopy { return &PacketFactoryCopy{ headerPool: &sync.Pool{ @@ -37,6 +37,7 @@ func NewPacketFactoryCopy() *PacketFactoryCopy { payloadPool: &sync.Pool{ New: func() interface{} { buf := make([]byte, maxPayloadLen) + return &buf }, }, @@ -44,13 +45,17 @@ func NewPacketFactoryCopy() *PacketFactoryCopy { } } -// NewPacket constructs a new RetainablePacket that can be added to the RTPBuffer -func (m *PacketFactoryCopy) NewPacket(header *rtp.Header, payload []byte, rtxSsrc uint32, rtxPayloadType uint8) (*RetainablePacket, error) { +// NewPacket constructs a new RetainablePacket that can be added to the RTPBuffer. +// +//nolint:cyclop +func (m *PacketFactoryCopy) NewPacket( + header *rtp.Header, payload []byte, rtxSsrc uint32, rtxPayloadType uint8, +) (*RetainablePacket, error) { if len(payload) > maxPayloadLen { return nil, io.ErrShortBuffer } - p := &RetainablePacket{ + retainablePacket := &RetainablePacket{ onRelease: m.releasePacket, sequenceNumber: header.SequenceNumber, // new packets have retain count of 1 @@ -58,53 +63,53 @@ func (m *PacketFactoryCopy) NewPacket(header *rtp.Header, payload []byte, rtxSsr } var ok bool - p.header, ok = m.headerPool.Get().(*rtp.Header) + retainablePacket.header, ok = m.headerPool.Get().(*rtp.Header) if !ok { return nil, errFailedToCastHeaderPool } - *p.header = header.Clone() + *retainablePacket.header = header.Clone() if payload != nil { - p.buffer, ok = m.payloadPool.Get().(*[]byte) + retainablePacket.buffer, ok = m.payloadPool.Get().(*[]byte) if !ok { return nil, errFailedToCastPayloadPool } if rtxSsrc != 0 && rtxPayloadType != 0 { - size := copy((*p.buffer)[rtxSsrcByteLength:], payload) - p.payload = (*p.buffer)[:size+rtxSsrcByteLength] + size := copy((*retainablePacket.buffer)[rtxSsrcByteLength:], payload) + retainablePacket.payload = (*retainablePacket.buffer)[:size+rtxSsrcByteLength] } else { - size := copy(*p.buffer, payload) - p.payload = (*p.buffer)[:size] + size := copy(*retainablePacket.buffer, payload) + retainablePacket.payload = (*retainablePacket.buffer)[:size] } } if rtxSsrc != 0 && rtxPayloadType != 0 { if payload == nil { - p.buffer, ok = m.payloadPool.Get().(*[]byte) + retainablePacket.buffer, ok = m.payloadPool.Get().(*[]byte) if !ok { return nil, errFailedToCastPayloadPool } - p.payload = (*p.buffer)[:rtxSsrcByteLength] + retainablePacket.payload = (*retainablePacket.buffer)[:rtxSsrcByteLength] } // Write the original sequence number at the beginning of the payload. - binary.BigEndian.PutUint16(p.payload, p.header.SequenceNumber) + binary.BigEndian.PutUint16(retainablePacket.payload, retainablePacket.header.SequenceNumber) // Rewrite the SSRC. - p.header.SSRC = rtxSsrc + retainablePacket.header.SSRC = rtxSsrc // Rewrite the payload type. - p.header.PayloadType = rtxPayloadType + retainablePacket.header.PayloadType = rtxPayloadType // Rewrite the sequence number. - p.header.SequenceNumber = m.rtxSequencer.NextSequenceNumber() + retainablePacket.header.SequenceNumber = m.rtxSequencer.NextSequenceNumber() // Remove padding if present. - if p.header.Padding && p.payload != nil && len(p.payload) > 0 { - paddingLength := int(p.payload[len(p.payload)-1]) - p.header.Padding = false - p.payload = (*p.buffer)[:len(p.payload)-paddingLength] + if retainablePacket.header.Padding && retainablePacket.payload != nil && len(retainablePacket.payload) > 0 { + paddingLength := int(retainablePacket.payload[len(retainablePacket.payload)-1]) + retainablePacket.header.Padding = false + retainablePacket.payload = (*retainablePacket.buffer)[:len(retainablePacket.payload)-paddingLength] } } - return p, nil + return retainablePacket, nil } func (m *PacketFactoryCopy) releasePacket(header *rtp.Header, payload *[]byte) { @@ -114,11 +119,13 @@ func (m *PacketFactoryCopy) releasePacket(header *rtp.Header, payload *[]byte) { } } -// PacketFactoryNoOp is a PacketFactory implementation that doesn't copy packets +// PacketFactoryNoOp is a PacketFactory implementation that doesn't copy packets. type PacketFactoryNoOp struct{} -// NewPacket constructs a new RetainablePacket that can be added to the RTPBuffer -func (f *PacketFactoryNoOp) NewPacket(header *rtp.Header, payload []byte, _ uint32, _ uint8) (*RetainablePacket, error) { +// NewPacket constructs a new RetainablePacket that can be added to the RTPBuffer. +func (f *PacketFactoryNoOp) NewPacket( + header *rtp.Header, payload []byte, _ uint32, _ uint8, +) (*RetainablePacket, error) { return &RetainablePacket{ onRelease: f.releasePacket, count: 1, diff --git a/internal/rtpbuffer/retainable_packet.go b/internal/rtpbuffer/retainable_packet.go index 5a9afd79..82ade097 100644 --- a/internal/rtpbuffer/retainable_packet.go +++ b/internal/rtpbuffer/retainable_packet.go @@ -9,7 +9,7 @@ import ( "github.com/pion/rtp" ) -// RetainablePacket is a referenced counted RTP packet +// RetainablePacket is a referenced counted RTP packet. type RetainablePacket struct { onRelease func(*rtp.Header, *[]byte) @@ -23,17 +23,17 @@ type RetainablePacket struct { sequenceNumber uint16 } -// Header returns the RTP Header of the RetainablePacket +// Header returns the RTP Header of the RetainablePacket. func (p *RetainablePacket) Header() *rtp.Header { return p.header } -// Payload returns the RTP Payload of the RetainablePacket +// Payload returns the RTP Payload of the RetainablePacket. func (p *RetainablePacket) Payload() []byte { return p.payload } -// Retain increases the reference count of the RetainablePacket +// Retain increases the reference count of the RetainablePacket. func (p *RetainablePacket) Retain() error { p.countMu.Lock() defer p.countMu.Unlock() @@ -42,10 +42,11 @@ func (p *RetainablePacket) Retain() error { return errPacketReleased } p.count++ + return nil } -// Release decreases the reference count of the RetainablePacket and frees if needed +// Release decreases the reference count of the RetainablePacket and frees if needed. func (p *RetainablePacket) Release() { p.countMu.Lock() defer p.countMu.Unlock() diff --git a/internal/rtpbuffer/rtpbuffer.go b/internal/rtpbuffer/rtpbuffer.go index 1deefeea..94c7adfe 100644 --- a/internal/rtpbuffer/rtpbuffer.go +++ b/internal/rtpbuffer/rtpbuffer.go @@ -9,13 +9,14 @@ import ( ) const ( - // Uint16SizeHalf is half of a math.Uint16 + // Uint16SizeHalf is half of a math.Uint16. Uint16SizeHalf = 1 << 15 maxPayloadLen = 1460 ) -// RTPBuffer stores RTP packets and allows custom logic around the lifetime of them via the PacketFactory +// RTPBuffer stores RTP packets and allows custom logic +// around the lifetime of them via the PacketFactory. type RTPBuffer struct { packets []*RetainablePacket size uint16 @@ -23,13 +24,14 @@ type RTPBuffer struct { started bool } -// NewRTPBuffer constructs a new RTPBuffer +// NewRTPBuffer constructs a new RTPBuffer. func NewRTPBuffer(size uint16) (*RTPBuffer, error) { allowedSizes := make([]uint16, 0) correctSize := false for i := 0; i < 16; i++ { if size == 1<= Uint16SizeHalf { @@ -99,5 +102,6 @@ func (r *RTPBuffer) Get(seq uint16) *RetainablePacket { return nil } } + return pkt } diff --git a/internal/rtpbuffer/rtpbuffer_test.go b/internal/rtpbuffer/rtpbuffer_test.go index aa59aec3..0326d5dc 100644 --- a/internal/rtpbuffer/rtpbuffer_test.go +++ b/internal/rtpbuffer/rtpbuffer_test.go @@ -12,7 +12,10 @@ import ( func TestRTPBuffer(t *testing.T) { pm := NewPacketFactoryCopy() - for _, start := range []uint16{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 511, 512, 513, 32767, 32768, 32769, 65527, 65528, 65529, 65530, 65531, 65532, 65533, 65534, 65535} { + for _, start := range []uint16{ + 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 511, 512, 513, 32767, 32768, 32769, + 65527, 65528, 65529, 65530, 65531, 65532, 65533, 65534, 65535, + } { start := start sb, err := NewRTPBuffer(8) @@ -34,6 +37,7 @@ func TestRTPBuffer(t *testing.T) { packet := sb.Get(seq) if packet == nil { t.Errorf("packet not found: %d", seq) + continue } if packet.Header().SequenceNumber != seq { @@ -72,7 +76,10 @@ func TestRTPBuffer(t *testing.T) { func TestRTPBuffer_WithRTX(t *testing.T) { pm := NewPacketFactoryCopy() - for _, start := range []uint16{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 511, 512, 513, 32767, 32768, 32769, 65527, 65528, 65529, 65530, 65531, 65532, 65533, 65534, 65535} { + for _, start := range []uint16{ + 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 511, 512, 513, 32767, 32768, 32769, + 65527, 65528, 65529, 65530, 65531, 65532, 65533, 65534, 65535, + } { start := start sb, err := NewRTPBuffer(8) @@ -94,10 +101,14 @@ func TestRTPBuffer_WithRTX(t *testing.T) { packet := sb.Get(seq) if packet == nil { t.Errorf("packet not found: %d", seq) + continue } if packet.Header().SSRC != 1 && packet.Header().PayloadType != 1 { - t.Errorf("packet for %d returned with incorrect SSRC : %d and PayloadType: %d", seq, packet.Header().SSRC, packet.Header().PayloadType) + t.Errorf( + "packet for %d returned with incorrect SSRC : %d and PayloadType: %d", + seq, packet.Header().SSRC, packet.Header().PayloadType, + ) } packet.Release() } diff --git a/internal/sequencenumber/unwrapper.go b/internal/sequencenumber/unwrapper.go index 311ff09d..48500b3c 100644 --- a/internal/sequencenumber/unwrapper.go +++ b/internal/sequencenumber/unwrapper.go @@ -9,7 +9,7 @@ const ( breakpoint = 32768 // half of max uint16 ) -// Unwrapper stores an unwrapped sequence number +// Unwrapper stores an unwrapped sequence number. type Unwrapper struct { init bool lastUnwrapped int64 @@ -19,18 +19,20 @@ func isNewer(value, previous uint16) bool { if value-previous == breakpoint { return value > previous } + return value != previous && (value-previous) < breakpoint } -// Unwrap unwraps the next sequencenumber +// Unwrap unwraps the next sequencenumber. func (u *Unwrapper) Unwrap(i uint16) int64 { if !u.init { u.init = true u.lastUnwrapped = int64(i) + return u.lastUnwrapped } - lastWrapped := uint16(u.lastUnwrapped) + lastWrapped := uint16(u.lastUnwrapped) //nolint:gosec // G115 delta := int64(i - lastWrapped) if isNewer(i, lastWrapped) { if delta < 0 { @@ -41,5 +43,6 @@ func (u *Unwrapper) Unwrap(i uint16) int64 { } u.lastUnwrapped += delta + return u.lastUnwrapped } diff --git a/internal/test/mock_stream.go b/internal/test/mock_stream.go index bf96e31b..83d07323 100644 --- a/internal/test/mock_stream.go +++ b/internal/test/mock_stream.go @@ -32,21 +32,21 @@ type MockStream struct { rtpInModified chan RTPWithError } -// RTPWithError is used to send an rtp packet or an error on a channel +// RTPWithError is used to send an rtp packet or an error on a channel. type RTPWithError struct { Packet *rtp.Packet Err error } -// RTCPWithError is used to send a batch of rtcp packets or an error on a channel +// RTCPWithError is used to send a batch of rtcp packets or an error on a channel. type RTCPWithError struct { Packets []rtcp.Packet Err error } -// NewMockStream creates a new MockStream +// NewMockStream creates a new MockStream. func NewMockStream(info *interceptor.StreamInfo, i interceptor.Interceptor) *MockStream { //nolint - s := &MockStream{ + mockStream := &MockStream{ interceptor: i, rtcpIn: make(chan []rtcp.Packet, 1000), rtpIn: make(chan *rtp.Packet, 1000), @@ -55,147 +55,168 @@ func NewMockStream(info *interceptor.StreamInfo, i interceptor.Interceptor) *Moc rtcpInModified: make(chan RTCPWithError, 1000), rtpInModified: make(chan RTPWithError, 1000), } - s.rtcpWriter = i.BindRTCPWriter(interceptor.RTCPWriterFunc(func(pkts []rtcp.Packet, _ interceptor.Attributes) (int, error) { - select { - case s.rtcpOutModified <- pkts: - default: - } + mockStream.rtcpWriter = i.BindRTCPWriter( + interceptor.RTCPWriterFunc(func(pkts []rtcp.Packet, _ interceptor.Attributes) (int, error) { + select { + case mockStream.rtcpOutModified <- pkts: + default: + } - return 0, nil - })) - s.rtcpReader = i.BindRTCPReader(interceptor.RTCPReaderFunc(func(b []byte, a interceptor.Attributes) (int, interceptor.Attributes, error) { - pkts, ok := <-s.rtcpIn - if !ok { - return 0, nil, io.EOF - } + return 0, nil + }), + ) + mockStream.rtcpReader = i.BindRTCPReader(interceptor.RTCPReaderFunc( + func(b []byte, attrs interceptor.Attributes) (int, interceptor.Attributes, error) { + pkts, ok := <-mockStream.rtcpIn + if !ok { + return 0, nil, io.EOF + } - marshaled, err := rtcp.Marshal(pkts) - if err != nil { - return 0, nil, io.EOF - } else if len(marshaled) > len(b) { - return 0, nil, io.ErrShortBuffer - } + marshaled, err := rtcp.Marshal(pkts) + if err != nil { + return 0, nil, io.EOF + } else if len(marshaled) > len(b) { + return 0, nil, io.ErrShortBuffer + } - copy(b, marshaled) - return len(marshaled), a, err - })) - s.rtpWriter = i.BindLocalStream(info, interceptor.RTPWriterFunc(func(header *rtp.Header, payload []byte, _ interceptor.Attributes) (int, error) { - select { - case s.rtpOutModified <- &rtp.Packet{Header: *header, Payload: payload}: - default: - } + copy(b, marshaled) + + return len(marshaled), attrs, err + }, + )) + mockStream.rtpWriter = i.BindLocalStream( + info, interceptor.RTPWriterFunc( + func(header *rtp.Header, payload []byte, _ interceptor.Attributes) (int, error) { + select { + case mockStream.rtpOutModified <- &rtp.Packet{Header: *header, Payload: payload}: + default: + } - return 0, nil - })) - s.rtpReader = i.BindRemoteStream(info, interceptor.RTPReaderFunc(func(b []byte, a interceptor.Attributes) (int, interceptor.Attributes, error) { - p, ok := <-s.rtpIn - if !ok { - return 0, nil, io.EOF - } + return 0, nil + }, + ), + ) + mockStream.rtpReader = i.BindRemoteStream( + info, interceptor.RTPReaderFunc( + func(b []byte, attrs interceptor.Attributes) (int, interceptor.Attributes, error) { + p, ok := <-mockStream.rtpIn + if !ok { + return 0, nil, io.EOF + } - marshaled, err := p.Marshal() - if err != nil { - return 0, nil, io.EOF - } else if len(marshaled) > len(b) { - return 0, nil, io.ErrShortBuffer - } + marshaled, err := p.Marshal() + if err != nil { + return 0, nil, io.EOF + } else if len(marshaled) > len(b) { + return 0, nil, io.ErrShortBuffer + } + + copy(b, marshaled) - copy(b, marshaled) - return len(marshaled), a, err - })) + return len(marshaled), attrs, err + }, + ), + ) go func() { buf := make([]byte, 1500) for { - i, _, err := s.rtcpReader.Read(buf, interceptor.Attributes{}) + i, _, err := mockStream.rtcpReader.Read(buf, interceptor.Attributes{}) if err != nil { if !errors.Is(err, io.EOF) { - s.rtcpInModified <- RTCPWithError{Err: err} + mockStream.rtcpInModified <- RTCPWithError{Err: err} } + return } pkts, err := rtcp.Unmarshal(buf[:i]) if err != nil { - s.rtcpInModified <- RTCPWithError{Err: err} + mockStream.rtcpInModified <- RTCPWithError{Err: err} + return } - s.rtcpInModified <- RTCPWithError{Packets: pkts} + mockStream.rtcpInModified <- RTCPWithError{Packets: pkts} } }() go func() { buf := make([]byte, 1500) for { - i, _, err := s.rtpReader.Read(buf, interceptor.Attributes{}) + i, _, err := mockStream.rtpReader.Read(buf, interceptor.Attributes{}) if err != nil { if err.Error() == "attempt to pop while buffering" { continue } if errors.Is(err, io.EOF) { - s.rtpInModified <- RTPWithError{Err: err} + mockStream.rtpInModified <- RTPWithError{Err: err} } + return } p := &rtp.Packet{} if err := p.Unmarshal(buf[:i]); err != nil { - s.rtpInModified <- RTPWithError{Err: err} + mockStream.rtpInModified <- RTPWithError{Err: err} + return } - s.rtpInModified <- RTPWithError{Packet: p} + mockStream.rtpInModified <- RTPWithError{Packet: p} } }() - return s + return mockStream } -// WriteRTCP writes a batch of rtcp packet to the stream, using the interceptor +// WriteRTCP writes a batch of rtcp packet to the stream, using the interceptor. func (s *MockStream) WriteRTCP(pkts []rtcp.Packet) error { _, err := s.rtcpWriter.Write(pkts, interceptor.Attributes{}) + return err } -// WriteRTP writes an rtp packet to the stream, using the interceptor +// WriteRTP writes an rtp packet to the stream, using the interceptor. func (s *MockStream) WriteRTP(p *rtp.Packet) error { _, err := s.rtpWriter.Write(&p.Header, p.Payload, interceptor.Attributes{}) + return err } -// ReceiveRTCP schedules a new rtcp batch, so it can be read by the stream +// ReceiveRTCP schedules a new rtcp batch, so it can be read by the stream. func (s *MockStream) ReceiveRTCP(pkts []rtcp.Packet) { s.rtcpIn <- pkts } -// ReceiveRTP schedules a rtp packet, so it can be read by the stream +// ReceiveRTP schedules a rtp packet, so it can be read by the stream. func (s *MockStream) ReceiveRTP(packet *rtp.Packet) { s.rtpIn <- packet } -// WrittenRTCP returns a channel containing the rtcp batches written, modified by the interceptor +// WrittenRTCP returns a channel containing the rtcp batches written, modified by the interceptor. func (s *MockStream) WrittenRTCP() chan []rtcp.Packet { return s.rtcpOutModified } -// WrittenRTP returns a channel containing rtp packets written, modified by the interceptor +// WrittenRTP returns a channel containing rtp packets written, modified by the interceptor. func (s *MockStream) WrittenRTP() chan *rtp.Packet { return s.rtpOutModified } -// ReadRTCP returns a channel containing the rtcp batched read, modified by the interceptor +// ReadRTCP returns a channel containing the rtcp batched read, modified by the interceptor. func (s *MockStream) ReadRTCP() chan RTCPWithError { return s.rtcpInModified } -// ReadRTP returns a channel containing the rtp packets read, modified by the interceptor +// ReadRTP returns a channel containing the rtp packets read, modified by the interceptor. func (s *MockStream) ReadRTP() chan RTPWithError { return s.rtpInModified } -// Close closes the stream and the underlying interceptor +// Close closes the stream and the underlying interceptor. func (s *MockStream) Close() error { close(s.rtcpIn) close(s.rtpIn) + return s.interceptor.Close() } diff --git a/internal/test/mock_stream_test.go b/internal/test/mock_stream_test.go index 819c2a33..ae6da3c2 100644 --- a/internal/test/mock_stream_test.go +++ b/internal/test/mock_stream_test.go @@ -13,38 +13,39 @@ import ( "github.com/stretchr/testify/assert" ) +//nolint:cyclop func TestMockStream(t *testing.T) { - s := NewMockStream(&interceptor.StreamInfo{}, &interceptor.NoOp{}) + mockStream := NewMockStream(&interceptor.StreamInfo{}, &interceptor.NoOp{}) - assert.NoError(t, s.WriteRTCP([]rtcp.Packet{&rtcp.PictureLossIndication{}})) + assert.NoError(t, mockStream.WriteRTCP([]rtcp.Packet{&rtcp.PictureLossIndication{}})) select { - case <-s.WrittenRTCP(): + case <-mockStream.WrittenRTCP(): case <-time.After(10 * time.Millisecond): t.Error("rtcp packet written but not found") } select { - case <-s.WrittenRTCP(): + case <-mockStream.WrittenRTCP(): t.Error("single rtcp packet written, but multiple found") case <-time.After(10 * time.Millisecond): } - assert.NoError(t, s.WriteRTP(&rtp.Packet{})) + assert.NoError(t, mockStream.WriteRTP(&rtp.Packet{})) select { - case <-s.WrittenRTP(): + case <-mockStream.WrittenRTP(): case <-time.After(10 * time.Millisecond): t.Error("rtp packet written but not found") } select { - case <-s.WrittenRTP(): + case <-mockStream.WrittenRTP(): t.Error("single rtp packet written, but multiple found") case <-time.After(10 * time.Millisecond): } - s.ReceiveRTCP([]rtcp.Packet{&rtcp.PictureLossIndication{}}) + mockStream.ReceiveRTCP([]rtcp.Packet{&rtcp.PictureLossIndication{}}) select { - case r := <-s.ReadRTCP(): + case r := <-mockStream.ReadRTCP(): if r.Err != nil { t.Errorf("read rtcp returned error: %v", r.Err) } @@ -52,14 +53,14 @@ func TestMockStream(t *testing.T) { t.Error("rtcp packet received but not read") } select { - case r := <-s.ReadRTCP(): + case r := <-mockStream.ReadRTCP(): t.Errorf("single rtcp packet received, but multiple read: %v", r) case <-time.After(10 * time.Millisecond): } - s.ReceiveRTP(&rtp.Packet{}) + mockStream.ReceiveRTP(&rtp.Packet{}) select { - case r := <-s.ReadRTP(): + case r := <-mockStream.ReadRTP(): if r.Err != nil { t.Errorf("read rtcp returned error: %v", r.Err) } @@ -67,10 +68,10 @@ func TestMockStream(t *testing.T) { t.Error("rtp packet received but not read") } select { - case r := <-s.ReadRTP(): + case r := <-mockStream.ReadRTP(): t.Errorf("single rtp packet received, but multiple read: %v", r) case <-time.After(10 * time.Millisecond): } - assert.NoError(t, s.Close()) + assert.NoError(t, mockStream.Close()) } diff --git a/internal/test/mock_ticker.go b/internal/test/mock_ticker.go index 9a306c2a..f8429a47 100644 --- a/internal/test/mock_ticker.go +++ b/internal/test/mock_ticker.go @@ -16,12 +16,12 @@ type MockTicker struct { func (t *MockTicker) Stop() { } -// Ch returns the tickers channel +// Ch returns the tickers channel. func (t *MockTicker) Ch() <-chan time.Time { return t.C } -// Tick sends now to the channel +// Tick sends now to the channel. func (t *MockTicker) Tick(now time.Time) { t.C <- now } diff --git a/internal/test/mock_time.go b/internal/test/mock_time.go index e22f36e3..b1702823 100644 --- a/internal/test/mock_time.go +++ b/internal/test/mock_time.go @@ -25,5 +25,6 @@ func (t *MockTime) SetNow(n time.Time) { func (t *MockTime) Now() time.Time { t.m.RLock() defer t.m.RUnlock() + return t.curNow } diff --git a/noop.go b/noop.go index b0fc2a69..964a330e 100644 --- a/noop.go +++ b/noop.go @@ -28,7 +28,8 @@ func (i *NoOp) BindLocalStream(_ *StreamInfo, writer RTPWriter) RTPWriter { // UnbindLocalStream is called when the Stream is removed. It can be used to clean up any data related to that track. func (i *NoOp) UnbindLocalStream(_ *StreamInfo) {} -// BindRemoteStream lets you modify any incoming RTP packets. It is called once for per RemoteStream. The returned method +// BindRemoteStream lets you modify any incoming RTP packets. +// It is called once for per RemoteStream. The returned method // will be called once per rtp packet. func (i *NoOp) BindRemoteStream(_ *StreamInfo, reader RTPReader) RTPReader { return reader diff --git a/pkg/cc/interceptor.go b/pkg/cc/interceptor.go index 252ab29f..2d478d68 100644 --- a/pkg/cc/interceptor.go +++ b/pkg/cc/interceptor.go @@ -11,10 +11,10 @@ import ( "github.com/pion/rtcp" ) -// Option can be used to set initial options on CC interceptors +// Option can be used to set initial options on CC interceptors. type Option func(*Interceptor) error -// BandwidthEstimatorFactory creates new BandwidthEstimators +// BandwidthEstimatorFactory creates new BandwidthEstimators. type BandwidthEstimatorFactory func() (BandwidthEstimator, error) // BandwidthEstimator is the interface that will be returned by a @@ -30,23 +30,24 @@ type BandwidthEstimator interface { } // NewPeerConnectionCallback returns the BandwidthEstimator for the -// PeerConnection with id +// PeerConnection with id. type NewPeerConnectionCallback func(id string, estimator BandwidthEstimator) -// InterceptorFactory is a factory for CC interceptors +// InterceptorFactory is a factory for CC interceptors. type InterceptorFactory struct { opts []Option bweFactory func() (BandwidthEstimator, error) addPeerConnection NewPeerConnectionCallback } -// NewInterceptor returns a new CC interceptor factory +// NewInterceptor returns a new CC interceptor factory. func NewInterceptor(factory BandwidthEstimatorFactory, opts ...Option) (*InterceptorFactory, error) { if factory == nil { factory = func() (BandwidthEstimator, error) { return gcc.NewSendSideBWE() } } + return &InterceptorFactory{ opts: opts, bweFactory: factory, @@ -60,13 +61,13 @@ func (f *InterceptorFactory) OnNewPeerConnection(cb NewPeerConnectionCallback) { f.addPeerConnection = cb } -// NewInterceptor returns a new CC interceptor +// NewInterceptor returns a new CC interceptor. func (f *InterceptorFactory) NewInterceptor(id string) (interceptor.Interceptor, error) { bwe, err := f.bweFactory() if err != nil { return nil, err } - i := &Interceptor{ + interceptorInstance := &Interceptor{ NoOp: interceptor.NoOp{}, estimator: bwe, feedback: make(chan []rtcp.Packet), @@ -74,18 +75,19 @@ func (f *InterceptorFactory) NewInterceptor(id string) (interceptor.Interceptor, } for _, opt := range f.opts { - if err := opt(i); err != nil { + if err := opt(interceptorInstance); err != nil { return nil, err } } if f.addPeerConnection != nil { - f.addPeerConnection(id, i.estimator) + f.addPeerConnection(id, interceptorInstance.estimator) } - return i, nil + + return interceptorInstance, nil } -// Interceptor implements Google Congestion Control +// Interceptor implements Google Congestion Control. type Interceptor struct { interceptor.NoOp estimator BandwidthEstimator @@ -124,7 +126,9 @@ func (c *Interceptor) BindRTCPReader(reader interceptor.RTCPReader) interceptor. // BindLocalStream lets you modify any outgoing RTP packets. It is called once // for per LocalStream. The returned method will be called once per rtp packet. -func (c *Interceptor) BindLocalStream(info *interceptor.StreamInfo, writer interceptor.RTPWriter) interceptor.RTPWriter { +func (c *Interceptor) BindLocalStream( + info *interceptor.StreamInfo, writer interceptor.RTPWriter, +) interceptor.RTPWriter { return c.estimator.AddStream(info, writer) } diff --git a/pkg/flexfec/encoder_interceptor.go b/pkg/flexfec/encoder_interceptor.go index 0210523c..a4dc0c64 100644 --- a/pkg/flexfec/encoder_interceptor.go +++ b/pkg/flexfec/encoder_interceptor.go @@ -39,41 +39,46 @@ func (r *FecInterceptorFactory) NewInterceptor(_ string) (interceptor.Intercepto packetBuffer: make([]rtp.Packet, 0), minNumMediaPackets: 5, } + return interceptor, nil } // BindLocalStream lets you modify any outgoing RTP packets. It is called once for per LocalStream. The returned method // will be called once per rtp packet. -func (r *FecInterceptor) BindLocalStream(info *interceptor.StreamInfo, writer interceptor.RTPWriter) interceptor.RTPWriter { +func (r *FecInterceptor) BindLocalStream( + info *interceptor.StreamInfo, writer interceptor.RTPWriter, +) interceptor.RTPWriter { // Chromium supports version flexfec-03 of existing draft, this is the one we will configure by default // although we should support configuring the latest (flexfec-20) as well. r.flexFecEncoder = NewFlexEncoder03(info.PayloadType, info.SSRC) - return interceptor.RTPWriterFunc(func(header *rtp.Header, payload []byte, attributes interceptor.Attributes) (int, error) { - r.packetBuffer = append(r.packetBuffer, rtp.Packet{ - Header: *header, - Payload: payload, - }) + return interceptor.RTPWriterFunc( + func(header *rtp.Header, payload []byte, attributes interceptor.Attributes) (int, error) { + r.packetBuffer = append(r.packetBuffer, rtp.Packet{ + Header: *header, + Payload: payload, + }) - // Send the media RTP packet - result, err := writer.Write(header, payload, attributes) + // Send the media RTP packet + result, err := writer.Write(header, payload, attributes) - // Send the FEC packets - var fecPackets []rtp.Packet - if len(r.packetBuffer) == int(r.minNumMediaPackets) { - fecPackets = r.flexFecEncoder.EncodeFec(r.packetBuffer, 2) + // Send the FEC packets + var fecPackets []rtp.Packet + if len(r.packetBuffer) == int(r.minNumMediaPackets) { + fecPackets = r.flexFecEncoder.EncodeFec(r.packetBuffer, 2) - for i := range fecPackets { - fecResult, fecErr := writer.Write(&(fecPackets[i].Header), fecPackets[i].Payload, attributes) + for i := range fecPackets { + fecResult, fecErr := writer.Write(&(fecPackets[i].Header), fecPackets[i].Payload, attributes) - if fecErr != nil && fecResult == 0 { - break + if fecErr != nil && fecResult == 0 { + break + } } + // Reset the packet buffer now that we've sent the corresponding FEC packets. + r.packetBuffer = nil } - // Reset the packet buffer now that we've sent the corresponding FEC packets. - r.packetBuffer = nil - } - return result, err - }) + return result, err + }, + ) } diff --git a/pkg/flexfec/flexfec_coverage.go b/pkg/flexfec/flexfec_coverage.go index c1e463dc..e82feffb 100644 --- a/pkg/flexfec/flexfec_coverage.go +++ b/pkg/flexfec/flexfec_coverage.go @@ -32,7 +32,7 @@ type ProtectionCoverage struct { // Fec packets that we will be generating to cover the list of mediaPackets. This allows us to know // how big the underlying map should be. func NewCoverage(mediaPackets []rtp.Packet, numFecPackets uint32) *ProtectionCoverage { - numMediaPackets := uint32(len(mediaPackets)) + numMediaPackets := uint32(len(mediaPackets)) //nolint:gosec // G115 // Basic sanity checks if numMediaPackets <= 0 || numMediaPackets > MaxMediaPackets { @@ -53,13 +53,14 @@ func NewCoverage(mediaPackets []rtp.Packet, numFecPackets uint32) *ProtectionCov } coverage.UpdateCoverage(mediaPackets, numFecPackets) + return coverage } // UpdateCoverage updates the ProtectionCoverage object with new bitmasks accounting for the numFecPackets // we want to use to protect the batch media packets. func (p *ProtectionCoverage) UpdateCoverage(mediaPackets []rtp.Packet, numFecPackets uint32) { - numMediaPackets := uint32(len(mediaPackets)) + numMediaPackets := uint32(len(mediaPackets)) //nolint:gosec // G115 // Basic sanity checks if numMediaPackets <= 0 || numMediaPackets > MaxMediaPackets { @@ -111,6 +112,7 @@ func (p *ProtectionCoverage) GetCoveredBy(fecPacketIndex uint32) *util.MediaPack coverage = append(coverage, mediaPacketIndex) } } + return util.NewMediaPacketIterator(p.mediaPackets, coverage) } @@ -120,7 +122,8 @@ func (p *ProtectionCoverage) ExtractMask1(fecPacketIndex uint32) uint16 { mask := p.packetMasks[fecPacketIndex] // We get the first 16 bits (64 - 16 -> shift by 48) and we shift once more for K field mask1 := mask.Lo >> 49 - return uint16(mask1) + + return uint16(mask1) //nolint:gosec // G115 } // ExtractMask2 returns the second section of the bitmask as defined by the FEC header. @@ -131,7 +134,8 @@ func (p *ProtectionCoverage) ExtractMask2(fecPacketIndex uint32) uint32 { mask2 := mask.Lo << 15 // We get the first 31 bits (64 - 31 -> shift by 33) and we shift once more for K field mask2 >>= 34 - return uint32(mask2) + + return uint32(mask2) //nolint:gosec // G115 } // ExtractMask3 returns the third section of the bitmask as defined by the FEC header. @@ -142,6 +146,7 @@ func (p *ProtectionCoverage) ExtractMask3(fecPacketIndex uint32) uint64 { maskLo := mask.Lo << 46 maskHi := mask.Hi >> 18 mask3 := maskLo | maskHi + return mask3 } @@ -155,5 +160,6 @@ func (p *ProtectionCoverage) ExtractMask3_03(fecPacketIndex uint32) uint64 { mask3 := maskLo | maskHi // We shift once for the K bit. mask3 >>= 1 + return mask3 } diff --git a/pkg/flexfec/flexfec_encoder.go b/pkg/flexfec/flexfec_encoder.go index 479832e5..6fd05d3b 100644 --- a/pkg/flexfec/flexfec_encoder.go +++ b/pkg/flexfec/flexfec_encoder.go @@ -92,10 +92,17 @@ func (flex *FlexEncoder20) encodeFlexFecPacket(fecPacketIndex uint32, mediaBaseS Payload: append(flexFecHeader, flexFecRepairPayload...), } flex.fecBaseSn++ + return packet } -func (flex *FlexEncoder20) encodeFlexFecHeader(mediaPackets *util.MediaPacketIterator, mask1 uint16, optionalMask2 uint32, optionalMask3 uint64, mediaBaseSn uint16) []byte { +func (flex *FlexEncoder20) encodeFlexFecHeader( + mediaPackets *util.MediaPacketIterator, + mask1 uint16, + optionalMask2 uint32, + optionalMask3 uint64, + mediaBaseSn uint16, +) []byte { /* 0 1 2 3 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 @@ -145,9 +152,9 @@ func (flex *FlexEncoder20) encodeFlexFecHeader(mediaPackets *util.MediaPacketIte flexFecHeader[1] ^= tmpMediaPacketBuf[1] // XOR the length recovery field - lengthRecoveryVal := uint16(mediaPacket.MarshalSize() - BaseRTPHeaderSize) - flexFecHeader[2] ^= uint8(lengthRecoveryVal >> 8) - flexFecHeader[3] ^= uint8(lengthRecoveryVal) + lengthRecoveryVal := uint16(mediaPacket.MarshalSize() - BaseRTPHeaderSize) //nolint:gosec // G115 + flexFecHeader[2] ^= uint8(lengthRecoveryVal >> 8) //nolint:gosec // G115 + flexFecHeader[3] ^= uint8(lengthRecoveryVal) //nolint:gosec // G115 // XOR the 5th to 8th bytes of the header: the timestamp field flexFecHeader[4] ^= flexFecHeader[4] @@ -170,6 +177,7 @@ func (flex *FlexEncoder20) encodeFlexFecHeader(mediaPackets *util.MediaPacketIte binary.BigEndian.PutUint64(flexFecHeader[16:24], optionalMask3) flexFecHeader[12] |= 0b10000000 } + return flexFecHeader } @@ -190,5 +198,6 @@ func (flex *FlexEncoder20) encodeFlexFecRepairPayload(mediaPackets *util.MediaPa flexFecPayload[byteIndex] ^= mediaPacketPayload[byteIndex] } } + return flexFecPayload } diff --git a/pkg/flexfec/flexfec_encoder_03.go b/pkg/flexfec/flexfec_encoder_03.go index 15958a4b..ef912d48 100644 --- a/pkg/flexfec/flexfec_encoder_03.go +++ b/pkg/flexfec/flexfec_encoder_03.go @@ -85,10 +85,17 @@ func (flex *FlexEncoder03) encodeFlexFecPacket(fecPacketIndex uint32, mediaBaseS Payload: append(flexFecHeader, flexFecRepairPayload...), } flex.fecBaseSn++ + return packet } -func (flex *FlexEncoder03) encodeFlexFecHeader(mediaPackets *util.MediaPacketIterator, mask1 uint16, optionalMask2 uint32, optionalMask3 uint64, mediaBaseSn uint16) []byte { +func (flex *FlexEncoder03) encodeFlexFecHeader( + mediaPackets *util.MediaPacketIterator, + mask1 uint16, + optionalMask2 uint32, + optionalMask3 uint64, + mediaBaseSn uint16, +) []byte { /* 0 1 2 3 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 @@ -149,9 +156,9 @@ func (flex *FlexEncoder03) encodeFlexFecHeader(mediaPackets *util.MediaPacketIte flexFecHeader[0] &= 0b00111111 // XOR the length recovery field - lengthRecoveryVal := uint16(mediaPacket.MarshalSize() - BaseRTPHeaderSize) - flexFecHeader[2] ^= uint8(lengthRecoveryVal >> 8) - flexFecHeader[3] ^= uint8(lengthRecoveryVal) + lengthRecoveryVal := uint16(mediaPacket.MarshalSize() - BaseRTPHeaderSize) //nolint:gosec // G115 + flexFecHeader[2] ^= uint8(lengthRecoveryVal >> 8) //nolint:gosec // G115 + flexFecHeader[3] ^= uint8(lengthRecoveryVal) //nolint:gosec // G115 // XOR the 5th to 8th bytes of the header: the timestamp field flexFecHeader[4] ^= tmpMediaPacketBuf[4] @@ -179,6 +186,7 @@ func (flex *FlexEncoder03) encodeFlexFecHeader(mediaPackets *util.MediaPacketIte if optionalMask2 == 0 { flexFecHeader[18] |= 0b10000000 + return flexFecHeader } binary.BigEndian.PutUint32(flexFecHeader[20:24], optionalMask2) @@ -209,5 +217,6 @@ func (flex *FlexEncoder03) encodeFlexFecRepairPayload(mediaPackets *util.MediaPa flexFecPayload[byteIndex] ^= mediaPacketPayload[byteIndex] } } + return flexFecPayload } diff --git a/pkg/flexfec/util/bitarray.go b/pkg/flexfec/util/bitarray.go index c081f0a4..0840aabf 100644 --- a/pkg/flexfec/util/bitarray.go +++ b/pkg/flexfec/util/bitarray.go @@ -33,6 +33,7 @@ func (b *BitArray) GetBit(bitIndex uint32) uint8 { if result > 0 { return 1 } + return 0 } @@ -41,5 +42,6 @@ func (b *BitArray) GetBit(bitIndex uint32) uint8 { if result > 0 { return 1 } + return 0 } diff --git a/pkg/flexfec/util/media_packet_iterator.go b/pkg/flexfec/util/media_packet_iterator.go index 3099a39f..16f93f67 100644 --- a/pkg/flexfec/util/media_packet_iterator.go +++ b/pkg/flexfec/util/media_packet_iterator.go @@ -25,6 +25,7 @@ func NewMediaPacketIterator(mediaPackets []rtp.Packet, coveredIndices []uint32) // Reset sets the starting iterating index back to 0. func (m *MediaPacketIterator) Reset() *MediaPacketIterator { m.nextIndex = 0 + return m } @@ -41,6 +42,7 @@ func (m *MediaPacketIterator) Next() *rtp.Packet { } packet := m.mediaPackets[m.nextIndex] m.nextIndex++ + return &packet } diff --git a/pkg/gcc/adaptive_threshold.go b/pkg/gcc/adaptive_threshold.go index 61d34d8f..e831c1db 100644 --- a/pkg/gcc/adaptive_threshold.go +++ b/pkg/gcc/adaptive_threshold.go @@ -31,7 +31,7 @@ func setInitialThreshold(t time.Duration) adaptiveThresholdOption { // See https://datatracker.ietf.org/doc/html/draft-ietf-rmcat-gcc-02#section-5.4 // or [Analysis and Design of the Google Congestion Control for Web Real-time // Communication (WebRTC)](https://c3lab.poliba.it/images/6/65/Gcc-analysis.pdf) -// for a more detailed description +// for a more detailed description. type adaptiveThreshold struct { thresh time.Duration overuseCoefficientUp float64 @@ -43,7 +43,7 @@ type adaptiveThreshold struct { } // newAdaptiveThreshold initializes a new adaptiveThreshold with default -// values taken from draft-ietf-rmcat-gcc-02 +// values taken from draft-ietf-rmcat-gcc-02. func newAdaptiveThreshold(opts ...adaptiveThresholdOption) *adaptiveThreshold { at := &adaptiveThreshold{ thresh: time.Duration(12500 * float64(time.Microsecond)), @@ -57,6 +57,7 @@ func newAdaptiveThreshold(opts ...adaptiveThresholdOption) *adaptiveThreshold { for _, opt := range opts { opt(at) } + return at } @@ -74,6 +75,7 @@ func (a *adaptiveThreshold) compare(estimate, _ time.Duration) (usage, time.Dura } thresh := a.thresh a.update(t) + return use, t, thresh } @@ -85,6 +87,7 @@ func (a *adaptiveThreshold) update(estimate time.Duration) { absEstimate := time.Duration(math.Abs(float64(estimate.Microseconds()))) * time.Microsecond if absEstimate > a.thresh+15*time.Millisecond { a.lastUpdate = now + return } k := a.overuseCoefficientUp @@ -92,7 +95,9 @@ func (a *adaptiveThreshold) update(estimate time.Duration) { k = a.overuseCoefficientDown } maxTimeDelta := 100 * time.Millisecond - timeDelta := time.Duration(minInt(int(now.Sub(a.lastUpdate).Milliseconds()), int(maxTimeDelta.Milliseconds()))) * time.Millisecond + timeDelta := time.Duration( + minInt(int(now.Sub(a.lastUpdate).Milliseconds()), int(maxTimeDelta.Milliseconds())), + ) * time.Millisecond d := absEstimate - a.thresh add := k * float64(d.Milliseconds()) * float64(timeDelta.Milliseconds()) a.thresh += time.Duration(add*1000) * time.Microsecond diff --git a/pkg/gcc/arrival_group.go b/pkg/gcc/arrival_group.go index 5aa1d89d..bf0f9e4c 100644 --- a/pkg/gcc/arrival_group.go +++ b/pkg/gcc/arrival_group.go @@ -34,5 +34,6 @@ func (g arrivalGroup) String() string { s += fmt.Sprintf("\tARRIVAL:\t%v\n", int64(float64(g.arrival.UnixNano())/1e+6)) s += fmt.Sprintf("\tDEPARTURE:\t%v\n", int64(float64(g.departure.UnixNano())/1e+6)) s += fmt.Sprintf("\tPACKETS:\n%v\n", g.packets) + return s } diff --git a/pkg/gcc/arrival_group_accumulator.go b/pkg/gcc/arrival_group_accumulator.go index c568ffdc..d16a1dfb 100644 --- a/pkg/gcc/arrival_group_accumulator.go +++ b/pkg/gcc/arrival_group_accumulator.go @@ -31,6 +31,7 @@ func (a *arrivalGroupAccumulator) run(in <-chan []cc.Acknowledgment, agWriter fu if !init { group = newArrivalGroup(next) init = true + continue } if next.Arrival.Before(group.arrival) { @@ -42,6 +43,7 @@ func (a *arrivalGroupAccumulator) run(in <-chan []cc.Acknowledgment, agWriter fu // constitute a group. if interDepartureTimePkt(group, next) <= a.interDepartureThreshold { group.add(next) + continue } @@ -51,6 +53,7 @@ func (a *arrivalGroupAccumulator) run(in <-chan []cc.Acknowledgment, agWriter fu if interArrivalTimePkt(group, next) <= a.interArrivalThreshold && interGroupDelayVariationPkt(group, next) < a.interGroupDelayVariationTreshold { group.add(next) + continue } @@ -69,6 +72,7 @@ func interDepartureTimePkt(group arrivalGroup, ack cc.Acknowledgment) time.Durat if len(group.packets) == 0 { return 0 } + return ack.Departure.Sub(group.departure) } diff --git a/pkg/gcc/delay_based_bwe.go b/pkg/gcc/delay_based_bwe.go index 0ae8f08c..62ef620f 100644 --- a/pkg/gcc/delay_based_bwe.go +++ b/pkg/gcc/delay_based_bwe.go @@ -12,7 +12,7 @@ import ( ) // DelayStats contains some internal statistics of the delay based congestion -// controller +// controller. type DelayStats struct { Measurement time.Duration Estimate time.Duration @@ -47,7 +47,7 @@ type delayControllerConfig struct { maxBitrate int } -func newDelayController(c delayControllerConfig) *delayController { +func newDelayController(delayConfig delayControllerConfig) *delayController { ackPipe := make(chan []cc.Acknowledgment) ackRatePipe := make(chan []cc.Acknowledgment) @@ -61,12 +61,15 @@ func newDelayController(c delayControllerConfig) *delayController { log: logging.NewDefaultLoggerFactory().NewLogger("gcc_delay_controller"), } - rateController := newRateController(c.nowFn, c.initialBitrate, c.minBitrate, c.maxBitrate, func(ds DelayStats) { - delayController.log.Infof("delaystats: %v", ds) - if delayController.onUpdateCallback != nil { - delayController.onUpdateCallback(ds) - } - }) + rateController := newRateController( + delayConfig.nowFn, delayConfig.initialBitrate, delayConfig.minBitrate, delayConfig.maxBitrate, + func(ds DelayStats) { + delayController.log.Infof("delaystats: %v", ds) + if delayController.onUpdateCallback != nil { + delayController.onUpdateCallback(ds) + } + }, + ) delayController.rateController = rateController overuseDetector := newOveruseDetector(newAdaptiveThreshold(), 10*time.Millisecond, rateController.onDelayStats) slopeEstimator := newSlopeEstimator(newKalman(), overuseDetector.onDelayStats) diff --git a/pkg/gcc/gcc.go b/pkg/gcc/gcc.go index 1db69390..58e72520 100644 --- a/pkg/gcc/gcc.go +++ b/pkg/gcc/gcc.go @@ -10,6 +10,7 @@ func minInt(a, b int) int { if a < b { return a } + return b } @@ -17,6 +18,7 @@ func maxInt(a, b int) int { if a > b { return a } + return b } diff --git a/pkg/gcc/kalman.go b/pkg/gcc/kalman.go index ca366e13..a009077c 100644 --- a/pkg/gcc/kalman.go +++ b/pkg/gcc/kalman.go @@ -66,6 +66,7 @@ func newKalman(opts ...kalmanOption) *kalman { for _, opt := range opts { opt(k) } + return k } @@ -91,5 +92,6 @@ func (k *kalman) updateEstimate(measurement time.Duration) time.Duration { k.estimate += time.Duration(k.gain * zms * float64(time.Millisecond)) k.estimateError = (1 - k.gain) * estimateUncertainty + return k.estimate } diff --git a/pkg/gcc/leaky_bucket_pacer.go b/pkg/gcc/leaky_bucket_pacer.go index 9773f97a..496ae50f 100644 --- a/pkg/gcc/leaky_bucket_pacer.go +++ b/pkg/gcc/leaky_bucket_pacer.go @@ -23,7 +23,7 @@ type item struct { attributes interceptor.Attributes } -// LeakyBucketPacer implements a leaky bucket pacing algorithm +// LeakyBucketPacer implements a leaky bucket pacing algorithm. type LeakyBucketPacer struct { log logging.LeveledLogger @@ -43,9 +43,9 @@ type LeakyBucketPacer struct { pool *sync.Pool } -// NewLeakyBucketPacer initializes a new LeakyBucketPacer +// NewLeakyBucketPacer initializes a new LeakyBucketPacer. func NewLeakyBucketPacer(initialBitrate int) *LeakyBucketPacer { - p := &LeakyBucketPacer{ + pacer := &LeakyBucketPacer{ log: logging.NewDefaultLoggerFactory().NewLogger("pacer"), f: 1.5, targetBitrate: initialBitrate, @@ -56,18 +56,20 @@ func NewLeakyBucketPacer(initialBitrate int) *LeakyBucketPacer { ssrcToWriter: map[uint32]interceptor.RTPWriter{}, pool: &sync.Pool{}, } - p.pool = &sync.Pool{ + pacer.pool = &sync.Pool{ New: func() interface{} { b := make([]byte, 1460) + return &b }, } - go p.Run() - return p + go pacer.Run() + + return pacer } -// AddStream adds a new stream and its corresponding writer to the pacer +// AddStream adds a new stream and its corresponding writer to the pacer. func (p *LeakyBucketPacer) AddStream(ssrc uint32, writer interceptor.RTPWriter) { p.writerLock.Lock() defer p.writerLock.Unlock() @@ -75,7 +77,7 @@ func (p *LeakyBucketPacer) AddStream(ssrc uint32, writer interceptor.RTPWriter) } // SetTargetBitrate updates the target bitrate at which the pacer is allowed to -// send packets. The pacer may exceed this limit by p.f +// send packets. The pacer may exceed this limit by p.f. func (p *LeakyBucketPacer) SetTargetBitrate(rate int) { p.targetBitrateLock.Lock() defer p.targetBitrateLock.Unlock() @@ -112,7 +114,7 @@ func (p *LeakyBucketPacer) Write(header *rtp.Header, payload []byte, attributes return header.MarshalSize() + len(payload), nil } -// Run starts the LeakyBucketPacer +// Run starts the LeakyBucketPacer. func (p *LeakyBucketPacer) Run() { ticker := time.NewTicker(p.pacingInterval) defer ticker.Stop() @@ -131,6 +133,7 @@ func (p *LeakyBucketPacer) Run() { p.qLock.Unlock() if !ok { p.log.Warnf("failed to access leaky bucket pacer queue, cast failed") + continue } @@ -141,6 +144,7 @@ func (p *LeakyBucketPacer) Run() { p.log.Warnf("no writer found for ssrc: %v", next.header.SSRC) p.pool.Put(next.payload) p.qLock.Lock() + continue } @@ -159,8 +163,9 @@ func (p *LeakyBucketPacer) Run() { } } -// Close closes the LeakyBucketPacer +// Close closes the LeakyBucketPacer. func (p *LeakyBucketPacer) Close() error { close(p.done) + return nil } diff --git a/pkg/gcc/loss_based_bwe.go b/pkg/gcc/loss_based_bwe.go index 500db884..bf99a418 100644 --- a/pkg/gcc/loss_based_bwe.go +++ b/pkg/gcc/loss_based_bwe.go @@ -24,7 +24,7 @@ const ( decreaseTimeThreshold = 200 * time.Millisecond ) -// LossStats contains internal statistics of the loss based controller +// LossStats contains internal statistics of the loss based controller. type LossStats struct { TargetBitrate int AverageLoss float64 @@ -94,11 +94,17 @@ func (e *lossBasedBandwidthEstimator) updateLossEstimate(results []cc.Acknowledg decreaseLoss := math.Min(e.averageLoss, lossRatio) if increaseLoss < increaseLossThreshold && time.Since(e.lastIncrease) > increaseTimeThreshold { - e.log.Infof("loss controller increasing; averageLoss: %v, decreaseLoss: %v, increaseLoss: %v", e.averageLoss, decreaseLoss, increaseLoss) + e.log.Infof( + "loss controller increasing; averageLoss: %v, decreaseLoss: %v, increaseLoss: %v", + e.averageLoss, decreaseLoss, increaseLoss, + ) e.lastIncrease = time.Now() e.bitrate = clampInt(int(increaseFactor*float64(e.bitrate)), e.minBitrate, e.maxBitrate) } else if decreaseLoss > decreaseLossThreshold && time.Since(e.lastDecrease) > decreaseTimeThreshold { - e.log.Infof("loss controller decreasing; averageLoss: %v, decreaseLoss: %v, increaseLoss: %v", e.averageLoss, decreaseLoss, increaseLoss) + e.log.Infof( + "loss controller decreasing; averageLoss: %v, decreaseLoss: %v, increaseLoss: %v", + e.averageLoss, decreaseLoss, increaseLoss, + ) e.lastDecrease = time.Now() e.bitrate = clampInt(int(float64(e.bitrate)*(1-0.5*decreaseLoss)), e.minBitrate, e.maxBitrate) } diff --git a/pkg/gcc/noop_pacer.go b/pkg/gcc/noop_pacer.go index 6e785ae1..a173e51c 100644 --- a/pkg/gcc/noop_pacer.go +++ b/pkg/gcc/noop_pacer.go @@ -13,16 +13,16 @@ import ( ) // ErrUnknownStream is returned when trying to send a packet with a SSRC that -// was never registered with any stream +// was never registered with any stream. var ErrUnknownStream = errors.New("unknown ssrc") -// NoOpPacer implements a pacer that always immediately sends incoming packets +// NoOpPacer implements a pacer that always immediately sends incoming packets. type NoOpPacer struct { lock sync.Mutex ssrcToWriter map[uint32]interceptor.RTPWriter } -// NewNoOpPacer initializes a new NoOpPacer +// NewNoOpPacer initializes a new NoOpPacer. func NewNoOpPacer() *NoOpPacer { return &NoOpPacer{ lock: sync.Mutex{}, @@ -35,14 +35,14 @@ func NewNoOpPacer() *NoOpPacer { func (p *NoOpPacer) SetTargetBitrate(int) { } -// AddStream adds a stream and corresponding writer to the p +// AddStream adds a stream and corresponding writer to the p. func (p *NoOpPacer) AddStream(ssrc uint32, writer interceptor.RTPWriter) { p.lock.Lock() defer p.lock.Unlock() p.ssrcToWriter[ssrc] = writer } -// Write sends a packet with header and payload to a previously added stream +// Write sends a packet with header and payload to a previously added stream. func (p *NoOpPacer) Write(header *rtp.Header, payload []byte, attributes interceptor.Attributes) (int, error) { p.lock.Lock() defer p.lock.Unlock() @@ -54,7 +54,7 @@ func (p *NoOpPacer) Write(header *rtp.Header, payload []byte, attributes interce return 0, fmt.Errorf("%w: %v", ErrUnknownStream, header.SSRC) } -// Close closes p +// Close closes p. func (p *NoOpPacer) Close() error { return nil } diff --git a/pkg/gcc/overuse_detector.go b/pkg/gcc/overuse_detector.go index e7abd9b5..2cab7d14 100644 --- a/pkg/gcc/overuse_detector.go +++ b/pkg/gcc/overuse_detector.go @@ -43,7 +43,7 @@ func (d *overuseDetector) onDelayStats(ds DelayStats) { thresholdUse, estimate, currentThreshold := d.threshold.compare(ds.Estimate, ds.LastReceiveDelta) use := usageNormal - if thresholdUse == usageOver { + if thresholdUse == usageOver { //nolint:nestif if d.increasingDuration == 0 { d.increasingDuration = delta / 2 } else { diff --git a/pkg/gcc/overuse_detector_test.go b/pkg/gcc/overuse_detector_test.go index daa52f8e..ddbeee65 100644 --- a/pkg/gcc/overuse_detector_test.go +++ b/pkg/gcc/overuse_detector_test.go @@ -19,6 +19,7 @@ func (t staticThreshold) compare(estimate, _ time.Duration) (usage, time.Duratio if estimate < -time.Duration(t) { return usageUnder, estimate, time.Duration(t) } + return usageNormal, estimate, time.Duration(t) } diff --git a/pkg/gcc/rate_calculator.go b/pkg/gcc/rate_calculator.go index 4c2b7dc5..9b7b2ba6 100644 --- a/pkg/gcc/rate_calculator.go +++ b/pkg/gcc/rate_calculator.go @@ -38,6 +38,7 @@ func (c *rateCalculator) run(in <-chan []cc.Acknowledgment, onRateUpdate func(in // which is by definition in the window that ends with the last // arrival time onRateUpdate(next.Size * 8) + continue } @@ -53,6 +54,7 @@ func (c *rateCalculator) run(in <-chan []cc.Acknowledgment, onRateUpdate func(in history = history[del:] if len(history) == 0 { onRateUpdate(0) + continue } dt := next.Arrival.Sub(history[0].Arrival) diff --git a/pkg/gcc/rate_calculator_test.go b/pkg/gcc/rate_calculator_test.go index 146d0106..64d83a2c 100644 --- a/pkg/gcc/rate_calculator_test.go +++ b/pkg/gcc/rate_calculator_test.go @@ -113,5 +113,6 @@ func getACKStream(length int, size int, interval time.Duration) []cc.Acknowledgm }) t0 = t0.Add(interval) } + return res } diff --git a/pkg/gcc/rate_controller.go b/pkg/gcc/rate_controller.go index c19a617b..01560463 100644 --- a/pkg/gcc/rate_controller.go +++ b/pkg/gcc/rate_controller.go @@ -50,7 +50,9 @@ func (a *exponentialMovingAverage) update(value float64) { } } -func newRateController(now now, initialTargetBitrate, minBitrate, maxBitrate int, dsw func(DelayStats)) *rateController { +func newRateController( + now now, initialTargetBitrate, minBitrate, maxBitrate int, dsw func(DelayStats), +) *rateController { return &rateController{ now: now, initialTargetBitrate: initialTargetBitrate, @@ -87,6 +89,7 @@ func (c *rateController) onDelayStats(ds DelayStats) { c.delayStats = ds c.delayStats.State = stateIncrease c.init = true + return } c.delayStats = ds @@ -134,7 +137,8 @@ func (c *rateController) onDelayStats(ds DelayStats) { } func (c *rateController) increase(now time.Time) int { - if c.latestDecreaseRate.average > 0 && float64(c.latestReceivedRate) > c.latestDecreaseRate.average-3*c.latestDecreaseRate.stdDeviation && + if c.latestDecreaseRate.average > 0 && + float64(c.latestReceivedRate) > c.latestDecreaseRate.average-3*c.latestDecreaseRate.stdDeviation && float64(c.latestReceivedRate) < c.latestDecreaseRate.average+3*c.latestDecreaseRate.stdDeviation { bitsPerFrame := float64(c.target) / 30.0 packetsPerFrame := math.Ceil(bitsPerFrame / (1200 * 8)) @@ -144,6 +148,7 @@ func (c *rateController) increase(now time.Time) int { alpha := 0.5 * math.Min(float64(now.Sub(c.lastUpdate).Milliseconds())/float64(responseTime.Milliseconds()), 1.0) increase := int(math.Max(1000.0, alpha*expectedPacketSizeBits)) c.lastUpdate = now + return int(math.Min(float64(c.target+increase), 1.5*float64(c.latestReceivedRate))) } eta := math.Pow(1.08, math.Min(float64(now.Sub(c.lastUpdate).Milliseconds())/1000, 1.0)) @@ -160,6 +165,7 @@ func (c *rateController) increase(now time.Time) int { if rate < c.target { return c.target } + return rate } @@ -167,5 +173,6 @@ func (c *rateController) decrease() int { target := int(beta * float64(c.latestReceivedRate)) c.latestDecreaseRate.update(float64(c.latestReceivedRate)) c.lastUpdate = c.now() + return target } diff --git a/pkg/gcc/rate_controller_test.go b/pkg/gcc/rate_controller_test.go index 46b3238c..18e4e461 100644 --- a/pkg/gcc/rate_controller_test.go +++ b/pkg/gcc/rate_controller_test.go @@ -40,6 +40,7 @@ func TestRateControllerRun(t *testing.T) { t0 := time.Time{} mockNoFn := func() time.Time { t0 = t0.Add(100 * time.Millisecond) + return t0 } for _, tc := range cases { diff --git a/pkg/gcc/send_side_bwe.go b/pkg/gcc/send_side_bwe.go index 4d6be4b3..5cc3d01c 100644 --- a/pkg/gcc/send_side_bwe.go +++ b/pkg/gcc/send_side_bwe.go @@ -23,10 +23,10 @@ const ( maxBitrate = 50_000_000 ) -// ErrSendSideBWEClosed is raised when SendSideBWE.WriteRTCP is called after SendSideBWE.Close +// ErrSendSideBWEClosed is raised when SendSideBWE.WriteRTCP is called after SendSideBWE.Close. var ErrSendSideBWEClosed = errors.New("SendSideBwe closed") -// Pacer is the interface implemented by packet pacers +// Pacer is the interface implemented by packet pacers. type Pacer interface { interceptor.RTPWriter AddStream(ssrc uint32, writer interceptor.RTPWriter) @@ -34,13 +34,13 @@ type Pacer interface { Close() error } -// Stats contains internal statistics of the bandwidth estimator +// Stats contains internal statistics of the bandwidth estimator. type Stats struct { LossStats DelayStats } -// SendSideBWE implements a combination of loss and delay based GCC +// SendSideBWE implements a combination of loss and delay based GCC. type SendSideBWE struct { pacer Pacer lossController *lossBasedBandwidthEstimator @@ -59,29 +59,32 @@ type SendSideBWE struct { closeLock sync.RWMutex } -// Option configures a bandwidth estimator +// Option configures a bandwidth estimator. type Option func(*SendSideBWE) error -// SendSideBWEInitialBitrate sets the initial bitrate of new GCC interceptors +// SendSideBWEInitialBitrate sets the initial bitrate of new GCC interceptors. func SendSideBWEInitialBitrate(rate int) Option { return func(e *SendSideBWE) error { e.latestBitrate = rate + return nil } } -// SendSideBWEMaxBitrate sets the initial bitrate of new GCC interceptors +// SendSideBWEMaxBitrate sets the initial bitrate of new GCC interceptors. func SendSideBWEMaxBitrate(rate int) Option { return func(e *SendSideBWE) error { e.maxBitrate = rate + return nil } } -// SendSideBWEMinBitrate sets the initial bitrate of new GCC interceptors +// SendSideBWEMinBitrate sets the initial bitrate of new GCC interceptors. func SendSideBWEMinBitrate(rate int) Option { return func(e *SendSideBWE) error { e.minBitrate = rate + return nil } } @@ -90,13 +93,14 @@ func SendSideBWEMinBitrate(rate int) Option { func SendSideBWEPacer(p Pacer) Option { return func(e *SendSideBWE) error { e.pacer = p + return nil } } -// NewSendSideBWE creates a new sender side bandwidth estimator +// NewSendSideBWE creates a new sender side bandwidth estimator. func NewSendSideBWE(opts ...Option) (*SendSideBWE, error) { - e := &SendSideBWE{ + send := &SendSideBWE{ pacer: nil, lossController: nil, delayController: nil, @@ -110,52 +114,59 @@ func NewSendSideBWE(opts ...Option) (*SendSideBWE, error) { close: make(chan struct{}), } for _, opt := range opts { - if err := opt(e); err != nil { + if err := opt(send); err != nil { return nil, err } } - if e.pacer == nil { - e.pacer = NewLeakyBucketPacer(e.latestBitrate) + if send.pacer == nil { + send.pacer = NewLeakyBucketPacer(send.latestBitrate) } - e.lossController = newLossBasedBWE(e.latestBitrate) - e.delayController = newDelayController(delayControllerConfig{ + send.lossController = newLossBasedBWE(send.latestBitrate) + send.delayController = newDelayController(delayControllerConfig{ nowFn: time.Now, - initialBitrate: e.latestBitrate, - minBitrate: e.minBitrate, - maxBitrate: e.maxBitrate, + initialBitrate: send.latestBitrate, + minBitrate: send.minBitrate, + maxBitrate: send.maxBitrate, }) - e.delayController.onUpdate(e.onDelayUpdate) + send.delayController.onUpdate(send.onDelayUpdate) - return e, nil + return send, nil } -// AddStream adds a new stream to the bandwidth estimator +// AddStream adds a new stream to the bandwidth estimator. func (e *SendSideBWE) AddStream(info *interceptor.StreamInfo, writer interceptor.RTPWriter) interceptor.RTPWriter { var hdrExtID uint8 for _, e := range info.RTPHeaderExtensions { if e.URI == transportCCURI { - hdrExtID = uint8(e.ID) + hdrExtID = uint8(e.ID) //nolint:gosec // G115 + break } } - e.pacer.AddStream(info.SSRC, interceptor.RTPWriterFunc(func(header *rtp.Header, payload []byte, attributes interceptor.Attributes) (int, error) { - if hdrExtID != 0 { - if attributes == nil { - attributes = make(interceptor.Attributes) + e.pacer.AddStream(info.SSRC, interceptor.RTPWriterFunc( + func(header *rtp.Header, payload []byte, attributes interceptor.Attributes) (int, error) { + if hdrExtID != 0 { + if attributes == nil { + attributes = make(interceptor.Attributes) + } + attributes.Set(cc.TwccExtensionAttributesKey, hdrExtID) } - attributes.Set(cc.TwccExtensionAttributesKey, hdrExtID) - } - if err := e.feedbackAdapter.OnSent(time.Now(), header, len(payload), attributes); err != nil { - return 0, err - } - return writer.Write(header, payload, attributes) - })) + if err := e.feedbackAdapter.OnSent(time.Now(), header, len(payload), attributes); err != nil { + return 0, err + } + + return writer.Write(header, payload, attributes) + }, + )) + return e.pacer } -// WriteRTCP adds some RTCP feedback to the bandwidth estimator +// WriteRTCP adds some RTCP feedback to the bandwidth estimator. +// +//nolint:cyclop func (e *SendSideBWE) WriteRTCP(pkts []rtcp.Packet, _ interceptor.Attributes) error { now := time.Now() e.closeLock.RLock() @@ -178,6 +189,7 @@ func (e *SendSideBWE) WriteRTCP(pkts []rtcp.Packet, _ interceptor.Attributes) er for i, ack := range acks { if i == 0 { feedbackSentTime = ack.Arrival + continue } if ack.Arrival.After(feedbackSentTime) { @@ -207,10 +219,11 @@ func (e *SendSideBWE) WriteRTCP(pkts []rtcp.Packet, _ interceptor.Attributes) er e.lossController.updateLossEstimate(acks) e.delayController.updateDelayEstimate(acks) } + return nil } -// GetTargetBitrate returns the current target bitrate in bits per second +// GetTargetBitrate returns the current target bitrate in bits per second. func (e *SendSideBWE) GetTargetBitrate() int { e.lock.Lock() defer e.lock.Unlock() @@ -218,7 +231,7 @@ func (e *SendSideBWE) GetTargetBitrate() int { return e.latestBitrate } -// GetStats returns some internal statistics of the bandwidth estimator +// GetStats returns some internal statistics of the bandwidth estimator. func (e *SendSideBWE) GetStats() map[string]interface{} { e.lock.Lock() defer e.lock.Unlock() @@ -236,12 +249,12 @@ func (e *SendSideBWE) GetStats() map[string]interface{} { } // OnTargetBitrateChange sets the callback that is called when the target -// bitrate in bits per second changes +// bitrate in bits per second changes. func (e *SendSideBWE) OnTargetBitrateChange(f func(bitrate int)) { e.onTargetBitrateChange = f } -// isClosed returns true if SendSideBWE is closed +// isClosed returns true if SendSideBWE is closed. func (e *SendSideBWE) isClosed() bool { select { case <-e.close: @@ -251,7 +264,7 @@ func (e *SendSideBWE) isClosed() bool { } } -// Close stops and closes the bandwidth estimator +// Close stops and closes the bandwidth estimator. func (e *SendSideBWE) Close() error { e.closeLock.Lock() defer e.closeLock.Unlock() @@ -260,6 +273,7 @@ func (e *SendSideBWE) Close() error { return err } close(e.close) + return e.pacer.Close() } diff --git a/pkg/gcc/send_side_bwe_test.go b/pkg/gcc/send_side_bwe_test.go index 5d563dc8..608c5885 100644 --- a/pkg/gcc/send_side_bwe_test.go +++ b/pkg/gcc/send_side_bwe_test.go @@ -16,7 +16,7 @@ import ( ) // mockTWCCResponder is a RTPWriter that writes -// TWCC feedback to a embedded SendSideBWE instantly +// TWCC feedback to a embedded SendSideBWE instantly. type mockTWCCResponder struct { bwe *SendSideBWE rtpChan chan []byte @@ -25,6 +25,7 @@ type mockTWCCResponder struct { func (m *mockTWCCResponder) Read(out []byte, _ interceptor.Attributes) (int, interceptor.Attributes, error) { pkt := <-m.rtpChan copy(out, pkt) + return len(pkt), nil, nil } @@ -33,7 +34,7 @@ func (m *mockTWCCResponder) Write(pkts []rtcp.Packet, attributes interceptor.Att } // mockGCCWriteStream receives RTP packets that have been paced by -// the congestion controller +// the congestion controller. type mockGCCWriteStream struct { twccResponder *mockTWCCResponder } @@ -45,6 +46,7 @@ func (m *mockGCCWriteStream) Write(header *rtp.Header, payload []byte, _ interce } m.twccResponder.rtpChan <- pkt + return 0, err } @@ -60,7 +62,7 @@ func TestSendSideBWE(t *testing.T) { require.NoError(t, err) require.NotNil(t, bwe) - m := &mockGCCWriteStream{ + gccMock := &mockGCCWriteStream{ &mockTWCCResponder{ bwe, make(chan []byte, 500), @@ -71,13 +73,13 @@ func TestSendSideBWE(t *testing.T) { require.NoError(t, err) require.NotNil(t, twccSender) - twccInboundRTP := twccSender.BindRemoteStream(streamInfo, m.twccResponder) - twccSender.BindRTCPWriter(m.twccResponder) + twccInboundRTP := twccSender.BindRemoteStream(streamInfo, gccMock.twccResponder) + twccSender.BindRTCPWriter(gccMock.twccResponder) require.Equal(t, latestBitrate, bwe.GetTargetBitrate()) require.NotEqual(t, 0, len(bwe.GetStats())) - rtpWriter := bwe.AddStream(streamInfo, m) + rtpWriter := bwe.AddStream(streamInfo, gccMock) require.NotNil(t, rtpWriter) twccWriter := twcc.HeaderExtensionInterceptor{} @@ -118,7 +120,7 @@ func BenchmarkSendSideBWE_WriteRTCP(b *testing.B) { require.NoError(b, err) require.NotNil(b, bwe) - r := twcc.NewRecorder(5000) + recorder := twcc.NewRecorder(5000) seq := uint16(0) arrivalTime := int64(0) @@ -130,11 +132,11 @@ func BenchmarkSendSideBWE_WriteRTCP(b *testing.B) { if rand.Intn(5) != 0 { //nolint:gosec arrivalTime += int64(rtcp.TypeTCCDeltaScaleFactor * (rand.Intn(128) + 1)) //nolint:gosec - r.Record(5000, seq, arrivalTime) + recorder.Record(5000, seq, arrivalTime) } } - rtcpPackets := r.BuildFeedbackPacket() + rtcpPackets := recorder.BuildFeedbackPacket() require.Equal(b, 1, len(rtcpPackets)) require.NoError(b, bwe.WriteRTCP(rtcpPackets, nil)) diff --git a/pkg/gcc/slope_estimator.go b/pkg/gcc/slope_estimator.go index 2b57f4da..04f5d5ab 100644 --- a/pkg/gcc/slope_estimator.go +++ b/pkg/gcc/slope_estimator.go @@ -35,6 +35,7 @@ func (e *slopeEstimator) onArrivalGroup(ag arrivalGroup) { if !e.init { e.group = ag e.init = true + return } measurement := interGroupDelayVariation(e.group, ag) diff --git a/pkg/gcc/state.go b/pkg/gcc/state.go index 75b231d8..8611ab95 100644 --- a/pkg/gcc/state.go +++ b/pkg/gcc/state.go @@ -13,10 +13,11 @@ const ( stateHold ) -func (s state) transition(u usage) state { +//nolint:cyclop +func (s state) transition(use usage) state { switch s { case stateHold: - switch u { + switch use { case usageOver: return stateDecrease case usageNormal: @@ -26,7 +27,7 @@ func (s state) transition(u usage) state { } case stateIncrease: - switch u { + switch use { case usageOver: return stateDecrease case usageNormal: @@ -36,7 +37,7 @@ func (s state) transition(u usage) state { } case stateDecrease: - switch u { + switch use { case usageOver: return stateDecrease case usageNormal: @@ -45,6 +46,7 @@ func (s state) transition(u usage) state { return stateHold } } + return stateIncrease } diff --git a/pkg/intervalpli/generator_interceptor.go b/pkg/intervalpli/generator_interceptor.go index cff8ebb7..d6d81759 100644 --- a/pkg/intervalpli/generator_interceptor.go +++ b/pkg/intervalpli/generator_interceptor.go @@ -12,12 +12,12 @@ import ( "github.com/pion/rtcp" ) -// ReceiverInterceptorFactory is a interceptor.Factory for a ReceiverInterceptor +// ReceiverInterceptorFactory is a interceptor.Factory for a ReceiverInterceptor. type ReceiverInterceptorFactory struct { opts []GeneratorOption } -// NewReceiverInterceptor returns a new ReceiverInterceptor +// NewReceiverInterceptor returns a new ReceiverInterceptor. func NewReceiverInterceptor(opts ...GeneratorOption) (*ReceiverInterceptorFactory, error) { return &ReceiverInterceptorFactory{ opts: opts, @@ -47,7 +47,7 @@ type GeneratorInterceptor struct { // NewGeneratorInterceptor returns a new GeneratorInterceptor interceptor. func NewGeneratorInterceptor(opts ...GeneratorOption) (*GeneratorInterceptor, error) { - r := &GeneratorInterceptor{ + generatorInterceptor := &GeneratorInterceptor{ interval: 3 * time.Second, log: logging.NewDefaultLoggerFactory().NewLogger("pli_generator"), immediatePLINeeded: make(chan []uint32, 1), @@ -55,12 +55,12 @@ func NewGeneratorInterceptor(opts ...GeneratorOption) (*GeneratorInterceptor, er } for _, opt := range opts { - if err := opt(r); err != nil { + if err := opt(generatorInterceptor); err != nil { return nil, err } } - return r, nil + return generatorInterceptor, nil } func (r *GeneratorInterceptor) isClosed() bool { @@ -128,6 +128,7 @@ func (r *GeneratorInterceptor) loop(rtcpWriter interceptor.RTCPWriter) { } ssrcs = append(ssrcs, key) + return true }) @@ -142,6 +143,7 @@ func (r *GeneratorInterceptor) loop(rtcpWriter interceptor.RTCPWriter) { func (r *GeneratorInterceptor) createLoopTicker() (*time.Ticker, <-chan time.Time) { if r.interval > 0 { ticker := time.NewTicker(r.interval) + return ticker, ticker.C } @@ -164,9 +166,12 @@ func (r *GeneratorInterceptor) writePLIs(rtcpWriter interceptor.RTCPWriter, ssrc } } -// BindRemoteStream lets you modify any incoming RTP packets. It is called once for per RemoteStream. The returned method +// BindRemoteStream lets you modify any incoming RTP packets. +// It is called once for per RemoteStream. The returned method // will be called once per rtp packet. -func (r *GeneratorInterceptor) BindRemoteStream(info *interceptor.StreamInfo, reader interceptor.RTPReader) interceptor.RTPReader { +func (r *GeneratorInterceptor) BindRemoteStream( + info *interceptor.StreamInfo, reader interceptor.RTPReader, +) interceptor.RTPReader { if !streamSupportPli(info) { return reader } diff --git a/pkg/intervalpli/generator_interceptor_test.go b/pkg/intervalpli/generator_interceptor_test.go index c8f72df1..39a44919 100644 --- a/pkg/intervalpli/generator_interceptor_test.go +++ b/pkg/intervalpli/generator_interceptor_test.go @@ -41,7 +41,7 @@ func TestPLIGeneratorInterceptor_Unsupported(t *testing.T) { } func TestPLIGeneratorInterceptor(t *testing.T) { - i, err := NewGeneratorInterceptor( + generatorInterceptor, err := NewGeneratorInterceptor( GeneratorInterval(time.Second*1), GeneratorLog(logging.NewDefaultLoggerFactory().NewLogger("test")), ) @@ -55,7 +55,7 @@ func TestPLIGeneratorInterceptor(t *testing.T) { RTCPFeedback: []interceptor.RTCPFeedback{ {Type: "nack", Parameter: "pli"}, }, - }, i) + }, generatorInterceptor) defer func() { assert.NoError(t, stream.Close()) }() diff --git a/pkg/intervalpli/generator_option.go b/pkg/intervalpli/generator_option.go index 18b127d0..b14469ce 100644 --- a/pkg/intervalpli/generator_option.go +++ b/pkg/intervalpli/generator_option.go @@ -16,6 +16,7 @@ type GeneratorOption func(r *GeneratorInterceptor) error func GeneratorLog(log logging.LeveledLogger) GeneratorOption { return func(r *GeneratorInterceptor) error { r.log = log + return nil } } @@ -24,6 +25,7 @@ func GeneratorLog(log logging.LeveledLogger) GeneratorOption { func GeneratorInterval(interval time.Duration) GeneratorOption { return func(r *GeneratorInterceptor) error { r.interval = interval + return nil } } diff --git a/pkg/intervalpli/pli.go b/pkg/intervalpli/pli.go index 4c3ccb51..c498244b 100644 --- a/pkg/intervalpli/pli.go +++ b/pkg/intervalpli/pli.go @@ -1,7 +1,8 @@ // SPDX-FileCopyrightText: 2023 The Pion community // SPDX-License-Identifier: MIT -// Package intervalpli is an interceptor that requests PLI on a static interval. Useful when bridging protocols that don't have receiver feedback +// Package intervalpli is an interceptor that requests PLI on a static interval. +// Useful when bridging protocols that don't have receiver feedback. package intervalpli import "github.com/pion/interceptor" diff --git a/pkg/jitterbuffer/jitter_buffer.go b/pkg/jitterbuffer/jitter_buffer.go index 863a0a80..2d460f29 100644 --- a/pkg/jitterbuffer/jitter_buffer.go +++ b/pkg/jitterbuffer/jitter_buffer.go @@ -12,34 +12,35 @@ import ( "github.com/pion/rtp" ) -// State tracks a JitterBuffer as either Buffering or Emitting +// State tracks a JitterBuffer as either Buffering or Emitting. type State uint16 -// Event represents all events a JitterBuffer can emit +// Event represents all events a JitterBuffer can emit. type Event string var ( - // ErrBufferUnderrun is returned when the buffer has no items + // ErrBufferUnderrun is returned when the buffer has no items. ErrBufferUnderrun = errors.New("invalid Peek: Empty jitter buffer") - // ErrPopWhileBuffering is returned if a jitter buffer is not in a playback state + // ErrPopWhileBuffering is returned if a jitter buffer is not in a playback state. ErrPopWhileBuffering = errors.New("attempt to pop while buffering") ) const ( - // Buffering is the state when the jitter buffer has not started emitting yet, or has hit an underflow and needs to re-buffer packets + // Buffering is the state when the jitter buffer has not started emitting yet, + // or has hit an underflow and needs to re-buffer packets. Buffering State = iota // Emitting is the state when the jitter buffer is operating nominally Emitting ) const ( - // StartBuffering is emitted when the buffer receives its first packet + // StartBuffering is emitted when the buffer receives its first packet. StartBuffering Event = "startBuffering" - // BeginPlayback is emitted when the buffer has satisfied its buffer length + // BeginPlayback is emitted when the buffer has satisfied its buffer length. BeginPlayback = "playing" - // BufferUnderflow is emitted when the buffer does not have enough packets to Pop + // BufferUnderflow is emitted when the buffer does not have enough packets to Pop. BufferUnderflow = "underflow" - // BufferOverflow is emitted when the buffer has exceeded its limit + // BufferOverflow is emitted when the buffer has exceeded its limit. BufferOverflow = "overflow" ) @@ -50,19 +51,20 @@ func (jbs State) String() string { case Emitting: return "Emitting" } + return "unknown" } type ( - // Option will Override JitterBuffer's defaults + // Option will Override JitterBuffer's defaults. Option func(jb *JitterBuffer) - // EventListener will be called when the corresponding Event occurs + // EventListener will be called when the corresponding Event occurs. EventListener func(event Event, jb *JitterBuffer) ) // A JitterBuffer will accept Pushed packets, put them in sequence number // order, and allows removing in either sequence number order or via a -// provided timestamp +// provided timestamp. type JitterBuffer struct { packets *PriorityQueue minStartCount uint16 @@ -81,14 +83,14 @@ type JitterBuffer struct { // without its predecessor being present // // underflowCount will provide the count of attempts to Pop an empty buffer -// overflowCount will track the number of times the jitter buffer exceeds its limit +// overflowCount will track the number of times the jitter buffer exceeds its limit. type Stats struct { outOfOrderCount uint32 underflowCount uint32 overflowCount uint32 } -// New will initialize a jitter buffer and its associated statistics +// New will initialize a jitter buffer and its associated statistics. func New(opts ...Option) *JitterBuffer { jb := &JitterBuffer{ state: Buffering, @@ -106,7 +108,7 @@ func New(opts ...Option) *JitterBuffer { } // WithMinimumPacketCount will set the required number of packets to be received before -// any attempt to pop a packet can succeed +// any attempt to pop a packet can succeed. func WithMinimumPacketCount(count uint16) Option { return func(jb *JitterBuffer) { jb.minStartCount = count @@ -115,12 +117,12 @@ func WithMinimumPacketCount(count uint16) Option { // Listen will register an event listener // The jitter buffer may emit events correspnding, interested listerns should -// look at Event for available events +// look at Event for available events. func (jb *JitterBuffer) Listen(event Event, cb EventListener) { jb.listeners[event] = append(jb.listeners[event], cb) } -// PlayoutHead returns the SequenceNumber that will be attempted to Pop next +// PlayoutHead returns the SequenceNumber that will be attempted to Pop next. func (jb *JitterBuffer) PlayoutHead() uint16 { jb.mutex.Lock() defer jb.mutex.Unlock() @@ -129,7 +131,7 @@ func (jb *JitterBuffer) PlayoutHead() uint16 { } // SetPlayoutHead allows you to manually specify the packet you wish to pop next -// If you have encountered a packet that hasn't resolved you can skip it +// If you have encountered a packet that hasn't resolved you can skip it. func (jb *JitterBuffer) SetPlayoutHead(playoutHead uint16) { jb.mutex.Lock() defer jb.mutex.Unlock() @@ -148,7 +150,7 @@ func (jb *JitterBuffer) updateStats(lastPktSeqNo uint16) { // Push an RTP packet into the jitter buffer, this does not clone // the data so if the memory is expected to be reused, the caller should -// take this in to account and pass a copy of the packet they wish to buffer +// take this in to account and pass a copy of the packet they wish to buffer. func (jb *JitterBuffer) Push(packet *rtp.Packet) { jb.mutex.Lock() defer jb.mutex.Unlock() @@ -198,10 +200,11 @@ func (jb *JitterBuffer) Peek(playoutHead bool) (*rtp.Packet, error) { if playoutHead && jb.state == Emitting { return jb.packets.Find(jb.playoutHead) } + return jb.packets.Find(jb.lastSequence) } -// Pop an RTP packet from the jitter buffer at the current playout head +// Pop an RTP packet from the jitter buffer at the current playout head. func (jb *JitterBuffer) Pop() (*rtp.Packet, error) { jb.mutex.Lock() defer jb.mutex.Unlock() @@ -212,14 +215,16 @@ func (jb *JitterBuffer) Pop() (*rtp.Packet, error) { if err != nil { jb.stats.underflowCount++ jb.emit(BufferUnderflow) + return nil, err } jb.playoutHead = (jb.playoutHead + 1) jb.updateState() + return packet, nil } -// PopAtSequence will pop an RTP packet from the jitter buffer at the specified Sequence +// PopAtSequence will pop an RTP packet from the jitter buffer at the specified Sequence. func (jb *JitterBuffer) PopAtSequence(sq uint16) (*rtp.Packet, error) { jb.mutex.Lock() defer jb.mutex.Unlock() @@ -230,15 +235,17 @@ func (jb *JitterBuffer) PopAtSequence(sq uint16) (*rtp.Packet, error) { if err != nil { jb.stats.underflowCount++ jb.emit(BufferUnderflow) + return nil, err } jb.playoutHead = (jb.playoutHead + 1) jb.updateState() + return packet, nil } // PeekAtSequence will return an RTP packet from the jitter buffer at the specified Sequence -// without removing it from the buffer +// without removing it from the buffer. func (jb *JitterBuffer) PeekAtSequence(sq uint16) (*rtp.Packet, error) { jb.mutex.Lock() defer jb.mutex.Unlock() @@ -246,11 +253,12 @@ func (jb *JitterBuffer) PeekAtSequence(sq uint16) (*rtp.Packet, error) { if err != nil { return nil, err } + return packet, nil } // PopAtTimestamp pops an RTP packet from the jitter buffer with the provided timestamp -// Call this method repeatedly to drain the buffer at the timestamp +// Call this method repeatedly to drain the buffer at the timestamp. func (jb *JitterBuffer) PopAtTimestamp(ts uint32) (*rtp.Packet, error) { jb.mutex.Lock() defer jb.mutex.Unlock() @@ -261,13 +269,15 @@ func (jb *JitterBuffer) PopAtTimestamp(ts uint32) (*rtp.Packet, error) { if err != nil { jb.stats.underflowCount++ jb.emit(BufferUnderflow) + return nil, err } jb.updateState() + return packet, nil } -// Clear will empty the buffer and optionally reset the state +// Clear will empty the buffer and optionally reset the state. func (jb *JitterBuffer) Clear(resetState bool) { jb.mutex.Lock() defer jb.mutex.Unlock() diff --git a/pkg/jitterbuffer/jitter_buffer_test.go b/pkg/jitterbuffer/jitter_buffer_test.go index f163a8f3..f2e2a61f 100644 --- a/pkg/jitterbuffer/jitter_buffer_test.go +++ b/pkg/jitterbuffer/jitter_buffer_test.go @@ -11,6 +11,7 @@ import ( "github.com/stretchr/testify/assert" ) +//nolint:cyclop,maintidx func TestJitterBuffer(t *testing.T) { assert := assert.New(t) @@ -52,7 +53,15 @@ func TestJitterBuffer(t *testing.T) { t.Run("Appends packets and begins playout", func(*testing.T) { jb := New() for i := 0; i < 100; i++ { - jb.Push(&rtp.Packet{Header: rtp.Header{SequenceNumber: uint16(5012 + i), Timestamp: uint32(512 + i)}, Payload: []byte{0x02}}) + jb.Push( + &rtp.Packet{ + Header: rtp.Header{ + SequenceNumber: uint16(5012 + i), //nolint:gosec // G115 + Timestamp: uint32(512 + i), //nolint:gosec // G115 + }, + Payload: []byte{0x02}, + }, + ) } assert.Equal(jb.packets.Length(), uint16(100)) assert.Equal(jb.state, Emitting) @@ -68,7 +77,16 @@ func TestJitterBuffer(t *testing.T) { events = append(events, event) }) for i := 0; i < 2; i++ { - jb.Push(&rtp.Packet{Header: rtp.Header{SequenceNumber: uint16(5012 + i), Timestamp: uint32(512 + i)}, Payload: []byte{0x02}}) + //nolint:gosec // G115 + jb.Push( + &rtp.Packet{ + Header: rtp.Header{ + SequenceNumber: uint16(5012 + i), + Timestamp: uint32(512 + i), + }, + Payload: []byte{0x02}, + }, + ) } assert.Equal(jb.packets.Length(), uint16(2)) assert.Equal(jb.state, Emitting) @@ -83,7 +101,8 @@ func TestJitterBuffer(t *testing.T) { t.Run("Wraps playout correctly", func(*testing.T) { jb := New() for i := 0; i < 100; i++ { - sqnum := uint16(math.MaxUint16 - 32 + i) + sqnum := uint16(math.MaxUint16 - 32 + i) //nolint:gosec // G115 + //nolint:gosec // G115 jb.Push(&rtp.Packet{Header: rtp.Header{SequenceNumber: sqnum, Timestamp: uint32(512 + i)}, Payload: []byte{0x02}}) } assert.Equal(jb.packets.Length(), uint16(100)) @@ -95,7 +114,7 @@ func TestJitterBuffer(t *testing.T) { for i := 0; i < 100; i++ { head, err := jb.Pop() if i < 99 { - assert.Equal(head.SequenceNumber, uint16((math.MaxUint16 - 31 + i))) + assert.Equal(head.SequenceNumber, uint16((math.MaxUint16 - 31 + i))) //nolint:gosec // G115 assert.Equal(err, nil) } else { assert.Equal(head, (*rtp.Packet)(nil)) @@ -106,7 +125,8 @@ func TestJitterBuffer(t *testing.T) { t.Run("Pops at timestamp correctly", func(*testing.T) { jb := New() for i := 0; i < 100; i++ { - sqnum := uint16((math.MaxUint16 - 32 + i)) + sqnum := uint16((math.MaxUint16 - 32 + i)) //nolint:gosec // G115 + //nolint:gosec // G115 jb.Push(&rtp.Packet{Header: rtp.Header{SequenceNumber: sqnum, Timestamp: uint32(512 + i)}, Payload: []byte{0x02}}) } assert.Equal(jb.packets.Length(), uint16(100)) @@ -132,7 +152,8 @@ func TestJitterBuffer(t *testing.T) { assert.Equal(pkt.SequenceNumber, uint16(5002)) assert.Equal(err, nil) for i := 0; i < 100; i++ { - sqnum := uint16((math.MaxUint16 - 32 + i)) + sqnum := uint16((math.MaxUint16 - 32 + i)) //nolint:gosec // G115 + //nolint:gosec // G115 jb.Push(&rtp.Packet{Header: rtp.Header{SequenceNumber: sqnum, Timestamp: uint32(512 + i)}, Payload: []byte{0x02}}) } pkt, err = jb.Peek(true) @@ -143,7 +164,8 @@ func TestJitterBuffer(t *testing.T) { t.Run("Pops at sequence with an invalid sequence number", func(*testing.T) { jb := New() for i := 0; i < 50; i++ { - sqnum := uint16((math.MaxUint16 - 32 + i)) + sqnum := uint16((math.MaxUint16 - 32 + i)) //nolint:gosec // G115 + //nolint:gosec // G115 jb.Push(&rtp.Packet{Header: rtp.Header{SequenceNumber: sqnum, Timestamp: uint32(512 + i)}, Payload: []byte{0x02}}) } jb.Push(&rtp.Packet{Header: rtp.Header{SequenceNumber: 1019, Timestamp: uint32(9000)}, Payload: []byte{0x02}}) @@ -158,7 +180,8 @@ func TestJitterBuffer(t *testing.T) { t.Run("Pops at timestamp with multiple packets", func(*testing.T) { jb := New() for i := 0; i < 50; i++ { - sqnum := uint16((math.MaxUint16 - 32 + i)) + sqnum := uint16((math.MaxUint16 - 32 + i)) //nolint:gosec // G115 + //nolint:gosec // G115 jb.Push(&rtp.Packet{Header: rtp.Header{SequenceNumber: sqnum, Timestamp: uint32(512 + i)}, Payload: []byte{0x02}}) } jb.Push(&rtp.Packet{Header: rtp.Header{SequenceNumber: 1019, Timestamp: uint32(9000)}, Payload: []byte{0x02}}) @@ -180,7 +203,8 @@ func TestJitterBuffer(t *testing.T) { t.Run("Peeks at timestamp with multiple packets", func(*testing.T) { jb := New() for i := 0; i < 50; i++ { - sqnum := uint16((math.MaxUint16 - 32 + i)) + sqnum := uint16((math.MaxUint16 - 32 + i)) //nolint:gosec // G115 + //nolint:gosec // G115 jb.Push(&rtp.Packet{Header: rtp.Header{SequenceNumber: sqnum, Timestamp: uint32(512 + i)}, Payload: []byte{0x02}}) } jb.Push(&rtp.Packet{Header: rtp.Header{SequenceNumber: 1019, Timestamp: uint32(9000)}, Payload: []byte{0x02}}) diff --git a/pkg/jitterbuffer/option.go b/pkg/jitterbuffer/option.go index 9a33c22e..7a09df85 100644 --- a/pkg/jitterbuffer/option.go +++ b/pkg/jitterbuffer/option.go @@ -7,13 +7,14 @@ import ( "github.com/pion/logging" ) -// ReceiverInterceptorOption can be used to configure ReceiverInterceptor +// ReceiverInterceptorOption can be used to configure ReceiverInterceptor. type ReceiverInterceptorOption func(d *ReceiverInterceptor) error -// Log sets a logger for the interceptor +// Log sets a logger for the interceptor. func Log(log logging.LeveledLogger) ReceiverInterceptorOption { return func(d *ReceiverInterceptor) error { d.log = log + return nil } } diff --git a/pkg/jitterbuffer/priority_queue.go b/pkg/jitterbuffer/priority_queue.go index 11a8679c..e8b9cf98 100644 --- a/pkg/jitterbuffer/priority_queue.go +++ b/pkg/jitterbuffer/priority_queue.go @@ -9,7 +9,7 @@ import ( "github.com/pion/rtp" ) -// PriorityQueue provides a linked list sorting of RTP packets by SequenceNumber +// PriorityQueue provides a linked list sorting of RTP packets by SequenceNumber. type PriorityQueue struct { next *node length uint16 @@ -23,15 +23,15 @@ type node struct { } var ( - // ErrInvalidOperation may be returned if a Pop or Find operation is performed on an empty queue + // ErrInvalidOperation may be returned if a Pop or Find operation is performed on an empty queue. ErrInvalidOperation = errors.New("attempt to find or pop on an empty list") - // ErrNotFound will be returned if the packet cannot be found in the queue + // ErrNotFound will be returned if the packet cannot be found in the queue. ErrNotFound = errors.New("priority not found") ) // NewQueue will create a new PriorityQueue whose order relies on monotonically // increasing Sequence Number, wrapping at MaxUint16, so -// a packet with sequence number MaxUint16 - 1 will be after 0 +// a packet with sequence number MaxUint16 - 1 will be after 0. func NewQueue() *PriorityQueue { return &PriorityQueue{ next: nil, @@ -49,7 +49,7 @@ func newNode(val *rtp.Packet, priority uint16) *node { } // Find a packet in the queue with the provided sequence number, -// regardless of position (the packet is retained in the queue) +// regardless of position (the packet is retained in the queue). func (q *PriorityQueue) Find(sqNum uint16) (*rtp.Packet, error) { next := q.next for next != nil { @@ -62,12 +62,13 @@ func (q *PriorityQueue) Find(sqNum uint16) (*rtp.Packet, error) { return nil, ErrNotFound } -// Push will insert a packet in to the queue in order of sequence number +// Push will insert a packet in to the queue in order of sequence number. func (q *PriorityQueue) Push(val *rtp.Packet, priority uint16) { newPq := newNode(val, priority) if q.next == nil { q.next = newPq q.length++ + return } if priority < q.next.priority { @@ -75,6 +76,7 @@ func (q *PriorityQueue) Push(val *rtp.Packet, priority uint16) { q.next.prev = newPq q.next = newPq q.length++ + return } head := q.next @@ -102,13 +104,13 @@ func (q *PriorityQueue) Push(val *rtp.Packet, priority uint16) { q.length++ } -// Length will get the total length of the queue +// Length will get the total length of the queue. func (q *PriorityQueue) Length() uint16 { return q.length } // Pop removes the first element from the queue, regardless -// sequence number +// sequence number. func (q *PriorityQueue) Pop() (*rtp.Packet, error) { if q.next == nil { return nil, ErrInvalidOperation @@ -117,10 +119,11 @@ func (q *PriorityQueue) Pop() (*rtp.Packet, error) { q.next.val = nil q.length-- q.next = q.next.next + return val, nil } -// PopAt removes an element at the specified sequence number (priority) +// PopAt removes an element at the specified sequence number (priority). func (q *PriorityQueue) PopAt(sqNum uint16) (*rtp.Packet, error) { if q.next == nil { return nil, ErrInvalidOperation @@ -130,6 +133,7 @@ func (q *PriorityQueue) PopAt(sqNum uint16) (*rtp.Packet, error) { q.next.val = nil q.next = q.next.next q.length-- + return val, nil } pos := q.next @@ -143,16 +147,18 @@ func (q *PriorityQueue) PopAt(sqNum uint16) (*rtp.Packet, error) { prev.next.prev = prev } q.length-- + return val, nil } prev = pos pos = pos.next } + return nil, ErrNotFound } // PopAtTimestamp removes and returns a packet at the given RTP Timestamp, regardless -// sequence number order +// sequence number order. func (q *PriorityQueue) PopAtTimestamp(timestamp uint32) (*rtp.Packet, error) { if q.next == nil { return nil, ErrInvalidOperation @@ -162,6 +168,7 @@ func (q *PriorityQueue) PopAtTimestamp(timestamp uint32) (*rtp.Packet, error) { q.next.val = nil q.next = q.next.next q.length-- + return val, nil } pos := q.next @@ -175,15 +182,17 @@ func (q *PriorityQueue) PopAtTimestamp(timestamp uint32) (*rtp.Packet, error) { prev.next.prev = prev } q.length-- + return val, nil } prev = pos pos = pos.next } + return nil, ErrNotFound } -// Clear will empty a PriorityQueue +// Clear will empty a PriorityQueue. func (q *PriorityQueue) Clear() { next := q.next q.length = 0 diff --git a/pkg/jitterbuffer/priority_queue_test.go b/pkg/jitterbuffer/priority_queue_test.go index 8b8d23e1..7f1888d0 100644 --- a/pkg/jitterbuffer/priority_queue_test.go +++ b/pkg/jitterbuffer/priority_queue_test.go @@ -28,13 +28,23 @@ func TestPriorityQueue(t *testing.T) { }) t.Run("Appends many in order", func(*testing.T) { - q := NewQueue() + queue := NewQueue() for i := 0; i < 100; i++ { - q.Push(&rtp.Packet{Header: rtp.Header{SequenceNumber: uint16(5012 + i), Timestamp: uint32(512 + i)}, Payload: []byte{0x02}}, uint16(5012+i)) + //nolint:gosec // G115 + queue.Push( + &rtp.Packet{ + Header: rtp.Header{ + SequenceNumber: uint16(5012 + i), + Timestamp: uint32(512 + i), + }, + Payload: []byte{0x02}, + }, + uint16(5012+i), + ) } - assert.Equal(uint16(100), q.Length()) + assert.Equal(uint16(100), queue.Length()) last := (*node)(nil) - cur := q.next + cur := queue.next for cur != nil { last = cur cur = cur.next @@ -42,66 +52,102 @@ func TestPriorityQueue(t *testing.T) { assert.Equal(cur.priority, last.priority+1) } } - assert.Equal(q.next.priority, uint16(5012)) + assert.Equal(queue.next.priority, uint16(5012)) assert.Equal(last.priority, uint16(5012+99)) }) t.Run("Can remove an element", func(*testing.T) { pkt := &rtp.Packet{Header: rtp.Header{SequenceNumber: 5000, Timestamp: 500}, Payload: []byte{0x02}} - q := NewQueue() - q.Push(pkt, pkt.SequenceNumber) + queue := NewQueue() + queue.Push(pkt, pkt.SequenceNumber) pkt2 := &rtp.Packet{Header: rtp.Header{SequenceNumber: 5004, Timestamp: 500}, Payload: []byte{0x02}} - q.Push(pkt2, pkt2.SequenceNumber) + queue.Push(pkt2, pkt2.SequenceNumber) for i := 0; i < 100; i++ { - q.Push(&rtp.Packet{Header: rtp.Header{SequenceNumber: uint16(5012 + i), Timestamp: uint32(512 + i)}, Payload: []byte{0x02}}, uint16(5012+i)) + //nolint:gosec // G115 + queue.Push( + &rtp.Packet{ + Header: rtp.Header{SequenceNumber: uint16(5012 + i), Timestamp: uint32(512 + i)}, + Payload: []byte{0x02}, + }, + uint16(5012+i), + ) } - popped, _ := q.Pop() + popped, _ := queue.Pop() assert.Equal(popped.SequenceNumber, uint16(5000)) - _, _ = q.Pop() - nextPop, _ := q.Pop() + _, _ = queue.Pop() + nextPop, _ := queue.Pop() assert.Equal(nextPop.SequenceNumber, uint16(5012)) }) t.Run("Appends in order", func(*testing.T) { - q := NewQueue() + queue := NewQueue() for i := 0; i < 100; i++ { - q.Push(&rtp.Packet{Header: rtp.Header{SequenceNumber: uint16(5012 + i), Timestamp: uint32(512 + i)}, Payload: []byte{0x02}}, uint16(5012+i)) + queue.Push( + &rtp.Packet{ + Header: rtp.Header{ + SequenceNumber: uint16(5012 + i), //nolint:gosec // G115 + Timestamp: uint32(512 + i), //nolint:gosec // G115 + }, + Payload: []byte{0x02}, + }, + uint16(5012+i), //nolint:gosec // G115 + ) } - assert.Equal(uint16(100), q.Length()) + assert.Equal(uint16(100), queue.Length()) pkt := &rtp.Packet{Header: rtp.Header{SequenceNumber: 5000, Timestamp: 500}, Payload: []byte{0x02}} - q.Push(pkt, pkt.SequenceNumber) - assert.Equal(pkt, q.next.val) - assert.Equal(uint16(101), q.Length()) - assert.Equal(q.next.priority, uint16(5000)) + queue.Push(pkt, pkt.SequenceNumber) + assert.Equal(pkt, queue.next.val) + assert.Equal(uint16(101), queue.Length()) + assert.Equal(queue.next.priority, uint16(5000)) }) t.Run("Can find", func(*testing.T) { - q := NewQueue() + queue := NewQueue() for i := 0; i < 100; i++ { - q.Push(&rtp.Packet{Header: rtp.Header{SequenceNumber: uint16(5012 + i), Timestamp: uint32(512 + i)}, Payload: []byte{0x02}}, uint16(5012+i)) + //nolint:gosec // G115 + queue.Push( + &rtp.Packet{ + Header: rtp.Header{ + SequenceNumber: uint16(5012 + i), + Timestamp: uint32(512 + i), + }, + Payload: []byte{0x02}, + }, + uint16(5012+i), + ) } - pkt, err := q.Find(5012) + pkt, err := queue.Find(5012) assert.Equal(pkt.SequenceNumber, uint16(5012)) assert.Equal(err, nil) }) t.Run("Updates the length when PopAt* are called", func(*testing.T) { pkt := &rtp.Packet{Header: rtp.Header{SequenceNumber: 5000, Timestamp: 500}, Payload: []byte{0x02}} - q := NewQueue() - q.Push(pkt, pkt.SequenceNumber) + queue := NewQueue() + queue.Push(pkt, pkt.SequenceNumber) pkt2 := &rtp.Packet{Header: rtp.Header{SequenceNumber: 5004, Timestamp: 500}, Payload: []byte{0x02}} - q.Push(pkt2, pkt2.SequenceNumber) + queue.Push(pkt2, pkt2.SequenceNumber) for i := 0; i < 100; i++ { - q.Push(&rtp.Packet{Header: rtp.Header{SequenceNumber: uint16(5012 + i), Timestamp: uint32(512 + i)}, Payload: []byte{0x02}}, uint16(5012+i)) + //nolint:gosec // G115 + queue.Push( + &rtp.Packet{ + Header: rtp.Header{ + SequenceNumber: uint16(5012 + i), + Timestamp: uint32(512 + i), + }, + Payload: []byte{0x02}, + }, + uint16(5012+i), + ) } - assert.Equal(uint16(102), q.Length()) - popped, _ := q.PopAt(uint16(5012)) + assert.Equal(uint16(102), queue.Length()) + popped, _ := queue.PopAt(uint16(5012)) assert.Equal(popped.SequenceNumber, uint16(5012)) - assert.Equal(uint16(101), q.Length()) + assert.Equal(uint16(101), queue.Length()) - popped, err := q.PopAtTimestamp(uint32(500)) + popped, err := queue.PopAtTimestamp(uint32(500)) assert.Equal(popped.SequenceNumber, uint16(5000)) - assert.Equal(uint16(100), q.Length()) + assert.Equal(uint16(100), queue.Length()) assert.Equal(err, nil) }) } @@ -151,11 +197,11 @@ func TestPriorityQueue_Unreference(t *testing.T) { numPkts := 100 for i := 0; i < numPkts; i++ { atomic.AddInt64(&refs, 1) - seq := uint16(i) + seq := uint16(i) //nolint:gosec // G115 p := rtp.Packet{ Header: rtp.Header{ SequenceNumber: seq, - Timestamp: uint32(i + 42), + Timestamp: uint32(i + 42), //nolint:gosec // G115 }, Payload: []byte{byte(i)}, } diff --git a/pkg/jitterbuffer/receiver_interceptor.go b/pkg/jitterbuffer/receiver_interceptor.go index b4c032b9..cd133e25 100644 --- a/pkg/jitterbuffer/receiver_interceptor.go +++ b/pkg/jitterbuffer/receiver_interceptor.go @@ -11,12 +11,12 @@ import ( "github.com/pion/rtp" ) -// InterceptorFactory is a interceptor.Factory for a GeneratorInterceptor +// InterceptorFactory is a interceptor.Factory for a GeneratorInterceptor. type InterceptorFactory struct { opts []ReceiverInterceptorOption } -// NewInterceptor constructs a new ReceiverInterceptor +// NewInterceptor constructs a new ReceiverInterceptor. func (g *InterceptorFactory) NewInterceptor(_ string) (interceptor.Interceptor, error) { i := &ReceiverInterceptor{ close: make(chan struct{}), @@ -59,14 +59,16 @@ type ReceiverInterceptor struct { log logging.LeveledLogger } -// NewInterceptor returns a new InterceptorFactory +// NewInterceptor returns a new InterceptorFactory. func NewInterceptor(opts ...ReceiverInterceptorOption) (*InterceptorFactory, error) { return &InterceptorFactory{opts}, nil } -// BindRemoteStream lets you modify any incoming RTP packets. It is called once for per RemoteStream. The returned method -// will be called once per rtp packet. -func (i *ReceiverInterceptor) BindRemoteStream(_ *interceptor.StreamInfo, reader interceptor.RTPReader) interceptor.RTPReader { +// BindRemoteStream lets you modify any incoming RTP packets. It is called once for per RemoteStream. +// The returned method will be called once per rtp packet. +func (i *ReceiverInterceptor) BindRemoteStream( + _ *interceptor.StreamInfo, reader interceptor.RTPReader, +) interceptor.RTPReader { return interceptor.RTPReaderFunc(func(b []byte, a interceptor.Attributes) (int, interceptor.Attributes, error) { buf := make([]byte, len(b)) n, attr, err := reader.Read(buf, a) @@ -86,8 +88,10 @@ func (i *ReceiverInterceptor) BindRemoteStream(_ *interceptor.StreamInfo, reader return 0, nil, err } nlen, err := newPkt.MarshalTo(b) + return nlen, attr, err } + return n, attr, ErrPopWhileBuffering }) } @@ -100,11 +104,12 @@ func (i *ReceiverInterceptor) UnbindRemoteStream(_ *interceptor.StreamInfo) { i.buffer.Clear(true) } -// Close closes the interceptor +// Close closes the interceptor. func (i *ReceiverInterceptor) Close() error { defer i.wg.Wait() i.m.Lock() defer i.m.Unlock() i.buffer.Clear(true) + return nil } diff --git a/pkg/jitterbuffer/receiver_interceptor_test.go b/pkg/jitterbuffer/receiver_interceptor_test.go index 58685966..5492dd3a 100644 --- a/pkg/jitterbuffer/receiver_interceptor_test.go +++ b/pkg/jitterbuffer/receiver_interceptor_test.go @@ -24,7 +24,7 @@ func TestBufferStart(t *testing.T) { ) assert.NoError(t, err) - i, err := factory.NewInterceptor("") + testInterceptor, err := factory.NewInterceptor("") assert.NoError(t, err) assert.Zero(t, buf.Len()) @@ -32,7 +32,7 @@ func TestBufferStart(t *testing.T) { stream := test.NewMockStream(&interceptor.StreamInfo{ SSRC: 123456, ClockRate: 90000, - }, i) + }, testInterceptor) defer func() { assert.NoError(t, stream.Close()) }() @@ -53,7 +53,7 @@ func TestBufferStart(t *testing.T) { default: // No data ready to read, this is what we expect } - err = i.Close() + err = testInterceptor.Close() assert.NoError(t, err) assert.Zero(t, buf.Len()) } @@ -66,7 +66,7 @@ func TestReceiverBuffersAndPlaysout(t *testing.T) { ) assert.NoError(t, err) - i, err := factory.NewInterceptor("") + testInterceptor, err := factory.NewInterceptor("") assert.NoError(t, err) assert.EqualValues(t, 0, buf.Len()) @@ -74,7 +74,7 @@ func TestReceiverBuffersAndPlaysout(t *testing.T) { stream := test.NewMockStream(&interceptor.StreamInfo{ SSRC: 123456, ClockRate: 90000, - }, i) + }, testInterceptor) stream.ReceiveRTCP([]rtcp.Packet{&rtcp.PictureLossIndication{ SenderSSRC: 123, @@ -82,7 +82,7 @@ func TestReceiverBuffersAndPlaysout(t *testing.T) { }}) for s := 0; s < 61; s++ { stream.ReceiveRTP(&rtp.Packet{Header: rtp.Header{ - SequenceNumber: uint16(s), + SequenceNumber: uint16(s), //nolint:gosec // G115 }}) } // Give time for packets to be handled and stream written to. @@ -90,9 +90,9 @@ func TestReceiverBuffersAndPlaysout(t *testing.T) { for s := 0; s < 10; s++ { read := <-stream.ReadRTP() seq := read.Packet.Header.SequenceNumber - assert.EqualValues(t, uint16(s), seq) + assert.EqualValues(t, uint16(s), seq) //nolint:gosec // G115 } assert.NoError(t, stream.Close()) - err = i.Close() + err = testInterceptor.Close() assert.NoError(t, err) } diff --git a/pkg/mock/factory.go b/pkg/mock/factory.go index da6bf944..ccde8bf4 100644 --- a/pkg/mock/factory.go +++ b/pkg/mock/factory.go @@ -10,7 +10,7 @@ type Factory struct { NewInterceptorFn func(id string) (interceptor.Interceptor, error) } -// NewInterceptor implements Interceptor +// NewInterceptor implements Interceptor. func (f *Factory) NewInterceptor(id string) (interceptor.Interceptor, error) { return f.NewInterceptorFn(id) } diff --git a/pkg/mock/interceptor.go b/pkg/mock/interceptor.go index b6213cc9..19f20163 100644 --- a/pkg/mock/interceptor.go +++ b/pkg/mock/interceptor.go @@ -26,6 +26,7 @@ func (i *Interceptor) BindRTCPReader(reader interceptor.RTCPReader) interceptor. if i.BindRTCPReaderFn != nil { return i.BindRTCPReaderFn(reader) } + return reader } @@ -34,14 +35,18 @@ func (i *Interceptor) BindRTCPWriter(writer interceptor.RTCPWriter) interceptor. if i.BindRTCPWriterFn != nil { return i.BindRTCPWriterFn(writer) } + return writer } // BindLocalStream implements Interceptor. -func (i *Interceptor) BindLocalStream(info *interceptor.StreamInfo, writer interceptor.RTPWriter) interceptor.RTPWriter { +func (i *Interceptor) BindLocalStream( + info *interceptor.StreamInfo, writer interceptor.RTPWriter, +) interceptor.RTPWriter { if i.BindLocalStreamFn != nil { return i.BindLocalStreamFn(info, writer) } + return writer } @@ -53,10 +58,13 @@ func (i *Interceptor) UnbindLocalStream(info *interceptor.StreamInfo) { } // BindRemoteStream implements Interceptor. -func (i *Interceptor) BindRemoteStream(info *interceptor.StreamInfo, reader interceptor.RTPReader) interceptor.RTPReader { +func (i *Interceptor) BindRemoteStream( + info *interceptor.StreamInfo, reader interceptor.RTPReader, +) interceptor.RTPReader { if i.BindRemoteStreamFn != nil { return i.BindRemoteStreamFn(info, reader) } + return reader } @@ -72,6 +80,7 @@ func (i *Interceptor) Close() error { if i.CloseFn != nil { return i.CloseFn() } + return nil } diff --git a/pkg/mock/interceptor_test.go b/pkg/mock/interceptor_test.go index 92e43c59..de97d7e7 100644 --- a/pkg/mock/interceptor_test.go +++ b/pkg/mock/interceptor_test.go @@ -10,6 +10,7 @@ import ( "github.com/pion/interceptor" ) +//nolint:cyclop func TestInterceptor(t *testing.T) { dummyRTPWriter := &RTPWriter{} dummyRTPReader := &RTPReader{} @@ -18,23 +19,23 @@ func TestInterceptor(t *testing.T) { dummyStreamInfo := &interceptor.StreamInfo{} t.Run("Default", func(t *testing.T) { - i := &Interceptor{} + testInterceptor := &Interceptor{} - if i.BindRTCPWriter(dummyRTCPWriter) != dummyRTCPWriter { + if testInterceptor.BindRTCPWriter(dummyRTCPWriter) != dummyRTCPWriter { t.Error("Default BindRTCPWriter should return given writer") } - if i.BindRTCPReader(dummyRTCPReader) != dummyRTCPReader { + if testInterceptor.BindRTCPReader(dummyRTCPReader) != dummyRTCPReader { t.Error("Default BindRTCPReader should return given reader") } - if i.BindLocalStream(dummyStreamInfo, dummyRTPWriter) != dummyRTPWriter { + if testInterceptor.BindLocalStream(dummyStreamInfo, dummyRTPWriter) != dummyRTPWriter { t.Error("Default BindLocalStream should return given writer") } - i.UnbindLocalStream(dummyStreamInfo) - if i.BindRemoteStream(dummyStreamInfo, dummyRTPReader) != dummyRTPReader { + testInterceptor.UnbindLocalStream(dummyStreamInfo) + if testInterceptor.BindRemoteStream(dummyStreamInfo, dummyRTPReader) != dummyRTPReader { t.Error("Default BindRemoteStream should return given reader") } - i.UnbindRemoteStream(dummyStreamInfo) - if i.Close() != nil { + testInterceptor.UnbindRemoteStream(dummyStreamInfo) + if testInterceptor.Close() != nil { t.Error("Default Close should return nil") } }) @@ -48,17 +49,20 @@ func TestInterceptor(t *testing.T) { cntUnbindRemoteStream uint32 cntClose uint32 ) - i := &Interceptor{ + testInterceptor := &Interceptor{ BindRTCPReaderFn: func(reader interceptor.RTCPReader) interceptor.RTCPReader { atomic.AddUint32(&cntBindRTCPReader, 1) + return reader }, BindRTCPWriterFn: func(writer interceptor.RTCPWriter) interceptor.RTCPWriter { atomic.AddUint32(&cntBindRTCPWriter, 1) + return writer }, BindLocalStreamFn: func(_ *interceptor.StreamInfo, writer interceptor.RTPWriter) interceptor.RTPWriter { atomic.AddUint32(&cntBindLocalStream, 1) + return writer }, UnbindLocalStreamFn: func(*interceptor.StreamInfo) { @@ -66,6 +70,7 @@ func TestInterceptor(t *testing.T) { }, BindRemoteStreamFn: func(_ *interceptor.StreamInfo, reader interceptor.RTPReader) interceptor.RTPReader { atomic.AddUint32(&cntBindRemoteStream, 1) + return reader }, UnbindRemoteStreamFn: func(*interceptor.StreamInfo) { @@ -73,25 +78,26 @@ func TestInterceptor(t *testing.T) { }, CloseFn: func() error { atomic.AddUint32(&cntClose, 1) + return nil }, } - if i.BindRTCPWriter(dummyRTCPWriter) != dummyRTCPWriter { + if testInterceptor.BindRTCPWriter(dummyRTCPWriter) != dummyRTCPWriter { t.Error("Mocked BindRTCPWriter should return given writer") } - if i.BindRTCPReader(dummyRTCPReader) != dummyRTCPReader { + if testInterceptor.BindRTCPReader(dummyRTCPReader) != dummyRTCPReader { t.Error("Mocked BindRTCPReader should return given reader") } - if i.BindLocalStream(dummyStreamInfo, dummyRTPWriter) != dummyRTPWriter { + if testInterceptor.BindLocalStream(dummyStreamInfo, dummyRTPWriter) != dummyRTPWriter { t.Error("Mocked BindLocalStream should return given writer") } - i.UnbindLocalStream(dummyStreamInfo) - if i.BindRemoteStream(dummyStreamInfo, dummyRTPReader) != dummyRTPReader { + testInterceptor.UnbindLocalStream(dummyStreamInfo) + if testInterceptor.BindRemoteStream(dummyStreamInfo, dummyRTPReader) != dummyRTPReader { t.Error("Mocked BindRemoteStream should return given reader") } - i.UnbindRemoteStream(dummyStreamInfo) - if i.Close() != nil { + testInterceptor.UnbindRemoteStream(dummyStreamInfo) + if testInterceptor.Close() != nil { t.Error("Mocked Close should return nil") } diff --git a/pkg/nack/generator_interceptor.go b/pkg/nack/generator_interceptor.go index ab2bb2c5..e8d6a1af 100644 --- a/pkg/nack/generator_interceptor.go +++ b/pkg/nack/generator_interceptor.go @@ -13,14 +13,14 @@ import ( "github.com/pion/rtcp" ) -// GeneratorInterceptorFactory is a interceptor.Factory for a GeneratorInterceptor +// GeneratorInterceptorFactory is a interceptor.Factory for a GeneratorInterceptor. type GeneratorInterceptorFactory struct { opts []GeneratorOption } -// NewInterceptor constructs a new ReceiverInterceptor +// NewInterceptor constructs a new ReceiverInterceptor. func (g *GeneratorInterceptorFactory) NewInterceptor(_ string) (interceptor.Interceptor, error) { - i := &GeneratorInterceptor{ + generatorInterceptor := &GeneratorInterceptor{ streamsFilter: streamSupportNack, size: 512, skipLastN: 0, @@ -33,16 +33,16 @@ func (g *GeneratorInterceptorFactory) NewInterceptor(_ string) (interceptor.Inte } for _, opt := range g.opts { - if err := opt(i); err != nil { + if err := opt(generatorInterceptor); err != nil { return nil, err } } - if _, err := newReceiveLog(i.size); err != nil { + if _, err := newReceiveLog(generatorInterceptor.size); err != nil { return nil, err } - return i, nil + return generatorInterceptor, nil } // GeneratorInterceptor interceptor generates nack feedback messages. @@ -63,13 +63,13 @@ type GeneratorInterceptor struct { receiveLogsMu sync.Mutex } -// NewGeneratorInterceptor returns a new GeneratorInterceptorFactory +// NewGeneratorInterceptor returns a new GeneratorInterceptorFactory. func NewGeneratorInterceptor(opts ...GeneratorOption) (*GeneratorInterceptorFactory, error) { return &GeneratorInterceptorFactory{opts}, nil } -// BindRTCPWriter lets you modify any outgoing RTCP packets. It is called once per PeerConnection. The returned method -// will be called once per packet batch. +// BindRTCPWriter lets you modify any outgoing RTCP packets. It is called once per PeerConnection. +// The returned method will be called once per packet batch. func (n *GeneratorInterceptor) BindRTCPWriter(writer interceptor.RTCPWriter) interceptor.RTCPWriter { n.m.Lock() defer n.m.Unlock() @@ -85,9 +85,11 @@ func (n *GeneratorInterceptor) BindRTCPWriter(writer interceptor.RTCPWriter) int return writer } -// BindRemoteStream lets you modify any incoming RTP packets. It is called once for per RemoteStream. The returned method -// will be called once per rtp packet. -func (n *GeneratorInterceptor) BindRemoteStream(info *interceptor.StreamInfo, reader interceptor.RTPReader) interceptor.RTPReader { +// BindRemoteStream lets you modify any incoming RTP packets. It is called once for per RemoteStream. +// The returned method will be called once per rtp packet. +func (n *GeneratorInterceptor) BindRemoteStream( + info *interceptor.StreamInfo, reader interceptor.RTPReader, +) interceptor.RTPReader { if !n.streamsFilter(info) { return reader } @@ -124,7 +126,7 @@ func (n *GeneratorInterceptor) UnbindRemoteStream(info *interceptor.StreamInfo) n.receiveLogsMu.Unlock() } -// Close closes the interceptor +// Close closes the interceptor. func (n *GeneratorInterceptor) Close() error { defer n.wg.Wait() n.m.Lock() @@ -137,7 +139,7 @@ func (n *GeneratorInterceptor) Close() error { return nil } -// nolint:gocognit +// nolint:gocognit,cyclop func (n *GeneratorInterceptor) loop(rtcpWriter interceptor.RTCPWriter) { defer n.wg.Done() @@ -185,6 +187,7 @@ func (n *GeneratorInterceptor) loop(rtcpWriter interceptor.RTCPWriter) { for _, missingSeq := range missing { if missingSeq == nackSeq { isMissing = true + break } } diff --git a/pkg/nack/generator_interceptor_test.go b/pkg/nack/generator_interceptor_test.go index e303c59a..fe4be29a 100644 --- a/pkg/nack/generator_interceptor_test.go +++ b/pkg/nack/generator_interceptor_test.go @@ -64,7 +64,8 @@ func TestGeneratorInterceptor(t *testing.T) { assert.True(t, ok, "TransportLayerNack rtcp packet expected, found: %T", pkts[0]) assert.Equal(t, uint16(13), p.Nacks[0].PacketID) - assert.Equal(t, rtcp.PacketBitmap(0b10), p.Nacks[0].LostPackets) // we want packets: 13, 15 (not packet 17, because skipLastN is setReceived to 2) + // we want packets: 13, 15 (not packet 17, because skipLastN is setReceived to 2) + assert.Equal(t, rtcp.PacketBitmap(0b10), p.Nacks[0].LostPackets) case <-time.After(10 * time.Millisecond): t.Fatal("written rtcp packet not found") } @@ -90,13 +91,13 @@ func TestGeneratorInterceptor_StreamFilter(t *testing.T) { ) assert.NoError(t, err) - i, err := f.NewInterceptor("") + testInterceptor, err := f.NewInterceptor("") assert.NoError(t, err) streamWithoutNacks := test.NewMockStream(&interceptor.StreamInfo{ SSRC: 1, RTCPFeedback: []interceptor.RTCPFeedback{{Type: "nack"}}, - }, i) + }, testInterceptor) defer func() { assert.NoError(t, streamWithoutNacks.Close()) }() @@ -104,7 +105,7 @@ func TestGeneratorInterceptor_StreamFilter(t *testing.T) { streamWithNacks := test.NewMockStream(&interceptor.StreamInfo{ SSRC: 2, RTCPFeedback: []interceptor.RTCPFeedback{{Type: "nack"}}, - }, i) + }, testInterceptor) defer func() { assert.NoError(t, streamWithNacks.Close()) }() diff --git a/pkg/nack/generator_option.go b/pkg/nack/generator_option.go index 5403e3ee..db84093a 100644 --- a/pkg/nack/generator_option.go +++ b/pkg/nack/generator_option.go @@ -10,56 +10,63 @@ import ( "github.com/pion/logging" ) -// GeneratorOption can be used to configure GeneratorInterceptor +// GeneratorOption can be used to configure GeneratorInterceptor. type GeneratorOption func(r *GeneratorInterceptor) error // GeneratorSize sets the size of the interceptor. -// Size must be one of: 64, 128, 256, 512, 1024, 2048, 4096, 8192, 16384, 32768 +// Size must be one of: 64, 128, 256, 512, 1024, 2048, 4096, 8192, 16384, 32768. func GeneratorSize(size uint16) GeneratorOption { return func(r *GeneratorInterceptor) error { r.size = size + return nil } } -// GeneratorSkipLastN sets the number of packets (n-1 packets before the last received packets) to ignore when generating -// nack requests. +// GeneratorSkipLastN sets the number of packets (n-1 packets before the last received packets) +// +// to ignore when generating nack requests. func GeneratorSkipLastN(skipLastN uint16) GeneratorOption { return func(r *GeneratorInterceptor) error { r.skipLastN = skipLastN + return nil } } // GeneratorMaxNacksPerPacket sets the maximum number of NACKs sent per missing packet, e.g. if set to 2, a missing -// packet will only be NACKed at most twice. If set to 0 (default), max number of NACKs is unlimited +// packet will only be NACKed at most twice. If set to 0 (default), max number of NACKs is unlimited. func GeneratorMaxNacksPerPacket(maxNacks uint16) GeneratorOption { return func(r *GeneratorInterceptor) error { r.maxNacksPerPacket = maxNacks + return nil } } -// GeneratorLog sets a logger for the interceptor +// GeneratorLog sets a logger for the interceptor. func GeneratorLog(log logging.LeveledLogger) GeneratorOption { return func(r *GeneratorInterceptor) error { r.log = log + return nil } } -// GeneratorInterval sets the nack send interval for the interceptor +// GeneratorInterval sets the nack send interval for the interceptor. func GeneratorInterval(interval time.Duration) GeneratorOption { return func(r *GeneratorInterceptor) error { r.interval = interval + return nil } } -// GeneratorStreamsFilter sets filter for generator streams +// GeneratorStreamsFilter sets filter for generator streams. func GeneratorStreamsFilter(filter func(info *interceptor.StreamInfo) bool) GeneratorOption { return func(r *GeneratorInterceptor) error { r.streamsFilter = filter + return nil } } diff --git a/pkg/nack/receive_log.go b/pkg/nack/receive_log.go index 313133e2..3bf5c49d 100644 --- a/pkg/nack/receive_log.go +++ b/pkg/nack/receive_log.go @@ -25,6 +25,7 @@ func newReceiveLog(size uint16) (*receiveLog, error) { for i := 6; i < 16; i++ { if size == 1<> 16) + stream.lastSenderReport = uint32(sr.NTPTime >> 16) //nolint:gosec // G115 stream.lastSenderReportTime = now } @@ -131,6 +134,7 @@ func (stream *receiverStream) generateReport(now time.Time) *rtcp.ReceiverReport ret++ } } + return ret }() stream.totalLost += totalLostSinceReport @@ -143,7 +147,7 @@ func (stream *receiverStream) generateReport(now time.Time) *rtcp.ReceiverReport stream.totalLost = 0xFFFFFF } - r := &rtcp.ReceiverReport{ + receiverReport := &rtcp.ReceiverReport{ SSRC: stream.receiverSSRC, Reports: []rtcp.ReceptionReport{ { @@ -156,6 +160,7 @@ func (stream *receiverStream) generateReport(now time.Time) *rtcp.ReceiverReport if stream.lastSenderReportTime.IsZero() { return 0 } + return uint32(now.Sub(stream.lastSenderReportTime).Seconds() * 65536) }(), Jitter: uint32(stream.jitter), @@ -165,5 +170,5 @@ func (stream *receiverStream) generateReport(now time.Time) *rtcp.ReceiverReport stream.lastReportSeqnum = stream.lastSeqnum - return r + return receiverReport } diff --git a/pkg/report/sender_interceptor.go b/pkg/report/sender_interceptor.go index 1b6f4b25..40c7f9f7 100644 --- a/pkg/report/sender_interceptor.go +++ b/pkg/report/sender_interceptor.go @@ -13,17 +13,17 @@ import ( "github.com/pion/rtp" ) -// TickerFactory is a factory to create new tickers +// TickerFactory is a factory to create new tickers. type TickerFactory func(d time.Duration) Ticker -// SenderInterceptorFactory is a interceptor.Factory for a SenderInterceptor +// SenderInterceptorFactory is a interceptor.Factory for a SenderInterceptor. type SenderInterceptorFactory struct { opts []SenderOption } -// NewInterceptor constructs a new SenderInterceptor +// NewInterceptor constructs a new SenderInterceptor. func (s *SenderInterceptorFactory) NewInterceptor(_ string) (interceptor.Interceptor, error) { - i := &SenderInterceptor{ + senderInterceptor := &SenderInterceptor{ interval: 1 * time.Second, now: time.Now, newTicker: func(d time.Duration) Ticker { @@ -34,15 +34,15 @@ func (s *SenderInterceptorFactory) NewInterceptor(_ string) (interceptor.Interce } for _, opt := range s.opts { - if err := opt(i); err != nil { + if err := opt(senderInterceptor); err != nil { return nil, err } } - return i, nil + return senderInterceptor, nil } -// NewSenderInterceptor returns a new SenderInterceptorFactory +// NewSenderInterceptor returns a new SenderInterceptorFactory. func NewSenderInterceptor(opts ...SenderOption) (*SenderInterceptorFactory, error) { return &SenderInterceptorFactory{opts}, nil } @@ -119,7 +119,9 @@ func (s *SenderInterceptor) loop(rtcpWriter interceptor.RTCPWriter) { s.streams.Range(func(_, value interface{}) bool { if stream, ok := value.(*senderStream); !ok { s.log.Warnf("failed to cast SenderInterceptor stream") - } else if _, err := rtcpWriter.Write([]rtcp.Packet{stream.generateReport(now)}, interceptor.Attributes{}); err != nil { + } else if _, err := rtcpWriter.Write( + []rtcp.Packet{stream.generateReport(now)}, interceptor.Attributes{}, + ); err != nil { s.log.Warnf("failed sending: %+v", err) } @@ -134,7 +136,9 @@ func (s *SenderInterceptor) loop(rtcpWriter interceptor.RTCPWriter) { // BindLocalStream lets you modify any outgoing RTP packets. It is called once for per LocalStream. The returned method // will be called once per rtp packet. -func (s *SenderInterceptor) BindLocalStream(info *interceptor.StreamInfo, writer interceptor.RTPWriter) interceptor.RTPWriter { +func (s *SenderInterceptor) BindLocalStream( + info *interceptor.StreamInfo, writer interceptor.RTPWriter, +) interceptor.RTPWriter { stream := newSenderStream(info.SSRC, info.ClockRate, s.useLatestPacket) s.streams.Store(info.SSRC, stream) diff --git a/pkg/report/sender_interceptor_test.go b/pkg/report/sender_interceptor_test.go index 11365084..96f27e0f 100644 --- a/pkg/report/sender_interceptor_test.go +++ b/pkg/report/sender_interceptor_test.go @@ -73,7 +73,7 @@ func TestSenderInterceptor(t *testing.T) { for i := 0; i < 10; i++ { assert.NoError(t, stream.WriteRTP(&rtp.Packet{ - Header: rtp.Header{SequenceNumber: uint16(i)}, + Header: rtp.Header{SequenceNumber: uint16(i)}, //nolint:gosec // G115 Payload: []byte("\x00\x00"), })) } @@ -116,8 +116,8 @@ func TestSenderInterceptor(t *testing.T) { for i := 0; i < 10; i++ { assert.NoError(t, stream.WriteRTP(&rtp.Packet{ Header: rtp.Header{ - SequenceNumber: uint16(i), - Timestamp: uint32(i), + SequenceNumber: uint16(i), //nolint:gosec // G115 + Timestamp: uint32(i), //nolint:gosec // G115 }, Payload: []byte("\x00\x00"), })) @@ -179,8 +179,8 @@ func TestSenderInterceptor(t *testing.T) { for i := 0; i < 10; i++ { assert.NoError(t, stream.WriteRTP(&rtp.Packet{ Header: rtp.Header{ - SequenceNumber: uint16(i), - Timestamp: uint32(i), + SequenceNumber: uint16(i), //nolint:gosec // G115 + Timestamp: uint32(i), //nolint:gosec // G115 }, Payload: []byte("\x00\x00"), })) diff --git a/pkg/report/sender_option.go b/pkg/report/sender_option.go index 1e489b0b..07a4f087 100644 --- a/pkg/report/sender_option.go +++ b/pkg/report/sender_option.go @@ -16,6 +16,7 @@ type SenderOption func(r *SenderInterceptor) error func SenderLog(log logging.LeveledLogger) SenderOption { return func(r *SenderInterceptor) error { r.log = log + return nil } } @@ -24,6 +25,7 @@ func SenderLog(log logging.LeveledLogger) SenderOption { func SenderInterval(interval time.Duration) SenderOption { return func(r *SenderInterceptor) error { r.interval = interval + return nil } } @@ -32,6 +34,7 @@ func SenderInterval(interval time.Duration) SenderOption { func SenderNow(f func() time.Time) SenderOption { return func(r *SenderInterceptor) error { r.now = f + return nil } } @@ -40,6 +43,7 @@ func SenderNow(f func() time.Time) SenderOption { func SenderTicker(f TickerFactory) SenderOption { return func(r *SenderInterceptor) error { r.newTicker = f + return nil } } @@ -49,6 +53,7 @@ func SenderTicker(f TickerFactory) SenderOption { func SenderUseLatestPacket() SenderOption { return func(r *SenderInterceptor) error { r.useLatestPacket = true + return nil } } @@ -58,6 +63,7 @@ func SenderUseLatestPacket() SenderOption { func enableStartTracking(startedCh chan struct{}) SenderOption { return func(r *SenderInterceptor) error { r.started = startedCh + return nil } } diff --git a/pkg/report/sender_stream.go b/pkg/report/sender_stream.go index 44658e24..6bb37dbf 100644 --- a/pkg/report/sender_stream.go +++ b/pkg/report/sender_stream.go @@ -48,7 +48,7 @@ func (stream *senderStream) processRTP(now time.Time, header *rtp.Header, payloa } stream.packetCount++ - stream.octetCount += uint32(len(payload)) + stream.octetCount += uint32(len(payload)) //nolint:gosec // G115 } func (stream *senderStream) generateReport(now time.Time) *rtcp.SenderReport { diff --git a/pkg/rfc8888/interceptor.go b/pkg/rfc8888/interceptor.go index efe2df29..5293999c 100644 --- a/pkg/rfc8888/interceptor.go +++ b/pkg/rfc8888/interceptor.go @@ -14,17 +14,17 @@ import ( "github.com/pion/rtcp" ) -// TickerFactory is a factory to create new tickers +// TickerFactory is a factory to create new tickers. type TickerFactory func(d time.Duration) ticker -// SenderInterceptorFactory is a interceptor.Factory for a SenderInterceptor +// SenderInterceptorFactory is a interceptor.Factory for a SenderInterceptor. type SenderInterceptorFactory struct { opts []Option } -// NewInterceptor constructs a new SenderInterceptor +// NewInterceptor constructs a new SenderInterceptor. func (s *SenderInterceptorFactory) NewInterceptor(_ string) (interceptor.Interceptor, error) { - i := &SenderInterceptor{ + senderInterceptor := &SenderInterceptor{ NoOp: interceptor.NoOp{}, log: logging.NewDefaultLoggerFactory().NewLogger("rfc8888_interceptor"), lock: sync.Mutex{}, @@ -40,12 +40,13 @@ func (s *SenderInterceptorFactory) NewInterceptor(_ string) (interceptor.Interce close: make(chan struct{}), } for _, opt := range s.opts { - err := opt(i) + err := opt(senderInterceptor) if err != nil { return nil, err } } - return i, nil + + return senderInterceptor, nil } // NewSenderInterceptor returns a new SenderInterceptorFactory configured with the given options. @@ -91,9 +92,12 @@ func (s *SenderInterceptor) BindRTCPWriter(writer interceptor.RTCPWriter) interc return writer } -// BindRemoteStream lets you modify any incoming RTP packets. It is called once for per RemoteStream. The returned method -// will be called once per rtp packet. -func (s *SenderInterceptor) BindRemoteStream(_ *interceptor.StreamInfo, reader interceptor.RTPReader) interceptor.RTPReader { +// BindRemoteStream lets you modify any incoming RTP packets. +// It is called once for per RemoteStream. The returned method +// will be called once per rtp packet.. +func (s *SenderInterceptor) BindRemoteStream( + _ *interceptor.StreamInfo, reader interceptor.RTPReader, +) interceptor.RTPReader { return interceptor.RTPReaderFunc(func(b []byte, a interceptor.Attributes) (int, interceptor.Attributes, error) { i, attr, err := reader.Read(b, a) if err != nil { @@ -115,6 +119,7 @@ func (s *SenderInterceptor) BindRemoteStream(_ *interceptor.StreamInfo, reader i ecn: 0, // ECN is not supported (yet). } s.packetChan <- p + return i, attr, nil }) } @@ -157,6 +162,7 @@ func (s *SenderInterceptor) loop(writer interceptor.RTCPWriter) { select { case <-s.close: t.Stop() + return case pkt := <-s.packetChan: @@ -167,6 +173,7 @@ func (s *SenderInterceptor) loop(writer interceptor.RTCPWriter) { s.log.Tracef("report triggered at %v", now) if writer == nil { s.log.Trace("no writer added, continue") + continue } pkts := s.recorder.BuildReport(now, int(s.maxReportSize)) diff --git a/pkg/rfc8888/interceptor_test.go b/pkg/rfc8888/interceptor_test.go index d914bd16..be735158 100644 --- a/pkg/rfc8888/interceptor_test.go +++ b/pkg/rfc8888/interceptor_test.go @@ -14,6 +14,7 @@ import ( "github.com/stretchr/testify/assert" ) +//nolint:maintidx,cyclop func TestInterceptor(t *testing.T) { t.Run("before any packet", func(t *testing.T) { f, err := NewSenderInterceptor() @@ -59,7 +60,7 @@ func TestInterceptor(t *testing.T) { Extension: false, Marker: false, PayloadType: 0, - SequenceNumber: uint16(i), + SequenceNumber: uint16(i), //nolint:gosec // G115 Timestamp: 0, SSRC: 123456, CSRC: []uint32{}, @@ -115,7 +116,7 @@ func TestInterceptor(t *testing.T) { mNow.SetNow(zero.Add(d)) stream.ReceiveRTP(&rtp.Packet{ Header: rtp.Header{ - SequenceNumber: uint16(i), + SequenceNumber: uint16(i), //nolint:gosec // G115 SSRC: 123456, }, }) @@ -200,7 +201,7 @@ func TestInterceptor(t *testing.T) { mNow.SetNow(zero.Add(time.Duration(sequenceNumberToDelay[i]) * time.Millisecond)) stream.ReceiveRTP(&rtp.Packet{ Header: rtp.Header{ - SequenceNumber: uint16(i), + SequenceNumber: uint16(i), //nolint:gosec // G115 SSRC: 123456, }, }) diff --git a/pkg/rfc8888/option.go b/pkg/rfc8888/option.go index 1214868b..236073b5 100644 --- a/pkg/rfc8888/option.go +++ b/pkg/rfc8888/option.go @@ -5,13 +5,14 @@ package rfc8888 import "time" -// An Option is a function that can be used to configure a SenderInterceptor +// An Option is a function that can be used to configure a SenderInterceptor. type Option func(*SenderInterceptor) error // SenderTicker sets an alternative for time.Ticker. func SenderTicker(f TickerFactory) Option { return func(i *SenderInterceptor) error { i.newTicker = f + return nil } } @@ -20,14 +21,16 @@ func SenderTicker(f TickerFactory) Option { func SenderNow(f func() time.Time) Option { return func(i *SenderInterceptor) error { i.now = f + return nil } } -// SendInterval sets the feedback send interval for the interceptor +// SendInterval sets the feedback send interval for the interceptor. func SendInterval(interval time.Duration) Option { return func(s *SenderInterceptor) error { s.interval = interval + return nil } } diff --git a/pkg/rfc8888/recorder.go b/pkg/rfc8888/recorder.go index e1dba0da..a1a2cf26 100644 --- a/pkg/rfc8888/recorder.go +++ b/pkg/rfc8888/recorder.go @@ -22,7 +22,7 @@ type Recorder struct { streams map[uint32]*streamLog } -// NewRecorder creates a new Recorder +// NewRecorder creates a new Recorder. func NewRecorder() *Recorder { return &Recorder{ streams: map[uint32]*streamLog{}, diff --git a/pkg/rfc8888/recorder_test.go b/pkg/rfc8888/recorder_test.go index 7e38b9a6..e032a78f 100644 --- a/pkg/rfc8888/recorder_test.go +++ b/pkg/rfc8888/recorder_test.go @@ -149,7 +149,7 @@ func TestRecorder(t *testing.T) { // Add 1000 packets on 10 different streams for i := 0; i < 10; i++ { for j := 0; j < 100; j++ { - recorder.AddPacket(now, uint32(i), uint16(j), 0) + recorder.AddPacket(now, uint32(i), uint16(j), 0) //nolint:gosec // G115 } } reports := recorder.BuildReport(time.Time{}, 1380) diff --git a/pkg/rfc8888/stream_log.go b/pkg/rfc8888/stream_log.go index 2d18d9c6..ed41f952 100644 --- a/pkg/rfc8888/stream_log.go +++ b/pkg/rfc8888/stream_log.go @@ -53,7 +53,7 @@ func (l *streamLog) metricsAfter(reference time.Time, maxReportBlocks int64) rtc if len(l.log) == 0 { return rtcp.CCFeedbackReportBlock{ MediaSSRC: l.ssrc, - BeginSequence: uint16(l.nextSequenceNumberToReport), + BeginSequence: uint16(l.nextSequenceNumberToReport), //nolint:gosec // G115 MetricBlocks: []rtcp.CCFeedbackMetricBlock{}, } } @@ -66,7 +66,7 @@ func (l *streamLog) metricsAfter(reference time.Time, maxReportBlocks int64) rtc offset := l.nextSequenceNumberToReport lastReceived := l.nextSequenceNumberToReport gapDetected := false - for i := offset; i <= l.lastSequenceNumberReceived; i++ { + for i := offset; i <= l.lastSequenceNumberReceived; i++ { //nolint:varnamelen // i int64 received := false ecn := uint8(0) ato := uint16(0) @@ -92,9 +92,10 @@ func (l *streamLog) metricsAfter(reference time.Time, maxReportBlocks int64) rtc } } } + return rtcp.CCFeedbackReportBlock{ MediaSSRC: l.ssrc, - BeginSequence: uint16(offset), + BeginSequence: uint16(offset), //nolint:gosec // G115 MetricBlocks: metricBlocks, } } @@ -107,5 +108,6 @@ func getArrivalTimeOffset(base time.Time, arrival time.Time) uint16 { if ato > 0x1FFD { return 0x1FFE } + return ato } diff --git a/pkg/rfc8888/stream_log_test.go b/pkg/rfc8888/stream_log_test.go index a3e43526..d5ac2c95 100644 --- a/pkg/rfc8888/stream_log_test.go +++ b/pkg/rfc8888/stream_log_test.go @@ -202,6 +202,7 @@ func TestStreamLogAdd(t *testing.T) { } } +//nolint:maintidx func TestStreamLogMetricsAfter(t *testing.T) { tests := []struct { name string @@ -575,7 +576,7 @@ func TestRemoveOldestPackets(t *testing.T) { now := time.Now().Add(10 * time.Second) for i := 2; i < 16386; i++ { now = now.Add(10 * time.Millisecond) - sl.add(now, uint16(i), 0) + sl.add(now, uint16(i), 0) //nolint:gosec // G115 } metrics := sl.metricsAfter(now, maxReportsPerReportBlock) assert.Equal(t, uint16(2), metrics.BeginSequence) diff --git a/pkg/stats/interceptor.go b/pkg/stats/interceptor.go index 2570c513..d308c31d 100644 --- a/pkg/stats/interceptor.go +++ b/pkg/stats/interceptor.go @@ -13,14 +13,15 @@ import ( "github.com/pion/rtp" ) -// Option can be used to configure the stats interceptor +// Option can be used to configure the stats interceptor. type Option func(*Interceptor) error // SetRecorderFactory sets the factory that is used to create new stats -// recorders for new streams +// recorders for new streams. func SetRecorderFactory(f RecorderFactory) Option { return func(i *Interceptor) error { i.RecorderFactory = f + return nil } } @@ -30,26 +31,27 @@ func SetRecorderFactory(f RecorderFactory) Option { func SetNowFunc(now func() time.Time) Option { return func(i *Interceptor) error { i.now = now + return nil } } -// Getter returns the most recent stats of a stream +// Getter returns the most recent stats of a stream. type Getter interface { Get(ssrc uint32) *Stats } // NewPeerConnectionCallback receives a new StatsGetter for a newly created -// PeerConnection +// PeerConnection. type NewPeerConnectionCallback func(string, Getter) -// InterceptorFactory is a interceptor.Factory for a stats Interceptor +// InterceptorFactory is a interceptor.Factory for a stats Interceptor. type InterceptorFactory struct { opts []Option addPeerConnection NewPeerConnectionCallback } -// NewInterceptor creates a new InterceptorFactory +// NewInterceptor creates a new InterceptorFactory. func NewInterceptor(opts ...Option) (*InterceptorFactory, error) { return &InterceptorFactory{ opts: opts, @@ -63,9 +65,9 @@ func (r *InterceptorFactory) OnNewPeerConnection(cb NewPeerConnectionCallback) { r.addPeerConnection = cb } -// NewInterceptor creates a new Interceptor +// NewInterceptor creates a new Interceptor. func (r *InterceptorFactory) NewInterceptor(id string) (interceptor.Interceptor, error) { - i := &Interceptor{ + interceptor := &Interceptor{ NoOp: interceptor.NoOp{}, now: time.Now, lock: sync.Mutex{}, @@ -76,19 +78,19 @@ func (r *InterceptorFactory) NewInterceptor(id string) (interceptor.Interceptor, wg: sync.WaitGroup{}, } for _, opt := range r.opts { - if err := opt(i); err != nil { + if err := opt(interceptor); err != nil { return nil, err } } if r.addPeerConnection != nil { - r.addPeerConnection(id, i) + r.addPeerConnection(id, interceptor) } - return i, nil + return interceptor, nil } -// Recorder is the interface of a statistics recorder +// Recorder is the interface of a statistics recorder. type Recorder interface { QueueIncomingRTP(ts time.Time, buf []byte, attr interceptor.Attributes) QueueIncomingRTCP(ts time.Time, buf []byte, attr interceptor.Attributes) @@ -99,10 +101,10 @@ type Recorder interface { Start() } -// RecorderFactory creates new Recorders to be used by the interceptor +// RecorderFactory creates new Recorders to be used by the interceptor. type RecorderFactory func(ssrc uint32, clockRate float64) Recorder -// Interceptor is the interceptor that collects stream stats +// Interceptor is the interceptor that collects stream stats. type Interceptor struct { interceptor.NoOp now func() time.Time @@ -112,14 +114,16 @@ type Interceptor struct { wg sync.WaitGroup } -// Get returns the statistics for the stream with ssrc +// Get returns the statistics for the stream with ssrc. func (r *Interceptor) Get(ssrc uint32) *Stats { r.lock.Lock() defer r.lock.Unlock() if rec, ok := r.recorders[ssrc]; ok { stats := rec.GetStats() + return &stats } + return nil } @@ -136,10 +140,11 @@ func (r *Interceptor) getRecorder(ssrc uint32, clockRate float64) Recorder { rec.Start() }() r.recorders[ssrc] = rec + return rec } -// Close closes the interceptor and associated stats recorders +// Close closes the interceptor and associated stats recorders. func (r *Interceptor) Close() error { defer r.wg.Wait() @@ -149,24 +154,28 @@ func (r *Interceptor) Close() error { for _, r := range r.recorders { r.Stop() } + return nil } // BindRTCPReader lets you modify any incoming RTCP packets. It is called once per sender/receiver, however this might // change in the future. The returned method will be called once per packet batch. func (r *Interceptor) BindRTCPReader(reader interceptor.RTCPReader) interceptor.RTCPReader { - return interceptor.RTCPReaderFunc(func(bytes []byte, attributes interceptor.Attributes) (int, interceptor.Attributes, error) { - n, attattributes, err := reader.Read(bytes, attributes) - if err != nil { - return 0, attattributes, err - } - r.lock.Lock() - for _, recorder := range r.recorders { - recorder.QueueIncomingRTCP(r.now(), bytes[:n], attributes) - } - r.lock.Unlock() - return n, attattributes, err - }) + return interceptor.RTCPReaderFunc( + func(bytes []byte, attributes interceptor.Attributes) (int, interceptor.Attributes, error) { + n, attattributes, err := reader.Read(bytes, attributes) + if err != nil { + return 0, attattributes, err + } + r.lock.Lock() + for _, recorder := range r.recorders { + recorder.QueueIncomingRTCP(r.now(), bytes[:n], attributes) + } + r.lock.Unlock() + + return n, attattributes, err + }, + ) } // BindRTCPWriter lets you modify any outgoing RTCP packets. It is called once per PeerConnection. The returned method @@ -178,30 +187,43 @@ func (r *Interceptor) BindRTCPWriter(writer interceptor.RTCPWriter) interceptor. recorder.QueueOutgoingRTCP(r.now(), pkts, attributes) } r.lock.Unlock() + return writer.Write(pkts, attributes) }) } -// BindLocalStream lets you modify any outgoing RTP packets. It is called once for per LocalStream. The returned method -// will be called once per rtp packet. -func (r *Interceptor) BindLocalStream(info *interceptor.StreamInfo, writer interceptor.RTPWriter) interceptor.RTPWriter { +// BindLocalStream lets you modify any outgoing RTP packets. It is called once for per LocalStream. +// The returned method will be called once per rtp packet. +func (r *Interceptor) BindLocalStream( + info *interceptor.StreamInfo, writer interceptor.RTPWriter, +) interceptor.RTPWriter { recorder := r.getRecorder(info.SSRC, float64(info.ClockRate)) - return interceptor.RTPWriterFunc(func(header *rtp.Header, payload []byte, attributes interceptor.Attributes) (int, error) { - recorder.QueueOutgoingRTP(r.now(), header, payload, attributes) - return writer.Write(header, payload, attributes) - }) + + return interceptor.RTPWriterFunc( + func(header *rtp.Header, payload []byte, attributes interceptor.Attributes) (int, error) { + recorder.QueueOutgoingRTP(r.now(), header, payload, attributes) + + return writer.Write(header, payload, attributes) + }, + ) } -// BindRemoteStream lets you modify any incoming RTP packets. It is called once for per RemoteStream. The returned method -// will be called once per rtp packet. -func (r *Interceptor) BindRemoteStream(info *interceptor.StreamInfo, reader interceptor.RTPReader) interceptor.RTPReader { +// BindRemoteStream lets you modify any incoming RTP packets. It is called once for per RemoteStream. +// The returned method will be called once per rtp packet. +func (r *Interceptor) BindRemoteStream( + info *interceptor.StreamInfo, reader interceptor.RTPReader, +) interceptor.RTPReader { recorder := r.getRecorder(info.SSRC, float64(info.ClockRate)) - return interceptor.RTPReaderFunc(func(bytes []byte, attributes interceptor.Attributes) (int, interceptor.Attributes, error) { - n, attributes, err := reader.Read(bytes, attributes) - if err != nil { - return 0, nil, err - } - recorder.QueueIncomingRTP(r.now(), bytes[:n], attributes) - return n, attributes, nil - }) + + return interceptor.RTPReaderFunc( + func(bytes []byte, attributes interceptor.Attributes) (int, interceptor.Attributes, error) { + n, attributes, err := reader.Read(bytes, attributes) + if err != nil { + return 0, nil, err + } + recorder.QueueIncomingRTP(r.now(), bytes[:n], attributes) + + return n, attributes, nil + }, + ) } diff --git a/pkg/stats/interceptor_test.go b/pkg/stats/interceptor_test.go index 2d506587..26c6fd55 100644 --- a/pkg/stats/interceptor_test.go +++ b/pkg/stats/interceptor_test.go @@ -14,6 +14,7 @@ import ( "github.com/stretchr/testify/assert" ) +//nolint:cyclop func TestInterceptor(t *testing.T) { t.Run("before any packets", func(t *testing.T) { f, err := NewInterceptor() @@ -46,7 +47,7 @@ func TestInterceptor(t *testing.T) { t.Run("records packets", func(t *testing.T) { mockRecorder := newMockRecorder() now := time.Now() - f, err := NewInterceptor( + testInterceptor, err := NewInterceptor( SetRecorderFactory(func(uint32, float64) Recorder { return mockRecorder }), @@ -56,13 +57,13 @@ func TestInterceptor(t *testing.T) { ) assert.NoError(t, err) statsCh := make(chan Getter) - f.OnNewPeerConnection(func(_ string, g Getter) { + testInterceptor.OnNewPeerConnection(func(_ string, g Getter) { go func() { statsCh <- g }() }) - i, err := f.NewInterceptor("") + i, err := testInterceptor.NewInterceptor("") assert.NoError(t, err) stream := test.NewMockStream(&interceptor.StreamInfo{SSRC: 0}, i) diff --git a/pkg/stats/received_stats.go b/pkg/stats/received_stats.go index 774821cd..45db241f 100644 --- a/pkg/stats/received_stats.go +++ b/pkg/stats/received_stats.go @@ -8,22 +8,23 @@ import ( "time" ) -// ReceivedRTPStreamStats contains common receiver stats of RTP streams +// ReceivedRTPStreamStats contains common receiver stats of RTP streams. type ReceivedRTPStreamStats struct { PacketsReceived uint64 PacketsLost int64 Jitter float64 } -// String returns a string representation of ReceivedRTPStreamStats +// String returns a string representation of ReceivedRTPStreamStats. func (s ReceivedRTPStreamStats) String() string { out := fmt.Sprintf("\tPacketsReceived: %v\n", s.PacketsReceived) out += fmt.Sprintf("\tPacketsLost: %v\n", s.PacketsLost) out += fmt.Sprintf("\tJitter: %v\n", s.Jitter) + return out } -// InboundRTPStreamStats contains stats of inbound RTP streams +// InboundRTPStreamStats contains stats of inbound RTP streams. type InboundRTPStreamStats struct { ReceivedRTPStreamStats @@ -35,7 +36,7 @@ type InboundRTPStreamStats struct { NACKCount uint32 } -// String returns a string representation of InboundRTPStreamStats +// String returns a string representation of InboundRTPStreamStats. func (s InboundRTPStreamStats) String() string { out := "InboundRTPStreamStats:\n" out += s.ReceivedRTPStreamStats.String() @@ -45,11 +46,12 @@ func (s InboundRTPStreamStats) String() string { out += fmt.Sprintf("\tFIRCount: %v\n", s.FIRCount) out += fmt.Sprintf("\tPLICount: %v\n", s.PLICount) out += fmt.Sprintf("\tNACKCount: %v\n", s.NACKCount) + return out } // RemoteInboundRTPStreamStats contains stats of inbound RTP streams of the -// remote peer +// remote peer. type RemoteInboundRTPStreamStats struct { ReceivedRTPStreamStats @@ -59,7 +61,7 @@ type RemoteInboundRTPStreamStats struct { RoundTripTimeMeasurements uint64 } -// String returns a string representation of RemoteInboundRTPStreamStats +// String returns a string representation of RemoteInboundRTPStreamStats. func (s RemoteInboundRTPStreamStats) String() string { out := "RemoteInboundRTPStreamStats:\n" out += s.ReceivedRTPStreamStats.String() @@ -67,5 +69,6 @@ func (s RemoteInboundRTPStreamStats) String() string { out += fmt.Sprintf("\tTotalRoundTripTime: %v\n", s.TotalRoundTripTime) out += fmt.Sprintf("\tFractionLost: %v\n", s.FractionLost) out += fmt.Sprintf("\tRoundTripTimeMeasurements: %v\n", s.RoundTripTimeMeasurements) + return out } diff --git a/pkg/stats/sent_stats.go b/pkg/stats/sent_stats.go index 2a91dce5..2404ef37 100644 --- a/pkg/stats/sent_stats.go +++ b/pkg/stats/sent_stats.go @@ -8,20 +8,21 @@ import ( "time" ) -// SentRTPStreamStats contains common sender stats of RTP streams +// SentRTPStreamStats contains common sender stats of RTP streams. type SentRTPStreamStats struct { PacketsSent uint64 BytesSent uint64 } -// String returns a string representation of SentRTPStreamStats +// String returns a string representation of SentRTPStreamStats. func (s SentRTPStreamStats) String() string { out := fmt.Sprintf("\tPacketsSent: %v\n", s.PacketsSent) out += fmt.Sprintf("\tBytesSent: %v\n", s.BytesSent) + return out } -// OutboundRTPStreamStats contains stats of outbound RTP streams +// OutboundRTPStreamStats contains stats of outbound RTP streams. type OutboundRTPStreamStats struct { SentRTPStreamStats @@ -31,7 +32,7 @@ type OutboundRTPStreamStats struct { PLICount uint32 } -// String returns a string representation of OutboundRTPStreamStats +// String returns a string representation of OutboundRTPStreamStats. func (s OutboundRTPStreamStats) String() string { out := "OutboundRTPStreamStats\n" out += s.SentRTPStreamStats.String() @@ -39,11 +40,12 @@ func (s OutboundRTPStreamStats) String() string { out += fmt.Sprintf("\tNACKCount: %v\n", s.NACKCount) out += fmt.Sprintf("\tFIRCount: %v\n", s.FIRCount) out += fmt.Sprintf("\tPLICount: %v\n", s.PLICount) + return out } // RemoteOutboundRTPStreamStats contains stats of outbound RTP streams of the -// remote peer +// remote peer. type RemoteOutboundRTPStreamStats struct { SentRTPStreamStats @@ -54,7 +56,7 @@ type RemoteOutboundRTPStreamStats struct { RoundTripTimeMeasurements uint64 } -// String returns a string representation of RemoteOutboundRTPStreamStats +// String returns a string representation of RemoteOutboundRTPStreamStats. func (s RemoteOutboundRTPStreamStats) String() string { out := "RemoteOutboundRTPStreamStats:\n" out += s.SentRTPStreamStats.String() @@ -63,5 +65,6 @@ func (s RemoteOutboundRTPStreamStats) String() string { out += fmt.Sprintf("\tRoundTripTime: %v\n", s.RoundTripTime) out += fmt.Sprintf("\tTotalRoundTripTime: %v\n", s.TotalRoundTripTime) out += fmt.Sprintf("\tRoundTripTimeMeasurements: %v\n", s.RoundTripTimeMeasurements) + return out } diff --git a/pkg/stats/stats_recorder.go b/pkg/stats/stats_recorder.go index 430019bc..4d00945c 100644 --- a/pkg/stats/stats_recorder.go +++ b/pkg/stats/stats_recorder.go @@ -16,7 +16,7 @@ import ( "github.com/pion/rtp" ) -// Stats contains all the available statistics of RTP streams +// Stats contains all the available statistics of RTP streams. type Stats struct { InboundRTPStreamStats OutboundRTPStreamStats @@ -107,6 +107,7 @@ func (r *recorder) Stop() { func (r *recorder) GetStats() Stats { r.ms.Lock() defer r.ms.Unlock() + return Stats{ InboundRTPStreamStats: r.latestStats.InboundRTPStreamStats, OutboundRTPStreamStats: r.latestStats.OutboundRTPStreamStats, @@ -115,11 +116,11 @@ func (r *recorder) GetStats() Stats { } } -func (r *recorder) recordIncomingRTP(latestStats internalStats, v *incomingRTP) internalStats { - if v.header.SSRC != r.ssrc { +func (r *recorder) recordIncomingRTP(latestStats internalStats, incoming *incomingRTP) internalStats { + if incoming.header.SSRC != r.ssrc { return latestStats } - sequenceNumber := latestStats.inboundSequencerNumber.Unwrap(v.header.SequenceNumber) + sequenceNumber := latestStats.inboundSequencerNumber.Unwrap(incoming.header.SequenceNumber) if !latestStats.inboundSequenceNumberInitialized { latestStats.inboundFirstSequenceNumber = sequenceNumber latestStats.inboundSequenceNumberInitialized = true @@ -130,29 +131,33 @@ func (r *recorder) recordIncomingRTP(latestStats internalStats, v *incomingRTP) latestStats.InboundRTPStreamStats.PacketsReceived++ expectedPackets := latestStats.inboundHighestSequenceNumber - latestStats.inboundFirstSequenceNumber + 1 - latestStats.InboundRTPStreamStats.PacketsLost = expectedPackets - int64(latestStats.InboundRTPStreamStats.PacketsReceived) + //nolint:gosec // G115 + latestStats.InboundRTPStreamStats.PacketsLost = expectedPackets - + int64(latestStats.InboundRTPStreamStats.PacketsReceived) if !latestStats.inboundLastArrivalInitialized { - latestStats.inboundLastArrival = v.ts + latestStats.inboundLastArrival = incoming.ts latestStats.inboundLastArrivalInitialized = true } else { - arrival := int(v.ts.Sub(latestStats.inboundLastArrival).Seconds() * r.clockRate) - transit := arrival - int(v.header.Timestamp) + arrival := int(incoming.ts.Sub(latestStats.inboundLastArrival).Seconds() * r.clockRate) + transit := arrival - int(incoming.header.Timestamp) d := transit - latestStats.inboundLastTransit latestStats.inboundLastTransit = transit if d < 0 { d = -d } latestStats.InboundRTPStreamStats.Jitter += (1.0 / 16.0) * (float64(d) - latestStats.InboundRTPStreamStats.Jitter) - latestStats.inboundLastArrival = v.ts + latestStats.inboundLastArrival = incoming.ts } - latestStats.LastPacketReceivedTimestamp = v.ts - latestStats.HeaderBytesReceived += uint64(v.header.MarshalSize()) - latestStats.BytesReceived += uint64(v.header.MarshalSize() + v.payloadLen) + latestStats.LastPacketReceivedTimestamp = incoming.ts + latestStats.HeaderBytesReceived += uint64(incoming.header.MarshalSize()) //nolint:gosec // G115 + latestStats.BytesReceived += uint64(incoming.header.MarshalSize() + incoming.payloadLen) //nolint:gosec // G115 + return latestStats } +//nolint:cyclop func (r *recorder) recordOutgoingRTCP(latestStats internalStats, v *outgoingRTCP) internalStats { for _, pkt := range v.pkts { // The SSRC check is performed for most of the cases but not all. The @@ -162,41 +167,50 @@ func (r *recorder) recordOutgoingRTCP(latestStats internalStats, v *outgoingRTCP case *rtcp.FullIntraRequest: if !contains(pkt.DestinationSSRC(), r.ssrc) { r.logger.Debugf("skipping outgoing RTCP pkt: %v", pkt) + continue } latestStats.InboundRTPStreamStats.FIRCount++ case *rtcp.PictureLossIndication: if !contains(pkt.DestinationSSRC(), r.ssrc) { r.logger.Debugf("skipping outgoing RTCP pkt: %v", pkt) + continue } latestStats.InboundRTPStreamStats.PLICount++ case *rtcp.TransportLayerNack: if !contains(pkt.DestinationSSRC(), r.ssrc) { r.logger.Debugf("skipping outgoing RTCP pkt: %v", pkt) + continue } latestStats.InboundRTPStreamStats.NACKCount++ case *rtcp.SenderReport: if !contains(pkt.DestinationSSRC(), r.ssrc) { r.logger.Debugf("skipping outgoing RTCP pkt: %v", pkt) + continue } latestStats.lastSenderReports = append(latestStats.lastSenderReports, rtcpPkt.NTPTime) if len(latestStats.lastSenderReports) > r.maxLastSenderReports { - latestStats.lastSenderReports = latestStats.lastSenderReports[len(latestStats.lastSenderReports)-r.maxLastSenderReports:] + latestStats.lastSenderReports = latestStats.lastSenderReports[len( + latestStats.lastSenderReports, + )-r.maxLastSenderReports:] } case *rtcp.ExtendedReport: for _, block := range rtcpPkt.Reports { if xr, ok := block.(*rtcp.ReceiverReferenceTimeReportBlock); ok { latestStats.lastReceiverReferenceTimes = append(latestStats.lastReceiverReferenceTimes, xr.NTPTimestamp) if len(latestStats.lastReceiverReferenceTimes) > r.maxLastReceiverReferenceTimes { - latestStats.lastReceiverReferenceTimes = latestStats.lastReceiverReferenceTimes[len(latestStats.lastReceiverReferenceTimes)-r.maxLastReceiverReferenceTimes:] + latestStats.lastReceiverReferenceTimes = latestStats.lastReceiverReferenceTimes[len( + latestStats.lastReceiverReferenceTimes, + )-r.maxLastReceiverReferenceTimes:] } } } } } + return latestStats } @@ -206,12 +220,13 @@ func (r *recorder) recordOutgoingRTP(latestStats internalStats, v *outgoingRTP) } headerSize := v.header.MarshalSize() latestStats.OutboundRTPStreamStats.PacketsSent++ - latestStats.OutboundRTPStreamStats.BytesSent += uint64(headerSize + v.payloadLen) - latestStats.HeaderBytesSent += uint64(headerSize) + latestStats.OutboundRTPStreamStats.BytesSent += uint64(headerSize + v.payloadLen) //nolint:gosec // G115 + latestStats.HeaderBytesSent += uint64(headerSize) //nolint:gosec // G115 if !latestStats.remoteInboundFirstSequenceNumberInitialized { latestStats.remoteInboundFirstSequenceNumber = int64(v.header.SequenceNumber) latestStats.remoteInboundFirstSequenceNumberInitialized = true } + return latestStats } @@ -221,7 +236,9 @@ func (r *recorder) recordIncomingRR(latestStats internalStats, pkt *rtcp.Receive cycles := uint64(report.LastSequenceNumber&0xFFFF0000) >> 16 nr := uint64(report.LastSequenceNumber & 0x0000FFFF) highest := cycles*(0xFFFF+1) + nr - latestStats.RemoteInboundRTPStreamStats.PacketsReceived = highest - uint64(report.TotalLost) - uint64(latestStats.remoteInboundFirstSequenceNumber) + 1 + //nolint:gosec // G115 + latestStats.RemoteInboundRTPStreamStats.PacketsReceived = highest - uint64(report.TotalLost) - + uint64(latestStats.remoteInboundFirstSequenceNumber) + 1 } latestStats.RemoteInboundRTPStreamStats.PacketsLost = int64(report.TotalLost) latestStats.RemoteInboundRTPStreamStats.Jitter = float64(report.Jitter) / r.clockRate @@ -234,12 +251,14 @@ func (r *recorder) recordIncomingRR(latestStats internalStats, pkt *rtcp.Receive latestStats.RemoteInboundRTPStreamStats.RoundTripTime = (ts.Add(-dlsr)).Sub(ntp.ToTime(lastReport)) latestStats.RemoteInboundRTPStreamStats.TotalRoundTripTime += latestStats.RemoteInboundRTPStreamStats.RoundTripTime latestStats.RemoteInboundRTPStreamStats.RoundTripTimeMeasurements++ + break } } } latestStats.FractionLost = float64(report.FractionLost) / 256.0 } + return latestStats } @@ -253,6 +272,7 @@ func (r *recorder) recordIncomingXR(latestStats internalStats, pkt *rtcp.Extende if (lastRR&0x0000FFFFFFFF0000)>>16 == uint64(xrReport.LastRR) { dlrr := time.Duration(float64(xrReport.DLRR) / 65536.0 * float64(time.Second)) latestStats.RemoteOutboundRTPStreamStats.RoundTripTime = (ts.Add(-dlrr)).Sub(ntp.ToTime(lastRR)) + //nolint:lll latestStats.RemoteOutboundRTPStreamStats.TotalRoundTripTime += latestStats.RemoteOutboundRTPStreamStats.RoundTripTime latestStats.RemoteOutboundRTPStreamStats.RoundTripTimeMeasurements++ } @@ -261,6 +281,7 @@ func (r *recorder) recordIncomingXR(latestStats internalStats, pkt *rtcp.Extende } } } + return latestStats } @@ -270,13 +291,15 @@ func contains(ls []uint32, e uint32) bool { return true } } + return false } -func (r *recorder) recordIncomingRTCP(latestStats internalStats, v *incomingRTCP) internalStats { - for _, pkt := range v.pkts { +func (r *recorder) recordIncomingRTCP(latestStats internalStats, incoming *incomingRTCP) internalStats { + for _, pkt := range incoming.pkts { if !contains(pkt.DestinationSSRC(), r.ssrc) { r.logger.Debugf("skipping incoming RTCP pkt: %v", pkt) + continue } switch pkt := pkt.(type) { @@ -287,7 +310,7 @@ func (r *recorder) recordIncomingRTCP(latestStats internalStats, v *incomingRTCP case *rtcp.PictureLossIndication: latestStats.OutboundRTPStreamStats.PLICount++ case *rtcp.ReceiverReport: - latestStats = r.recordIncomingRR(latestStats, pkt, v.ts) + latestStats = r.recordIncomingRR(latestStats, pkt, incoming.ts) case *rtcp.SenderReport: latestStats.RemoteOutboundRTPStreamStats.PacketsSent = uint64(pkt.PacketCount) latestStats.RemoteOutboundRTPStreamStats.BytesSent = uint64(pkt.OctetCount) @@ -295,9 +318,10 @@ func (r *recorder) recordIncomingRTCP(latestStats internalStats, v *incomingRTCP latestStats.ReportsSent++ case *rtcp.ExtendedReport: - return r.recordIncomingXR(latestStats, pkt, v.ts) + return r.recordIncomingXR(latestStats, pkt, incoming.ts) } } + return latestStats } @@ -315,6 +339,7 @@ func (r *recorder) QueueIncomingRTP(ts time.Time, buf []byte, attr interceptor.A header, err := attr.GetRTPHeader(buf) if err != nil { r.logger.Warnf("failed to get RTP Header, skipping incoming RTP packet in stats calculation: %v", err) + return } hdr := header.Clone() @@ -338,6 +363,7 @@ func (r *recorder) QueueIncomingRTCP(ts time.Time, buf []byte, attr interceptor. pkts, err := attr.GetRTCPPackets(buf) if err != nil { r.logger.Warnf("failed to get RTCP packets, skipping incoming RTCP packet in stats calculation: %v", err) + return } r.ms.Lock() @@ -381,5 +407,6 @@ func minInt(a, b int) int { if a < b { return a } + return b } diff --git a/pkg/stats/stats_recorder_test.go b/pkg/stats/stats_recorder_test.go index 3aa5b4cf..ef310844 100644 --- a/pkg/stats/stats_recorder_test.go +++ b/pkg/stats/stats_recorder_test.go @@ -18,17 +18,22 @@ import ( ) func mustMarshalRTP(t *testing.T, pkt rtp.Packet) []byte { + t.Helper() buf, err := pkt.Marshal() assert.NoError(t, err) + return buf } func mustMarshalRTCPs(t *testing.T, pkt rtcp.Packet) []byte { + t.Helper() buf, err := pkt.Marshal() assert.NoError(t, err) + return buf } +//nolint:maintidx func TestStatsRecorder(t *testing.T) { cname := &rtcp.SourceDescription{ Chunks: []rtcp.SourceDescriptionChunk{{ @@ -228,7 +233,8 @@ func TestStatsRecorder(t *testing.T) { &rtcp.ReceiverReport{ SSRC: 0, Reports: []rtcp.ReceptionReport{{ - SSRC: 0, + SSRC: 0, + //nolint:gosec // G115 LastSenderReport: uint32((ntp.ToNTP(now) & 0x0000FFFFFFFF0000) >> 16), Delay: 1 * 65536.0, }}, @@ -303,7 +309,8 @@ func TestStatsRecorder(t *testing.T) { &rtcp.DLRRReportBlock{ Reports: []rtcp.DLRRReport{ { - SSRC: 0, + SSRC: 0, + //nolint:gosec // G115 LastRR: uint32((ntp.ToNTP(now) >> 16) & 0xFFFFFFFF), DLRR: 1 * 65536.0, }, @@ -785,30 +792,30 @@ func TestStatsRecorder(t *testing.T) { }, } { t.Run(fmt.Sprintf("%v:%v", i, cc.name), func(t *testing.T) { - r := newRecorder(0, 90_000) + recorder := newRecorder(0, 90_000) - r.Start() + recorder.Start() for _, record := range cc.records { switch v := record.content.(type) { case incomingRTP: - r.QueueIncomingRTP(record.ts, mustMarshalRTP(t, rtp.Packet{Header: v.header}), v.attr) + recorder.QueueIncomingRTP(record.ts, mustMarshalRTP(t, rtp.Packet{Header: v.header}), v.attr) case incomingRTCP: pkts := make(rtcp.CompoundPacket, len(v.pkts)) copy(pkts, v.pkts) - r.QueueIncomingRTCP(record.ts, mustMarshalRTCPs(t, &pkts), v.attr) + recorder.QueueIncomingRTCP(record.ts, mustMarshalRTCPs(t, &pkts), v.attr) case outgoingRTP: - r.QueueOutgoingRTP(record.ts, &v.header, []byte{}, v.attr) + recorder.QueueOutgoingRTP(record.ts, &v.header, []byte{}, v.attr) case outgoingRTCP: - r.QueueOutgoingRTCP(record.ts, v.pkts, v.attr) + recorder.QueueOutgoingRTCP(record.ts, v.pkts, v.attr) default: assert.FailNow(t, "invalid test case") } } - s := r.GetStats() + s := recorder.GetStats() - r.Stop() + recorder.Stop() assert.Equal(t, cc.expectedInboundRTPStreamStats, s.InboundRTPStreamStats) assert.Equal(t, cc.expectedOutboundRTPStreamStats, s.OutboundRTPStreamStats) @@ -819,7 +826,7 @@ func TestStatsRecorder(t *testing.T) { } func TestStatsRecorder_DLRR_Precision(t *testing.T) { - r := newRecorder(0, 90_000) + recorder := newRecorder(0, 90_000) report := &rtcp.ExtendedReport{ Reports: []rtcp.ReportBlock{ @@ -835,7 +842,7 @@ func TestStatsRecorder_DLRR_Precision(t *testing.T) { }, } - s := r.recordIncomingXR(internalStats{ + s := recorder.recordIncomingXR(internalStats{ lastReceiverReferenceTimes: []uint64{50000000}, }, report, time.Time{}) @@ -863,7 +870,7 @@ func TestGetStatsNotBlocking(t *testing.T) { } func TestQueueNotBlocking(t *testing.T) { - for _, i := range []struct { + for _, testCase := range []struct { f func(r *recorder) name string }{ @@ -892,7 +899,7 @@ func TestQueueNotBlocking(t *testing.T) { name: "QueueOutgoingRTCP", }, } { - t.Run(i.name+"NotBlocking", func(t *testing.T) { + t.Run(testCase.name+"NotBlocking", func(t *testing.T) { r := newRecorder(0, 90_000) ctx, cancel := context.WithTimeout(context.Background(), time.Second) @@ -901,7 +908,7 @@ func TestQueueNotBlocking(t *testing.T) { go func() { defer cancel() r.Start() - i.f(r) + testCase.f(r) }() go r.Stop() diff --git a/pkg/twcc/arrival_time_map.go b/pkg/twcc/arrival_time_map.go index d3e2a8af..a496ad73 100644 --- a/pkg/twcc/arrival_time_map.go +++ b/pkg/twcc/arrival_time_map.go @@ -12,6 +12,8 @@ const ( // of the arrival times of packets. It is used by the TWCC interceptor to build feedback // packets. // See https://source.chromium.org/chromium/chromium/src/+/refs/heads/main:third_party/webrtc/modules/remote_bitrate_estimator/packet_arrival_map.h;drc=b5cd13bb6d5d157a5fbe3628b2dd1c1e106203c6 +// +//nolint:lll type packetArrivalTimeMap struct { // arrivalTimes is a circular buffer, where the packet with sequence number sn is stored // in slot sn % len(arrivalTimes) @@ -31,12 +33,14 @@ func (m *packetArrivalTimeMap) AddPacket(sequenceNumber int64, arrivalTime int64 m.beginSequenceNumber = sequenceNumber m.endSequenceNumber = sequenceNumber + 1 m.arrivalTimes[m.index(sequenceNumber)] = arrivalTime + return } if sequenceNumber >= m.beginSequenceNumber && sequenceNumber < m.endSequenceNumber { // The packet is within the buffer, no need to resize. m.arrivalTimes[m.index(sequenceNumber)] = arrivalTime + return } @@ -53,6 +57,7 @@ func (m *packetArrivalTimeMap) AddPacket(sequenceNumber int64, arrivalTime int64 m.arrivalTimes[m.index(sequenceNumber)] = arrivalTime m.setNotReceived(sequenceNumber+1, m.beginSequenceNumber) m.beginSequenceNumber = sequenceNumber + return } @@ -64,6 +69,7 @@ func (m *packetArrivalTimeMap) AddPacket(sequenceNumber int64, arrivalTime int64 m.beginSequenceNumber = sequenceNumber m.endSequenceNumber = newEndSequenceNumber m.arrivalTimes[m.index(sequenceNumber)] = arrivalTime + return } @@ -99,12 +105,15 @@ func (m *packetArrivalTimeMap) EndSequenceNumber() int64 { // FindNextAtOrAfter returns the sequence number and timestamp of the first received packet that has a sequence number // greator or equal to sequenceNumber. -func (m *packetArrivalTimeMap) FindNextAtOrAfter(sequenceNumber int64) (foundSequenceNumber int64, arrivalTime int64, ok bool) { +func (m *packetArrivalTimeMap) FindNextAtOrAfter(sequenceNumber int64) ( + foundSequenceNumber int64, arrivalTime int64, ok bool, +) { for sequenceNumber = m.Clamp(sequenceNumber); sequenceNumber < m.endSequenceNumber; sequenceNumber++ { if t := m.get(sequenceNumber); t >= 0 { return sequenceNumber, t, true } } + return -1, -1, false } @@ -116,6 +125,7 @@ func (m *packetArrivalTimeMap) EraseTo(sequenceNumber int64) { if sequenceNumber >= m.endSequenceNumber { // Erase all. m.beginSequenceNumber = m.endSequenceNumber + return } // Remove some @@ -138,7 +148,7 @@ func (m *packetArrivalTimeMap) HasReceived(sequenceNumber int64) bool { return m.get(sequenceNumber) >= 0 } -// Clamp returns sequenceNumber clamped to [beginSequenceNumber, endSequenceNumber] +// Clamp returns sequenceNumber clamped to [beginSequenceNumber, endSequenceNumber]. func (m *packetArrivalTimeMap) Clamp(sequenceNumber int64) int64 { if sequenceNumber < m.beginSequenceNumber { return m.beginSequenceNumber @@ -146,6 +156,7 @@ func (m *packetArrivalTimeMap) Clamp(sequenceNumber int64) int64 { if m.endSequenceNumber < sequenceNumber { return m.endSequenceNumber } + return sequenceNumber } @@ -153,6 +164,7 @@ func (m *packetArrivalTimeMap) get(sequenceNumber int64) int64 { if sequenceNumber < m.beginSequenceNumber || sequenceNumber >= m.endSequenceNumber { return -1 } + return m.arrivalTimes[m.index(sequenceNumber)] } diff --git a/pkg/twcc/arrival_time_map_test.go b/pkg/twcc/arrival_time_map_test.go index cf2103d9..bf00df2e 100644 --- a/pkg/twcc/arrival_time_map_test.go +++ b/pkg/twcc/arrival_time_map_test.go @@ -9,261 +9,262 @@ import ( "github.com/stretchr/testify/assert" ) +//nolint:maintidx func TestArrivalTimeMap(t *testing.T) { t.Run("consistent when empty", func(t *testing.T) { - var m packetArrivalTimeMap - assert.Equal(t, m.BeginSequenceNumber(), m.EndSequenceNumber()) - assert.False(t, m.HasReceived(0)) - assert.Equal(t, int64(0), m.Clamp(-5)) - assert.Equal(t, int64(0), m.Clamp(5)) + var arrivalTimeMap packetArrivalTimeMap + assert.Equal(t, arrivalTimeMap.BeginSequenceNumber(), arrivalTimeMap.EndSequenceNumber()) + assert.False(t, arrivalTimeMap.HasReceived(0)) + assert.Equal(t, int64(0), arrivalTimeMap.Clamp(-5)) + assert.Equal(t, int64(0), arrivalTimeMap.Clamp(5)) }) t.Run("inserts first item into map", func(t *testing.T) { - var m packetArrivalTimeMap - m.AddPacket(42, 10) - assert.Equal(t, int64(42), m.BeginSequenceNumber()) - assert.Equal(t, int64(43), m.EndSequenceNumber()) - - assert.False(t, m.HasReceived(41)) - assert.True(t, m.HasReceived(42)) - assert.False(t, m.HasReceived(43)) - assert.False(t, m.HasReceived(44)) - - assert.Equal(t, int64(42), m.Clamp(-100)) - assert.Equal(t, int64(42), m.Clamp(42)) - assert.Equal(t, int64(43), m.Clamp(100)) + var arrivalTimeMap packetArrivalTimeMap + arrivalTimeMap.AddPacket(42, 10) + assert.Equal(t, int64(42), arrivalTimeMap.BeginSequenceNumber()) + assert.Equal(t, int64(43), arrivalTimeMap.EndSequenceNumber()) + + assert.False(t, arrivalTimeMap.HasReceived(41)) + assert.True(t, arrivalTimeMap.HasReceived(42)) + assert.False(t, arrivalTimeMap.HasReceived(43)) + assert.False(t, arrivalTimeMap.HasReceived(44)) + + assert.Equal(t, int64(42), arrivalTimeMap.Clamp(-100)) + assert.Equal(t, int64(42), arrivalTimeMap.Clamp(42)) + assert.Equal(t, int64(43), arrivalTimeMap.Clamp(100)) }) t.Run("inserts with gaps", func(t *testing.T) { - var m packetArrivalTimeMap - m.AddPacket(42, 0) - m.AddPacket(45, 11) - assert.Equal(t, int64(42), m.BeginSequenceNumber()) - assert.Equal(t, int64(46), m.EndSequenceNumber()) - - assert.False(t, m.HasReceived(41)) - assert.True(t, m.HasReceived(42)) - assert.False(t, m.HasReceived(43)) - assert.False(t, m.HasReceived(44)) - assert.True(t, m.HasReceived(45)) - assert.False(t, m.HasReceived(46)) - - assert.Equal(t, int64(0), m.get(42)) - assert.Less(t, m.get(43), int64(0)) - assert.Less(t, m.get(44), int64(0)) - assert.Equal(t, int64(11), m.get(45)) - - assert.Equal(t, int64(42), m.Clamp(-100)) - assert.Equal(t, int64(44), m.Clamp(44)) - assert.Equal(t, int64(46), m.Clamp(100)) + var arrivalTimeMap packetArrivalTimeMap + arrivalTimeMap.AddPacket(42, 0) + arrivalTimeMap.AddPacket(45, 11) + assert.Equal(t, int64(42), arrivalTimeMap.BeginSequenceNumber()) + assert.Equal(t, int64(46), arrivalTimeMap.EndSequenceNumber()) + + assert.False(t, arrivalTimeMap.HasReceived(41)) + assert.True(t, arrivalTimeMap.HasReceived(42)) + assert.False(t, arrivalTimeMap.HasReceived(43)) + assert.False(t, arrivalTimeMap.HasReceived(44)) + assert.True(t, arrivalTimeMap.HasReceived(45)) + assert.False(t, arrivalTimeMap.HasReceived(46)) + + assert.Equal(t, int64(0), arrivalTimeMap.get(42)) + assert.Less(t, arrivalTimeMap.get(43), int64(0)) + assert.Less(t, arrivalTimeMap.get(44), int64(0)) + assert.Equal(t, int64(11), arrivalTimeMap.get(45)) + + assert.Equal(t, int64(42), arrivalTimeMap.Clamp(-100)) + assert.Equal(t, int64(44), arrivalTimeMap.Clamp(44)) + assert.Equal(t, int64(46), arrivalTimeMap.Clamp(100)) }) t.Run("find next at or after with gaps", func(t *testing.T) { - var m packetArrivalTimeMap - m.AddPacket(42, 0) - m.AddPacket(45, 11) + var arrivalTimeMap packetArrivalTimeMap + arrivalTimeMap.AddPacket(42, 0) + arrivalTimeMap.AddPacket(45, 11) - seq, ts, ok := m.FindNextAtOrAfter(42) + seq, ts, ok := arrivalTimeMap.FindNextAtOrAfter(42) assert.Equal(t, int64(42), seq) assert.Equal(t, int64(0), ts) assert.True(t, ok) - seq, ts, ok = m.FindNextAtOrAfter(43) + seq, ts, ok = arrivalTimeMap.FindNextAtOrAfter(43) assert.Equal(t, int64(45), seq) assert.Equal(t, int64(11), ts) assert.True(t, ok) }) t.Run("inserts within buffer", func(t *testing.T) { - var m packetArrivalTimeMap - m.AddPacket(42, 10) - m.AddPacket(45, 11) - - m.AddPacket(43, 12) - m.AddPacket(44, 13) - - assert.False(t, m.HasReceived(41)) - assert.True(t, m.HasReceived(42)) - assert.True(t, m.HasReceived(43)) - assert.True(t, m.HasReceived(44)) - assert.True(t, m.HasReceived(45)) - assert.False(t, m.HasReceived(46)) - - assert.Equal(t, int64(10), m.get(42)) - assert.Equal(t, int64(12), m.get(43)) - assert.Equal(t, int64(13), m.get(44)) - assert.Equal(t, int64(11), m.get(45)) + var arrivalTimeMap packetArrivalTimeMap + arrivalTimeMap.AddPacket(42, 10) + arrivalTimeMap.AddPacket(45, 11) + + arrivalTimeMap.AddPacket(43, 12) + arrivalTimeMap.AddPacket(44, 13) + + assert.False(t, arrivalTimeMap.HasReceived(41)) + assert.True(t, arrivalTimeMap.HasReceived(42)) + assert.True(t, arrivalTimeMap.HasReceived(43)) + assert.True(t, arrivalTimeMap.HasReceived(44)) + assert.True(t, arrivalTimeMap.HasReceived(45)) + assert.False(t, arrivalTimeMap.HasReceived(46)) + + assert.Equal(t, int64(10), arrivalTimeMap.get(42)) + assert.Equal(t, int64(12), arrivalTimeMap.get(43)) + assert.Equal(t, int64(13), arrivalTimeMap.get(44)) + assert.Equal(t, int64(11), arrivalTimeMap.get(45)) }) t.Run("grows buffer and removes old", func(t *testing.T) { - var m packetArrivalTimeMap + var arrivalTimeMap packetArrivalTimeMap var largeSeqNum int64 = 42 + maxNumberOfPackets - m.AddPacket(42, 10) - m.AddPacket(43, 11) - m.AddPacket(44, 12) - m.AddPacket(45, 13) - m.AddPacket(largeSeqNum, 12) - - assert.Equal(t, int64(43), m.BeginSequenceNumber()) - assert.Equal(t, largeSeqNum+1, m.EndSequenceNumber()) - - assert.False(t, m.HasReceived(41)) - assert.False(t, m.HasReceived(42)) - assert.True(t, m.HasReceived(43)) - assert.True(t, m.HasReceived(44)) - assert.True(t, m.HasReceived(45)) - assert.False(t, m.HasReceived(46)) - assert.True(t, m.HasReceived(largeSeqNum)) - assert.False(t, m.HasReceived(largeSeqNum+1)) + arrivalTimeMap.AddPacket(42, 10) + arrivalTimeMap.AddPacket(43, 11) + arrivalTimeMap.AddPacket(44, 12) + arrivalTimeMap.AddPacket(45, 13) + arrivalTimeMap.AddPacket(largeSeqNum, 12) + + assert.Equal(t, int64(43), arrivalTimeMap.BeginSequenceNumber()) + assert.Equal(t, largeSeqNum+1, arrivalTimeMap.EndSequenceNumber()) + + assert.False(t, arrivalTimeMap.HasReceived(41)) + assert.False(t, arrivalTimeMap.HasReceived(42)) + assert.True(t, arrivalTimeMap.HasReceived(43)) + assert.True(t, arrivalTimeMap.HasReceived(44)) + assert.True(t, arrivalTimeMap.HasReceived(45)) + assert.False(t, arrivalTimeMap.HasReceived(46)) + assert.True(t, arrivalTimeMap.HasReceived(largeSeqNum)) + assert.False(t, arrivalTimeMap.HasReceived(largeSeqNum+1)) }) t.Run("sequence number jump deletes all", func(t *testing.T) { - var m packetArrivalTimeMap + var arrivalTimeMap packetArrivalTimeMap var largeSeqNum int64 = 42 + 2*maxNumberOfPackets - m.AddPacket(42, 10) - m.AddPacket(largeSeqNum, 12) + arrivalTimeMap.AddPacket(42, 10) + arrivalTimeMap.AddPacket(largeSeqNum, 12) - assert.Equal(t, largeSeqNum, m.BeginSequenceNumber()) - assert.Equal(t, largeSeqNum+1, m.EndSequenceNumber()) + assert.Equal(t, largeSeqNum, arrivalTimeMap.BeginSequenceNumber()) + assert.Equal(t, largeSeqNum+1, arrivalTimeMap.EndSequenceNumber()) - assert.False(t, m.HasReceived(42)) - assert.True(t, m.HasReceived(largeSeqNum)) - assert.False(t, m.HasReceived(largeSeqNum+1)) + assert.False(t, arrivalTimeMap.HasReceived(42)) + assert.True(t, arrivalTimeMap.HasReceived(largeSeqNum)) + assert.False(t, arrivalTimeMap.HasReceived(largeSeqNum+1)) }) t.Run("expands before beginning", func(t *testing.T) { - var m packetArrivalTimeMap - m.AddPacket(42, 10) - m.AddPacket(-1000, 13) - assert.Equal(t, int64(-1000), m.BeginSequenceNumber()) - assert.Equal(t, int64(43), m.EndSequenceNumber()) - - assert.False(t, m.HasReceived(-1001)) - assert.True(t, m.HasReceived(-1000)) - assert.False(t, m.HasReceived(-999)) - assert.True(t, m.HasReceived(42)) - assert.False(t, m.HasReceived(43)) + var arrivalTimeMap packetArrivalTimeMap + arrivalTimeMap.AddPacket(42, 10) + arrivalTimeMap.AddPacket(-1000, 13) + assert.Equal(t, int64(-1000), arrivalTimeMap.BeginSequenceNumber()) + assert.Equal(t, int64(43), arrivalTimeMap.EndSequenceNumber()) + + assert.False(t, arrivalTimeMap.HasReceived(-1001)) + assert.True(t, arrivalTimeMap.HasReceived(-1000)) + assert.False(t, arrivalTimeMap.HasReceived(-999)) + assert.True(t, arrivalTimeMap.HasReceived(42)) + assert.False(t, arrivalTimeMap.HasReceived(43)) }) t.Run("expanding before beginning keeps received", func(t *testing.T) { - var m packetArrivalTimeMap + var arrivalTimeMap packetArrivalTimeMap var smallSeqNum int64 = 42 - 2*maxNumberOfPackets - m.AddPacket(42, 10) - m.AddPacket(smallSeqNum, 13) + arrivalTimeMap.AddPacket(42, 10) + arrivalTimeMap.AddPacket(smallSeqNum, 13) - assert.Equal(t, int64(42), m.BeginSequenceNumber()) - assert.Equal(t, int64(43), m.EndSequenceNumber()) + assert.Equal(t, int64(42), arrivalTimeMap.BeginSequenceNumber()) + assert.Equal(t, int64(43), arrivalTimeMap.EndSequenceNumber()) }) t.Run("erase to removes elements", func(t *testing.T) { - var m packetArrivalTimeMap - m.AddPacket(42, 10) - m.AddPacket(43, 11) - m.AddPacket(44, 12) - m.AddPacket(45, 13) + var arrivalTimeMap packetArrivalTimeMap + arrivalTimeMap.AddPacket(42, 10) + arrivalTimeMap.AddPacket(43, 11) + arrivalTimeMap.AddPacket(44, 12) + arrivalTimeMap.AddPacket(45, 13) - m.EraseTo(44) + arrivalTimeMap.EraseTo(44) - assert.Equal(t, int64(44), m.BeginSequenceNumber()) - assert.Equal(t, int64(46), m.EndSequenceNumber()) + assert.Equal(t, int64(44), arrivalTimeMap.BeginSequenceNumber()) + assert.Equal(t, int64(46), arrivalTimeMap.EndSequenceNumber()) - assert.False(t, m.HasReceived(43)) - assert.True(t, m.HasReceived(44)) - assert.True(t, m.HasReceived(45)) - assert.False(t, m.HasReceived(46)) + assert.False(t, arrivalTimeMap.HasReceived(43)) + assert.True(t, arrivalTimeMap.HasReceived(44)) + assert.True(t, arrivalTimeMap.HasReceived(45)) + assert.False(t, arrivalTimeMap.HasReceived(46)) }) t.Run("erases in empty map", func(t *testing.T) { - var m packetArrivalTimeMap + var arrivalTimeMap packetArrivalTimeMap - assert.Equal(t, m.BeginSequenceNumber(), m.EndSequenceNumber()) + assert.Equal(t, arrivalTimeMap.BeginSequenceNumber(), arrivalTimeMap.EndSequenceNumber()) - m.EraseTo(m.EndSequenceNumber()) - assert.Equal(t, m.BeginSequenceNumber(), m.EndSequenceNumber()) + arrivalTimeMap.EraseTo(arrivalTimeMap.EndSequenceNumber()) + assert.Equal(t, arrivalTimeMap.BeginSequenceNumber(), arrivalTimeMap.EndSequenceNumber()) }) t.Run("is tolerant to wrong arguments for erase", func(t *testing.T) { - var m packetArrivalTimeMap - m.AddPacket(42, 10) - m.AddPacket(43, 11) + var arrivalTimeMap packetArrivalTimeMap + arrivalTimeMap.AddPacket(42, 10) + arrivalTimeMap.AddPacket(43, 11) - m.EraseTo(1) + arrivalTimeMap.EraseTo(1) - assert.Equal(t, int64(42), m.BeginSequenceNumber()) - assert.Equal(t, int64(44), m.EndSequenceNumber()) + assert.Equal(t, int64(42), arrivalTimeMap.BeginSequenceNumber()) + assert.Equal(t, int64(44), arrivalTimeMap.EndSequenceNumber()) - m.EraseTo(100) + arrivalTimeMap.EraseTo(100) - assert.Equal(t, int64(44), m.BeginSequenceNumber()) - assert.Equal(t, int64(44), m.EndSequenceNumber()) + assert.Equal(t, int64(44), arrivalTimeMap.BeginSequenceNumber()) + assert.Equal(t, int64(44), arrivalTimeMap.EndSequenceNumber()) }) //nolint:dupl t.Run("erase all remembers beginning sequence number", func(t *testing.T) { - var m packetArrivalTimeMap - m.AddPacket(42, 10) - m.AddPacket(43, 11) - m.AddPacket(44, 12) - m.AddPacket(45, 13) - - m.EraseTo(46) - m.AddPacket(50, 10) - - assert.Equal(t, int64(46), m.BeginSequenceNumber()) - assert.Equal(t, int64(51), m.EndSequenceNumber()) - - assert.False(t, m.HasReceived(45)) - assert.False(t, m.HasReceived(46)) - assert.False(t, m.HasReceived(47)) - assert.False(t, m.HasReceived(48)) - assert.False(t, m.HasReceived(49)) - assert.True(t, m.HasReceived(50)) - assert.False(t, m.HasReceived(51)) + var arrivalTimeMap packetArrivalTimeMap + arrivalTimeMap.AddPacket(42, 10) + arrivalTimeMap.AddPacket(43, 11) + arrivalTimeMap.AddPacket(44, 12) + arrivalTimeMap.AddPacket(45, 13) + + arrivalTimeMap.EraseTo(46) + arrivalTimeMap.AddPacket(50, 10) + + assert.Equal(t, int64(46), arrivalTimeMap.BeginSequenceNumber()) + assert.Equal(t, int64(51), arrivalTimeMap.EndSequenceNumber()) + + assert.False(t, arrivalTimeMap.HasReceived(45)) + assert.False(t, arrivalTimeMap.HasReceived(46)) + assert.False(t, arrivalTimeMap.HasReceived(47)) + assert.False(t, arrivalTimeMap.HasReceived(48)) + assert.False(t, arrivalTimeMap.HasReceived(49)) + assert.True(t, arrivalTimeMap.HasReceived(50)) + assert.False(t, arrivalTimeMap.HasReceived(51)) }) //nolint:dupl t.Run("erase to missing sequence number", func(t *testing.T) { - var m packetArrivalTimeMap - m.AddPacket(37, 10) - m.AddPacket(39, 11) - m.AddPacket(40, 12) - m.AddPacket(41, 13) - - m.EraseTo(38) - - m.AddPacket(42, 40) - - assert.Equal(t, int64(38), m.BeginSequenceNumber()) - assert.Equal(t, int64(43), m.EndSequenceNumber()) - - assert.False(t, m.HasReceived(37)) - assert.False(t, m.HasReceived(38)) - assert.True(t, m.HasReceived(39)) - assert.True(t, m.HasReceived(40)) - assert.True(t, m.HasReceived(41)) - assert.True(t, m.HasReceived(42)) - assert.False(t, m.HasReceived(43)) + var arrivalTimeMap packetArrivalTimeMap + arrivalTimeMap.AddPacket(37, 10) + arrivalTimeMap.AddPacket(39, 11) + arrivalTimeMap.AddPacket(40, 12) + arrivalTimeMap.AddPacket(41, 13) + + arrivalTimeMap.EraseTo(38) + + arrivalTimeMap.AddPacket(42, 40) + + assert.Equal(t, int64(38), arrivalTimeMap.BeginSequenceNumber()) + assert.Equal(t, int64(43), arrivalTimeMap.EndSequenceNumber()) + + assert.False(t, arrivalTimeMap.HasReceived(37)) + assert.False(t, arrivalTimeMap.HasReceived(38)) + assert.True(t, arrivalTimeMap.HasReceived(39)) + assert.True(t, arrivalTimeMap.HasReceived(40)) + assert.True(t, arrivalTimeMap.HasReceived(41)) + assert.True(t, arrivalTimeMap.HasReceived(42)) + assert.False(t, arrivalTimeMap.HasReceived(43)) }) t.Run("remove old packets", func(t *testing.T) { - var m packetArrivalTimeMap - m.AddPacket(37, 10) - m.AddPacket(39, 11) - m.AddPacket(40, 12) - m.AddPacket(41, 13) + var arrivalTimeMap packetArrivalTimeMap + arrivalTimeMap.AddPacket(37, 10) + arrivalTimeMap.AddPacket(39, 11) + arrivalTimeMap.AddPacket(40, 12) + arrivalTimeMap.AddPacket(41, 13) - m.RemoveOldPackets(42, 11) + arrivalTimeMap.RemoveOldPackets(42, 11) - assert.Equal(t, int64(40), m.BeginSequenceNumber()) - assert.Equal(t, int64(42), m.EndSequenceNumber()) + assert.Equal(t, int64(40), arrivalTimeMap.BeginSequenceNumber()) + assert.Equal(t, int64(42), arrivalTimeMap.EndSequenceNumber()) - assert.False(t, m.HasReceived(39)) - assert.True(t, m.HasReceived(40)) - assert.True(t, m.HasReceived(41)) - assert.False(t, m.HasReceived(42)) + assert.False(t, arrivalTimeMap.HasReceived(39)) + assert.True(t, arrivalTimeMap.HasReceived(40)) + assert.True(t, arrivalTimeMap.HasReceived(41)) + assert.False(t, arrivalTimeMap.HasReceived(42)) }) t.Run("shrinks buffer when necessary", func(t *testing.T) { diff --git a/pkg/twcc/header_extension_interceptor.go b/pkg/twcc/header_extension_interceptor.go index 791b1459..52d9e84a 100644 --- a/pkg/twcc/header_extension_interceptor.go +++ b/pkg/twcc/header_extension_interceptor.go @@ -13,20 +13,20 @@ import ( var errHeaderIsNil = errors.New("header is nil") -// HeaderExtensionInterceptorFactory is a interceptor.Factory for a HeaderExtensionInterceptor +// HeaderExtensionInterceptorFactory is a interceptor.Factory for a HeaderExtensionInterceptor. type HeaderExtensionInterceptorFactory struct{} -// NewInterceptor constructs a new HeaderExtensionInterceptor +// NewInterceptor constructs a new HeaderExtensionInterceptor. func (h *HeaderExtensionInterceptorFactory) NewInterceptor(_ string) (interceptor.Interceptor, error) { return &HeaderExtensionInterceptor{}, nil } -// NewHeaderExtensionInterceptor returns a HeaderExtensionInterceptorFactory +// NewHeaderExtensionInterceptor returns a HeaderExtensionInterceptorFactory. func NewHeaderExtensionInterceptor() (*HeaderExtensionInterceptorFactory, error) { return &HeaderExtensionInterceptorFactory{}, nil } -// HeaderExtensionInterceptor adds transport wide sequence numbers as header extension to each RTP packet +// HeaderExtensionInterceptor adds transport wide sequence numbers as header extension to each RTP packet. type HeaderExtensionInterceptor struct { interceptor.NoOp nextSequenceNr uint32 @@ -36,31 +36,39 @@ const transportCCURI = "http://www.ietf.org/id/draft-holmer-rmcat-transport-wide // BindLocalStream returns a writer that adds a rtp.TransportCCExtension // header with increasing sequence numbers to each outgoing packet. -func (h *HeaderExtensionInterceptor) BindLocalStream(info *interceptor.StreamInfo, writer interceptor.RTPWriter) interceptor.RTPWriter { +func (h *HeaderExtensionInterceptor) BindLocalStream( + info *interceptor.StreamInfo, + writer interceptor.RTPWriter, +) interceptor.RTPWriter { var hdrExtID uint8 for _, e := range info.RTPHeaderExtensions { if e.URI == transportCCURI { - hdrExtID = uint8(e.ID) + hdrExtID = uint8(e.ID) //nolint:gosec // G115 + break } } if hdrExtID == 0 { // Don't add header extension if ID is 0, because 0 is an invalid extension ID return writer } - return interceptor.RTPWriterFunc(func(header *rtp.Header, payload []byte, attributes interceptor.Attributes) (int, error) { - sequenceNumber := atomic.AddUint32(&h.nextSequenceNr, 1) - 1 - tcc, err := (&rtp.TransportCCExtension{TransportSequence: uint16(sequenceNumber)}).Marshal() - if err != nil { - return 0, err - } - if header == nil { - return 0, errHeaderIsNil - } - err = header.SetExtension(hdrExtID, tcc) - if err != nil { - return 0, err - } - return writer.Write(header, payload, attributes) - }) + return interceptor.RTPWriterFunc( + func(header *rtp.Header, payload []byte, attributes interceptor.Attributes) (int, error) { + sequenceNumber := atomic.AddUint32(&h.nextSequenceNr, 1) - 1 + //nolint:gosec // G115 + tcc, err := (&rtp.TransportCCExtension{TransportSequence: uint16(sequenceNumber)}).Marshal() + if err != nil { + return 0, err + } + if header == nil { + return 0, errHeaderIsNil + } + err = header.SetExtension(hdrExtID, tcc) + if err != nil { + return 0, err + } + + return writer.Write(header, payload, attributes) + }, + ) } diff --git a/pkg/twcc/header_extension_interceptor_test.go b/pkg/twcc/header_extension_interceptor_test.go index 0d85563a..e4b29f81 100644 --- a/pkg/twcc/header_extension_interceptor_test.go +++ b/pkg/twcc/header_extension_interceptor_test.go @@ -72,7 +72,7 @@ func TestHeaderExtensionInterceptor(t *testing.T) { panic("written rtp packet not found") } } - }(pChan, uint16(i+1)) + }(pChan, uint16(i+1)) //nolint:gosec // G115 } wg.Wait() close(pChan) diff --git a/pkg/twcc/sender_interceptor.go b/pkg/twcc/sender_interceptor.go index d7906fc6..229330ad 100644 --- a/pkg/twcc/sender_interceptor.go +++ b/pkg/twcc/sender_interceptor.go @@ -14,16 +14,16 @@ import ( "github.com/pion/rtp" ) -// SenderInterceptorFactory is a interceptor.Factory for a SenderInterceptor +// SenderInterceptorFactory is a interceptor.Factory for a SenderInterceptor. type SenderInterceptorFactory struct { opts []Option } var errClosed = errors.New("interceptor is closed") -// NewInterceptor constructs a new SenderInterceptor +// NewInterceptor constructs a new SenderInterceptor. func (s *SenderInterceptorFactory) NewInterceptor(_ string) (interceptor.Interceptor, error) { - i := &SenderInterceptor{ + senderInterceptor := &SenderInterceptor{ log: logging.NewDefaultLoggerFactory().NewLogger("twcc_sender_interceptor"), packetChan: make(chan packet), close: make(chan struct{}), @@ -32,13 +32,13 @@ func (s *SenderInterceptorFactory) NewInterceptor(_ string) (interceptor.Interce } for _, opt := range s.opts { - err := opt(i) + err := opt(senderInterceptor) if err != nil { return nil, err } } - return i, nil + return senderInterceptor, nil } // NewSenderInterceptor returns a new SenderInterceptorFactory configured with the given options. @@ -64,7 +64,7 @@ type SenderInterceptor struct { packetChan chan packet } -// An Option is a function that can be used to configure a SenderInterceptor +// An Option is a function that can be used to configure a SenderInterceptor. type Option func(*SenderInterceptor) error // SendInterval sets the interval at which the interceptor @@ -72,6 +72,7 @@ type Option func(*SenderInterceptor) error func SendInterval(interval time.Duration) Option { return func(s *SenderInterceptor) error { s.interval = interval + return nil } } @@ -102,54 +103,63 @@ type packet struct { ssrc uint32 } -// BindRemoteStream lets you modify any incoming RTP packets. It is called once for per RemoteStream. The returned method +// BindRemoteStream lets you modify any incoming RTP packets. +// It is called once for per RemoteStream. The returned method // will be called once per rtp packet. -func (s *SenderInterceptor) BindRemoteStream(info *interceptor.StreamInfo, reader interceptor.RTPReader) interceptor.RTPReader { +// +//nolint:cyclop +func (s *SenderInterceptor) BindRemoteStream( + info *interceptor.StreamInfo, reader interceptor.RTPReader, +) interceptor.RTPReader { var hdrExtID uint8 for _, e := range info.RTPHeaderExtensions { if e.URI == transportCCURI { - hdrExtID = uint8(e.ID) + hdrExtID = uint8(e.ID) //nolint:gosec // G115 + break } } if hdrExtID == 0 { // Don't try to read header extension if ID is 0, because 0 is an invalid extension ID return reader } - return interceptor.RTPReaderFunc(func(buf []byte, attributes interceptor.Attributes) (int, interceptor.Attributes, error) { - i, attr, err := reader.Read(buf, attributes) - if err != nil { - return 0, nil, err - } - if attr == nil { - attr = make(interceptor.Attributes) - } - header, err := attr.GetRTPHeader(buf[:i]) - if err != nil { - return 0, nil, err - } - var tccExt rtp.TransportCCExtension - if ext := header.GetExtension(hdrExtID); ext != nil { - err = tccExt.Unmarshal(ext) + return interceptor.RTPReaderFunc( + func(buf []byte, attributes interceptor.Attributes) (int, interceptor.Attributes, error) { + i, attr, err := reader.Read(buf, attributes) if err != nil { return 0, nil, err } - p := packet{ - hdr: header, - sequenceNumber: tccExt.TransportSequence, - arrivalTime: time.Since(s.startTime).Microseconds(), - ssrc: info.SSRC, + if attr == nil { + attr = make(interceptor.Attributes) + } + header, err := attr.GetRTPHeader(buf[:i]) + if err != nil { + return 0, nil, err } - select { - case <-s.close: - return 0, nil, errClosed - case s.packetChan <- p: + var tccExt rtp.TransportCCExtension + if ext := header.GetExtension(hdrExtID); ext != nil { + err = tccExt.Unmarshal(ext) + if err != nil { + return 0, nil, err + } + + p := packet{ + hdr: header, + sequenceNumber: tccExt.TransportSequence, + arrivalTime: time.Since(s.startTime).Microseconds(), + ssrc: info.SSRC, + } + select { + case <-s.close: + return 0, nil, errClosed + case s.packetChan <- p: + } } - } - return i, attr, nil - }) + return i, attr, nil + }, + ) } // Close closes the interceptor. @@ -174,7 +184,7 @@ func (s *SenderInterceptor) isClosed() bool { } } -func (s *SenderInterceptor) loop(w interceptor.RTCPWriter) { +func (s *SenderInterceptor) loop(writer interceptor.RTCPWriter) { defer s.wg.Done() select { @@ -189,6 +199,7 @@ func (s *SenderInterceptor) loop(w interceptor.RTCPWriter) { select { case <-s.close: ticker.Stop() + return case p := <-s.packetChan: s.recorder.Record(p.ssrc, p.sequenceNumber, p.arrivalTime) @@ -199,7 +210,7 @@ func (s *SenderInterceptor) loop(w interceptor.RTCPWriter) { if len(pkts) == 0 { continue } - if _, err := w.Write(pkts, nil); err != nil { + if _, err := writer.Write(pkts, nil); err != nil { s.log.Error(err.Error()) } } diff --git a/pkg/twcc/sender_interceptor_test.go b/pkg/twcc/sender_interceptor_test.go index b188862e..08e508ca 100644 --- a/pkg/twcc/sender_interceptor_test.go +++ b/pkg/twcc/sender_interceptor_test.go @@ -15,6 +15,7 @@ import ( "github.com/stretchr/testify/assert" ) +//nolint:maintidx func TestSenderInterceptor(t *testing.T) { t.Run("before any packets", func(t *testing.T) { f, err := NewSenderInterceptor() @@ -60,6 +61,7 @@ func TestSenderInterceptor(t *testing.T) { for i := 0; i < 10; i++ { hdr := rtp.Header{} + //nolint:gosec // G115 tcc, err := (&rtp.TransportCCExtension{TransportSequence: uint16(i)}).Marshal() assert.NoError(t, err) err = hdr.SetExtension(1, tcc) @@ -103,6 +105,7 @@ func TestSenderInterceptor(t *testing.T) { time.Sleep(time.Duration(d) * time.Millisecond) hdr := rtp.Header{} + //nolint:gosec // G115 tcc, err := (&rtp.TransportCCExtension{TransportSequence: uint16(i)}).Marshal() assert.NoError(t, err) err = hdr.SetExtension(1, tcc) @@ -159,6 +162,7 @@ func TestSenderInterceptor(t *testing.T) { time.Sleep(time.Duration(d) * time.Millisecond) hdr := rtp.Header{} + //nolint:gosec // G115 tcc, err := (&rtp.TransportCCExtension{TransportSequence: uint16(i)}).Marshal() assert.NoError(t, err) err = hdr.SetExtension(1, tcc) @@ -226,6 +230,7 @@ func TestSenderInterceptor(t *testing.T) { for _, i := range []int{65530, 65534, 65535, 1, 2, 10} { hdr := rtp.Header{} + //nolint:gosec // G115 tcc, err := (&rtp.TransportCCExtension{TransportSequence: uint16(i)}).Marshal() assert.NoError(t, err) err = hdr.SetExtension(1, tcc) @@ -280,7 +285,7 @@ func TestSenderInterceptor_Leak(t *testing.T) { f, err := NewSenderInterceptor(SendInterval(200 * time.Millisecond)) assert.NoError(t, err) - i, err := f.NewInterceptor("") + testInterceptor, err := f.NewInterceptor("") assert.NoError(t, err) stream := test.NewMockStream(&interceptor.StreamInfo{RTPHeaderExtensions: []interceptor.RTPHeaderExtension{ @@ -288,14 +293,15 @@ func TestSenderInterceptor_Leak(t *testing.T) { URI: transportCCURI, ID: 1, }, - }}, i) + }}, testInterceptor) defer func() { assert.NoError(t, stream.Close()) }() - assert.NoError(t, i.Close()) + assert.NoError(t, testInterceptor.Close()) for _, i := range []int{0, 1, 2, 3, 4, 5} { hdr := rtp.Header{} + //nolint:gosec // G115 tcc, err := (&rtp.TransportCCExtension{TransportSequence: uint16(i)}).Marshal() assert.NoError(t, err) diff --git a/pkg/twcc/twcc.go b/pkg/twcc/twcc.go index 6464c77b..750e4541 100644 --- a/pkg/twcc/twcc.go +++ b/pkg/twcc/twcc.go @@ -71,12 +71,13 @@ func (r *Recorder) Record(mediaSSRC uint32, sequenceNumber uint16, arrivalTime i } func (r *Recorder) maybeCullOldPackets(sequenceNumber int64, arrivalTime int64) { - if r.startSequenceNumber != nil && *r.startSequenceNumber >= r.arrivalTimeMap.EndSequenceNumber() && arrivalTime >= packetWindowMicroseconds { + if r.startSequenceNumber != nil && *r.startSequenceNumber >= r.arrivalTimeMap.EndSequenceNumber() && + arrivalTime >= packetWindowMicroseconds { r.arrivalTimeMap.RemoveOldPackets(sequenceNumber, arrivalTime-packetWindowMicroseconds) } } -// PacketsHeld returns the number of received packets currently held by the recorder +// PacketsHeld returns the number of received packets currently held by the recorder. func (r *Recorder) PacketsHeld() int { return r.packetsHeld } @@ -101,6 +102,7 @@ func (r *Recorder) BuildFeedbackPacket() []rtcp.Packet { // old. } r.packetsHeld = 0 + return feedbacks } @@ -109,6 +111,7 @@ func (r *Recorder) BuildFeedbackPacket() []rtcp.Packet { func (r *Recorder) maybeBuildFeedbackPacket(beginSeqNumInclusive, endSeqNumExclusive int64) *feedback { // NOTE: The logic of this method is inspired by the implementation in Chrome. // See https://source.chromium.org/chromium/chromium/src/+/refs/heads/main:third_party/webrtc/modules/remote_bitrate_estimator/remote_estimator_proxy.cc;l=276;drc=b5cd13bb6d5d157a5fbe3628b2dd1c1e106203c6 + //nolint:lll startSNInclusive, endSNExclusive := r.arrivalTimeMap.Clamp(beginSeqNumInclusive), r.arrivalTimeMap.Clamp(endSeqNumExclusive) // Create feedback on demand, as we don't yet know if there are packets in the range that have been @@ -136,18 +139,19 @@ func (r *Recorder) maybeBuildFeedbackPacket(beginSeqNumInclusive, endSeqNumExclu // baseSequenceNumber is the expected first sequence number. This is known, // but we may not have actually received it, so the base time should be the time // of the first received packet in the feedback. - fb.setBase(uint16(baseSequenceNumber), arrivalTime) + fb.setBase(uint16(baseSequenceNumber), arrivalTime) //nolint:gosec // G115 - if !fb.addReceived(uint16(seq), arrivalTime) { + if !fb.addReceived(uint16(seq), arrivalTime) { //nolint:gosec // G115 // Could not add a single received packet to the feedback. // This is unexpected to actually occur, but if it does, we'll // try again after skipping any missing packets. // NOTE: It's fine that we already incremented fbPktCnt, as in essence // we did actually "skip" a feedback (and this matches Chrome's behavior). r.startSequenceNumber = &seq + return nil } - } else if !fb.addReceived(uint16(seq), arrivalTime) { + } else if !fb.addReceived(uint16(seq), arrivalTime) { //nolint:gosec // G115 // Could not add timestamp. Packet may be full. Return // and try again with a fresh packet. break @@ -157,6 +161,7 @@ func (r *Recorder) maybeBuildFeedbackPacket(beginSeqNumInclusive, endSeqNumExclu } r.startSequenceNumber = &nextSequenceNumber + return fb } @@ -192,7 +197,7 @@ func (f *feedback) setBase(sequenceNumber uint16, timeUS int64) { func (f *feedback) getRTCP() *rtcp.TransportLayerCC { f.rtcp.PacketStatusCount = f.sequenceNumberCount - f.rtcp.ReferenceTime = uint32(f.refTimestamp64MS) + f.rtcp.ReferenceTime = uint32(f.refTimestamp64MS) //nolint:gosec // G115 f.rtcp.BaseSequenceNumber = f.baseSequenceNumber for len(f.lastChunk.deltas) > 0 { f.chunks = append(f.chunks, f.lastChunk.encode()) @@ -200,7 +205,8 @@ func (f *feedback) getRTCP() *rtcp.TransportLayerCC { f.rtcp.PacketChunks = append(f.rtcp.PacketChunks, f.chunks...) f.rtcp.RecvDeltas = f.deltas - padLen := 20 + len(f.rtcp.PacketChunks)*2 + f.len // 4 bytes header + 16 bytes twcc header + 2 bytes for each chunk + length of deltas + // 4 bytes header + 16 bytes twcc header + 2 bytes for each chunk + length of deltas + padLen := 20 + len(f.rtcp.PacketChunks)*2 + f.len padding := padLen%4 != 0 for padLen%4 != 0 { padLen++ @@ -209,7 +215,7 @@ func (f *feedback) getRTCP() *rtcp.TransportLayerCC { Count: rtcp.FormatTCC, Type: rtcp.TypeTransportSpecificFeedback, Padding: padding, - Length: uint16((padLen / 4) - 1), + Length: uint16((padLen / 4) - 1), //nolint:gosec // G115 } return f.rtcp @@ -223,7 +229,8 @@ func (f *feedback) addReceived(sequenceNumber uint16, timestampUS int64) bool { } else { delta250US = (deltaUS - rtcp.TypeTCCDeltaScaleFactor/2) / rtcp.TypeTCCDeltaScaleFactor } - if delta250US < math.MinInt16 || delta250US > math.MaxInt16 { // delta doesn't fit into 16 bit, need to create new packet + // delta doesn't fit into 16 bit, need to create new packet + if delta250US < math.MinInt16 || delta250US > math.MaxInt16 { return false } deltaUSRounded := delta250US * rtcp.TypeTCCDeltaScaleFactor @@ -257,6 +264,7 @@ func (f *feedback) addReceived(sequenceNumber uint16, timestampUS int64) bool { f.lastTimestampUS += deltaUSRounded f.sequenceNumberCount++ f.nextSequenceNumber++ + return true } @@ -282,6 +290,7 @@ func (c *chunk) canAdd(delta uint16) bool { if len(c.deltas) < maxRunLengthCap && !c.hasDifferentTypes && delta == c.deltas[0] { return true } + return false } @@ -294,13 +303,15 @@ func (c *chunk) add(delta uint16) { func (c *chunk) encode() rtcp.PacketStatusChunk { if !c.hasDifferentTypes { defer c.reset() + return &rtcp.RunLengthChunk{ PacketStatusSymbol: c.deltas[0], - RunLength: uint16(len(c.deltas)), + RunLength: uint16(len(c.deltas)), //nolint:gosec // G115 } } if len(c.deltas) == maxOneBitCap { defer c.reset() + return &rtcp.StatusVectorChunk{ SymbolSize: rtcp.TypeTCCSymbolSizeOneBit, SymbolList: c.deltas, @@ -341,6 +352,7 @@ func maxInt(a, b int) int { if a > b { return a } + return b } @@ -348,6 +360,7 @@ func minInt(a, b int) int { if a < b { return a } + return b } @@ -355,6 +368,7 @@ func max64(a, b int64) int64 { if a > b { return a } + return b } @@ -362,5 +376,6 @@ func min64(a, b int64) int64 { if a < b { return a } + return b } diff --git a/pkg/twcc/twcc_test.go b/pkg/twcc/twcc_test.go index 7e413529..11eca446 100644 --- a/pkg/twcc/twcc_test.go +++ b/pkg/twcc/twcc_test.go @@ -12,6 +12,8 @@ import ( ) func rtcpToTwcc(t *testing.T, in []rtcp.Packet) []*rtcp.TransportLayerCC { + t.Helper() + out := make([]*rtcp.TransportLayerCC, len(in)) var ok bool for i, pkt := range in { @@ -25,20 +27,20 @@ func rtcpToTwcc(t *testing.T, in []rtcp.Packet) []*rtcp.TransportLayerCC { func Test_chunk_add(t *testing.T) { t.Run("fill with not received", func(t *testing.T) { - c := &chunk{} + testChunk := &chunk{} for i := 0; i < maxRunLengthCap; i++ { - assert.True(t, c.canAdd(rtcp.TypeTCCPacketNotReceived), i) - c.add(rtcp.TypeTCCPacketNotReceived) + assert.True(t, testChunk.canAdd(rtcp.TypeTCCPacketNotReceived), i) + testChunk.add(rtcp.TypeTCCPacketNotReceived) } - assert.Equal(t, make([]uint16, maxRunLengthCap), c.deltas) - assert.False(t, c.hasDifferentTypes) + assert.Equal(t, make([]uint16, maxRunLengthCap), testChunk.deltas) + assert.False(t, testChunk.hasDifferentTypes) - assert.False(t, c.canAdd(rtcp.TypeTCCPacketNotReceived)) - assert.False(t, c.canAdd(rtcp.TypeTCCPacketReceivedSmallDelta)) - assert.False(t, c.canAdd(rtcp.TypeTCCPacketReceivedLargeDelta)) + assert.False(t, testChunk.canAdd(rtcp.TypeTCCPacketNotReceived)) + assert.False(t, testChunk.canAdd(rtcp.TypeTCCPacketReceivedSmallDelta)) + assert.False(t, testChunk.canAdd(rtcp.TypeTCCPacketReceivedLargeDelta)) - statusChunk := c.encode() + statusChunk := testChunk.encode() assert.IsType(t, &rtcp.RunLengthChunk{}, statusChunk) buf, err := statusChunk.Marshal() @@ -47,20 +49,20 @@ func Test_chunk_add(t *testing.T) { }) t.Run("fill with small delta", func(t *testing.T) { - c := &chunk{} + testChunk := &chunk{} for i := 0; i < maxOneBitCap; i++ { - assert.True(t, c.canAdd(rtcp.TypeTCCPacketReceivedSmallDelta), i) - c.add(rtcp.TypeTCCPacketReceivedSmallDelta) + assert.True(t, testChunk.canAdd(rtcp.TypeTCCPacketReceivedSmallDelta), i) + testChunk.add(rtcp.TypeTCCPacketReceivedSmallDelta) } - assert.Equal(t, c.deltas, []uint16{1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1}) - assert.False(t, c.hasDifferentTypes) + assert.Equal(t, testChunk.deltas, []uint16{1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1}) + assert.False(t, testChunk.hasDifferentTypes) - assert.False(t, c.canAdd(rtcp.TypeTCCPacketReceivedLargeDelta)) - assert.False(t, c.canAdd(rtcp.TypeTCCPacketNotReceived)) + assert.False(t, testChunk.canAdd(rtcp.TypeTCCPacketReceivedLargeDelta)) + assert.False(t, testChunk.canAdd(rtcp.TypeTCCPacketNotReceived)) - statusChunk := c.encode() + statusChunk := testChunk.encode() assert.IsType(t, &rtcp.RunLengthChunk{}, statusChunk) buf, err := statusChunk.Marshal() @@ -69,21 +71,21 @@ func Test_chunk_add(t *testing.T) { }) t.Run("fill with large delta", func(t *testing.T) { - c := &chunk{} + testChunk := &chunk{} for i := 0; i < maxTwoBitCap; i++ { - assert.True(t, c.canAdd(rtcp.TypeTCCPacketReceivedLargeDelta), i) - c.add(rtcp.TypeTCCPacketReceivedLargeDelta) + assert.True(t, testChunk.canAdd(rtcp.TypeTCCPacketReceivedLargeDelta), i) + testChunk.add(rtcp.TypeTCCPacketReceivedLargeDelta) } - assert.Equal(t, c.deltas, []uint16{2, 2, 2, 2, 2, 2, 2}) - assert.True(t, c.hasLargeDelta) - assert.False(t, c.hasDifferentTypes) + assert.Equal(t, testChunk.deltas, []uint16{2, 2, 2, 2, 2, 2, 2}) + assert.True(t, testChunk.hasLargeDelta) + assert.False(t, testChunk.hasDifferentTypes) - assert.False(t, c.canAdd(rtcp.TypeTCCPacketReceivedSmallDelta)) - assert.False(t, c.canAdd(rtcp.TypeTCCPacketNotReceived)) + assert.False(t, testChunk.canAdd(rtcp.TypeTCCPacketReceivedSmallDelta)) + assert.False(t, testChunk.canAdd(rtcp.TypeTCCPacketNotReceived)) - statusChunk := c.encode() + statusChunk := testChunk.encode() assert.IsType(t, &rtcp.RunLengthChunk{}, statusChunk) buf, err := statusChunk.Marshal() @@ -92,27 +94,27 @@ func Test_chunk_add(t *testing.T) { }) t.Run("fill with different types", func(t *testing.T) { - c := &chunk{} - - assert.True(t, c.canAdd(rtcp.TypeTCCPacketReceivedSmallDelta)) - c.add(rtcp.TypeTCCPacketReceivedSmallDelta) - assert.True(t, c.canAdd(rtcp.TypeTCCPacketReceivedSmallDelta)) - c.add(rtcp.TypeTCCPacketReceivedSmallDelta) - assert.True(t, c.canAdd(rtcp.TypeTCCPacketReceivedSmallDelta)) - c.add(rtcp.TypeTCCPacketReceivedSmallDelta) - assert.True(t, c.canAdd(rtcp.TypeTCCPacketReceivedSmallDelta)) - c.add(rtcp.TypeTCCPacketReceivedSmallDelta) - - assert.True(t, c.canAdd(rtcp.TypeTCCPacketReceivedLargeDelta)) - c.add(rtcp.TypeTCCPacketReceivedLargeDelta) - assert.True(t, c.canAdd(rtcp.TypeTCCPacketReceivedLargeDelta)) - c.add(rtcp.TypeTCCPacketReceivedLargeDelta) - assert.True(t, c.canAdd(rtcp.TypeTCCPacketReceivedLargeDelta)) - c.add(rtcp.TypeTCCPacketReceivedLargeDelta) - - assert.False(t, c.canAdd(rtcp.TypeTCCPacketReceivedLargeDelta)) - - statusChunk := c.encode() + testChunk := &chunk{} + + assert.True(t, testChunk.canAdd(rtcp.TypeTCCPacketReceivedSmallDelta)) + testChunk.add(rtcp.TypeTCCPacketReceivedSmallDelta) + assert.True(t, testChunk.canAdd(rtcp.TypeTCCPacketReceivedSmallDelta)) + testChunk.add(rtcp.TypeTCCPacketReceivedSmallDelta) + assert.True(t, testChunk.canAdd(rtcp.TypeTCCPacketReceivedSmallDelta)) + testChunk.add(rtcp.TypeTCCPacketReceivedSmallDelta) + assert.True(t, testChunk.canAdd(rtcp.TypeTCCPacketReceivedSmallDelta)) + testChunk.add(rtcp.TypeTCCPacketReceivedSmallDelta) + + assert.True(t, testChunk.canAdd(rtcp.TypeTCCPacketReceivedLargeDelta)) + testChunk.add(rtcp.TypeTCCPacketReceivedLargeDelta) + assert.True(t, testChunk.canAdd(rtcp.TypeTCCPacketReceivedLargeDelta)) + testChunk.add(rtcp.TypeTCCPacketReceivedLargeDelta) + assert.True(t, testChunk.canAdd(rtcp.TypeTCCPacketReceivedLargeDelta)) + testChunk.add(rtcp.TypeTCCPacketReceivedLargeDelta) + + assert.False(t, testChunk.canAdd(rtcp.TypeTCCPacketReceivedLargeDelta)) + + statusChunk := testChunk.encode() assert.IsType(t, &rtcp.StatusVectorChunk{}, statusChunk) buf, err := statusChunk.Marshal() @@ -121,38 +123,38 @@ func Test_chunk_add(t *testing.T) { }) t.Run("overfill and encode", func(t *testing.T) { - c := chunk{} - - assert.True(t, c.canAdd(rtcp.TypeTCCPacketReceivedSmallDelta)) - c.add(rtcp.TypeTCCPacketReceivedSmallDelta) - assert.True(t, c.canAdd(rtcp.TypeTCCPacketNotReceived)) - c.add(rtcp.TypeTCCPacketNotReceived) - assert.True(t, c.canAdd(rtcp.TypeTCCPacketNotReceived)) - c.add(rtcp.TypeTCCPacketNotReceived) - assert.True(t, c.canAdd(rtcp.TypeTCCPacketNotReceived)) - c.add(rtcp.TypeTCCPacketNotReceived) - assert.True(t, c.canAdd(rtcp.TypeTCCPacketNotReceived)) - c.add(rtcp.TypeTCCPacketNotReceived) - assert.True(t, c.canAdd(rtcp.TypeTCCPacketNotReceived)) - c.add(rtcp.TypeTCCPacketNotReceived) - assert.True(t, c.canAdd(rtcp.TypeTCCPacketNotReceived)) - c.add(rtcp.TypeTCCPacketNotReceived) - assert.True(t, c.canAdd(rtcp.TypeTCCPacketNotReceived)) - c.add(rtcp.TypeTCCPacketNotReceived) - - assert.False(t, c.canAdd(rtcp.TypeTCCPacketReceivedLargeDelta)) - - statusChunk1 := c.encode() + testChunk := chunk{} + + assert.True(t, testChunk.canAdd(rtcp.TypeTCCPacketReceivedSmallDelta)) + testChunk.add(rtcp.TypeTCCPacketReceivedSmallDelta) + assert.True(t, testChunk.canAdd(rtcp.TypeTCCPacketNotReceived)) + testChunk.add(rtcp.TypeTCCPacketNotReceived) + assert.True(t, testChunk.canAdd(rtcp.TypeTCCPacketNotReceived)) + testChunk.add(rtcp.TypeTCCPacketNotReceived) + assert.True(t, testChunk.canAdd(rtcp.TypeTCCPacketNotReceived)) + testChunk.add(rtcp.TypeTCCPacketNotReceived) + assert.True(t, testChunk.canAdd(rtcp.TypeTCCPacketNotReceived)) + testChunk.add(rtcp.TypeTCCPacketNotReceived) + assert.True(t, testChunk.canAdd(rtcp.TypeTCCPacketNotReceived)) + testChunk.add(rtcp.TypeTCCPacketNotReceived) + assert.True(t, testChunk.canAdd(rtcp.TypeTCCPacketNotReceived)) + testChunk.add(rtcp.TypeTCCPacketNotReceived) + assert.True(t, testChunk.canAdd(rtcp.TypeTCCPacketNotReceived)) + testChunk.add(rtcp.TypeTCCPacketNotReceived) + + assert.False(t, testChunk.canAdd(rtcp.TypeTCCPacketReceivedLargeDelta)) + + statusChunk1 := testChunk.encode() assert.IsType(t, &rtcp.StatusVectorChunk{}, statusChunk1) - assert.Equal(t, 1, len(c.deltas)) + assert.Equal(t, 1, len(testChunk.deltas)) - assert.True(t, c.canAdd(rtcp.TypeTCCPacketReceivedLargeDelta)) - c.add(rtcp.TypeTCCPacketReceivedLargeDelta) + assert.True(t, testChunk.canAdd(rtcp.TypeTCCPacketReceivedLargeDelta)) + testChunk.add(rtcp.TypeTCCPacketReceivedLargeDelta) - statusChunk2 := c.encode() + statusChunk2 := testChunk.encode() assert.IsType(t, &rtcp.StatusVectorChunk{}, statusChunk2) - assert.Equal(t, 0, len(c.deltas)) + assert.Equal(t, 0, len(testChunk.deltas)) assert.Equal(t, &rtcp.StatusVectorChunk{ SymbolSize: rtcp.TypeTCCSymbolSizeTwoBit, @@ -177,23 +179,23 @@ func Test_feedback(t *testing.T) { }) t.Run("add received 1", func(t *testing.T) { - f := &feedback{} - f.setBase(1, 1000*1000) + testFeedback := &feedback{} + testFeedback.setBase(1, 1000*1000) - got := f.addReceived(1, 1023*1000) + got := testFeedback.addReceived(1, 1023*1000) assert.True(t, got) - assert.Equal(t, uint16(2), f.nextSequenceNumber) - assert.Equal(t, int64(15), f.refTimestamp64MS) + assert.Equal(t, uint16(2), testFeedback.nextSequenceNumber) + assert.Equal(t, int64(15), testFeedback.refTimestamp64MS) - got = f.addReceived(4, 1086*1000) + got = testFeedback.addReceived(4, 1086*1000) assert.True(t, got) - assert.Equal(t, uint16(5), f.nextSequenceNumber) - assert.Equal(t, int64(15), f.refTimestamp64MS) + assert.Equal(t, uint16(5), testFeedback.nextSequenceNumber) + assert.Equal(t, int64(15), testFeedback.refTimestamp64MS) - assert.True(t, f.lastChunk.hasDifferentTypes) - assert.Equal(t, 4, len(f.lastChunk.deltas)) - assert.NotContains(t, f.lastChunk.deltas, rtcp.TypeTCCPacketReceivedLargeDelta) + assert.True(t, testFeedback.lastChunk.hasDifferentTypes) + assert.Equal(t, 4, len(testFeedback.lastChunk.deltas)) + assert.NotContains(t, testFeedback.lastChunk.deltas, rtcp.TypeTCCPacketReceivedLargeDelta) }) t.Run("add received 2", func(t *testing.T) { @@ -263,7 +265,7 @@ func Test_feedback(t *testing.T) { f.setBase(5, base) for i := int64(0); i < 5; i++ { - got := f.addReceived(5+uint16(i+1), base+deltaUS*i) + got := f.addReceived(5+uint16(i+1), base+deltaUS*i) //nolint:gosec // G115 assert.True(t, got) } @@ -402,6 +404,8 @@ func Test_feedback(t *testing.T) { } func addRun(t *testing.T, r *Recorder, sequenceNumbers []uint16, arrivalTimes []int64) { + t.Helper() + assert.Equal(t, len(sequenceNumbers), len(arrivalTimes)) for i := range sequenceNumbers { @@ -415,25 +419,29 @@ const ( func increaseTime(arrivalTime *int64, increaseAmount int64) int64 { *arrivalTime += increaseAmount + return *arrivalTime } func marshalAll(t *testing.T, pkts []rtcp.Packet) { + t.Helper() + for _, pkt := range pkts { marshaled, err := pkt.Marshal() assert.NoError(t, err) // Chrome expects feedback packets to always be 18 bytes or more. // https://source.chromium.org/chromium/chromium/src/+/main:third_party/webrtc/modules/rtp_rtcp/source/rtcp_packet/transport_feedback.cc;l=423?q=transport_feedback.cc&ss=chromium%2Fchromium%2Fsrc + //nolint:lll assert.GreaterOrEqual(t, len(marshaled), 18) } } func TestBuildFeedbackPacket(t *testing.T) { - r := NewRecorder(5000) + recoder := NewRecorder(5000) arrivalTime := int64(scaleFactorReferenceTime) - addRun(t, r, []uint16{0, 1, 2, 3, 4, 5, 6, 7}, []int64{ + addRun(t, recoder, []uint16{0, 1, 2, 3, 4, 5, 6, 7}, []int64{ scaleFactorReferenceTime, increaseTime(&arrivalTime, rtcp.TypeTCCDeltaScaleFactor), increaseTime(&arrivalTime, rtcp.TypeTCCDeltaScaleFactor), @@ -444,7 +452,7 @@ func TestBuildFeedbackPacket(t *testing.T) { increaseTime(&arrivalTime, rtcp.TypeTCCDeltaScaleFactor*256), }) - rtcpPackets := r.BuildFeedbackPacket() + rtcpPackets := recoder.BuildFeedbackPacket() assert.Equal(t, 1, len(rtcpPackets)) assert.Equal(t, &rtcp.TransportLayerCC{ @@ -487,25 +495,25 @@ func TestBuildFeedbackPacket(t *testing.T) { } func TestBuildFeedbackPacket_Rolling(t *testing.T) { - r := NewRecorder(5000) + recoder := NewRecorder(5000) arrivalTime := int64(scaleFactorReferenceTime) - addRun(t, r, []uint16{65534, 65535}, []int64{ + addRun(t, recoder, []uint16{65534, 65535}, []int64{ arrivalTime, increaseTime(&arrivalTime, rtcp.TypeTCCDeltaScaleFactor), }) - rtcpPackets := r.BuildFeedbackPacket() + rtcpPackets := recoder.BuildFeedbackPacket() assert.Equal(t, 1, len(rtcpPackets)) - addRun(t, r, []uint16{0, 4, 5, 6}, []int64{ + addRun(t, recoder, []uint16{0, 4, 5, 6}, []int64{ increaseTime(&arrivalTime, rtcp.TypeTCCDeltaScaleFactor), increaseTime(&arrivalTime, rtcp.TypeTCCDeltaScaleFactor), increaseTime(&arrivalTime, rtcp.TypeTCCDeltaScaleFactor), increaseTime(&arrivalTime, rtcp.TypeTCCDeltaScaleFactor), }) - rtcpPackets = r.BuildFeedbackPacket() + rtcpPackets = recoder.BuildFeedbackPacket() assert.Equal(t, 1, len(rtcpPackets)) assert.Equal(t, &rtcp.TransportLayerCC{ @@ -585,30 +593,36 @@ func TestBuildFeedbackPacket_MinInput(t *testing.T) { } func TestBuildFeedbackPacket_MissingPacketsBetweenFeedbacks(t *testing.T) { - r := NewRecorder(5000) + recorder := NewRecorder(5000) // Create a run of received packets. arrivalTime := int64(scaleFactorReferenceTime) - addRun(t, r, []uint16{0, 1, 2, 3}, []int64{ + addRun(t, recorder, []uint16{0, 1, 2, 3}, []int64{ scaleFactorReferenceTime, increaseTime(&arrivalTime, rtcp.TypeTCCDeltaScaleFactor), increaseTime(&arrivalTime, rtcp.TypeTCCDeltaScaleFactor), increaseTime(&arrivalTime, rtcp.TypeTCCDeltaScaleFactor), }) - rtcpPackets := r.BuildFeedbackPacket() + rtcpPackets := recorder.BuildFeedbackPacket() assert.Equal(t, 1, len(rtcpPackets)) // Now create another run of received packets, but with a gap. - addRun(t, r, []uint16{7, 8, 9}, []int64{ + addRun(t, recorder, []uint16{7, 8, 9}, []int64{ increaseTime(&arrivalTime, rtcp.TypeTCCDeltaScaleFactor*256), increaseTime(&arrivalTime, rtcp.TypeTCCDeltaScaleFactor), increaseTime(&arrivalTime, rtcp.TypeTCCDeltaScaleFactor), }) - rtcpPackets = r.BuildFeedbackPacket() + rtcpPackets = recorder.BuildFeedbackPacket() require.Equal(t, 1, len(rtcpPackets)) twccPacket := rtcpToTwcc(t, rtcpPackets)[0] - assert.Equal(t, uint16(4), twccPacket.BaseSequenceNumber, "Base sequence should be one after the end of the previous feedback") - assert.Equal(t, uint16(6), twccPacket.PacketStatusCount, "Feedback should include status for both the lost and received packets") + assert.Equal( + t, uint16(4), twccPacket.BaseSequenceNumber, + "Base sequence should be one after the end of the previous feedback", + ) + assert.Equal( + t, uint16(6), twccPacket.PacketStatusCount, + "Feedback should include status for both the lost and received packets", + ) expectedPacketChunks := []rtcp.PacketStatusChunk{ &rtcp.StatusVectorChunk{ Type: rtcp.TypeTCCRunLengthChunk, @@ -628,26 +642,26 @@ func TestBuildFeedbackPacket_MissingPacketsBetweenFeedbacks(t *testing.T) { } func TestBuildFeedbackPacketCount(t *testing.T) { - r := NewRecorder(5000) + recorder := NewRecorder(5000) arrivalTime := int64(scaleFactorReferenceTime) - addRun(t, r, []uint16{0, 1}, []int64{ + addRun(t, recorder, []uint16{0, 1}, []int64{ arrivalTime, arrivalTime, }) - pkts := r.BuildFeedbackPacket() + pkts := recorder.BuildFeedbackPacket() assert.Len(t, pkts, 1) twcc := rtcpToTwcc(t, pkts)[0] assert.Equal(t, uint8(0), twcc.FbPktCount) - addRun(t, r, []uint16{0, 1}, []int64{ + addRun(t, recorder, []uint16{0, 1}, []int64{ arrivalTime, arrivalTime, }) - pkts = r.BuildFeedbackPacket() + pkts = recorder.BuildFeedbackPacket() assert.Len(t, pkts, 1) twcc = rtcpToTwcc(t, pkts)[0] @@ -700,10 +714,10 @@ func TestDuplicatePackets(t *testing.T) { func TestShortDeltas(t *testing.T) { t.Run("SplitsOneBitDeltas", func(t *testing.T) { - r := NewRecorder(5000) + recorder := NewRecorder(5000) arrivalTime := int64(scaleFactorReferenceTime) - addRun(t, r, []uint16{3, 4, 5, 7, 6, 8, 10, 11, 13, 14}, []int64{ + addRun(t, recorder, []uint16{3, 4, 5, 7, 6, 8, 10, 11, 13, 14}, []int64{ arrivalTime, arrivalTime, arrivalTime, @@ -716,7 +730,7 @@ func TestShortDeltas(t *testing.T) { arrivalTime, }) - rtcpPackets := r.BuildFeedbackPacket() + rtcpPackets := recorder.BuildFeedbackPacket() assert.Equal(t, 1, len(rtcpPackets)) pkt := rtcpToTwcc(t, rtcpPackets)[0] @@ -821,10 +835,10 @@ func TestShortDeltas(t *testing.T) { } func TestReorderedPackets(t *testing.T) { - r := NewRecorder(5000) + recorder := NewRecorder(5000) arrivalTime := int64(scaleFactorReferenceTime) - addRun(t, r, []uint16{3, 4, 5, 7, 6, 8, 10, 11, 13, 14}, []int64{ + addRun(t, recorder, []uint16{3, 4, 5, 7, 6, 8, 10, 11, 13, 14}, []int64{ increaseTime(&arrivalTime, rtcp.TypeTCCDeltaScaleFactor), increaseTime(&arrivalTime, rtcp.TypeTCCDeltaScaleFactor), increaseTime(&arrivalTime, rtcp.TypeTCCDeltaScaleFactor), @@ -837,7 +851,7 @@ func TestReorderedPackets(t *testing.T) { increaseTime(&arrivalTime, rtcp.TypeTCCDeltaScaleFactor), }) - rtcpPackets := r.BuildFeedbackPacket() + rtcpPackets := recorder.BuildFeedbackPacket() assert.Equal(t, 1, len(rtcpPackets)) pkt := rtcpToTwcc(t, rtcpPackets)[0] @@ -890,23 +904,23 @@ func TestReorderedPackets(t *testing.T) { } func TestPacketsHheld(t *testing.T) { - r := NewRecorder(5000) - assert.Zero(t, r.PacketsHeld()) + recorder := NewRecorder(5000) + assert.Zero(t, recorder.PacketsHeld()) arrivalTime := int64(scaleFactorReferenceTime) - addRun(t, r, []uint16{0, 1, 2}, []int64{ + addRun(t, recorder, []uint16{0, 1, 2}, []int64{ arrivalTime, increaseTime(&arrivalTime, rtcp.TypeTCCDeltaScaleFactor), increaseTime(&arrivalTime, rtcp.TypeTCCDeltaScaleFactor), }) - assert.Equal(t, r.PacketsHeld(), 3) + assert.Equal(t, recorder.PacketsHeld(), 3) - addRun(t, r, []uint16{3, 4}, []int64{ + addRun(t, recorder, []uint16{3, 4}, []int64{ increaseTime(&arrivalTime, rtcp.TypeTCCDeltaScaleFactor), increaseTime(&arrivalTime, rtcp.TypeTCCDeltaScaleFactor), }) - assert.Equal(t, r.PacketsHeld(), 5) + assert.Equal(t, recorder.PacketsHeld(), 5) - r.BuildFeedbackPacket() - assert.Zero(t, r.PacketsHeld()) + recorder.BuildFeedbackPacket() + assert.Zero(t, recorder.PacketsHeld()) } diff --git a/registry.go b/registry.go index e36ef6bf..9c120142 100644 --- a/registry.go +++ b/registry.go @@ -13,7 +13,7 @@ func (r *Registry) Add(f Factory) { r.factories = append(r.factories, f) } -// Build constructs a single Interceptor from a InterceptorRegistry +// Build constructs a single Interceptor from a InterceptorRegistry. func (r *Registry) Build(id string) (Interceptor, error) { if len(r.factories) == 0 { return &NoOp{}, nil diff --git a/streaminfo.go b/streaminfo.go index cb261304..d622312f 100644 --- a/streaminfo.go +++ b/streaminfo.go @@ -9,7 +9,7 @@ type RTPHeaderExtension struct { ID int } -// StreamInfo is the Context passed when a StreamLocal or StreamRemote has been Binded or Unbinded +// StreamInfo is the Context passed when a StreamLocal or StreamRemote has been Binded or Unbinded. type StreamInfo struct { ID string Attributes Attributes