When building with Clang 10 on Fedora 32, we get:
CC audio/mixeng.o
audio/mixeng.c:274:34: error: implicit conversion from 'unsigned int' to 'float' changes value from 4294967295 to 4294967296 [-Werror,-Wimplicit-int-float-conversion]
static const float float_scale = UINT_MAX / 2.f;
^~~~~~~~ ~
/usr/lib64/clang/10.0.0/include/limits.h:56:37: note: expanded from macro 'UINT_MAX'
#define UINT_MAX (__INT_MAX__ *2U +1U)
~~~~~~~~~~~~~~~~~^~~
Fix by using a 64-bit float for the conversion, before casting
back to 32-bit float.
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
audio/mixeng.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/audio/mixeng.c b/audio/mixeng.c
index 739a500449..9946bfeaec 100644
--- a/audio/mixeng.c
+++ b/audio/mixeng.c
@@ -271,7 +271,7 @@ f_sample *mixeng_clip[2][2][2][3] = {
#define CONV_NATURAL_FLOAT(x) (x)
#define CLIP_NATURAL_FLOAT(x) (x)
#else
-static const float float_scale = UINT_MAX / 2.f;
+static const float float_scale = UINT_MAX / 2.;
#define CONV_NATURAL_FLOAT(x) ((x) * float_scale)
#ifdef RECIPROCAL
--
2.21.3
On Sun, 3 May 2020, Philippe Mathieu-Daudé wrote:
> When building with Clang 10 on Fedora 32, we get:
>
> CC audio/mixeng.o
> audio/mixeng.c:274:34: error: implicit conversion from 'unsigned int' to 'float' changes value from 4294967295 to 4294967296 [-Werror,-Wimplicit-int-float-conversion]
> static const float float_scale = UINT_MAX / 2.f;
> ^~~~~~~~ ~
> /usr/lib64/clang/10.0.0/include/limits.h:56:37: note: expanded from macro 'UINT_MAX'
> #define UINT_MAX (__INT_MAX__ *2U +1U)
> ~~~~~~~~~~~~~~~~~^~~
>
> Fix by using a 64-bit float for the conversion, before casting
> back to 32-bit float.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> audio/mixeng.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/audio/mixeng.c b/audio/mixeng.c
> index 739a500449..9946bfeaec 100644
> --- a/audio/mixeng.c
> +++ b/audio/mixeng.c
> @@ -271,7 +271,7 @@ f_sample *mixeng_clip[2][2][2][3] = {
> #define CONV_NATURAL_FLOAT(x) (x)
> #define CLIP_NATURAL_FLOAT(x) (x)
> #else
> -static const float float_scale = UINT_MAX / 2.f;
> +static const float float_scale = UINT_MAX / 2.;
Maybe writing it as 2.0 is easier to read and looks nicer.
Regards,
BALATON Zoltan
On 5/3/20 4:32 AM, Philippe Mathieu-Daudé wrote: > When building with Clang 10 on Fedora 32, we get: > > CC audio/mixeng.o > audio/mixeng.c:274:34: error: implicit conversion from 'unsigned int' to 'float' changes value from 4294967295 to 4294967296 [-Werror,-Wimplicit-int-float-conversion] > static const float float_scale = UINT_MAX / 2.f; > ^~~~~~~~ ~ > /usr/lib64/clang/10.0.0/include/limits.h:56:37: note: expanded from macro 'UINT_MAX' > #define UINT_MAX (__INT_MAX__ *2U +1U) > ~~~~~~~~~~~~~~~~~^~~ > > Fix by using a 64-bit float for the conversion, before casting > back to 32-bit float. > > Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > --- > audio/mixeng.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) No, this should be fixed properly. First, the warning is in the !FLOAT_MIXENG branch. IMO that means we should not be using floating point at all, and this should be a simple integral multiply/shift. I had a brief look at this before the 5.0 release. The arithmetic all through audio looks confused to me. There's a combination of shifting and masking (implying a scale by 1<<32), and multiplication and division by UINT32_MAX. I'm reasonably certain that every appearance of UINT32_MAX in this code is an off-by-one bug, or a misuse of the constant. r~
> Fix by using a 64-bit float for the conversion, before casting
> back to 32-bit float.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> audio/mixeng.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/audio/mixeng.c b/audio/mixeng.c
> index 739a500449..9946bfeaec 100644
> --- a/audio/mixeng.c
> +++ b/audio/mixeng.c
> @@ -271,7 +271,7 @@ f_sample *mixeng_clip[2][2][2][3] = {
> #define CONV_NATURAL_FLOAT(x) (x)
> #define CLIP_NATURAL_FLOAT(x) (x)
> #else
> -static const float float_scale = UINT_MAX / 2.f;
> +static const float float_scale = UINT_MAX / 2.;
I would prefer an explicit cast of UINT_MAX to float. This is what we already have in audio/mixeng_template.h in the conf_* and clip_* functions with FLOAT_MIXENG defined. I think similar functions should look similar.
> #define CONV_NATURAL_FLOAT(x) ((x) * float_scale)
>
> #ifdef RECIPROCAL
Please don't forget to fix the RECIPROCAL case.
Btw. the problem was reported here:
https://lists.nongnu.org/archive/html/qemu-devel/2020-03/msg02270.html
With best regards,
Volker
On 5/4/20 7:49 AM, Volker Rümelin wrote:
>
>> Fix by using a 64-bit float for the conversion, before casting
>> back to 32-bit float.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>> audio/mixeng.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/audio/mixeng.c b/audio/mixeng.c
>> index 739a500449..9946bfeaec 100644
>> --- a/audio/mixeng.c
>> +++ b/audio/mixeng.c
>> @@ -271,7 +271,7 @@ f_sample *mixeng_clip[2][2][2][3] = {
>> #define CONV_NATURAL_FLOAT(x) (x)
>> #define CLIP_NATURAL_FLOAT(x) (x)
>> #else
>> -static const float float_scale = UINT_MAX / 2.f;
>> +static const float float_scale = UINT_MAX / 2.;
>
> I would prefer an explicit cast of UINT_MAX to float. This is what we already have in audio/mixeng_template.h in the conf_* and clip_* functions with FLOAT_MIXENG defined. I think similar functions should look similar.
>
>> #define CONV_NATURAL_FLOAT(x) ((x) * float_scale)
>>
>> #ifdef RECIPROCAL
>
> Please don't forget to fix the RECIPROCAL case.
>
>
> Btw. the problem was reported here:
> https://lists.nongnu.org/archive/html/qemu-devel/2020-03/msg02270.html
Ah I missed that, I now feel safer knowing developers who understand
this code are already trying to fix it, thanks Volker!
>
> With best regards,
> Volker
>
>
© 2016 - 2026 Red Hat, Inc.