Add support for packets with floating ptime & refactor #2
Conversation
| sampleCnt int64 | ||
| } | ||
|
|
||
| type Jitter struct { |
There was a problem hiding this comment.
Buffer 인터페이스와 구별하기 위해 이름 변경
| return pkt, true | ||
| } else { | ||
| b.loss.Set(targetTime, nil) | ||
| b.current += b.defaultTickInterval |
There was a problem hiding this comment.
패킷 loss 시 미리 정한 defualtTickInterval 만큼 증가시킴. 문제가 예상됌.
- loss 시에는 해당 타임스탬프 패킷의 ptime 을 알 수 없음
- 한 번 loss 가 나서 defaultTickInterval 만큼 임의로 증가시키는 경우 싱크가 깨지게 됌. 다음에 오는 패킷의 타임스탬프와 일치하지 않을 수 있음.
- 타임스탬프가 같은 패킷만 반환하는 것이 아니라 speex 처럼 좀 더 똑똑하게 반환할 필요가 있어 보임
There was a problem hiding this comment.
[targetTime, targetTime + defaultInterval ) 범위의 패킷을 모두 반환하게끔 변경
| func TestOverflow(t *testing.T) { | ||
| factory := NewFactory(100, 400, 20*50, 20) | ||
| packetBuffer := NewPacketBuffer(factory) | ||
|
|
||
| packetBuffer.Put(&Packet{Timestamp: 1<<32 - 20, Data: []byte{1}, SampleCnt: 20}) | ||
|
|
||
| packetBuffer.Put(&Packet{Timestamp: 0, Data: []byte{1}, SampleCnt: 20}) | ||
| assert.Equal(t, int64(1<<32), packetBuffer.lastTimestamp()) | ||
|
|
||
| packetBuffer.Put(&Packet{Timestamp: 20, Data: []byte{1}, SampleCnt: 20}) | ||
| assert.Equal(t, int64(1<<32+20), packetBuffer.lastTimestamp()) | ||
| } |
There was a problem hiding this comment.
packet buffer 의 책임에 대한 테스트 별개로 분리
There was a problem hiding this comment.
mockery 써서 decorate 대상인 내부 Buffer 인터페이스를 모킹해서 오버플로우시에도 단조 증가하는 타임스탬프가 들어오는지 확인해주는게 더 좋을듯.
pkg/jitter/jitter_test.go
Outdated
|
|
||
| b.Put(packet(0)) | ||
| b.Put(packet(1)) | ||
| b.Put(packet(2)) | ||
|
|
||
| assertLoss(t, b, 920) | ||
| assertLoss(t, b, 940) | ||
| assertLoss(t, b, 960) | ||
| assertLoss(t, b, 980) | ||
| assertLoss(t, b, 1000) | ||
|
|
||
| assertGet(t, b, 1, 1020) | ||
| assertGet(t, b, 2, 1040) | ||
| assertGet(t, b, 3, 1060) | ||
|
|
||
| assert.Equal(t, b.targetTime(), int64(1060)) | ||
| assert.Equal(t, b.latency, int64(100)) |
| type deltaWithSampleCnt struct { | ||
| delta int64 | ||
| sampleCnt int64 | ||
| } |
There was a problem hiding this comment.
네이밍이 영 좋지 못한듯..! 더 나은 이름 없을까요
pkg/jitter/api.go
Outdated
| type Packet struct { | ||
| Data []byte | ||
| SampleCnt int64 | ||
| Timestamp int64 | ||
| SSRC uint32 | ||
| } | ||
|
|
||
| type Buffer interface { | ||
| Put(p *Packet) | ||
| Get() (*Packet, bool) | ||
| } | ||
|
|
||
| type BufferFactory interface { | ||
| CreateBuffer() Buffer | ||
| } |
There was a problem hiding this comment.
PacketBuffer 와 Jitter 를 Buffer 인터페이스로 추상화하고 PacketBuffer 를 Jitter 에 대한 데코레이터로 의도함.
외부 의존성인 rtp.Packet 대신 내부적으로 DTO 하나 만들어서 쓰는게 낫다고 생각하였음.
kjs001127
changed the title
| func NewJitter(minLatency, maxLatency, window, defaultTickInterval int64) *Jitter { | ||
| b := &Jitter{ |
There was a problem hiding this comment.
모든 파라미터는 샘플 갯수 (timestamp) 기반!
| if b.sumTsOfLatePackets() > b.window*2/100 { // late 패킷들의 ptime 합이 윈도우의 2% 를 초과시 | ||
| candidate := b.latency + maxInList(b.late) |
There was a problem hiding this comment.
late 인 패킷들의 sampleCnt 를 모두 더해 윈도우의 2% 와 비교
pkg/jitter/jitter.go
Outdated
| if delta < 0 { | ||
| b.normal.Set(p.Timestamp, deltaWithSampleCnt{delta: -delta, sampleCnt: p.SampleCnt}) | ||
| } else if delta > 0 && delta < b.maxLatency { // 늦게 온 것이면, 단 너무 늦으면 버림 | ||
| b.late.Set(p.Timestamp, deltaWithSampleCnt{delta: delta, sampleCnt: p.SampleCnt}) // 늦은 시간을 기록 | ||
| } |
There was a problem hiding this comment.
delta 값과 sample 갯수를 모두 저장해야 함
|
git rename 안쓰고 IDE refactor->rename 했더니 diff 가 새로 찍혀버려서 변경된 부분들에 코멘트 달아두었습니다! |
No description provided.