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 745667 - volume: Unable to set the volume with gcc-4.9 on arm platform
volume: Unable to set the volume with gcc-4.9 on arm platform
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-base
1.4.1
Other Linux
: Normal major
: 1.5.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2015-03-05 11:11 UTC by Stephane Cerveau
Modified: 2015-03-06 10:23 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
volume: Explicitly cast integers to doubles and then back to integers after multiplication (1.40 KB, patch)
2015-03-05 11:31 UTC, Sebastian Dröge (slomo)
none Details | Review

Description Stephane Cerveau 2015-03-05 11:11:22 UTC
The volume can not be set with gstvolume using gstreamer-1.4.1 and gcc-4.9 on armv7 with hardfloat activated.

Whith this pipeline:

gst-launch-1.0 audiotestsrc ! volume volume=0.5 ! alsasink

The sound level is equal to 0 instead of half of the standard volume. If i
put volume=1 this is working properly.
Affter investigation i understood that it is related to this line

gstvolume.c:251 self->current_volume = volume; in volume_update_volume

If i put any log just after this line, the behaviour is coming back to
normal. If i compile with -00 it's also working fine. 

The bug can be reproduced on either rpi2 or imx6q.
Comment 1 Sebastian Dröge (slomo) 2015-03-05 11:29:03 UTC
If that line is really the problem, or the lines below it... then I would say this is a compiler bug (or undefined C behaviour I didn't know yet).

What happens afterwards is that we multiple a float with signed integers, and my guess is that the compiler breaks that by first casting the float to an integer (thus giving 0 for values < 1.0)... instead of casting the integer to a float first.

Does the attached patch fix it for you?
Comment 2 Sebastian Dröge (slomo) 2015-03-05 11:31:56 UTC
Created attachment 298625 [details] [review]
volume: Explicitly cast integers to doubles and then back to integers after multiplication
Comment 3 Sebastian Dröge (slomo) 2015-03-05 11:40:38 UTC
Or maybe the compiler does the right thing here, and did the wrong thing before by casting the integer to a float first?
Comment 4 Jan Schmidt 2015-03-05 11:51:41 UTC
Seems like a compiler bug - ints should always promote to floats/doubles/long doubles if the other operand is one of those types. (Section 6.3 of ISO 9899:1999)
Comment 5 Luis de Bethencourt 2015-03-05 12:15:29 UTC
gcc 4.9.2 has been out for enough time that it is very suprising to find no other mentions of this compiler bug in the web.

Is assuming the standard will happen defined behaviour?

Is there any cons, besides legibility to apply Sebastian's patch of forced casting?
Comment 6 Sebastian Dröge (slomo) 2015-03-05 12:16:36 UTC
We don't even know yet if the patch works :)
Comment 7 Stephane Cerveau 2015-03-05 12:58:15 UTC
The patch seems ok. I compiled with -O2 and i can set the volume as before.
Comment 8 Sebastian Dröge (slomo) 2015-03-05 13:22:30 UTC
commit 40f4daffd12b84dceb23942a992815e11b5a0839
Author: Sebastian Dröge <sebastian@centricular.com>
Date:   Thu Mar 5 12:31:06 2015 +0100

    volume: Explicitly cast integers to doubles and then back to integers after multiplication
    
    gcc 4.9.1 on ARM seems to have a bug that causes it to cast the float to an
    integer first, resulting in a 0 scale factor for volume < 1.0.
    
    As a side effect this change here will also improve accuracy of the result a
    bit because we go via doubles instead of floats.
    
    https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65325
    https://bugzilla.gnome.org/show_bug.cgi?id=745667
Comment 9 Luis de Bethencourt 2015-03-06 10:23:11 UTC
Thanks Sebastian :)