[Spice-commits] 2 commits - src/channel-display-gst.c

GitLab Mirror gitlab-mirror at kemper.freedesktop.org
Tue Jun 18 06:35:27 UTC 2019


 src/channel-display-gst.c |  144 +++++++++++++++++++++++++---------------------
 1 file changed, 80 insertions(+), 64 deletions(-)

New commits:
commit 20f576932e2974365fe8a7266ff8426895f6dafa
Author: Francois Gouget <fgouget at codeweavers.com>
Date:   Mon Jun 17 19:32:03 2019 +0200

    gstreamer: Fix the spice_gst_decoder_queue_frame() documentation
    
    Take into account changes made in 8835e757922c, 8c2101254051 and
    possibly other other commits.
    Add MAX_DECODED_FRAMES to define how many decoded frames GStreamer
    should 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 ec8a7d3..9306462 100644
--- a/src/channel-display-gst.c
+++ b/src/channel-display-gst.c
@@ -62,6 +62,9 @@ typedef struct SpiceGstDecoder {
 #define VALID_VIDEO_CODEC_TYPE(codec) \
     (codec > 0 && codec < G_N_ELEMENTS(gst_opts))
 
+/* Decoded frames are big so limit how many are queued by GStreamer */
+#define MAX_DECODED_FRAMES 2
+
 /* GstPlayFlags enum is in plugin's header which should not be exported.
  * https://bugzilla.gnome.org/show_bug.cgi?id=784279
  */
@@ -320,11 +323,14 @@ static void schedule_frame(SpiceGstDecoder *decoder)
 
 /* GStreamer thread
  *
- * We cannot use GStreamer's signals because they are not always run in
- * the main context. So use a callback (lower overhead) and have it pull
- * the sample to avoid a race with free_pipeline(). This means queuing the
- * decoded frames outside GStreamer. So while we're at it, also schedule
- * the frame display ourselves in schedule_frame().
+ * Decoded frames are big so we rely on GStreamer to limit how many are
+ * buffered (see MAX_DECODED_FRAMES). This means we must not pull the samples
+ * as soon as they become available. Instead just increment pending_samples so
+ * schedule_frame() knows whether it can pull a new sample when it needs one.
+ *
+ * Note that GStreamer's signals are not always run in the main context, hence
+ * the schedule_frame() + display_frame() mechanism. So we might as well use
+ * a callback here (lower overhead).
  */
 static GstFlowReturn new_sample(GstAppSink *gstappsink, gpointer video_decoder)
 {
@@ -535,7 +541,7 @@ static gboolean create_pipeline(SpiceGstDecoder *decoder)
         GstAppSinkCallbacks appsink_cbs = { NULL };
         appsink_cbs.new_sample = new_sample;
         gst_app_sink_set_callbacks(decoder->appsink, &appsink_cbs, decoder, NULL);
-        gst_app_sink_set_max_buffers(decoder->appsink, 2);
+        gst_app_sink_set_max_buffers(decoder->appsink, MAX_DECODED_FRAMES);
         gst_app_sink_set_drop(decoder->appsink, FALSE);
     }
     bus = gst_pipeline_get_bus(GST_PIPELINE(decoder->pipeline));
@@ -612,37 +618,44 @@ static void spice_gst_decoder_destroy(VideoDecoder *video_decoder)
  *
  * 1) frame->data, which contains the compressed frame data, is wrapped in a GstBuffer
  *    (encoded_buffer) which owns the SpiceFrame.
- * 2) A SpiceGstFrame is created to keep track of SpiceFrame (encoded_frame), and some
- *    additional metadata. The encoded_buffer is referenced and the SpiceGstFrame is then
- *    pushed into the decoding_queue.
+ * 2) A SpiceGstFrame is created to keep track of SpiceFrame (encoded_frame),
+ *    and additional metadata among which GStreamer's encoded_buffer the
+ *    refcount of which is incremented. The SpiceGstFrame is then pushed into
+ *    the decoding_queue.
  *
  * If GstVideoOverlay is used (window handle was obtained successfully at the widget):
  *   3) Decompressed frames will be rendered to widget directly from GStreamer's pipeline
  *      using some GStreamer sink plugin which implements the GstVideoOverlay interface
  *      (last step).
  *   4) As soon as GStreamer's pipeline no longer needs the compressed frame it will
- *      unref the encoded_buffer
- *   5) Once a decoded buffer arrives to the sink (notified by probe event) we will pop
+ *      unref the encoded_buffer.
+ *   5) Once a decoded buffer arrives to the sink sink_event_probe() will pop
  *      its matching SpiceGstFrame from the decoding_queue and free it using
- *      free_gst_frame, this will also unref the encoded_buffer which will allow
- *      GStreamer to call spice_frame_free and free its encoded_frame.
+ *      free_gst_frame(). This will also unref the encoded_buffer which will
+ *      allow GStreamer to call spice_frame_free() and free its encoded_frame.
  *
  * Otherwise appsink is used:
  *   3) Once the decompressed frame is available the GStreamer pipeline calls
  *      new_sample() in the GStreamer thread.
