Conversation
One is fixed and determined on track start up, the other needs to be updated on SRs
| gopkg.in/yaml.v3 v3.0.1 // indirect | ||
| ) | ||
|
|
||
| tool github.com/maxbrunsfeld/counterfeiter/v6 |
There was a problem hiding this comment.
is this required? Guess it makes things repeatable/consistent?
There was a problem hiding this comment.
go get -tool github.com/maxbrunsfeld/counterfeiter/v6
is what it got it added there - I think it's fine to have it available there for go tool for this module.
| // offsets | ||
| currentPTSOffset time.Duration // presentation timestamp offset (used for a/v sync) | ||
| desiredPTSOffset time.Duration // desired presentation timestamp offset (used for a/v sync) | ||
| basePTSOffset time.Duration // component of the desired PTS offset (set initially to preseve initial offset) |
| return | ||
| } | ||
|
|
||
| t.desiredPTSOffset += offset |
There was a problem hiding this comment.
Is there a chance that PTS can move backward because of this?
There was a problem hiding this comment.
yes - it could keep accumulating in both directions depending on SRs. The version before was simply overwriting t.desiredPTSOffset which obviously breaks track starting late.
|
|
||
| adj0, err := tsync.GetPTS(pkt(ts)) | ||
| require.NoError(t, err) | ||
| near(t, adj0, 0, 5*time.Millisecond) |
There was a problem hiding this comment.
is 5ms enough? some of our GH runners are super slow. Ideally, using a mock clock would be great, but that is not easy to do.
There was a problem hiding this comment.
this is good point - most of variation comes from potentially variable time needed between setting t.startTime and calculating estimatedPTS few instructions later (don't expect huge diff there unless the gh runner is super super slow). I could bump the tolerance a bit higher and keep an eye on this - and if flakiness appear I could introduce mock clock (which due to its invasiveness would like to skip if we can get away without it). wdyt?
There was a problem hiding this comment.
yeah, good strategy, mock clock will require a lot of changes.
Most of the time it should be fine. Once in a while, runner is super slow and probably have some noisy neighbour hogging cpu and causing flakiness, but it should be very rare.
3 participants
While the recent change made sure packet PTSs don't jump unexpectedly, it also added wrong cumulative handling of offsets.
Desired PTS offset needs to be split in 2 components:
In addition to the change adding some synchronizer unit tests to cover basic PTS generation logic.