Skip to content

Commit

Permalink
Simplify report result types
Browse files Browse the repository at this point in the history
  • Loading branch information
@mengelbart
mengelbart committed Jan 27, 2025
1 parent f2d1167 commit 48f8054
Show file tree
Hide file tree
Showing 8 changed files with 178 additions and 234 deletions.
18 changes: 5 additions & 13 deletions pkg/ccfb/ccfb_receiver.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,24 +14,19 @@ type acknowledgement struct {
ecn rtcp.ECN
}

type acknowledgementList struct {
ts time.Time
acks []acknowledgement
}

func convertCCFB(ts time.Time, feedback *rtcp.CCFeedbackReport) (time.Time, map[uint32]acknowledgementList) {
func convertCCFB(ts time.Time, feedback *rtcp.CCFeedbackReport) (time.Time, map[uint32][]acknowledgement) {
if feedback == nil {
return time.Time{}, nil
}
result := map[uint32]acknowledgementList{}
result := map[uint32][]acknowledgement{}
referenceTime := ntp.ToTime32(feedback.ReportTimestamp, ts)
for _, rb := range feedback.ReportBlocks {
result[rb.MediaSSRC] = convertMetricBlock(ts, referenceTime, rb.BeginSequence, rb.MetricBlocks)
result[rb.MediaSSRC] = convertMetricBlock(referenceTime, rb.BeginSequence, rb.MetricBlocks)
}
return referenceTime, result
}

func convertMetricBlock(ts time.Time, reference time.Time, seqNrOffset uint16, blocks []rtcp.CCFeedbackMetricBlock) acknowledgementList {
func convertMetricBlock(reference time.Time, seqNrOffset uint16, blocks []rtcp.CCFeedbackMetricBlock) []acknowledgement {
reports := make([]acknowledgement, len(blocks))
for i, mb := range blocks {
if mb.Received {
Expand Down Expand Up @@ -60,8 +55,5 @@ func convertMetricBlock(ts time.Time, reference time.Time, seqNrOffset uint16, b
}
}
}
return acknowledgementList{
ts: ts,
acks: reports,
}
return reports
}
131 changes: 59 additions & 72 deletions pkg/ccfb/ccfb_receiver_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ func TestConvertCCFB(t *testing.T) {
cases := []struct {
ts time.Time
feedback *rtcp.CCFeedbackReport
expect map[uint32]acknowledgementList
expect map[uint32][]acknowledgement
expectTS time.Time
}{
{},
Expand All @@ -38,16 +38,13 @@ func TestConvertCCFB(t *testing.T) {
},
ReportTimestamp: ntp.ToNTP32(timeZero.Add(time.Second)),
},
expect: map[uint32]acknowledgementList{
2: {
ts: timeZero.Add(2 * time.Second),
acks: []acknowledgement{
{
seqNr: 17,
arrived: true,
arrival: timeZero.Add(500 * time.Millisecond),
ecn: 0,
},
expect: map[uint32][]acknowledgement{
2: []acknowledgement{
{
seqNr: 17,
arrived: true,
arrival: timeZero.Add(500 * time.Millisecond),
ecn: 0,
},
},
},
Expand All @@ -63,13 +60,12 @@ func TestConvertCCFB(t *testing.T) {
// Can't directly check equality since arrival timestamp conversions
// may be slightly off due to ntp conversions.
assert.Equal(t, len(tc.expect), len(res))
for i, ee := range tc.expect {
assert.Equal(t, ee.ts, res[i].ts)
for j, ack := range ee.acks {
assert.Equal(t, ack.seqNr, res[i].acks[j].seqNr)
assert.Equal(t, ack.arrived, res[i].acks[j].arrived)
assert.Equal(t, ack.ecn, res[i].acks[j].ecn)
assert.InDelta(t, ack.arrival.UnixNano(), res[i].acks[j].arrival.UnixNano(), float64(time.Millisecond.Nanoseconds()))
for i, acks := range tc.expect {
for j, ack := range acks {
assert.Equal(t, ack.seqNr, res[i][j].seqNr)
assert.Equal(t, ack.arrived, res[i][j].arrived)
assert.Equal(t, ack.ecn, res[i][j].ecn)
assert.InDelta(t, ack.arrival.UnixNano(), res[i][j].arrival.UnixNano(), float64(time.Millisecond.Nanoseconds()))
}
}
})
Expand All @@ -82,17 +78,14 @@ func TestConvertMetricBlock(t *testing.T) {
reference time.Time
seqNrOffset uint16
blocks []rtcp.CCFeedbackMetricBlock
expected acknowledgementList
expected []acknowledgement
}{
{
ts: time.Time{},
reference: time.Time{},
seqNrOffset: 0,
blocks: []rtcp.CCFeedbackMetricBlock{},
expected: acknowledgementList{
ts: time.Time{},
acks: []acknowledgement{},
},
expected: []acknowledgement{},
},
{
ts: time.Time{}.Add(2 * time.Second),
Expand All @@ -115,27 +108,24 @@ func TestConvertMetricBlock(t *testing.T) {
ArrivalTimeOffset: 0,
},
},
expected: acknowledgementList{
ts: time.Time{}.Add(2 * time.Second),
acks: []acknowledgement{
{
seqNr: 3,
arrived: true,
arrival: time.Time{}.Add(500 * time.Millisecond),
ecn: 0,
},
{
seqNr: 4,
arrived: false,
arrival: time.Time{},
ecn: 0,
},
{
seqNr: 5,
arrived: true,
arrival: time.Time{}.Add(time.Second),
ecn: 0,
},
expected: []acknowledgement{
{
seqNr: 3,
arrived: true,
arrival: time.Time{}.Add(500 * time.Millisecond),
ecn: 0,
},
{
seqNr: 4,
arrived: false,
arrival: time.Time{},
ecn: 0,
},
{
seqNr: 5,
arrived: true,
arrival: time.Time{}.Add(time.Second),
ecn: 0,
},
},
},
Expand Down Expand Up @@ -165,41 +155,38 @@ func TestConvertMetricBlock(t *testing.T) {
ArrivalTimeOffset: 0x1FFF,
},
},
expected: acknowledgementList{
ts: time.Time{}.Add(2 * time.Second),
acks: []acknowledgement{
{
seqNr: 3,
arrived: true,
arrival: time.Time{}.Add(500 * time.Millisecond),
ecn: 0,
},
{
seqNr: 4,
arrived: false,
arrival: time.Time{},
ecn: 0,
},
{
seqNr: 5,
arrived: true,
arrival: time.Time{}.Add(time.Second),
ecn: 0,
},
{
seqNr: 6,
arrived: true,
arrival: time.Time{},
ecn: 0,
},
expected: []acknowledgement{
{
seqNr: 3,
arrived: true,
arrival: time.Time{}.Add(500 * time.Millisecond),
ecn: 0,
},
{
seqNr: 4,
arrived: false,
arrival: time.Time{},
ecn: 0,
},
{
seqNr: 5,
arrived: true,
arrival: time.Time{}.Add(time.Second),
ecn: 0,
},
{
seqNr: 6,
arrived: true,
arrival: time.Time{},
ecn: 0,
},
},
},
}

for i, tc := range cases {
t.Run(fmt.Sprintf("%v", i), func(t *testing.T) {
res := convertMetricBlock(tc.ts, tc.reference, tc.seqNrOffset, tc.blocks)
res := convertMetricBlock(tc.reference, tc.seqNrOffset, tc.blocks)
assert.Equal(t, tc.expected, res)
})
}
Expand Down
18 changes: 4 additions & 14 deletions pkg/ccfb/history.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,6 @@ import (
"github.com/pion/rtcp"
)

type PacketReportList struct {
Arrival time.Time
Departure time.Time
Reports []PacketReport
}

type PacketReport struct {
SeqNr int64
Size uint16
Expand Down Expand Up @@ -85,12 +79,12 @@ func (h *historyList) removeOldest() {
}
}

func (h *historyList) getReportForAck(al acknowledgementList) PacketReportList {
func (h *historyList) getReportForAck(acks []acknowledgement) []PacketReport {
h.lock.Lock()
defer h.lock.Unlock()

var reports []PacketReport
for _, pr := range al.acks {
reports := []PacketReport{}
for _, pr := range acks {
sn := h.ackedSeqNr.Unwrap(pr.seqNr)
ent, ok := h.seqNrToPacket[sn]
// Ignore report for unknown packets (migth have been dropped from
Expand All @@ -108,9 +102,5 @@ func (h *historyList) getReportForAck(al acknowledgementList) PacketReportList {
}
}
}

return PacketReportList{
Arrival: al.ts,
Reports: reports,
}
return reports
}
43 changes: 17 additions & 26 deletions pkg/ccfb/history_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ func TestHistory(t *testing.T) {
size uint16
ts time.Time
}
acks acknowledgementList
expectedReport PacketReportList
acks []acknowledgement
expectedReport []PacketReport
expectedHistorySize int
}{
{
Expand All @@ -33,8 +33,8 @@ func TestHistory(t *testing.T) {
size uint16
ts time.Time
}{},
acks: acknowledgementList{},
expectedReport: PacketReportList{},
acks: []acknowledgement{},
expectedReport: []PacketReport{},
expectedHistorySize: 0,
},
{
Expand All @@ -48,8 +48,8 @@ func TestHistory(t *testing.T) {
{2, 1200, time.Time{}.Add(3 * time.Millisecond)},
{3, 1200, time.Time{}.Add(4 * time.Millisecond)},
},
acks: acknowledgementList{},
expectedReport: PacketReportList{},
acks: []acknowledgement{},
expectedReport: []PacketReport{},
expectedHistorySize: 4,
},
{
Expand All @@ -63,21 +63,15 @@ func TestHistory(t *testing.T) {
{2, 1200, time.Time{}.Add(3 * time.Millisecond)},
{3, 1200, time.Time{}.Add(4 * time.Millisecond)},
},
acks: acknowledgementList{
ts: time.Time{}.Add(time.Second),
acks: []acknowledgement{
{1, true, time.Time{}.Add(3 * time.Millisecond), 0},
{2, false, time.Time{}, 0},
{3, true, time.Time{}.Add(5 * time.Millisecond), 0},
},
acks: []acknowledgement{
{1, true, time.Time{}.Add(3 * time.Millisecond), 0},
{2, false, time.Time{}, 0},
{3, true, time.Time{}.Add(5 * time.Millisecond), 0},
},
expectedReport: PacketReportList{
Arrival: time.Time{}.Add(time.Second),
Reports: []PacketReport{
{1, 1200, time.Time{}.Add(2 * time.Millisecond), true, time.Time{}.Add(3 * time.Millisecond), 0},
{2, 1200, time.Time{}.Add(3 * time.Millisecond), false, time.Time{}, 0},
{3, 1200, time.Time{}.Add(4 * time.Millisecond), true, time.Time{}.Add(5 * time.Millisecond), 0},
},
expectedReport: []PacketReport{
{1, 1200, time.Time{}.Add(2 * time.Millisecond), true, time.Time{}.Add(3 * time.Millisecond), 0},
{2, 1200, time.Time{}.Add(3 * time.Millisecond), false, time.Time{}, 0},
{3, 1200, time.Time{}.Add(4 * time.Millisecond), true, time.Time{}.Add(5 * time.Millisecond), 0},
},
expectedHistorySize: 4,
},
Expand All @@ -103,20 +97,17 @@ func TestHistory(t *testing.T) {
assert.NoError(t, h.add(i, 1200, time.Time{}.Add(time.Duration(i)*time.Millisecond)))
}

acks := acknowledgementList{
ts: time.Time{}.Add(time.Second),
acks: []acknowledgement{},
}
acks := []acknowledgement{}
for i := uint16(200); i < 290; i++ {
acks.acks = append(acks.acks, acknowledgement{
acks = append(acks, acknowledgement{
seqNr: i,
arrived: true,
arrival: time.Time{}.Add(time.Duration(500+i) * time.Millisecond),
ecn: 0,
})
}
prl := h.getReportForAck(acks)
assert.Len(t, prl.Reports, 90)
assert.Len(t, prl, 90)
assert.Equal(t, 200, len(h.seqNrToPacket))
assert.Equal(t, 200, h.evictList.Len())
})
Expand Down
Loading

0 comments on commit 48f8054

Please sign in to comment.