[Spice-commits] 2 commits - src/channel-display-gst.c
GitLab Mirror
gitlab-mirror at kemper.freedesktop.org
Tue Jun 18 07:06:51 UTC 2019
src/channel-display-gst.c | 114 +++++++++++++++++++++++++++-------------------
1 file changed, 68 insertions(+), 46 deletions(-)
New commits:
commit dc23171f8832cccb7254783efbd9fd4f316bfb5f
Author: Francois Gouget <fgouget at codeweavers.com>
Date: Mon Jun 17 19:32:15 2019 +0200
gstreamer: Add the encoded frame's order the statistics
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>
Acked-by: Frediano Ziglio <fziglio at redhat.com>
diff --git a/src/channel-display-gst.c b/src/channel-display-gst.c
index c2b24ea..449b9cb 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 queue_len;
};
static SpiceGstFrame *create_gst_frame(GstBuffer *buffer, SpiceFrame *frame)
@@ -440,11 +441,18 @@ sink_event_probe(GstPad *pad, GstPadProbeInfo *info, gpointer data)
SpiceGstFrame *gstframe = l->data;
const SpiceFrame *frame = gstframe->encoded_frame;
int64_t duration = g_get_monotonic_time() - frame->creation_time;
+ /* Note that queue_len (the length of the queue prior to adding
+ * this frame) 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.
+ */
record(frames_stats,
"frame mm_time %u size %u creation time %" PRId64
- " decoded time %" PRId64 " queue %u",
+ " decoded time %" PRId64 " queue %u before %u",
frame->mm_time, frame->size, frame->creation_time, duration,
- decoder->decoding_queue->length);
+ decoder->decoding_queue->length, gstframe->queue_len);
if (!decoder->appsink) {
/* The sink will display the frame directly so this
@@ -729,6 +737,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->queue_len = decoder->decoding_queue->length;
g_queue_push_tail(decoder->decoding_queue, gst_frame);
g_mutex_unlock(&decoder->queues_mutex);
commit b44b58d6b05bec8d927a18ce6182b9af23a39a3d
Author: Francois Gouget <fgouget at codeweavers.com>
Date: Mon Jun 17 19:31:50 2019 +0200
gstreamer: Improve the statistics collection
By the time schedule_frame() pulls a sample off GStreamer's pipeline the
frame may have been decoded for hundreds of milliseconds, making the
decoding time all but meaningless.
This patch ensures the statistics are always collected by
sink_event_probe() which is called as soon as the decoded frame is
available.
Note that even so the decoding time may be overestimated as the frame
may have been waiting for a while in encoded form for a spot to be freed
in the GStreamer pipeline's sink queue.
Signed-off-by: Francois Gouget <fgouget at codeweavers.com>
Acked-by: Frediano Ziglio <fziglio at redhat.com>
diff --git a/src/channel-display-gst.c b/src/channel-display-gst.c
index 9306462..c2b24ea 100644
--- a/src/channel-display-gst.c
+++ b/src/channel-display-gst.c
@@ -1,6 +1,6 @@
/* -*- Mode: C; c-basic-offset: 4; indent-tabs-mode: nil -*- */
/*
- Copyright (C) 2015-2016 CodeWeavers, Inc
+ Copyright (C) 2015-2016, 2019 CodeWeavers, Inc
This library is free software; you can redistribute it and/or
modify it under the terms of the GNU Lesser General Public
@@ -185,13 +185,16 @@ static gboolean display_frame(gpointer video_decoder)
return G_SOURCE_REMOVE;
}
-/* Get the decoded frame relative to buffer or NULL if not found.
- * Dequeue the frame from decoding_queue and return it, caller
- * is responsible to free the pointer.
+/* Returns the decoding queue entry that matches the specified GStreamer buffer.
+ *
+ * The entry is identified based on the buffer timestamp. However sometimes
+ * GStreamer returns the same buffer twice (and the second time the entry may
+ * have been removed already) or buffers that have a modified, and thus
+ * unrecognizable, timestamp. In such cases NULL is returned.
*
* queues_mutex must be held.
*/
-static SpiceGstFrame *get_decoded_frame(SpiceGstDecoder *decoder, GstBuffer *buffer)
+static GList *find_frame_entry(SpiceGstDecoder *decoder, GstBuffer *buffer)
{
GstClockTime buffer_ts = GST_BUFFER_PTS(buffer);
#if GST_CHECK_VERSION(1,14,0)
@@ -203,49 +206,34 @@ static SpiceGstFrame *get_decoded_frame(SpiceGstDecoder *decoder, GstBuffer *buf
}
#endif
- /* Gstreamer sometimes returns the same buffer twice
- * or buffers that have a modified, and thus unrecognizable, PTS.
- * Blindly removing frames from the decoding_queue until we find a
- * match would only empty the queue, resulting in later buffers not
- * finding a match either, etc. So check the buffer has a matching
- * frame first.
- */
- SpiceGstFrame *gstframe = NULL;
GList *l = g_queue_peek_head_link(decoder->decoding_queue);
while (l) {
- gstframe = l->data;
+ const SpiceGstFrame *gstframe = l->data;
if (gstframe->timestamp == buffer_ts) {
- break;
+ return l;
}
- gstframe = NULL;
l = l->next;
}
- if (gstframe != NULL) {
- /* Now that we know there is a match, remove it and the older
- * frames from the decoding queue */
- SpiceGstFrame *late_frame;
- guint num_frames_dropped = 0;
-
- /* The GStreamer pipeline dropped the corresponding buffer. */
- while ((late_frame = g_queue_pop_head(decoder->decoding_queue)) != gstframe) {
- num_frames_dropped++;
- free_gst_frame(late_frame);
- }
+ return NULL;
+}
- if (num_frames_dropped != 0) {
- SPICE_DEBUG("the GStreamer pipeline dropped %u frames", num_frames_dropped);
- }
+/* Pops the queued frames up to and including the specified frame.
+ * All frames are freed except that last frame which belongs to the caller.
+ * Returns the number of freed frames.
+ *
+ * queues_mutex must be held.
+ */
+static guint32 pop_up_to_frame(SpiceGstDecoder *decoder, const SpiceGstFrame *popframe)
+{
+ SpiceGstFrame *gstframe;
+ guint32 freed = 0;
- const SpiceFrame *frame = gstframe->encoded_frame;
- 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",
- frame->mm_time, frame->size, frame->creation_time,
- duration, decoder->decoding_queue->length);
+ while ((gstframe = g_queue_pop_head(decoder->decoding_queue)) != popframe) {
+ free_gst_frame(gstframe);
+ freed++;
}
- return gstframe;
+ return freed;
}
/* Helper for schedule_frame().
@@ -268,8 +256,16 @@ static void fetch_pending_sample(SpiceGstDecoder *decoder)
* finding a match either, etc. So check the buffer has a matching
* frame first.
*/
- SpiceGstFrame *gstframe = get_decoded_frame(decoder, buffer);
- if (gstframe) {
+ GList *l = find_frame_entry(decoder, buffer);
+ if (l) {
+ SpiceGstFrame *gstframe = l->data;
+
+ /* Dequeue this and any dropped frames */
+ guint32 dropped = pop_up_to_frame(decoder, gstframe);
+ if (dropped) {
+ SPICE_DEBUG("the GStreamer pipeline dropped %u frames", dropped);
+ }
+
/* The frame is now ready for display */
gstframe->decoded_sample = sample;
decoder->display_frame = gstframe;
@@ -438,10 +434,28 @@ sink_event_probe(GstPad *pad, GstPadProbeInfo *info, gpointer data)
if (info->type & GST_PAD_PROBE_TYPE_BUFFER) { // Buffer arrived
GstBuffer *buffer = GST_PAD_PROBE_INFO_BUFFER(info);
g_mutex_lock(&decoder->queues_mutex);
- SpiceGstFrame *gstframe = get_decoded_frame(decoder, buffer);
- if (gstframe) {
- free_gst_frame(gstframe);
+
+ GList *l = find_frame_entry(decoder, buffer);
+ if (l) {
+ SpiceGstFrame *gstframe = l->data;
+ const SpiceFrame *frame = gstframe->encoded_frame;
+ 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",
+ frame->mm_time, frame->size, frame->creation_time, duration,
+ decoder->decoding_queue->length);
+
+ if (!decoder->appsink) {
+ /* The sink will display the frame directly so this
+ * SpiceGstFrame and those of any dropped frame are no longer
+ * needed.
+ */
+ pop_up_to_frame(decoder, gstframe);
+ free_gst_frame(gstframe);
+ }
}
+
g_mutex_unlock(&decoder->queues_mutex);
}
return GST_PAD_PROBE_OK;
@@ -452,9 +466,8 @@ static void
deep_element_added_cb(GstBin *pipeline, GstBin *bin, GstElement *element,
SpiceGstDecoder *decoder)
{
- /* attach a probe to the sink in case we don't have appsink (that
- * is the frames does not go to new_sample */
- if (GST_IS_BASE_SINK(element) && decoder->appsink == NULL) {
+ /* Attach a probe to the sink to update the statistics */
+ if (GST_IS_BASE_SINK(element)) {
GstPad *pad = gst_element_get_static_pad(element, "sink");
gst_pad_add_probe(pad, GST_PAD_PROBE_TYPE_BUFFER, sink_event_probe, decoder, NULL);
gst_object_unref(pad);
More information about the Spice-commits
mailing list