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

[Seppo Ingalsuo]

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
>