After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 784250 - rtpstats: Glib CRITICAL _gst_util_uint64_scale_int: assertion 'denom > 0' failed
rtpstats: Glib CRITICAL _gst_util_uint64_scale_int: assertion 'denom > 0' failed
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
git master
Other All
: Normal normal
: 1.12.3
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2017-06-27 14:23 UTC by Julien Isorce
Modified: 2017-07-25 09:29 UTC
See Also:
GNOME target: ---
GNOME version: ---

Attachments
rtpstats: fix assertion 'denom > 0' failed (1008 bytes, patch)
2017-06-27 15:03 UTC, Julien Isorce 		none Details | Review
rtpstats: fix assertion 'denom > 0' failed (946 bytes, patch)
2017-06-29 16:22 UTC, Julien Isorce 		committed Details | Review

Description Julien Isorce 2017-06-27 14:23:33 UTC 
In gst-plugins-good/gst/rtpmanager/rtpstats.c, ctx->clock_rate is a guint32 whereas "denom" parameter of gst_util _uint64_scale_int is a gint.

Indeed ctx->clock_rate can be set to -1 when rtpsource.c calls gst_rtp_packet_rate_ctx_reset with -1.
Comment 1 Julien Isorce 2017-06-27 15:03:34 UTC 
Created attachment 354573 [details] [review]
rtpstats: fix assertion 'denom > 0' failed
Comment 2 Nicolas Dufresne (ndufresne) 2017-06-29 13:30:03 UTC 
Review of attachment 354573 [details] [review]:

::: gst/rtpmanager/rtpstats.c
@@ +39,3 @@
   guint32 new_packet_rate;
 
+  if ((gint) ctx->clock_rate <= 0) {

I think Miguel just got it wrong when he wrote this stats. Just change rtpstats.h to store clock_rate as a gint, this is a superset of gstrtpjitterbuffer (gint32) and rtpsource (gint). I've check other places, like in rtpbin, they use uint but that's taken from the caps, so that's correct.
Comment 3 Julien Isorce 2017-06-29 16:22:27 UTC 
Created attachment 354698 [details] [review]
rtpstats: fix assertion 'denom > 0' failed

It fixes the issue and all tests in gst-plugins-good and gst-rtsp-server still pass.
Comment 4 Nicolas Dufresne (ndufresne) 2017-06-29 17:23:13 UTC 
Review of attachment 354698 [details] [review]:

Good.
Comment 5 Nicolas Dufresne (ndufresne) 2017-06-29 19:59:14 UTC 
Thanks.

Attachment 354698 [details] pushed as afbabae - rtpstats: fix assertion 'denom > 0' failed
Comment 6 Julien Isorce 2017-07-25 09:11:06 UTC 
Can I push above patch to 1.12 branch ?
Comment 7 Sebastian Dröge (slomo) 2017-07-25 09:16:32 UTC 
Sure
Comment 8 Julien Isorce 2017-07-25 09:24:59 UTC 
commit c9465db1b73432a92f49f59911219aed60a11407
Author:     Julien Isorce <jisorce@oblong.com>
AuthorDate: Tue Jun 27 15:59:18 2017 +0100
Commit:     Julien Isorce <jisorce@oblong.com>
CommitDate: Tue Jul 25 10:24:11 2017 +0100

    rtpstats: fix assertion 'denom > 0' failed
    
    gst_util_uint64_scale_int takes a gint as denom parameter
    whereas ctx->clock_rate is a guint32.
    
    It happens when gst_rtp_packet_rate_ctx_reset set clock_rate
    to -1.
    
    So just define clock_rate as gint like it is done in rtpsource.h
    
    https://bugzilla.gnome.org/show_bug.cgi?id=784250