[Spice-devel] [client] gstreamer: Fix the decoding time when not using GstVideoOverlay
Francois Gouget
fgouget at codeweavers.com
Tue Jun 11 18:42:44 UTC 2019
schedule_frame() only pulls frames out of GStreamer's pipeline once all
previous decoded frames have been displayed. Thus when the video delay
is high a decoded frame may have to wait for several frame intervals
before get_decoded_frame() looks at it and computes its decoding time.
So attach decoded frame samples to the corresponding decoding_queue
entry as soon as new_sample() is called, and compute the decoding time
and associated statistics then. Similarly, match sink_event_probe()'s
new buffer to a decoding_queue entry and update the statistics in the
process. This ensures that there is no extra delay in either case.
Signed-off-by: Francois Gouget <fgouget at codeweavers.com>
---
src/channel-display-gst.c | 183 ++++++++++++++++++++------------------
1 file changed, 96 insertions(+), 87 deletions(-)
diff --git a/src/channel-display-gst.c b/src/channel-display-gst.c
index 91ece0fa..fed73592 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
@@ -54,9 +54,9 @@ typedef struct SpiceGstDecoder {
GMutex queues_mutex;
GQueue *decoding_queue;
+ guint decoded_frames;
SpiceGstFrame *display_frame;
guint timer_id;
- guint pending_samples;
} SpiceGstDecoder;
#define VALID_VIDEO_CODEC_TYPE(codec) \
@@ -120,8 +120,6 @@ static void free_gst_frame(SpiceGstFrame *gstframe)
/* ---------- GStreamer pipeline ---------- */
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");
@@ -191,21 +189,26 @@ static void schedule_frame(SpiceGstDecoder *decoder)
g_mutex_lock(&decoder->queues_mutex);
while (!decoder->timer_id) {
- while (decoder->display_frame == NULL && decoder->pending_samples) {
- fetch_pending_sample(decoder);
+ GList *head = g_queue_peek_head_link(decoder->decoding_queue);
+ if (!head) {
+ /* No frame in the queue */
+ break;
}
-
- SpiceGstFrame *gstframe = decoder->display_frame;
- if (!gstframe) {
+ SpiceGstFrame *gstframe = head->data;
+ if (!gstframe->decoded_sample) {
+ /* This frame has not been decoded yet */
break;
}
+ g_queue_pop_head(decoder->decoding_queue);
+ decoder->display_frame = gstframe;
+ decoder->decoded_frames--;
if (spice_mmtime_diff(gstframe->encoded_frame->mm_time, now) >= 0) {
decoder->timer_id = g_timeout_add(gstframe->encoded_frame->mm_time - now,
display_frame, decoder);
- } else if (decoder->display_frame && !decoder->pending_samples) {
- /* Still attempt to display the least out of date frame so the
- * video is not completely frozen for an extended period of time.
+ } else if (decoder->display_frame && decoder->decoded_frames == 0) {
+ /* This is the least out of date frame. Display it since that's
+ * better than having frozen video for an extended period of time.
*/
decoder->timer_id = g_timeout_add(0, display_frame, decoder);
} else {
@@ -221,12 +224,16 @@ static void schedule_frame(SpiceGstDecoder *decoder)
g_mutex_unlock(&decoder->queues_mutex);
}
-/* 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_decoded_entry(SpiceGstDecoder *decoder, GstBuffer *buffer)
{
GstClockTime buffer_ts = GST_BUFFER_PTS(buffer);
#if GST_CHECK_VERSION(1,14,0)
@@ -238,81 +245,71 @@ 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;
+ SpiceGstFrame *gstframe = l->data;
if (gstframe->timestamp == buffer_ts) {
- break;
+ /* Update the statistics */
+ 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);
+ 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);
- }
-
- if (num_frames_dropped != 0) {
- SPICE_DEBUG("the GStreamer pipeline dropped %u frames", num_frames_dropped);
- }
-
- 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);
- }
- return gstframe;
+ return NULL;
}
-static void fetch_pending_sample(SpiceGstDecoder *decoder)
+/* Attaches the specified GStreamer sample to the corresponding SpiceGstFrame
+ * in the decoding queue and returns TRUE. Returns FALSE if no match was found.
+ *
+ * This also removes any older frame that have not been decoded yet as
+ * it means GStreamer dropped them. This ensures the decoded frames form a
+ * contiguous block at the head of the decoding queue.
+ *
+ * queues_mutex must be held.
+ */
+static gboolean attach_decoded_sample(SpiceGstDecoder *decoder, GstSample *sample)
{
- GstSample *sample = gst_app_sink_pull_sample(decoder->appsink);
- if (sample) {
- // account for the fetched sample
- decoder->pending_samples--;
+ GstBuffer *buffer = gst_sample_get_buffer(sample);
+ GList *l = find_decoded_entry(decoder, buffer);
+ if (l == NULL) {
+ return FALSE;
+ }
- GstBuffer *buffer = gst_sample_get_buffer(sample);
+ SpiceGstFrame *gstframe = l->data;
+ gstframe->decoded_sample = sample;
+ decoder->decoded_frames++;
- /* gst_app_sink_pull_sample() 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 = get_decoded_frame(decoder, buffer);
- if (gstframe) {
- /* The frame is now ready for display */
- gstframe->decoded_sample = sample;
- decoder->display_frame = gstframe;
- } else {
- spice_warning("got an unexpected decoded buffer!");
- gst_sample_unref(sample);
+ /* Any older undecoded frame must have been dropped by the GStreamer
+ * pipeline so there is no point in keeping it around.
+ */
+ guint num_frames_dropped = 0;
+ l = l->prev;
+ while (l) {
+ gstframe = l->data;
+ if (gstframe->decoded_sample) {
+ /* It's only decoded frames from there to the first one */
+ break;
}
- } else {
- // no more samples to get, possibly some sample was dropped
- decoder->pending_samples = 0;
- spice_warning("GStreamer error: could not pull sample");
+
+ num_frames_dropped++;
+ free_gst_frame(gstframe);
+ GList *p = l->prev;
+ g_queue_delete_link(decoder->decoding_queue, l);
+
+ l = p;
}
+ if (num_frames_dropped) {
+ SPICE_DEBUG("the GStreamer pipeline dropped %u frames", num_frames_dropped);
+ }
+
+ return TRUE;
}
/* GStreamer thread
@@ -327,15 +324,18 @@ static GstFlowReturn new_sample(GstAppSink *gstappsink, gpointer video_decoder)
{
SpiceGstDecoder *decoder = video_decoder;
- g_mutex_lock(&decoder->queues_mutex);
- decoder->pending_samples++;
- if (decoder->timer_id && decoder->display_frame) {
+ GstSample *sample = gst_app_sink_pull_sample(decoder->appsink);
+ if (sample) {
+ g_mutex_lock(&decoder->queues_mutex);
+ if (!attach_decoded_sample(decoder, sample) || decoder->timer_id) {
+ g_mutex_unlock(&decoder->queues_mutex);
+ return GST_FLOW_OK;
+
+ }
g_mutex_unlock(&decoder->queues_mutex);
- return GST_FLOW_OK;
- }
- g_mutex_unlock(&decoder->queues_mutex);
- schedule_frame(decoder);
+ schedule_frame(decoder);
+ }
return GST_FLOW_OK;
}
@@ -429,10 +429,19 @@ 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);
+
+ /* As a side-effect this updates the decoder statistics */
+ GList *l = find_decoded_entry(decoder, buffer);
+
+ /* Drop all entries up to this one */
+ while (l) {
+ free_gst_frame((SpiceGstFrame*)l->data);
+
+ GList *p = l->prev;
+ g_queue_delete_link(decoder->decoding_queue, l);
+ l = p;
}
+
g_mutex_unlock(&decoder->queues_mutex);
}
return GST_PAD_PROBE_OK;
--
2.20.1
More information about the Spice-devel
mailing list