Skip to content

WIP: rtpjitterbuffer: add DTX support

Guillaume Desmottes requested to merge gdesmott/gst-plugins-good:rtp-dtx into master

This MR is another attempt to fix gst-plugins-base#566

It's partially based on !600 from which I cherry-picked one commit and squashed some extra cleanups from other commits (I hope you don't mind @hgr).

This MR adds a dtx property to rtpopuspay asking the payloader to drop empty frames, making it easier to test DTX.

But the core of the changes are in rtpjitterbuffer:

  • new packets spacing heuristic so the jitterbuffer isn't confused because DTX big gaps.
  • changed the seqnum logic so the first packet after a DTX gap isn't dropped, as seqnums are contiguous with DTX.
  • added a new rtx-on-lost property (better naming suggestions welcome) so a new RTX timer is setup each time a packet is lost, allowing us to do PLC for each missing packet.

With those changes, the following pipeline keeps producing a steady audio stream even when the input microphone is muted, just by relying on RTX/lost timers and the existing GstRTPPacketLost event which is already handled by GstRTPBaseDepayload.

gst-launch-1.0 pulsesrc ! opusenc dtx=true bitrate-type=vbr ! rtpopuspay dtx=true ! rtpjitterbuffer do-lost=true do-retransmission=true rtx-on-lost=true ! rtpopusdepay ! opusdec plc=true ! fakesink silent=false -v

Things I'm not sure:

  • do-retransmission=true is required for DTX so RTX timers are setup, despite not actually caring about re-transmissions. It's a bit unfortunante but maybe that's ok? I considered always enabling the RTX timers if do-lost=true but this is a very significant changes breaking a lot of tests, so I figured it may be safer to not do so.
  • I also considered not having rtx-on-lost and having this behavior always enabled, but here again this was breaking a lot of tests relying on the exact rtx/lost events sequence. The jitterbuffer is a complex beast and I figured it may be safer to not automatically those big changes to prevent unexpected side effects.
  • Maybe those two behaviors could be grouped together in a dtx-support property or something, making it easier for end users?

Merge request reports