[Spice-devel] [PATCH spice-gtk v2] channel-display-gst: Use recorder for frame statistics

[Marc-André Lureau]

Hi

On Tue, Feb 19, 2019 at 5:19 PM Frediano Ziglio <fziglio at redhat.com> wrote:
>
> >
> > Hi
> >
> > On Wed, Jan 30, 2019 at 4:00 PM Frediano Ziglio <fziglio at redhat.com> wrote:
> > >
> > > Allows to handle these statistics in a more flexible way.
> > >
> > > Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> > > ---
> > >  src/channel-display-gst.c | 12 ++++++++----
> > >  1 file changed, 8 insertions(+), 4 deletions(-)
> > >
> > > Changes since v1:
> > > - change formatting constants to system one instead of GLib
> > >   to fix build on Windows.
> > >
> > > diff --git a/src/channel-display-gst.c b/src/channel-display-gst.c
> > > index 4272ade8..5bb53b6f 100644
> > > --- a/src/channel-display-gst.c
> > > +++ b/src/channel-display-gst.c
> > > @@ -20,6 +20,7 @@
> > >  #include "spice-client.h"
> > >  #include "spice-common.h"
> > >  #include "spice-channel-priv.h"
> > > +#include "common/recorder.h"
> > >
> > >  #include "channel-display-priv.h"
> > >
> > > @@ -109,6 +110,8 @@ static void schedule_frame(SpiceGstDecoder *decoder);
> > >  static void fetch_pending_sample(SpiceGstDecoder *decoder);
> > >  static SpiceGstFrame *get_decoded_frame(SpiceGstDecoder *decoder,
> > >  GstBuffer *buffer);
> > >
> > > +RECORDER(frames_stats, 64, "Frames statistics");
> > > +
> >
> > What is this 64 value for?
> >
>
> It's the ring size.

Ok, but why 64?

>
> > >  static int spice_gst_buffer_get_stride(GstBuffer *buffer)
> > >  {
> > >      GstVideoMeta *video = gst_buffer_get_video_meta(buffer);
> > > @@ -248,10 +251,11 @@ static SpiceGstFrame
> > > *get_decoded_frame(SpiceGstDecoder *decoder, GstBuffer *buf
> > >
> > >          const SpiceFrame *frame = gstframe->encoded_frame;
> > >          int64_t duration = g_get_monotonic_time() - frame->creation_time;
> > > -        SPICE_DEBUG("frame mm_time %u size %u creation time %"
> > > G_GINT64_FORMAT
> > > -                    " decoded time %" G_GINT64_FORMAT " queue %u",
> > > -                    frame->mm_time, frame->size, frame->creation_time,
> > > -                    duration, decoder->decoding_queue->length);
> > > +        record(frames_stats,
> > > +               "frame mm_time %u size %u creation time %" PRId64
> > > +               " decoded time %" PRId64 " queue %u",
> > > +               frame->mm_time, frame->size, frame->creation_time,
> > > +               duration, decoder->decoding_queue->length);
> >
> > Why SPICE_DEBUG log is removed?
> >
> > Why is the "recorder" stuff necessary here?
> >
>
> Measurement instrumentation

Ok, but can we get have the regular log as well?

>
> > >      }
> > >      return gstframe;
> > >  }
>
> Frediano



-- 
Marc-André Lureau