Skip to content

Commit 0555d58

Browse files
mengelbartmengelbart
committed
Simplify report result types
1 parent d8f51d4 commit 0555d58

File tree

8 files changed

+178
-234
lines changed

8 files changed

+178
-234
lines changed

‎pkg/ccfb/ccfb_receiver.go‎

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -14,24 +14,19 @@ type acknowledgement struct {
1414
ecn rtcp.ECN
1515
}
1616

17-
type acknowledgementList struct {
18-
ts time.Time
19-
acks []acknowledgement
20-
}
21-
22-
func convertCCFB(ts time.Time, feedback *rtcp.CCFeedbackReport) (time.Time, map[uint32]acknowledgementList) {
17+
func convertCCFB(ts time.Time, feedback *rtcp.CCFeedbackReport) (time.Time, map[uint32][]acknowledgement) {
2318
if feedback == nil {
2419
return time.Time{}, nil
2520
}
26-
result := map[uint32]acknowledgementList{}
21+
result := map[uint32][]acknowledgement{}
2722
referenceTime := ntp.ToTime32(feedback.ReportTimestamp, ts)
2823
for _, rb := range feedback.ReportBlocks {
29-
result[rb.MediaSSRC] = convertMetricBlock(ts, referenceTime, rb.BeginSequence, rb.MetricBlocks)
24+
result[rb.MediaSSRC] = convertMetricBlock(referenceTime, rb.BeginSequence, rb.MetricBlocks)
3025
}
3126
return referenceTime, result
3227
}
3328

34-
func convertMetricBlock(ts time.Time, reference time.Time, seqNrOffset uint16, blocks []rtcp.CCFeedbackMetricBlock) acknowledgementList {
29+
func convertMetricBlock(reference time.Time, seqNrOffset uint16, blocks []rtcp.CCFeedbackMetricBlock) []acknowledgement {
3530
reports := make([]acknowledgement, len(blocks))
3631
for i, mb := range blocks {
3732
if mb.Received {
@@ -60,8 +55,5 @@ func convertMetricBlock(ts time.Time, reference time.Time, seqNrOffset uint16, b
6055
}
6156
}
6257
}
63-
return acknowledgementList{
64-
ts: ts,
65-
acks: reports,
66-
}
58+
return reports
6759
}

‎pkg/ccfb/ccfb_receiver_test.go‎

Lines changed: 59 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ func TestConvertCCFB(t *testing.T) {
1515
cases := []struct {
1616
ts time.Time
1717
feedback *rtcp.CCFeedbackReport
18-
expect map[uint32]acknowledgementList
18+
expect map[uint32][]acknowledgement
1919
expectTS time.Time
2020
}{
2121
{},
@@ -38,16 +38,13 @@ func TestConvertCCFB(t *testing.T) {
3838
},
3939
ReportTimestamp: ntp.ToNTP32(timeZero.Add(time.Second)),
4040
},
41-
expect: map[uint32]acknowledgementList{
42-
2: {
43-
ts: timeZero.Add(2 * time.Second),
44-
acks: []acknowledgement{
45-
{
46-
seqNr: 17,
47-
arrived: true,
48-
arrival: timeZero.Add(500 * time.Millisecond),
49-
ecn: 0,
50-
},
41+
expect: map[uint32][]acknowledgement{
42+
2: []acknowledgement{
43+
{
44+
seqNr: 17,
45+
arrived: true,
46+
arrival: timeZero.Add(500 * time.Millisecond),
47+
ecn: 0,
5148
},
5249
},
5350
},
@@ -63,13 +60,12 @@ func TestConvertCCFB(t *testing.T) {
6360
// Can't directly check equality since arrival timestamp conversions
6461
// may be slightly off due to ntp conversions.
6562
assert.Equal(t, len(tc.expect), len(res))
66-
for i, ee := range tc.expect {
67-
assert.Equal(t, ee.ts, res[i].ts)
68-
for j, ack := range ee.acks {
69-
assert.Equal(t, ack.seqNr, res[i].acks[j].seqNr)
70-
assert.Equal(t, ack.arrived, res[i].acks[j].arrived)
71-
assert.Equal(t, ack.ecn, res[i].acks[j].ecn)
72-
assert.InDelta(t, ack.arrival.UnixNano(), res[i].acks[j].arrival.UnixNano(), float64(time.Millisecond.Nanoseconds()))
63+
for i, acks := range tc.expect {
64+
for j, ack := range acks {
65+
assert.Equal(t, ack.seqNr, res[i][j].seqNr)
66+
assert.Equal(t, ack.arrived, res[i][j].arrived)
67+
assert.Equal(t, ack.ecn, res[i][j].ecn)
68+
assert.InDelta(t, ack.arrival.UnixNano(), res[i][j].arrival.UnixNano(), float64(time.Millisecond.Nanoseconds()))
7369
}
7470
}
7571
})
@@ -82,17 +78,14 @@ func TestConvertMetricBlock(t *testing.T) {
8278
reference time.Time
8379
seqNrOffset uint16
8480
blocks []rtcp.CCFeedbackMetricBlock
85-
expected acknowledgementList
81+
expected []acknowledgement
8682
}{
8783
{
8884
ts: time.Time{},
8985
reference: time.Time{},
9086
seqNrOffset: 0,
9187
blocks: []rtcp.CCFeedbackMetricBlock{},
92-
expected: acknowledgementList{
93-
ts: time.Time{},
94-
acks: []acknowledgement{},
95-
},
88+
expected: []acknowledgement{},
9689
},
9790
{
9891
ts: time.Time{}.Add(2 * time.Second),
@@ -115,27 +108,24 @@ func TestConvertMetricBlock(t *testing.T) {
115108
ArrivalTimeOffset: 0,
116109
},
117110
},
118-
expected: acknowledgementList{
119-
ts: time.Time{}.Add(2 * time.Second),
120-
acks: []acknowledgement{
121-
{
122-
seqNr: 3,
123-
arrived: true,
124-
arrival: time.Time{}.Add(500 * time.Millisecond),
125-
ecn: 0,
126-
},
127-
{
128-
seqNr: 4,
129-
arrived: false,
130-
arrival: time.Time{},
131-
ecn: 0,
132-
},
133-
{
134-
seqNr: 5,
135-
arrived: true,
136-
arrival: time.Time{}.Add(time.Second),
137-
ecn: 0,
138-
},
111+
expected: []acknowledgement{
112+
{
113+
seqNr: 3,
114+
arrived: true,
115+
arrival: time.Time{}.Add(500 * time.Millisecond),
116+
ecn: 0,
117+
},
118+
{
119+
seqNr: 4,
120+
arrived: false,
121+
arrival: time.Time{},
122+
ecn: 0,
123+
},
124+
{
125+
seqNr: 5,
126+
arrived: true,
127+
arrival: time.Time{}.Add(time.Second),
128+
ecn: 0,
139129
},
140130
},
141131
},
@@ -165,41 +155,38 @@ func TestConvertMetricBlock(t *testing.T) {
165155
ArrivalTimeOffset: 0x1FFF,
166156
},
167157
},
168-
expected: acknowledgementList{
169-
ts: time.Time{}.Add(2 * time.Second),
170-
acks: []acknowledgement{
171-
{
172-
seqNr: 3,
173-
arrived: true,
174-
arrival: time.Time{}.Add(500 * time.Millisecond),
175-
ecn: 0,
176-
},
177-
{
178-
seqNr: 4,
179-
arrived: false,
180-
arrival: time.Time{},
181-
ecn: 0,
182-
},
183-
{
184-
seqNr: 5,
185-
arrived: true,
186-
arrival: time.Time{}.Add(time.Second),
187-
ecn: 0,
188-
},
189-
{
190-
seqNr: 6,
191-
arrived: true,
192-
arrival: time.Time{},
193-
ecn: 0,
194-
},
158+
expected: []acknowledgement{
159+
{
160+
seqNr: 3,
161+
arrived: true,
162+
arrival: time.Time{}.Add(500 * time.Millisecond),
163+
ecn: 0,
164+
},
165+
{
166+
seqNr: 4,
167+
arrived: false,
168+
arrival: time.Time{},
169+
ecn: 0,
170+
},
171+
{
172+
seqNr: 5,
173+
arrived: true,
174+
arrival: time.Time{}.Add(time.Second),
175+
ecn: 0,
176+
},
177+
{
178+
seqNr: 6,
179+
arrived: true,
180+
arrival: time.Time{},
181+
ecn: 0,
195182
},
196183
},
197184
},
198185
}
199186

200187
for i, tc := range cases {
201188
t.Run(fmt.Sprintf("%v", i), func(t *testing.T) {
202-
res := convertMetricBlock(tc.ts, tc.reference, tc.seqNrOffset, tc.blocks)
189+
res := convertMetricBlock(tc.reference, tc.seqNrOffset, tc.blocks)
203190
assert.Equal(t, tc.expected, res)
204191
})
205192
}

‎pkg/ccfb/history.go‎

Lines changed: 4 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,6 @@ import (
1010
"github.com/pion/rtcp"
1111
)
1212

13-
type PacketReportList struct {
14-
Arrival time.Time
15-
Departure time.Time
16-
Reports []PacketReport
17-
}
18-
1913
type PacketReport struct {
2014
SeqNr int64
2115
Size uint16
@@ -85,12 +79,12 @@ func (h *historyList) removeOldest() {
8579
}
8680
}
8781

88-
func (h *historyList) getReportForAck(al acknowledgementList) PacketReportList {
82+
func (h *historyList) getReportForAck(acks []acknowledgement) []PacketReport {
8983
h.lock.Lock()
9084
defer h.lock.Unlock()
9185

92-
var reports []PacketReport
93-
for _, pr := range al.acks {
86+
reports := []PacketReport{}
87+
for _, pr := range acks {
9488
sn := h.ackedSeqNr.Unwrap(pr.seqNr)
9589
ent, ok := h.seqNrToPacket[sn]
9690
// Ignore report for unknown packets (migth have been dropped from
@@ -108,9 +102,5 @@ func (h *historyList) getReportForAck(al acknowledgementList) PacketReportList {
108102
}
109103
}
110104
}
111-
112-
return PacketReportList{
113-
Arrival: al.ts,
114-
Reports: reports,
115-
}
105+
return reports
116106
}

‎pkg/ccfb/history_test.go‎

Lines changed: 17 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,8 @@ func TestHistory(t *testing.T) {
2323
size uint16
2424
ts time.Time
2525
}
26-
acks acknowledgementList
27-
expectedReport PacketReportList
26+
acks []acknowledgement
27+
expectedReport []PacketReport
2828
expectedHistorySize int
2929
}{
3030
{
@@ -33,8 +33,8 @@ func TestHistory(t *testing.T) {
3333
size uint16
3434
ts time.Time
3535
}{},
36-
acks: acknowledgementList{},
37-
expectedReport: PacketReportList{},
36+
acks: []acknowledgement{},
37+
expectedReport: []PacketReport{},
3838
expectedHistorySize: 0,
3939
},
4040
{
@@ -48,8 +48,8 @@ func TestHistory(t *testing.T) {
4848
{2, 1200, time.Time{}.Add(3 * time.Millisecond)},
4949
{3, 1200, time.Time{}.Add(4 * time.Millisecond)},
5050
},
51-
acks: acknowledgementList{},
52-
expectedReport: PacketReportList{},
51+
acks: []acknowledgement{},
52+
expectedReport: []PacketReport{},
5353
expectedHistorySize: 4,
5454
},
5555
{
@@ -63,21 +63,15 @@ func TestHistory(t *testing.T) {
6363
{2, 1200, time.Time{}.Add(3 * time.Millisecond)},
6464
{3, 1200, time.Time{}.Add(4 * time.Millisecond)},
6565
},
66-
acks: acknowledgementList{
67-
ts: time.Time{}.Add(time.Second),
68-
acks: []acknowledgement{
69-
{1, true, time.Time{}.Add(3 * time.Millisecond), 0},
70-
{2, false, time.Time{}, 0},
71-
{3, true, time.Time{}.Add(5 * time.Millisecond), 0},
72-
},
66+
acks: []acknowledgement{
67+
{1, true, time.Time{}.Add(3 * time.Millisecond), 0},
68+
{2, false, time.Time{}, 0},
69+
{3, true, time.Time{}.Add(5 * time.Millisecond), 0},
7370
},
74-
expectedReport: PacketReportList{
75-
Arrival: time.Time{}.Add(time.Second),
76-
Reports: []PacketReport{
77-
{1, 1200, time.Time{}.Add(2 * time.Millisecond), true, time.Time{}.Add(3 * time.Millisecond), 0},
78-
{2, 1200, time.Time{}.Add(3 * time.Millisecond), false, time.Time{}, 0},
79-
{3, 1200, time.Time{}.Add(4 * time.Millisecond), true, time.Time{}.Add(5 * time.Millisecond), 0},
80-
},
71+
expectedReport: []PacketReport{
72+
{1, 1200, time.Time{}.Add(2 * time.Millisecond), true, time.Time{}.Add(3 * time.Millisecond), 0},
73+
{2, 1200, time.Time{}.Add(3 * time.Millisecond), false, time.Time{}, 0},
74+
{3, 1200, time.Time{}.Add(4 * time.Millisecond), true, time.Time{}.Add(5 * time.Millisecond), 0},
8175
},
8276
expectedHistorySize: 4,
8377
},
@@ -103,20 +97,17 @@ func TestHistory(t *testing.T) {
10397
assert.NoError(t, h.add(i, 1200, time.Time{}.Add(time.Duration(i)*time.Millisecond)))
10498
}
10599

106-
acks := acknowledgementList{
107-
ts: time.Time{}.Add(time.Second),
108-
acks: []acknowledgement{},
109-
}
100+
acks := []acknowledgement{}
110101
for i := uint16(200); i < 290; i++ {
111-
acks.acks = append(acks.acks, acknowledgement{
102+
acks = append(acks, acknowledgement{
112103
seqNr: i,
113104
arrived: true,
114105
arrival: time.Time{}.Add(time.Duration(500+i) * time.Millisecond),
115106
ecn: 0,
116107
})
117108
}
118109
prl := h.getReportForAck(acks)
119-
assert.Len(t, prl.Reports, 90)
110+
assert.Len(t, prl, 90)
120111
assert.Equal(t, 200, len(h.seqNrToPacket))
121112
assert.Equal(t, 200, h.evictList.Len())
122113
})

0 commit comments

Comments
 (0)