Refactor JitterBuffer

[hrosa]

The current implementation of JitterBuffer is very prone to overflows which cause glitches in audio. It cannot handle bursts of RTP packets coming from the network and drops older packets, leading to gaps in media stream.

There is also a known issue where the Jitter Buffer consumer thread may not be very responsive at times (mainly when Media Server is under load), causing the buffer to pile up for a small amount of time. A good jitter buffer implementation should help attenuate this issue and increase the threshold temporarily according to some criteria.

[jehanzebqayyum]

Hi,

In my view this is how it should look like once refactored to use pluggable algorithms for jitter handling. Any comments?

[hrosa]

Hi @jehanzebqayyum

Conceptually this seems correct to me, but it fails to represent the pluggable algorithms. How do you plan to implement that?

[jehanzebqayyum]

We need to derive the actual impl from config. Looking into the practices being followed in the code, there is a provider and corresponding factory for such cases. Do we really need factory when provider does the same?

v0.2

[hrosa]

Looking into the practices being followed in the code, there is a provider and corresponding factory for such cases.

I'm aware it adds a bit of boilerplate code but we use both regular factories and Guice Providers because we don't wish to be tied to Guice. Dependency Injection (the Providers) exist in Bootstrap module only.

We need to derive the actual impl from config.

Yes, you can have a new section in the configuration file for the specific jitter buffer configuration.

The diagram still does not represent pluggable jitter algorithms.

[jehanzebqayyum]

The factory will instantiate the actual jitter strategy from classpath based on the config. In which other way you are trying to make it pluggable?

Thanks

On Wed, 1 Mar 2017 at 4:22 PM, Henrique Rosa notifications@github.com wrote:

Looking into the practices being followed in the code, there is a provider and corresponding factory for such cases.

I'm aware it adds a bit of boilerplate code but we use both regular factories and Guice Providers because we don't wish to be tied to Guice. Dependency Injection (the Providers) exist in Bootstrap module only.

We need to derive the actual impl from config.

Yes, you can have a new section in the configuration file for the specific jitter buffer configuration.

The diagram still does not represent pluggable jitter algorithms.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/RestComm/mediaserver/issues/174#issuecomment-283326308, or mute the thread https://github.com/notifications/unsubscribe-auth/AFNfSp4N12LzWy0_v3SHvsmPfChyez8nks5rhWMOgaJpZM4Iw3xD .

--

Best Regards, Jehanzeb Qayyum +971 52 898 2285 +92 3311337064

[hrosa]

The factory will instantiate the actual jitter strategy from classpath based on the config.

This sounds correct. But you need to distinguish between Fixed and Adaptive buffers. Then, the adaptive buffer can be configured to work with any given algorithm.

So what I'd like to know is how you plan to make the adaptive jitter buffer generic enough to plug-in any algorithm we desire.

[jehanzebqayyum]

i was thinking to leave each algorithm independently handle the buffer. e.g. AdapativeJitterBuffer above can be in fact e.g. QoSAdapativeJitterBuffer etc. As long as an algorithm implements JitterBuffer inteface, we do not need to distinguish much. e.g. we can also have NoJitterBuffer to disable jitter buffer completely. Otherwise we can decouple the management of adaptive jitter buffer as below which again will be driven by provider/factory from media server config:

v0.3

[hrosa]

OK @jehanzebqayyum sounds good :+1:

[jehanzebqayyum]

Trying to implement classical algorithm 1 from Adaptive Playout Mechanisms for Packetized Audio Applications in Wide-Area Networks. These set of algorithms require

• detection of talkspurt begin and end, it means detection of silence.
• removal and insertion of silence packets.

As cited in the paper I assume each packet is 20 ms.

[jehanzebqayyum]

For talkspurt detection i am planning to use marker bit. And if required to add silence, i am looking into using rtp payload for comfort noise.

[jehanzebqayyum]

Almost all the adaptive jitter buffer algorithms determine playout time of a packet or talkspurt instead of calculating buffer size. Which means there would be no limit for buffer and packets will keep on accumulating in it until the playout time upon which the wakeup call will be made (scheduled) to RtpInput.

Though translating playout time to buffer size would be a simpler strategy inline with current implementation. If anyone has ideas please do share. thanks

[hrosa]

Hi @jehanzebqayyum

Sorry for late reply, been really busy lately.

Very cool that you're implementing talkspurt detection. Make sure it's decoupled because I see that feature being very useful in other use cases like Recording or Conferencing (identify N active speakers).

Which means there would be no limit for buffer and packets will keep on accumulating in it until the playout time upon which the wakeup call will be made (scheduled) to RtpInput.

That sounds OK to me as jitter buffer size is configurable by users. Usually jitter buffer size is something like 50-60ms of audio. Just be careful here though, there's no point waiting forever until a playout time is filled, as you may accidentally insert gaps in media stream or delay conversation. It's usually OK for jitter buffers to drop delayed packets. Adaptive jitter buffers shine here because they're more tolerant when burst of packet occurs.

We're mostly looking for a jitter buffer implementation that is more tolerant to network unpredictability, because current impl is too prone to drop packets. It would also be cool if new implementation would be more efficient in terms of concurrency:

• we currently use too many locks for extensive periods;
• maybe a priority queue is more efficient than array list;
• etc

Seems to me you heading in right direction. Maybe you can do a PR so we can have a sneak peak and provide more detailed feedback?

Regards

[jehanzebqayyum]

so is it invalid to assume that the beginning of talkspurt will have marker bit on? this is the case when sender is using silence suppression.

[hrosa]

@jehanzebqayyum You can try to use the marker for first iteration, should be OK. We can see how that goes and if it's reliable.

Marker bit definition can be a bit vague, it's just assumed it's used to mark "events of interest" within the media stream. Beginning of talkspurt would be good usage for it, just not sure if we can blindly rely on clients usage of the marker bit.

Don't worry too much about it for now.

[jehanzebqayyum]

can some one review i just pushed some changes