Hi, On Thu, Jul 13, 2017 at 04:57:27PM +0200, Christophe de Dinechin wrote: > From: Christophe de Dinechin <dinechin@xxxxxxxxxx> > > Experience has shown that if the machine running the guest is overloaded, > it may pile up a lot of backlog in the frames queue. This patch clears > the queue if it exceeds 100 entries. This value is arbitrary. It > corresponds to a few seconds on a highly overloaded machine. I guess it depends on the stream. On 4K it might be too small, for 800x600, too big. Shouldn't we take in consideration the payload size? To the commit log, I'd also add the situation you had (OOM) and the outcome with this patch. Any glitches? Black screen? > Signed-off-by: Christophe de Dinechin <dinechin@xxxxxxxxxx> > --- > src/channel-display-gst.c | 13 +++++++++++++ > 1 file changed, 13 insertions(+) > > diff --git a/src/channel-display-gst.c b/src/channel-display-gst.c > index 3cf451b..17c2847 100644 > --- a/src/channel-display-gst.c > +++ b/src/channel-display-gst.c > @@ -498,6 +498,9 @@ static void spice_gst_decoder_destroy(VideoDecoder *video_decoder) > * 9) display_frame() then frees the SpiceGstFrame, which frees the SpiceFrame > * and decompressed frame with it. > */ > + > +SPICE_TWEAK_DEFINE(gst_queue_max_length, 100, "Max length of GStreamer queue"); > + > static gboolean spice_gst_decoder_queue_frame(VideoDecoder *video_decoder, > SpiceFrame *frame, int latency) > { > @@ -541,6 +544,16 @@ static gboolean spice_gst_decoder_queue_frame(VideoDecoder *video_decoder, > } > #endif > > + if (SPICE_TWEAK(gst_queue_max_length) && > + decoder->decoding_queue->length > SPICE_TWEAK(gst_queue_max_length)) { I'm not sure if we should mix things here. The main point of this patch is to avoid holding too much memory. The SPICE_TWEAK() is a proposal and not something used in the code base (yet!). > + SpiceGstFrame *gstframe; > + g_mutex_lock(&decoder->queues_mutex); > + while ((gstframe = g_queue_pop_head(decoder->decoding_queue))) > + free_gst_frame(gstframe); g_queue_free_full() > + g_mutex_unlock(&decoder->queues_mutex); > + return TRUE; Not sure *how* but it would be great to identify an i-frame as we are possibly dropping it... maybe a FIXME or a TODO about it.. not sure if really necessary :) Cheers, toso > + } > + > /* ref() the frame data for the buffer */ > frame->ref_data(frame->data_opaque); > GstBuffer *buffer = gst_buffer_new_wrapped_full(GST_MEMORY_FLAG_PHYSICALLY_CONTIGUOUS, > -- > 2.11.0 (Apple Git-81) > > _______________________________________________ > Spice-devel mailing list > Spice-devel@xxxxxxxxxxxxxxxxxxxxx > https://lists.freedesktop.org/mailman/listinfo/spice-devel
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel