[Sound-open-firmware] [PATCH] SRC: Fix use of int and uint types and remove unused function parameters

[Seppo Ingalsuo]

Sorry, new patch version is needed, please ignore this submission. This 
patch contains a bug that makes this check to fail:

         /* Run as many times as buffers allow */
-       while (((int)source->avail >= need_source) && ((int)sink->free 
 >= need_sink)) {
+       while (((int)(source->avail) >= need_source) && 
((int)(sink->free) >= need_sink)) {

I noticed this from flood of errors into trace.


Thanks,
Seppo


On 27.09.2017 19:02, Seppo Ingalsuo wrote:
>
> On 27.09.2017 18:52, Liam Girdwood wrote:
>> On Wed, 2017-09-27 at 16:49 +0300, Seppo Ingalsuo wrote:
>>> This patch fixes the warning messages shown with gcc option -Wextra.
>>>
>>> Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo at linux.intel.com>
>>> ---
>>>   src/audio/src.c | 16 +++++++---------
>>>   1 file changed, 7 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/src/audio/src.c b/src/audio/src.c
>>> index 3fd9614..3723b4e 100644
>>> --- a/src/audio/src.c
>>> +++ b/src/audio/src.c
>>> @@ -65,8 +65,7 @@ struct comp_data {
>>>       void (*src_func)(struct comp_dev *dev,
>>>           struct comp_buffer *source,
>>>           struct comp_buffer *sink,
>>> -        uint32_t source_frames,
>>> -        uint32_t sink_frames);
>>> +        int source_frames);
>> Should this be unsigned ?
>
> No, it's intentional. The frames count is used for +/- arithmetics ops 
> in the code so it's better to keep it signed integer. With unsigned 
> int type I would need more casts in the code. In this case the lower 
> positive range of int32 is not an issue.
>
> Generally for DSPs the signed integer is natural type to use. 
> Multiplications instructions are better available for signed integer 
> type so why not prefer them instead.
>
> Cheers,
> Seppo
>
>
>
>>
>>>   };
>>>     /* Common mute function for 2s and 1s SRC. This preserves the same
>>> @@ -113,8 +112,7 @@ static void src_muted_s32(struct comp_buffer 
>>> *source, struct comp_buffer *sink,
>>>   static void fallback_s32(struct comp_dev *dev,
>>>       struct comp_buffer *source,
>>>       struct comp_buffer *sink,
>>> -    uint32_t source_frames,
>>> -    uint32_t sink_frames)
>>> +    int source_frames)
>> ditto
>>>   {
>>>         struct comp_data *cd = comp_get_drvdata(dev);
>>> @@ -129,7 +127,7 @@ static void fallback_s32(struct comp_dev *dev,
>>>   /* Normal 2 stage SRC */
>>>   static void src_2s_s32_default(struct comp_dev *dev,
>>>       struct comp_buffer *source, struct comp_buffer *sink,
>>> -    uint32_t source_frames, uint32_t sink_frames)
>>> +    int source_frames)
>>>   {
>>>       int i;
>>>       int j;
>>> @@ -203,7 +201,7 @@ static void src_2s_s32_default(struct comp_dev 
>>> *dev,
>>>   /* 1 stage SRC for simple conversions */
>>>   static void src_1s_s32_default(struct comp_dev *dev,
>>>       struct comp_buffer *source, struct comp_buffer *sink,
>>> -    uint32_t source_frames, uint32_t sink_frames)
>>> +    int source_frames)
>>>   {
>>>       int i;
>>>       int j;
>>> @@ -440,7 +438,7 @@ static int src_params(struct comp_dev *dev)
>>>        * be too long.
>>>        */
>>>       q = need.blk_out / dev->frames;
>>> -    if (q * dev->frames < need.blk_out)
>>> +    if (q * (int)dev->frames < need.blk_out)
>> probably wont need to cast if unsigned too.
>>
>>>           ++q;
>>>         if (q * dev->frames < need.blk_out + dev->frames)
>>> @@ -543,9 +541,9 @@ static int src_copy(struct comp_dev *dev)
>>>       need_sink = blk_out * dev->frame_bytes;
>>>         /* Run as many times as buffers allow */
>>> -    while ((source->avail >= need_source) && (sink->free >= 
>>> need_sink)) {
>>> +    while (((int)source->avail >= need_source) && ((int)sink->free 
>>> >= need_sink)) {
>>>           /* Run src */
>>> -        cd->src_func(dev, source, sink, blk_in, blk_out);
>>> +        cd->src_func(dev, source, sink, blk_in);
>>>             /* calc new free and available  */
>>>           comp_update_buffer_consume(source, 0);
>>
>> ---------------------------------------------------------------------
>> Intel Corporation (UK) Limited
>> Registered No. 1134945 (England)
>> Registered Office: Pipers Way, Swindon SN3 1RJ
>> VAT No: 860 2173 47
>>
>> This e-mail and any attachments may contain confidential material for
>> the sole use of the intended recipient(s). Any review or distribution
>> by others is strictly prohibited. If you are not the intended
>> recipient, please contact the sender and delete all copies.
>> _______________________________________________
>> Sound-open-firmware mailing list
>> Sound-open-firmware at alsa-project.org
>> http://mailman.alsa-project.org/mailman/listinfo/sound-open-firmware
>>
>
> _______________________________________________
> Sound-open-firmware mailing list
> Sound-open-firmware at alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/sound-open-firmware
>