[Spice-devel] [PATCH] gstreamer: Fix infinite loop in get_period_bit_rate

[Uri Lublin]

Hi Frediano,

On 08/02/2016 03:45 PM, Frediano Ziglio wrote:
> This was discovered by change by me and Uri trying some remote connection

s/change/chance/

> with slow network (8Mbit) and high latency

Mostly the high latency

>
>   $ ping 10.10.48.87 -c 3
>   3 packets transmitted, 3 received, 0% packet loss, time 2002ms
>   rtt min/avg/max/mdev = 281.069/316.758/374.413/41.153 ms
>

[gdb output removed]

>
> You can see that encoder->history[encoder->history_first].mm_time == to
> cause the check index == encoder->history_first to not be executed.
>
> This code change was suggested by Uri.
>
> Reported-by: Uri Lublin <uril at redhat.com>
> Signed-off-by: Frediano Ziglio <fziglio at redhat.com>

Now, also (lightly) tested (playing etr on guest).
(lightly= Before: 1/1 infinite loop
           After:  0/1 infinite loop)

> ---
>  server/gstreamer-encoder.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/server/gstreamer-encoder.c b/server/gstreamer-encoder.c
> index 396687d..75eb06e 100644
> --- a/server/gstreamer-encoder.c
> +++ b/server/gstreamer-encoder.c
> @@ -434,19 +434,19 @@ static uint64_t get_period_bit_rate(SpiceGstEncoder *encoder, uint32_t from,
>              sum += encoder->history[index].size;
>              return (sum - 1) * 8 * MSEC_PER_SEC / (last_mm_time - from);
>
> -        } else if (index == encoder->history_first) {
> +        } else if (sum > 0) {
> +            sum += encoder->history[index].size;
> +
> +        } else {
> +            last_mm_time = encoder->history[index].mm_time;
> +        }

Please add an empty line here.

Thanks,
     Uri.

> +        if (index == encoder->history_first) {
>              /* This period is outside the recorded history */
>              spice_debug("period (%u-%u) outside known history (%u-%u)",
>                          from, to,
>                          encoder->history[encoder->history_first].mm_time,
>                          encoder->history[encoder->history_last].mm_time);
>             return 0;
> -
> -        } else if (sum > 0) {
> -            sum += encoder->history[index].size;
> -
> -        } else {
> -            last_mm_time = encoder->history[index].mm_time;
>          }
>          index = (index ? index : SPICE_GST_HISTORY_SIZE) - 1;
>      }
>