[Bug 772740] rtpbin: receiving RTP bundle support

[GStreamer (GNOME Bugzilla)]

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

--- Comment #10 from Philippe Normand <philn at igalia.com> ---
(In reply to Olivier Crête from comment #9)
> Review of attachment 337827 [details] [review]:
> 
> Except for the little nits below, this generally looks good. I'd really
> appreciate if there was a unit test to make sure this doesn't get broken in
> the future. I think it would also be beneficial to avoid plugin the extra
> ssrcdemux if the signal is not connected (or if it returns 0).
> 

With the current semantics, 0 means use the existing session. But thanks for
the feedback, I'll work on a test and implement lazy ssrcdemux plugging :)

> ::: gst/rtpmanager/gstrtpbin.c
> @@ +672,3 @@
> +  g_value_init (&result, G_TYPE_UINT);
> +  g_value_init (&params[0], GST_TYPE_ELEMENT);
> +  g_value_set_object (&params[0], rtpbin);
> 
> You need to unset the params[0] after emitting the signal otherwise you'll
> leak the element.
> 

Ok!

> @@ +678,3 @@
> +  g_signal_emitv (params,
> +      gst_rtp_bin_signals[SIGNAL_ON_BUNDLED_SSRC], 0, &result);
> +  if (G_VALUE_HOLDS_UINT (&result)) {
> 
> This will always be true, you just initialized it to G_TYPE_UINT earlier.
> 

Ok!

> @@ +3560,3 @@
> +    goto no_name;
> +
> +  GST_DEBUG_OBJECT (rtpbin, "finding session %d", sessid);
> 
> Use %u for debug since it's a guint.

The rtpbin code is crippled with these %d where %u should be used. But yeah I
can fix it!

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