GNOME Bugzilla – Bug 780904
rtprtxqueue: implement handling of the max-size-time property
Last modified: 2017-04-11 07:00:10 UTC
Created attachment 349241 [details] [review] rtprtxqueue: implement handling of the max-size-time property The max-size-time property in rtprtxqueue is not doing anything. This patch implements it like in rtprtxsend.
Review of attachment 349241 [details] [review]: This patch is wrong, the GST_BUFFER_TIMESTAMP() is in nanoseconds, not in the RTP clock. And you can't just use the timestamps, you must convert them to the running time using the segment. Look at how the queue, aggregator or multiqueue are doing it. ::: gst/rtpmanager/gstrtprtxqueue.c @@ +349,3 @@ + + /* it needs to work if ts wraps */ + if (high_ts >= low_ts) { GStreamer timestamps are 64-bit, they don't wrap-around !
Indeed, classic case of copy-paste gone wrong. I wonder if it is better to use the buffer timestamps or the RTP clock, like rtprtxsend does, though.
Using the gst timestamps is probably easier, you dont need to worry about about wrap-arounds, etc.
Created attachment 349477 [details] [review] rtprtxqueue: implement handling of the max-size-time property
Created attachment 349478 [details] [review] tests/check/rtprtx: add checks for rtprtxqueue's max-size-{time,packets} properties
Review of attachment 349477 [details] [review]: You need to queue the segment events... and to read them at the entrance and and the exit of the queue (and then have two member GstSegment in the object struct). The buffers could be on different segments!
Created attachment 349612 [details] [review] rtprtxqueue: implement handling of the max-size-time property
Review of attachment 349612 [details] [review]: Looks good!
Review of attachment 349478 [details] [review]: Looks good too
commit 21f532f1c66f27e1d097cab12a48b2763e0d93ac Author: George Kiagiadakis <george.kiagiadakis@collabora.com> Date: Fri Apr 7 16:33:18 2017 +0300 tests/check/rtprtx: add checks for rtprtxqueue's max-size-{time,packets} properties https://bugzilla.gnome.org/show_bug.cgi?id=780867 commit 7f6c7839301b7a13aae79b260deb4e619248d270 Author: George Kiagiadakis <george.kiagiadakis@collabora.com> Date: Tue Apr 4 17:33:31 2017 +0300 rtprtxqueue: implement handling of the max-size-time property https://bugzilla.gnome.org/show_bug.cgi?id=780867
oh damn, I pasted the wrong bug link on the commits...