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 595133 - gst/gstutils check fails
gst/gstutils check fails
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
0.10.x
Other Linux
: Normal blocker
: 0.10.25
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2009-09-14 07:55 UTC by Götz Waschk
Modified: 2009-09-15 09:31 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
requested output (3.69 KB, text/plain)
2009-09-14 14:28 UTC, Götz Waschk
  Details
0001-utils-Fix-GMP-scaling-unit-test.patch (2.62 KB, patch)
2009-09-15 07:43 UTC, Sebastian Dröge (slomo)
none Details | Review
0001-utils-Fix-GMP-scaling-unit-test.patch (2.63 KB, patch)
2009-09-15 08:30 UTC, Sebastian Dröge (slomo)
committed Details | Review

Description Götz Waschk 2009-09-14 07:55:25 UTC
This is on Mandriva Cooker i586 with gstreamer 0.10.24.2:

Running suite(s): GstUtils
93%: Checks: 16, Failures: 1, Errors: 0
gst/gstutils.c:835:F:general:test_math_scale_gmp:0: error: gst_util_uint64_scale_ceil(): 2625204072 * 4273619712024848820 / 963185059 = 11647942174097927414, correct = 3308274324

FAIL: gst/gstutils
Comment 1 Sebastian Dröge (slomo) 2009-09-14 07:57:53 UTC
Could you attach the output of configure here or the output of
grep UINT128 config.h

?
Comment 2 Götz Waschk 2009-09-14 07:59:56 UTC
/* #undef HAVE_UINT128_T */
Comment 3 Wim Taymans 2009-09-14 14:25:42 UTC
can you post the output of:

echo "" | gcc -E -dM -
Comment 4 Götz Waschk 2009-09-14 14:28:25 UTC
Created attachment 143161 [details]
requested output
Comment 5 Wim Taymans 2009-09-14 14:42:28 UTC
so this is on 32bits. I wonder if it happens on all 32bits. I tried recompiling it with the 64bits optimisations disabled but it still works fine here.
Comment 6 Sebastian Dröge (slomo) 2009-09-14 16:09:06 UTC
Ok, I can reproduce this here with:

gst_util_uint64_scale_ceil (2625204072, 4273619712024848820, 963185059)
Comment 7 Sebastian Dröge (slomo) 2009-09-14 16:20:51 UTC
Another variant of this:

gst/gstutils.c:835:F:general:test_math_scale_gmp:0: error: gst_util_uint64_scale_ceil(): 881023593 * 3005106402520480590 / 1954429577 = 1354650825618311888, correct = 1833887913
Comment 8 Sebastian Dröge (slomo) 2009-09-15 06:13:23 UTC
While looking at this a bit more it looks like a problem in the unit test instead because the calculated result is of course correct... I'll fix this ;)
Comment 9 Sebastian Dröge (slomo) 2009-09-15 07:43:41 UTC
Created attachment 143208 [details] [review]
0001-utils-Fix-GMP-scaling-unit-test.patch

Possible patch. Could you check if this really fixes it for you? Problem was, that GMP only uses "unsigned long int", which is 32 bit on 32 bit architectures and can't hold a guint64.
Comment 10 Götz Waschk 2009-09-15 08:15:41 UTC
I'm sorry, but your patch does not help. First, there is this warning, that is promoted to an error by the default -Werror flag:

  CC    gstutils.o
cc1: warnings being treated as errors
gst/gstutils.c: In function 'gmp_get_uint64':
gst/gstutils.c:810: error: left shift count >= width of type
make[1]: *** [gstutils.o] Error 1


After removing Werror I get this:
Running suite(s): GstUtils
87%: Checks: 16, Failures: 2, Errors: 0
gst/gstutils.c:886:F:general:test_math_scale_gmp:0: error: gst_util_uint64_scale_ceil(): 2882883178 * 4273619712024848820 / 963185059 = 12791255700910577540, correct = 4227317709

gst/gstutils.c:916:F:general:test_math_scale_gmp_int:0: error: gst_util_uint64_scale_int(): 3767122049 * 2021794878 / 1473265340 = 5169705589, correct = 874738293

FAIL: gst/gstutils
Comment 11 Sebastian Dröge (slomo) 2009-09-15 08:30:02 UTC
Created attachment 143215 [details] [review]
0001-utils-Fix-GMP-scaling-unit-test.patch

Oh, stupid me. This should get rid of the warning and hopefully also fixes the unit test suite ;)
Comment 12 Götz Waschk 2009-09-15 09:10:37 UTC
It fixes all mentioned problems, thank you.
Comment 13 Sebastian Dröge (slomo) 2009-09-15 09:25:14 UTC
Ok, thanks for testing :)
Comment 14 Sebastian Dröge (slomo) 2009-09-15 09:31:35 UTC
commit 7464062ff2f71ae77cba631489b0890e91109910
Author: Sebastian Dröge <sebastian.droege@collabora.co.uk>
Date:   Tue Sep 15 09:41:28 2009 +0200

    utils: Fix GMP scaling unit test
    
    GMP only uses "unsigned long int", which is 32 bit
    on 32 bit architectures and can't hold a guint64.
    This resulted in false unit test failures on 32 bit architectures.
    
    Fixes bug #595133.