jitterbuffer: Use jitterbuffer in SampleBuilder

[thatsnotright]

Description

Reference issue

Fixes #...

[codecov]

Codecov Report

Attention: Patch coverage is 80.64516% with 6 lines in your changes missing coverage. Please review.

Project coverage is 77.62%. Comparing base (271ab55) to head (9b32354).
Report is 9 commits behind head on master.

Files with missing lines	Patch %	Lines
pkg/media/samplebuilder/samplebuilder.go	80.64%	4 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2959      +/-   ##
==========================================
- Coverage   79.01%   77.62%   -1.40%     
==========================================
  Files          89       89              
  Lines        8584    10529    +1945     
==========================================
+ Hits         6783     8173    +1390     
- Misses       1311     1859     +548     
- Partials      490      497       +7     

Flag	Coverage Δ
go	79.17% <80.64%> (-1.40%)	⬇️
wasm	63.54% <80.64%> (-0.26%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[jech]

The current JitterBuffer induces a fixed delay of 50 packets. I don't think that it's a good idea to use it in SampleBuilder until that is fixed.

[thatsnotright]

The current JitterBuffer induces a fixed delay of 50 packets. I don't think that it's a good idea to use it in SampleBuilder until that is fixed.

@jech What would you propose the interface look like, or mechanism be for adjusting that delay?

[jech]

What would you propose the interface look like, or mechanism be for adjusting that delay?

Before we discuss the interface, we need to understand what is required.

For an interactive display, the jitter buffer delay should be set to something like smoothed_RTT + 4*variance(RTT). The RTT and its variance can be recomputed at various times: when we receive a receiver report, when we receive TWCC feedback, or when we receive an ICE consent. Yeah, it's a mess.

What is more, the delay might be influenced by external factors, such as when a receiver is performing audio-video sync.

For the samplebuilder, the amount of delay will depend on the application: it will be as above for low-latency applications, but it can be much larger than that for semi-interactive applications (such as streaming). For non-interactive applications, such as saving to disk, there is no need for a fixed delay, the samplebuilder can add as much delay as required to avoid packet loss.

In summary: there are many contradictory requirements, and meeting them all without having dozens of special cases will require good taste and careful thought. I am not able to propose a fully general interface at this time.

[Sean-Der]

@jech This is the behavior we have today (unfortunately)

I agree we should improve it! We shouldn't block merging this for that though.

Correct me if I'm am wrong @thatsnotright, but this causes 0 behavior change. We are just trying to push as much 'RTP code' into pion/interceptor. That lets people doing non-WebRTC things use it also.

[thatsnotright]

Correct me if I'm am wrong @thatsnotright, but this causes 0 behavior change.

Aside from potentially a little more delay there shouldn't be, it was designed to strictly replace how the current SampleBuilder buffered packets. It should reduce the amount of memory used.

[jech]

I see, thanks. I hold no opinion on whether this patch is desirable. I do have some misgivings about including in Pion an API that is not useful in its current state without as much as a warning in the comments, but that ship has sailed.