GNOME Bugzilla – Bug 323017
While(1) loop with sleep(0) in basertpdepayload.c
Last modified: 2005-12-12 12:40:53 UTC
Please describe the problem:
Steps to reproduce:
Does this happen every time?
Marking as blocker, as it's effectively a 100% CPU loop and both the problem and
the fix seem rather straight-forward.
Looks fine to me. Go ahead and commit.
The bug here is that GST_NSECOND is '1', because nanoseconds are GStreamer's
I'm not sure what the units of clock_rate are so it's hard to know what the
statement is TRYING to calculate. As you say (GST_NSECOND / filter->clock_rate)
is the wrong thing to do. I'd guess you probably wanted (GST_SECOND /
filter->clock_rate) instead, which would sleep for 1^9/filter->clock_rate
Making the filter getrange based doesn't sound like the right approach, since
it's not a random access stream.
Jan's change looks more correct -- clock_rate appears to be in per-second units.
2005-12-05 Andy Wingo <email@example.com>
patch by: Kai Vehmanen <kv2004 eca cx>
(gst_base_rtp_depayload_thread): Fix busy loop (#323017).
The original intent was probably 'GST_SECOND/filter->clock_rate', but this also
is far from optimal. For example most current depayloaders in gstreamer use a
clock-rate of 8000, and packetization interval of 20 or 30ms (for audio, much
longer intervals for video). Sleeping GST_SECOND/8000 gets us 0.12msec sleep
interval, although packets are received at most every ~20msec. Thus the
depayloader thread is woken up unnecessarily roughly 160 times (with no work to
Ideally the data should be read from the depayloader/jitter buffer when the
pipeline sink (which is consuming the data) is ready to receive more data. The
next option is to match the wakeup frequency to the packetization (or packet
arrival) frequency. Unfortunately this might not match the consumption rate in
When data is read from a reliable source (a file, TCP connection, etc), the
application can set 'queue-delay' to zero, and disable the jitter-buffering
I don't know enough about gstreamer architecture to comment on the usability of
getrange in this case. Still, especially for audio, using sleep() to schedule
buffers for playback, does not seem like the right way to go. The
depayloader/jitter buffer should be waken up (=> polled) when more data is
needed for playback.
As I see it, the post 0.10 fix will be to remove this thread and jitter buffer
completely, add a small dynamic buffer to basertpdepayloader solely for
reordering packets when a depayloader requires multiple packets per frame. Then
we will generate a clock from the rtp stream and clock the pipeline to this,
then the rest of the reordering and jitter correction can be left to the audio sink.
We have to consider the case when an audio sink isnt used. It can be video, the
the stream can be saved to a file or taken somewhere else. We can't rely on the
audio sink. The jitter is very important for live audio, but its not so
important when streaming to a file or for video. The ordering on the other hand
is important for both, and it probably needs to be done before any sinks.
After some talk, it has been decided to move the whole reordering/dropping
algorithms into the rtprecv element instead of the depayloader. The depayloader
base class simply has to put together the packets into frames and probably also
still do the RTP TS->GST TS conversion, or even that might get moved out.
As for releasing the queue that will be in rtprecv, a simple algorithm that
calculates the average time difference between the rtp packets is enough to
determine the interval of release.
A new proposal (based on comments to this bug) for playout buffering, has been
now documented at:
About reordering at sink:
Reordering must happen before codecs (i.e. not in the sinks), because some
decoders are stateful and need to be fed with frames in correct order.