[Bug 751311] rtp: Dynamic dropout / reorder limits

[GStreamer (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=751311

Sebastian Dröge (slomo) <slomo at coaxion.net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #306390|none                        |reviewed
             status|                            |

--- Comment #20 from Sebastian Dröge (slomo) <slomo at coaxion.net> ---
Review of attachment 306390:
 --> (https://bugzilla.gnome.org/review?bug=751311&attachment=306390)

::: gst/rtpmanager/rtpstats.c
@@ +39,3 @@
+guint32
+gst_rtp_packet_rate_ctx_update (RTPPacketRateCtx * ctx, guint16 seqnum,
+    guint32 ts, gint32 clock_rate)

Shouldn't the clock_rate be in some kind of "init" function only?

@@ +60,3 @@
+  }
+
+  diff_ts = ts - ctx->last_ts;

This doesn't handle timestamps wraparounds, or you have to require the caller
to provide ext RTP timestamps already (but those should be guint64 then)

Also can't the caller just provide nanoseconds time? All callers should already
calculate that

@@ +63,3 @@
+  diff_ts = gst_util_uint64_scale_int (diff_ts, GST_USECOND, clock_rate);
+  ctx->avg_packet_rate =
+      ((7 * (ctx->avg_packet_rate + 1)) + (1000 * diff_seqnum / diff_ts)) / 8;

Why USECOND? Why not

diff_ts = gst_util_uint64_scale_int (diff_ts, GST_SECOND, clock_rate);
/* now diff_ts is nanoseconds */
avg_packet_rate = (7 * (avg_packet_rate + 1) + gst_util_uint64_scale
(diff_seqnum, GST_SECOND, diff_ts)) / 8

::: gst/rtpmanager/rtpstats.h
@@ +209,3 @@
+void gst_rtp_packet_rate_ctx_destroy (RTPPacketRateCtx *ctx);
+guint32 gst_rtp_packet_rate_ctx_update (RTPPacketRateCtx *ctx, guint16 seqnum,
guint32 ts, gint32 clock_rate);
+guint32 gst_rtp_packet_rate_ctx_get (RTPPacketRateCtx *ctx);

Maybe require the caller to allocate that, and then only provide a
gst_rtp_packet_rate_ctx_reset() that sets default values to all fields.

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.