- *   4) new_sample() then matches the decompressed frame to a SpiceGstFrame from
- *      the decoding queue using the GStreamer timestamp information to deal with
- *      dropped frames. The SpiceGstFrame is popped from the decoding_queue.
- *   5) new_sample() then attaches the decompressed frame to the SpiceGstFrame,
- *      set into display_frame and calls schedule_frame().
- *   6) schedule_frame() then uses gstframe->encoded_frame->mm_time to arrange for
- *      display_frame() to be called, in the main thread, at the right time for
- *      the next frame.
- *   7) display_frame() uses SpiceGstFrame from display_frame and calls
+ *   4) new_sample() then increments the pending_samples count and calls
+ *      schedule_frame().
+ *   5) schedule_frame() is called whenever a new frame might need to be
+ *      displayed. If that is the case and pending_samples is non-zero it calls
+ *      fetch_pending_sample().
+ *   6) fetch_pending_sample() grabs GStreamer's latest sample and then calls
+ *      get_decoded_frame() which compares the GStreamer's buffer timestamp to
+ *      gstframe->encoded_frame->mm_time to match it with a decoding_queue
+ *      entry.
+ *   7) fetch_pending_sample() then attaches the sample to the SpiceGstFrame,
+ *      and sets display_frame.
+ *   8) schedule_frame() then uses display_frame->encoded_frame->mm_time to
+ *      arrange for display_frame() to be called, in the main thread, at the
+ *      right time.
+ *   9) display_frame() uses SpiceGstFrame from display_frame and calls
  *      stream_display_frame().
- *   8) display_frame() then calls to free_gst_frame to free the SpiceGstFrame and
- *      unref the encoded_buffer which allows GStreamer to call spice_frame_free
- *      and free its encoded_frame.
+ *  10) display_frame() then calls free_gst_frame() to free the SpiceGstFrame
+ *      and unref the encoded_buffer which allows GStreamer to call
+ *      spice_frame_free() and free its encoded_frame.
  */
 static gboolean spice_gst_decoder_queue_frame(VideoDecoder *video_decoder,
                                               SpiceFrame *frame, int latency)
commit 70fee133772d445b39f8dfdce35c850bd1aebf09
Author: Francois Gouget <fgouget at codeweavers.com>
Date:   Mon Jun 17 19:31:27 2019 +0200

    gstreamer: Avoid a couple of forward function declarations
    
    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 91ece0f..ec8a7d3 100644
--- a/src/channel-display-gst.c
+++ b/src/channel-display-gst.c
@@ -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");
 
@@ -184,46 +182,10 @@ static gboolean display_frame(gpointer video_decoder)
     return G_SOURCE_REMOVE;
 }
 
-/* main loop or GStreamer streaming thread */
-static void schedule_frame(SpiceGstDecoder *decoder)
-{
-    guint32 now = stream_get_time(decoder->base.stream);
-    g_mutex_lock(&decoder->queues_mutex);
-
-    while (!decoder->timer_id) {
-        while (decoder->display_frame == NULL && decoder->pending_samples) {
-            fetch_pending_sample(decoder);
-        }
-
-        SpiceGstFrame *gstframe = decoder->display_frame;
-        if (!gstframe) {
-            break;
-        }
-
-        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.
-             */
-            decoder->timer_id = g_timeout_add(0, display_frame, decoder);
-        } else {
-            SPICE_DEBUG("%s: rendering too late by %u ms (ts: %u, mmtime: %u), dropping",
-                        __FUNCTION__, now - gstframe->encoded_frame->mm_time,
-                        gstframe->encoded_frame->mm_time, now);
-            stream_dropped_frame_on_playback(decoder->base.stream);
-            decoder->display_frame = NULL;
-            free_gst_frame(gstframe);
-        }
-    }
-
-    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.
+ *
  * queues_mutex must be held.
  */
 static SpiceGstFrame *get_decoded_frame(SpiceGstDecoder *decoder, GstBuffer *buffer)
@@ -283,6 +245,10 @@ static SpiceGstFrame *get_decoded_frame(SpiceGstDecoder *decoder, GstBuffer *buf
     return gstframe;
 }
 
+/* Helper for schedule_frame().
+ *
+ * queues_mutex must be held.
+ */
 static void fetch_pending_sample(SpiceGstDecoder *decoder)
 {
     GstSample *sample = gst_app_sink_pull_sample(decoder->appsink);
@@ -315,6 +281,43 @@ static void fetch_pending_sample(SpiceGstDecoder *decoder)
     }
 }
 
+/* main loop or GStreamer streaming thread */
+static void schedule_frame(SpiceGstDecoder *decoder)
+{
+    guint32 now = stream_get_time(decoder->base.stream);
+    g_mutex_lock(&decoder->queues_mutex);
+
+    while (!decoder->timer_id) {
+        while (decoder->display_frame == NULL && decoder->pending_samples) {
+            fetch_pending_sample(decoder);
+        }
+
+        SpiceGstFrame *gstframe = decoder->display_frame;
+        if (!gstframe) {
+            break;
+        }
+
+        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.
+             */
+            decoder->timer_id = g_timeout_add(0, display_frame, decoder);
+        } else {
+            SPICE_DEBUG("%s: rendering too late by %u ms (ts: %u, mmtime: %u), dropping",
+                        __FUNCTION__, now - gstframe->encoded_frame->mm_time,
+                        gstframe->encoded_frame->mm_time, now);
+            stream_dropped_frame_on_playback(decoder->base.stream);
+            decoder->display_frame = NULL;
+            free_gst_frame(gstframe);
+        }
+    }
+
+    g_mutex_unlock(&decoder->queues_mutex);
+}
+
 /* GStreamer thread
  *
  * We cannot use GStreamer's signals because they are not always run in


More information about the Spice-commits mailing list