[Spice-devel] [PATCH] gstreamer-encoder: fix compiler warning with Fedora 30

[Uri Lublin]

On 7/1/19 6:19 PM, Frediano Ziglio wrote:
>>
>> Fedora 30 / gcc 9.1.1 20190503 (Red Hat 9.1.1-1) fails to build
>> because of this error/warning:
>>
>>> gstreamer-encoder.c: In function 'set_video_bit_rate':
>>> gstreamer-encoder.c:518:17: error: taking the absolute value of unsigned
>>> type 'uint64_t' {aka 'long unsigned int'} has no effect
>>> [-Werror=absolute-value]
>>>   518 |     } else  if (abs(bit_rate - encoder->video_bit_rate) >
>>>   encoder->video_bit_rate * SPICE_GST_VIDEO_BITRATE_MARGIN) {
>>>       |                 ^~~
>>> gstreamer-encoder.c:518:17: error: absolute value function 'abs' given an
>>> argument of type 'uint64_t' {aka 'long unsigned int'}
>>
>> This patches solves these two warnings:
>>
>> 1) cast the substraction to a signed type (int64_t instead of
>> uint64_t) to preserve the operation meaning;
>>
>> 2) use the long int labs() instead of the int version abs() to avoid
>> data trunction.
> 
> Not working for all platform we support, maybe a
> 
> static inline int64_t i64abs(int64_t value)
> {
>      if (sizeof(int) == sizeof(value)) {
>          return (int64_t) abs((int) value);
>      }
>      if (sizeof(long int) == sizeof(value)) {
>          return (int64_t) labs((long int) value);
>      }
>      return (int64_t) llabs((long long int) value);
> }

Or a simpler
     return (value >= 0) ? value : -value;

Uri.

> 
>> ---
>>
>> resending this patch with the comments addressed
>>
>> ---
>>   server/gstreamer-encoder.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/server/gstreamer-encoder.c b/server/gstreamer-encoder.c
>> index 6416b688..da73c5ee 100644
>> --- a/server/gstreamer-encoder.c
>> +++ b/server/gstreamer-encoder.c
>> @@ -515,7 +515,7 @@ static void set_video_bit_rate(SpiceGstEncoder *encoder,
>> uint64_t bit_rate)
>>           encoder->video_bit_rate = bit_rate;
>>           set_gstenc_bitrate(encoder);
>>
>> -    } else  if (abs(bit_rate - encoder->video_bit_rate) >
>> encoder->video_bit_rate * SPICE_GST_VIDEO_BITRATE_MARGIN) {
>> +    } else  if (labs((int64_t)(bit_rate - encoder->video_bit_rate)) >
>> encoder->video_bit_rate * SPICE_GST_VIDEO_BITRATE_MARGIN) {
>>           encoder->video_bit_rate = bit_rate;
>>           set_pipeline_changes(encoder, SPICE_GST_VIDEO_PIPELINE_BITRATE);
>>       }
> 
> Frediano
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
>