1. Patchew
  2. QEMU
  3. [PATCH v7 0/6] coreaudio fixes
  4. View patch

[PATCH v7 1/6] coreaudio: Remove unnecessary explicit casts

Akihiko Odaki posted 6 patches 1 week, 5 days ago
[PATCH v7 1/6] coreaudio: Remove unnecessary explicit casts
Posted by Akihiko Odaki 1 week, 5 days ago 
coreaudio had unnecessary explicit casts and they had extra whitespaces
around them so remove them.

Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
 audio/coreaudio.m | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/audio/coreaudio.m b/audio/coreaudio.m
index cadd729d50537850d81718b9284efed5877d9185..0b67347ad7e8c43a77af308a1a3a654dd7084083 100644
--- a/audio/coreaudio.m
+++ b/audio/coreaudio.m
@@ -309,7 +309,7 @@ static OSStatus audioDeviceIOProc(
     UInt32 frameCount, pending_frames;
     void *out = outOutputData->mBuffers[0].mData;
     HWVoiceOut *hw = hwptr;
-    coreaudioVoiceOut *core = (coreaudioVoiceOut *) hwptr;
+    coreaudioVoiceOut *core = hwptr;
     size_t len;
 
     if (coreaudio_buf_lock (core, "audioDeviceIOProc")) {
@@ -392,10 +392,10 @@ static OSStatus init_out_device(coreaudioVoiceOut *core)
     }
 
     if (frameRange.mMinimum > core->frameSizeSetting) {
-        core->audioDevicePropertyBufferFrameSize = (UInt32) frameRange.mMinimum;
+        core->audioDevicePropertyBufferFrameSize = frameRange.mMinimum;
         dolog ("warning: Upsizing Buffer Frames to %f\n", frameRange.mMinimum);
     } else if (frameRange.mMaximum < core->frameSizeSetting) {
-        core->audioDevicePropertyBufferFrameSize = (UInt32) frameRange.mMaximum;
+        core->audioDevicePropertyBufferFrameSize = frameRange.mMaximum;
         dolog ("warning: Downsizing Buffer Frames to %f\n", frameRange.mMaximum);
     } else {
         core->audioDevicePropertyBufferFrameSize = core->frameSizeSetting;

-- 
2.48.1
Re: [PATCH v7 1/6] coreaudio: Remove unnecessary explicit casts
Posted by Christian Schoenebeck 1 week, 5 days ago 
On Friday, January 24, 2025 6:12:04 AM CET Akihiko Odaki wrote:
> coreaudio had unnecessary explicit casts and they had extra whitespaces
> around them so remove them.
> 
> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> ---
>  audio/coreaudio.m | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/audio/coreaudio.m b/audio/coreaudio.m
> index cadd729d50537850d81718b9284efed5877d9185..0b67347ad7e8c43a77af308a1a3a654dd7084083 100644
> --- a/audio/coreaudio.m
> +++ b/audio/coreaudio.m
> @@ -309,7 +309,7 @@ static OSStatus audioDeviceIOProc(
>      UInt32 frameCount, pending_frames;
>      void *out = outOutputData->mBuffers[0].mData;
>      HWVoiceOut *hw = hwptr;
> -    coreaudioVoiceOut *core = (coreaudioVoiceOut *) hwptr;
> +    coreaudioVoiceOut *core = hwptr;

Well, hwptr is void*, so both versions are fine.

struct name 'coreaudioVoiceOut' should start with upper case BTW.

>      size_t len;
>  
>      if (coreaudio_buf_lock (core, "audioDeviceIOProc")) {
> @@ -392,10 +392,10 @@ static OSStatus init_out_device(coreaudioVoiceOut *core)
>      }
>  
>      if (frameRange.mMinimum > core->frameSizeSetting) {
> -        core->audioDevicePropertyBufferFrameSize = (UInt32) frameRange.mMinimum;
> +        core->audioDevicePropertyBufferFrameSize = frameRange.mMinimum;
>          dolog ("warning: Upsizing Buffer Frames to %f\n", frameRange.mMinimum);
>      } else if (frameRange.mMaximum < core->frameSizeSetting) {
> -        core->audioDevicePropertyBufferFrameSize = (UInt32) frameRange.mMaximum;
> +        core->audioDevicePropertyBufferFrameSize = frameRange.mMaximum;
>          dolog ("warning: Downsizing Buffer Frames to %f\n", frameRange.mMaximum);
>      } else {
>          core->audioDevicePropertyBufferFrameSize = core->frameSizeSetting;

Those casts are actually necessary, as AudioValueRange's members are Float64
(a.k.a. double) types.

/Christian
Re: [PATCH v7 1/6] coreaudio: Remove unnecessary explicit casts
Posted by Akihiko Odaki 1 week, 4 days ago 
On 2025/01/24 18:39, Christian Schoenebeck wrote:
> On Friday, January 24, 2025 6:12:04 AM CET Akihiko Odaki wrote:
>> coreaudio had unnecessary explicit casts and they had extra whitespaces
>> around them so remove them.
>>
>> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
>> ---
>>   audio/coreaudio.m | 6 +++---
>>   1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/audio/coreaudio.m b/audio/coreaudio.m
>> index cadd729d50537850d81718b9284efed5877d9185..0b67347ad7e8c43a77af308a1a3a654dd7084083 100644
>> --- a/audio/coreaudio.m
>> +++ b/audio/coreaudio.m
>> @@ -309,7 +309,7 @@ static OSStatus audioDeviceIOProc(
>>       UInt32 frameCount, pending_frames;
>>       void *out = outOutputData->mBuffers[0].mData;
>>       HWVoiceOut *hw = hwptr;
>> -    coreaudioVoiceOut *core = (coreaudioVoiceOut *) hwptr;
>> +    coreaudioVoiceOut *core = hwptr;
> 
> Well, hwptr is void*, so both versions are fine.
> 
> struct name 'coreaudioVoiceOut' should start with upper case BTW.
> 
>>       size_t len;
>>   
>>       if (coreaudio_buf_lock (core, "audioDeviceIOProc")) {
>> @@ -392,10 +392,10 @@ static OSStatus init_out_device(coreaudioVoiceOut *core)
>>       }
>>   
>>       if (frameRange.mMinimum > core->frameSizeSetting) {
>> -        core->audioDevicePropertyBufferFrameSize = (UInt32) frameRange.mMinimum;
>> +        core->audioDevicePropertyBufferFrameSize = frameRange.mMinimum;
>>           dolog ("warning: Upsizing Buffer Frames to %f\n", frameRange.mMinimum);
>>       } else if (frameRange.mMaximum < core->frameSizeSetting) {
>> -        core->audioDevicePropertyBufferFrameSize = (UInt32) frameRange.mMaximum;
>> +        core->audioDevicePropertyBufferFrameSize = frameRange.mMaximum;
>>           dolog ("warning: Downsizing Buffer Frames to %f\n", frameRange.mMaximum);
>>       } else {
>>           core->audioDevicePropertyBufferFrameSize = core->frameSizeSetting;
> 
> Those casts are actually necessary, as AudioValueRange's members are Float64
> (a.k.a. double) types.

Explicit casts are unnecessary. Implicit casts still happen at every 
line changed with this patch.

Regards,
Akihiko Odaki
Re: [PATCH v7 1/6] coreaudio: Remove unnecessary explicit casts
Posted by Christian Schoenebeck 1 week, 4 days ago 
On Saturday, January 25, 2025 6:58:30 AM CET Akihiko Odaki wrote:
> On 2025/01/24 18:39, Christian Schoenebeck wrote:
> > On Friday, January 24, 2025 6:12:04 AM CET Akihiko Odaki wrote:
> >> coreaudio had unnecessary explicit casts and they had extra whitespaces
> >> around them so remove them.
> >>
> >> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> >> ---
> >>   audio/coreaudio.m | 6 +++---
> >>   1 file changed, 3 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/audio/coreaudio.m b/audio/coreaudio.m
> >> index cadd729d50537850d81718b9284efed5877d9185..0b67347ad7e8c43a77af308a1a3a654dd7084083 100644
> >> --- a/audio/coreaudio.m
> >> +++ b/audio/coreaudio.m
> >> @@ -309,7 +309,7 @@ static OSStatus audioDeviceIOProc(
> >>       UInt32 frameCount, pending_frames;
> >>       void *out = outOutputData->mBuffers[0].mData;
> >>       HWVoiceOut *hw = hwptr;
> >> -    coreaudioVoiceOut *core = (coreaudioVoiceOut *) hwptr;
> >> +    coreaudioVoiceOut *core = hwptr;
> > 
> > Well, hwptr is void*, so both versions are fine.
> > 
> > struct name 'coreaudioVoiceOut' should start with upper case BTW.
> > 
> >>       size_t len;
> >>   
> >>       if (coreaudio_buf_lock (core, "audioDeviceIOProc")) {
> >> @@ -392,10 +392,10 @@ static OSStatus init_out_device(coreaudioVoiceOut *core)
> >>       }
> >>   
> >>       if (frameRange.mMinimum > core->frameSizeSetting) {
> >> -        core->audioDevicePropertyBufferFrameSize = (UInt32) frameRange.mMinimum;
> >> +        core->audioDevicePropertyBufferFrameSize = frameRange.mMinimum;
> >>           dolog ("warning: Upsizing Buffer Frames to %f\n", frameRange.mMinimum);
> >>       } else if (frameRange.mMaximum < core->frameSizeSetting) {
> >> -        core->audioDevicePropertyBufferFrameSize = (UInt32) frameRange.mMaximum;
> >> +        core->audioDevicePropertyBufferFrameSize = frameRange.mMaximum;
> >>           dolog ("warning: Downsizing Buffer Frames to %f\n", frameRange.mMaximum);
> >>       } else {
> >>           core->audioDevicePropertyBufferFrameSize = core->frameSizeSetting;
> > 
> > Those casts are actually necessary, as AudioValueRange's members are Float64
> > (a.k.a. double) types.
> 
> Explicit casts are unnecessary. Implicit casts still happen at every 
> line changed with this patch.

Wooo, I wasn't aware that QEMU doesn't use -Wconversion. I am not used to 
that. To me it makes sense to warn especially for things like implicit casts
from floating point to integer, as it would be the case here.

/Christian
Re: [PATCH v7 1/6] coreaudio: Remove unnecessary explicit casts
Posted by Akihiko Odaki 1 week, 3 days ago 
On 2025/01/25 19:41, Christian Schoenebeck wrote:
> On Saturday, January 25, 2025 6:58:30 AM CET Akihiko Odaki wrote:
>> On 2025/01/24 18:39, Christian Schoenebeck wrote:
>>> On Friday, January 24, 2025 6:12:04 AM CET Akihiko Odaki wrote:
>>>> coreaudio had unnecessary explicit casts and they had extra whitespaces
>>>> around them so remove them.
>>>>
>>>> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
>>>> ---
>>>>    audio/coreaudio.m | 6 +++---
>>>>    1 file changed, 3 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/audio/coreaudio.m b/audio/coreaudio.m
>>>> index cadd729d50537850d81718b9284efed5877d9185..0b67347ad7e8c43a77af308a1a3a654dd7084083 100644
>>>> --- a/audio/coreaudio.m
>>>> +++ b/audio/coreaudio.m
>>>> @@ -309,7 +309,7 @@ static OSStatus audioDeviceIOProc(
>>>>        UInt32 frameCount, pending_frames;
>>>>        void *out = outOutputData->mBuffers[0].mData;
>>>>        HWVoiceOut *hw = hwptr;
>>>> -    coreaudioVoiceOut *core = (coreaudioVoiceOut *) hwptr;
>>>> +    coreaudioVoiceOut *core = hwptr;
>>>
>>> Well, hwptr is void*, so both versions are fine.
>>>
>>> struct name 'coreaudioVoiceOut' should start with upper case BTW.
>>>
>>>>        size_t len;
>>>>    
>>>>        if (coreaudio_buf_lock (core, "audioDeviceIOProc")) {
>>>> @@ -392,10 +392,10 @@ static OSStatus init_out_device(coreaudioVoiceOut *core)
>>>>        }
>>>>    
>>>>        if (frameRange.mMinimum > core->frameSizeSetting) {
>>>> -        core->audioDevicePropertyBufferFrameSize = (UInt32) frameRange.mMinimum;
>>>> +        core->audioDevicePropertyBufferFrameSize = frameRange.mMinimum;
>>>>            dolog ("warning: Upsizing Buffer Frames to %f\n", frameRange.mMinimum);
>>>>        } else if (frameRange.mMaximum < core->frameSizeSetting) {
>>>> -        core->audioDevicePropertyBufferFrameSize = (UInt32) frameRange.mMaximum;
>>>> +        core->audioDevicePropertyBufferFrameSize = frameRange.mMaximum;
>>>>            dolog ("warning: Downsizing Buffer Frames to %f\n", frameRange.mMaximum);
>>>>        } else {
>>>>            core->audioDevicePropertyBufferFrameSize = core->frameSizeSetting;
>>>
>>> Those casts are actually necessary, as AudioValueRange's members are Float64
>>> (a.k.a. double) types.
>>
>> Explicit casts are unnecessary. Implicit casts still happen at every
>> line changed with this patch.
> 
> Wooo, I wasn't aware that QEMU doesn't use -Wconversion. I am not used to
> that. To me it makes sense to warn especially for things like implicit casts
> from floating point to integer, as it would be the case here.

I tried '--extra-cflags=-Wconversion -Wno-sign-conversion -Wno-error'. 
It may be actually spotting real bugs, there are just too many warnings.

For this particular case, the implicit casts will never change the 
values because the actual values are integers.

AudioHardware.h says kAudioDevicePropertyBufferFrameSizeRange is "an 
AudioValueRange indicating the minimum and maximum values, inclusive, 
for kAudioDevicePropertyBufferFrameSize." 
kAudioDevicePropertyBufferFrameSize is UInt32 so the values should 
always be in the range of UInt32. The number of frames cannot be a 
fractional value after all. They have Float64 values probably because 
Apple were so lazy that they reused the AudioValueRange type that has 
Float64 members.

Regards,
Akihiko Odaki

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.