rtpjitterbuffer: Heuristic to detect equidistant packets is broken
As the code comment says, this is what is currently done.
/* Guess whether stream currently uses equidistant packet spacing. If we
* often see identical timestamps it means the packets are not
* equidistant. */
This assumes that the underlying codec data has a constant rate, and only considers fragmented packets as the only cause of non-equidistant packets. That's of course wrong.
It breaks for example if there are simply gaps in an audio stream (and e.g. Opus DTX, see also gst-plugins-base#566), but even for video if the framerate is not constant.
A solution would be to also keep track of the difference in RTP timestamps between consecutive packets, which is what "equidistant" is about in the end. Implementing that is a bit tricky though as in this part of the code we're looking at packets in the order in which they arrive, not in sequence number order.
The equidistant flag is used to only for calculating the duration of a sequence number gap, by assuming that each packet had the same duration. So if there was a gap of D seconds and X packets missing, we assume that each lost packet had a duration of D/X. This is then used for the LOST timer, as well as later for the GstRTPPacketLost
event and as a result becomes the duration for the GAP
event created by the depayloader, and would be used for producing PLC data.
This means that if this duration is calculated wrong, then we would potentially produce more PLC data than actual data missing, or less. The latter part should probably be based instead on the difference in RTP timestamps between the packet around the gap.