[Spice-devel] [spice-server v1 2/3] clang: Fix possible wrong computation

[Victor Toso]

From: Victor Toso <me at victortoso.com>

Clang's warning about absolute value below is not entirely correct as
the difference between the two bitrate values could be negative but
using abs() could be wrong.

Wrapping between MAX() - MIN() is the simplest option.

> gstreamer-encoder.c:513:17: error: taking the absolute value of unsigned
> type 'unsigned long' has no effect
>     } else  if (abs(bit_rate - encoder->video_bit_rate) > encoder->video_bit_rate * SPICE_GST_VIDEO_BITRATE_MARGIN) {
>                 ^
> gstreamer-encoder.c:513:17: note: remove the call to 'abs' since unsigned
> values cannot be negative
>     } else  if (abs(bit_rate - encoder->video_bit_rate) > encoder->video_bit_rate * SPICE_GST_VIDEO_BITRATE_MARGIN) {
>                 ^~~
                                        1
Signed-off-by: Victor Toso <victortoso at redhat.com>
---
 server/gstreamer-encoder.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/server/gstreamer-encoder.c b/server/gstreamer-encoder.c
index 04f0c02f..869acf11 100644
--- a/server/gstreamer-encoder.c
+++ b/server/gstreamer-encoder.c
@@ -505,12 +505,17 @@ static void set_gstenc_bitrate(SpiceGstEncoder *encoder);
 
 static void set_video_bit_rate(SpiceGstEncoder *encoder, uint64_t bit_rate)
 {
+    uint64_t diff_rate;
+
     if (encoder->video_bit_rate != bit_rate &&
         encoder->gstenc_bitrate_is_dynamic) {
         encoder->video_bit_rate = bit_rate;
         set_gstenc_bitrate(encoder);
+        return;
+    }
 
-    } else  if (abs(bit_rate - encoder->video_bit_rate) > encoder->video_bit_rate * SPICE_GST_VIDEO_BITRATE_MARGIN) {
+    diff_rate = MAX(bit_rate, encoder->video_bit_rate) - MIN(bit_rate, encoder->video_bit_rate);
+    if (diff_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);
     }
-- 
2.15.1