[Spice-devel] [spice-gtk v1 05/11] channel-display: add spice_frame_free() helper

[Victor Toso]

From: Victor Toso <me at victortoso.com>

The SpiceFrame is created in channel-display.c but it is currently
freed at each decoders' end. A helper function can reduce some code
and makes it easier to check if the function is called, what time was
called, etc.

In channel-display-mjpeg.c this means removing free_spice_frame()
function;

In channel-display-gst.c, SpiceFrame is under SpiceGstFrame so we just
need to be careful to call spice_frame_free() once by removing the
unref function parameter to gst_buffer_new_wrapped_full();

Note that I'm using g_clear_pointer() everywhere that makes sense
(checking for NULL before calling free and setting pointer to NULL
afterwards)

Signed-off-by: Victor Toso <victortoso at redhat.com>
---
 src/channel-display-gst.c   |  8 +++-----
 src/channel-display-mjpeg.c | 21 ++++-----------------
 src/channel-display-priv.h  |  1 +
 src/channel-display.c       | 11 +++++++++++
 4 files changed, 19 insertions(+), 22 deletions(-)

diff --git a/src/channel-display-gst.c b/src/channel-display-gst.c
index 8b23036..d2847ec 100644
--- a/src/channel-display-gst.c
+++ b/src/channel-display-gst.c
@@ -91,10 +91,8 @@ static SpiceGstFrame *create_gst_frame(GstBuffer *buffer, SpiceFrame *frame)
 
 static void free_gst_frame(SpiceGstFrame *gstframe)
 {
-    gstframe->frame->free(gstframe->frame);
-    if (gstframe->sample) {
-        gst_sample_unref(gstframe->sample);
-    }
+    g_clear_pointer(&gstframe->frame, spice_frame_free);
+    g_clear_pointer(&gstframe->sample, gst_sample_unref);
     g_free(gstframe);
 }
 
@@ -553,7 +551,7 @@ static gboolean spice_gst_decoder_queue_frame(VideoDecoder *video_decoder,
     frame->ref_data(frame->data_opaque);
     GstBuffer *buffer = gst_buffer_new_wrapped_full(GST_MEMORY_FLAG_PHYSICALLY_CONTIGUOUS,
                                                     frame->data, frame->size, 0, frame->size,
-                                                    frame->data_opaque, frame->unref_data);
+                                                    frame->data_opaque, NULL);
 
     GST_BUFFER_DURATION(buffer) = GST_CLOCK_TIME_NONE;
     GST_BUFFER_DTS(buffer) = GST_CLOCK_TIME_NONE;
diff --git a/src/channel-display-mjpeg.c b/src/channel-display-mjpeg.c
index f0d55f6..66a6eb8 100644
--- a/src/channel-display-mjpeg.c
+++ b/src/channel-display-mjpeg.c
@@ -75,15 +75,6 @@ static void mjpeg_src_term(struct jpeg_decompress_struct *cinfo)
 }
 
 
-/* ---------- A SpiceFrame helper ---------- */
-
-static void free_spice_frame(SpiceFrame *frame)
-{
-    frame->unref_data(frame->data_opaque);
-    frame->free(frame);
-}
-
-
 /* ---------- Decoder proper ---------- */
 
 static void mjpeg_decoder_schedule(MJpegDecoder *decoder);
@@ -177,8 +168,7 @@ static gboolean mjpeg_decoder_decode_frame(gpointer video_decoder)
     /* Display the frame and dispose of it */
     stream_display_frame(decoder->base.stream, decoder->cur_frame,
                          width, height, SPICE_UNKNOWN_STRIDE, decoder->out_frame);
-    free_spice_frame(decoder->cur_frame);
-    decoder->cur_frame = NULL;
+    g_clear_pointer(&decoder->cur_frame, spice_frame_free);
     decoder->timer_id = 0;
 
     /* Schedule the next frame */
@@ -212,7 +202,7 @@ static void mjpeg_decoder_schedule(MJpegDecoder *decoder)
                         __FUNCTION__, time - frame->mm_time,
                         frame->mm_time, time);
             stream_dropped_frame_on_playback(decoder->base.stream);
-            free_spice_frame(frame);
+            spice_frame_free(frame);
         }
         frame = g_queue_pop_head(decoder->msgq);
     } while (frame);
@@ -222,7 +212,7 @@ static void mjpeg_decoder_schedule(MJpegDecoder *decoder)
 /* mjpeg_decoder_drop_queue() helper */
 static void _msg_in_unref_func(gpointer data, gpointer user_data)
 {
-    free_spice_frame((SpiceFrame*)data);
+    spice_frame_free(data);
 }
 
 static void mjpeg_decoder_drop_queue(MJpegDecoder *decoder)
@@ -231,10 +221,7 @@ static void mjpeg_decoder_drop_queue(MJpegDecoder *decoder)
         g_source_remove(decoder->timer_id);
         decoder->timer_id = 0;
     }
-    if (decoder->cur_frame) {
-        free_spice_frame(decoder->cur_frame);
-        decoder->cur_frame = NULL;
-    }
+    g_clear_pointer(&decoder->cur_frame, spice_frame_free);
     g_queue_foreach(decoder->msgq, _msg_in_unref_func, NULL);
     g_queue_clear(decoder->msgq);
 }
diff --git a/src/channel-display-priv.h b/src/channel-display-priv.h
index 6a90a78..95ad7d8 100644
--- a/src/channel-display-priv.h
+++ b/src/channel-display-priv.h
@@ -196,6 +196,7 @@ void stream_dropped_frame_on_playback(display_stream *st);
 #define SPICE_UNKNOWN_STRIDE 0
 void stream_display_frame(display_stream *st, SpiceFrame *frame, uint32_t width, uint32_t height, int stride, uint8_t* data);
 
+void spice_frame_free(SpiceFrame *frame);
 
 G_END_DECLS
 
diff --git a/src/channel-display.c b/src/channel-display.c
index ec94cf8..a134516 100644
--- a/src/channel-display.c
+++ b/src/channel-display.c
@@ -1485,6 +1485,17 @@ static void display_session_mm_time_reset_cb(SpiceSession *session, gpointer dat
 
 #define STREAM_PLAYBACK_SYNC_DROP_SEQ_LEN_LIMIT 5
 
+G_GNUC_INTERNAL
+void spice_frame_free(SpiceFrame *frame)
+{
+    if (frame == NULL) {
+        return;
+    }
+
+    frame->unref_data(frame->data_opaque);
+    frame->free(frame);
+}
+
 /* coroutine context */
 static void display_handle_stream_data(SpiceChannel *channel, SpiceMsgIn *in)
 {
-- 
2.16.2