[Spice-devel] [spice-gtk v1 10/11] channel-display-gst: summarize number of frames dropped

[Frediano Ziglio]

> 
> From: Victor Toso <me at victortoso.com>
> 
> For example, this has produced 9 lines of debug below instead of 31.
> 
> GSpice-DEBUG: channel-display-gst.c:247 the GStreamer pipeline dropped 4
> frames
> GSpice-DEBUG: channel-display-gst.c:247 the GStreamer pipeline dropped 5
> frames
> GSpice-DEBUG: channel-display-gst.c:247 the GStreamer pipeline dropped 4
> frames
> GSpice-DEBUG: channel-display-gst.c:247 the GStreamer pipeline dropped 4
> frames
> GSpice-DEBUG: channel-display-gst.c:247 the GStreamer pipeline dropped 4
> frames
> GSpice-DEBUG: channel-display-gst.c:247 the GStreamer pipeline dropped 4
> frames
> GSpice-DEBUG: channel-display-gst.c:247 the GStreamer pipeline dropped 3
> frames
> GSpice-DEBUG: channel-display-gst.c:247 the GStreamer pipeline dropped 2
> frames
> GSpice-DEBUG: channel-display-gst.c:247 the GStreamer pipeline dropped 1
> frames
> 
> Signed-off-by: Victor Toso <victortoso at redhat.com>
> ---
>  src/channel-display-gst.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/src/channel-display-gst.c b/src/channel-display-gst.c
> index d2847ec..72b5a16 100644
> --- a/src/channel-display-gst.c
> +++ b/src/channel-display-gst.c
> @@ -207,6 +207,7 @@ static GstFlowReturn new_sample(GstAppSink *gstappsink,
> gpointer video_decoder)
>      GstSample *sample = gst_app_sink_pull_sample(decoder->appsink);
>      if (sample) {
>          GstBuffer *buffer = gst_sample_get_buffer(sample);
> +        guint num_frames_dropped = 0;
>          g_mutex_lock(&decoder->queues_mutex);
>  
>          /* gst_app_sink_pull_sample() sometimes returns the same buffer
>          twice
> @@ -235,13 +236,16 @@ static GstFlowReturn new_sample(GstAppSink *gstappsink,
> gpointer video_decoder)
>                      /* The GStreamer pipeline dropped the corresponding
>                       * buffer.
>                       */
> -                    SPICE_DEBUG("the GStreamer pipeline dropped a frame");
> +                    num_frames_dropped++;
>                      free_gst_frame(gstframe);
>                  }
>                  break;
>              }
>              l = l->next;
>          }
> +        if (num_frames_dropped != 0) {
> +            SPICE_DEBUG("the GStreamer pipeline dropped %u frames",
> num_frames_dropped);
> +        }
>          if (!l) {
>              spice_warning("got an unexpected decoded buffer!");
>              gst_sample_unref(sample);

I think is more a bug (or lack) of protocol specification!

The protocol state (comments included):

    message {
        StreamDataHeader base;
        uint32 data_size;
        uint8 data[data_size] @end @nomarshal;
    } stream_data;

now... where is the "frame" ?? The client is expecting the "stream_data" to contain
a full frame information and calls the stream_data message "frame" and report the
problem. The server is currently splitting the byte stream into multiple chunks
which is fine from the protocol specification and the client is complaining.
I think we should start putting more efforts on protocol specification and
avoid to having hidden assumption all over!

I would propose to just drop the SPICE_DEBUG line.

Frediano