GNOME Bugzilla – Bug 751636
rtpjitterbuffer: wastes CPU processing too many unnecessary timers
Last modified: 2018-11-03 15:01:37 UTC
I have found a case where the jitterbuffer gets a lot of timers and it spends a lot of CPU processing them. I open this bug to provide some patches to fix this problem.
Created attachment 306268 [details] [review] 0001-gstrtpjitterbuffer-consider-timers-len-to-compare-wi.patch
Related to this also http://cgit.freedesktop.org/gstreamer/gst-plugins-good/commit/?id=de5cd0995bfa9fe7852532573e0b7cab9498725a Putting the timers into a sorted data structure is not too trivial though: - get_timeout() depends not only on the timeout but also on the latency property, ts-offset and out_offset (buffering mode). This could be solved be resorting the complete sequence whenever those change (which happens not too often hopefully, and usually shouldn't change the order so should be O(n)) - timers are searched for by (type, seqnum) and pointer value. If we sort the timers by (timeout, seqnum), then these other two would be O(n) again. We could add two hash tables for that, but that also seems too much overhead.
Comment on attachment 306268 [details] [review] 0001-gstrtpjitterbuffer-consider-timers-len-to-compare-wi.patch This doesn't seem right as it considers all possible timers. Some timer types shouldn't be considered a gap (the expected timers for future packets)
Created attachment 306271 [details] [review] rtpjitterbuffer: If possible, always update the current time before looping over all timers If we have a clock, update "now" now with the very latest running time we have. If timers are unscheduled below we otherwise wouldn't update now (it's only updated when timers expire), and also for the very first loop iteration now would otherwise always be 0.
Created attachment 306292 [details] [review] 0001-gstrtpjitterbuffer-consider-timers-len-to-compare-wi.patch
Created attachment 306293 [details] [review] 0001-gstrtpjitterbuffer-consider-timers-len-to-compare-wi.patch
Created attachment 306296 [details] [review] rtpjitterbuffer: If possible, always update the current time before looping over all timers If we have a clock, update "now" now with the very latest running time we have. If timers are unscheduled below we otherwise wouldn't update now (it's only updated when timers expire), and also for the very first loop iteration now would otherwise always be 0.
Created attachment 306648 [details] [review] rtpjitterbuffer: Consider timers len to compare with RTP_MAX_DROPOUT When there are a lot of small gaps, we can consider that there is a big gap (too losses) to reset the buffer.
Created attachment 306649 [details] [review] rtpjitterbuffer: Consider timers len to compare with RTP_MAX_DROPOUT When there are a lot of small gaps, we can consider that there is a big gap (too losses) to reset the buffer.
What about this? Your patch before would've never reset in case we had: big gap, 1 packet, big gap, 1 packet, etc.. Before resetting it would wait for 5 consecutive seqnums, which would never happen in this case.
Created attachment 306652 [details] [review] rtpjitterbuffer: Consider timers len to compare with RTP_MAX_DROPOUT When there are a lot of small gaps, we can consider that there is a big gap (too losses) to reset the buffer.
Created attachment 306653 [details] [review] rtpjitterbuffer: Consider timers len to compare with RTP_MAX_DROPOUT When there are a lot of small gaps, we can consider that there is a big gap (too losses) to reset the buffer.
For making the timer handling more efficient we could probably go with two GSequences. One sorted by (< seqnum), the other sorted by (< (timeout, seqnum)). The second needs to be resorted whenever the properties change. Both can be used to find by "ptr" by just looking for the seqnum / timeout value.
The Pexip folks also presented something based on a proper priority queue at the GStreamer conference. It would be good to get that merged here. What's the status of that?
-- GitLab Migration Automatic Message -- This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/gstreamer/gst-plugins-good/issues/197.