[Spice-devel] [spice] gstreamer-encoder: Return the average frame size as a 32 bit int

Frediano Ziglio fziglio at redhat.com
Thu May 16 07:20:40 UTC 2019


> 
> It makes no sense to expect average frame sizes anywhere close to 2GB.
> 
> Signed-off-by: Francois Gouget <fgouget at codeweavers.com>

Sure but 256 kb are possible.

> ---
>  server/gstreamer-encoder.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/server/gstreamer-encoder.c b/server/gstreamer-encoder.c
> index e319eea22..6130781da 100644
> --- a/server/gstreamer-encoder.c
> +++ b/server/gstreamer-encoder.c
> @@ -396,7 +396,7 @@ static uint64_t get_average_encoding_time(SpiceGstEncoder
> *encoder)
>      return encoder->stat_duration_sum / count;
>  }
>  
> -static uint64_t get_average_frame_size(SpiceGstEncoder *encoder)
> +static uint32_t get_average_frame_size(SpiceGstEncoder *encoder)
>  {
>      uint32_t count = encoder->history_last +
>          (encoder->history_last < encoder->stat_first ?
>          SPICE_GST_HISTORY_SIZE : 0) -
> @@ -520,7 +520,7 @@ static uint32_t get_min_playback_delay(SpiceGstEncoder
> *encoder)
>       * an I frame) and an average frame. This also takes into account the
>       * frames dropped by the encoder bit rate control.
>       */
> -    uint64_t size = get_maximum_frame_size(encoder) +
> get_average_frame_size(encoder);
> +    uint32_t size = get_maximum_frame_size(encoder) +
> get_average_frame_size(encoder);
>      uint32_t send_time = MSEC_PER_SEC * size * 8 / encoder->bit_rate;
>  

Here you have 8000 * 2 * frame_size so could overflow uint32_t with
frame_size >= ~256kb.
I agree get_average_frame_size can safely returns uint32_t but you should
change above line to

   uint32_t send_time = (uint32_t) ((uint64_t) (MSEC_PER_SEC * 8) * size / encoder->bit_rate);

or leave size uint64_t.

>      /* Also factor in the network latency with a margin for jitter. */

Frediano


More information about the Spice-devel mailing list