[Spice-devel] [client 5/5] gstreamer: Add the encoded frame's rank to the statistics

[Frediano Ziglio]

> 
> The number of frames that were sitting in the decoding_queue before the
> current frame was added is crucial to correctly interpret the decoding
> time:
> * Less than MAX_DECODED_FRAMES means nothing blocked the decoding of
>   that frame.
> * More than MAX_DECODED_FRAMES means decoding was delayed by one or more
>   frame intervals.
> 
> Signed-off-by: Francois Gouget <fgouget at codeweavers.com>
> ---
>  src/channel-display-gst.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/src/channel-display-gst.c b/src/channel-display-gst.c
> index fc338dff..b8f0c2ee 100644
> --- a/src/channel-display-gst.c
> +++ b/src/channel-display-gst.c
> @@ -90,6 +90,7 @@ struct SpiceGstFrame {
>      GstBuffer *encoded_buffer;
>      SpiceFrame *encoded_frame;
>      GstSample *decoded_sample;
> +    guint rank;

Maybe put some comment in the code and not only on commit message?

Honestly "rank" does not sound that great name to me, I'm not an English native
speaker and I had a look at a dictionary for potential other meanings but
I didn't find one were this "rank" sounds right.

>  };
>  
>  static SpiceGstFrame *create_gst_frame(GstBuffer *buffer, SpiceFrame *frame)
> @@ -442,9 +443,9 @@ sink_event_probe(GstPad *pad, GstPadProbeInfo *info,
> gpointer data)
>              int64_t duration = g_get_monotonic_time() -
>              frame->creation_time;
>              record(frames_stats,
>                     "frame mm_time %u size %u creation time %" PRId64
> -                   " decoded time %" PRId64 " queue %u",
> +                   " decoded time %" PRId64 " rank %u queue %u",

Why not adding to the end? I'm asking because I have a tool which uses
that output. Is it a problem adding that to the end?

>                     frame->mm_time, frame->size, frame->creation_time,
>                     duration,
> -                   g_queue_get_length(decoder->decoding_queue));
> +                   gstframe->rank,
> g_queue_get_length(decoder->decoding_queue));
>  
>              if (!decoder->appsink) {
>                  /* The sink will display the frame directly so this
> @@ -729,6 +730,7 @@ static gboolean
> spice_gst_decoder_queue_frame(VideoDecoder *video_decoder,
>  
>      SpiceGstFrame *gst_frame = create_gst_frame(buffer, frame);
>      g_mutex_lock(&decoder->queues_mutex);
> +    gst_frame->rank = g_queue_get_length(decoder->decoding_queue);
>      g_queue_push_tail(decoder->decoding_queue, gst_frame);
>      g_mutex_unlock(&decoder->queues_mutex);
>  

Frediano