> On Mon, 2019-02-11 at 16:14 +0200, Snir Sheriber wrote: > > --- > > src/channel-display-gst.c | 39 ++++++++++++++++++++++--------------- > > -- > > 1 file changed, 22 insertions(+), 17 deletions(-) > > > > diff --git a/src/channel-display-gst.c b/src/channel-display-gst.c > > index 4272ade..c63592b 100644 > > --- a/src/channel-display-gst.c > > +++ b/src/channel-display-gst.c > > @@ -580,34 +580,39 @@ static void > > spice_gst_decoder_destroy(VideoDecoder *video_decoder) > > /* spice_gst_decoder_queue_frame() queues the SpiceFrame for > > decoding and > > * displaying. The steps it goes through are as follows: > > * > > - * 1) A SpiceGstFrame is created to keep track of SpiceFrame and > > some additional > > - * metadata. The SpiceGstFrame is then pushed to the > > decoding_queue. > > - * 2) frame->data, which contains the compressed frame data, is > > reffed and > > - * wrapped in a GstBuffer which is pushed to the GStreamer > > pipeline for > > - * decoding. > > - * 3) As soon as the GStreamer pipeline no longer needs the > > compressed frame it > > - * will call frame->unref_data() to free it. > > + * 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. > > * > > * If GstVideoOverlay is used (window handle was obtained > > successfully at the widget): > > - * 4) Decompressed frames will be renderd to widget directly from > > gstreamer's pipeline > > - * using some gstreamer sink plugin which implements the > > GstVideoOverlay interface > > + * 3) Decompressed frames will be rendered to widget directly from > > GStreamer's pipeline > > "to *the* widget" sounds more correct. Otherwise I'd say the rest of > this patch looks fine. I only reviewed it for language, though. I > didn't check that the description actually matches the code. > > Reviewed-by: Jonathon Jongsma <jjongsma@xxxxxxxxxx> > > Description is fine, Acked-by: Frediano Ziglio <fziglio@xxxxxxxxxx> Frediano > > > + * 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 > > + * 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. > > * > > * Otherwise appsink is used: > > - * 4) Once the decompressed frame is available the GStreamer > > pipeline calls > > + * 3) Once the decompressed frame is available the GStreamer > > pipeline calls > > * new_sample() in the GStreamer thread. > > - * 5) new_sample() then matches the decompressed frame to a > > SpiceGstFrame from > > + * 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. > > - * 6) new_sample() then attaches the decompressed frame to the > > SpiceGstFrame, > > + * 5) new_sample() then attaches the decompressed frame to the > > SpiceGstFrame, > > * set into display_frame and calls schedule_frame(). > > - * 7) schedule_frame() then uses gstframe->frame->mm_time to > > arrange for > > + * 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. > > - * 8) display_frame() use SpiceGstFrame from display_frame and > > - * calls stream_display_frame(). > > - * 9) display_frame() then frees the SpiceGstFrame, which frees > > the SpiceFrame > > - * and decompressed frame with it. > > + * 7) 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. > > */ > > static gboolean spice_gst_decoder_queue_frame(VideoDecoder > > *video_decoder, > > SpiceFrame *frame, int > > latency) _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel