[Spice-devel] [client] gstreamer: Fix the decoding time when not using GstVideoOverlay

[Francois Gouget]

On Wed, 12 Jun 2019, Snir Sheriber wrote:

> Hi,
> 
> On 6/11/19 9:42 PM, Francois Gouget wrote:
> > schedule_frame() only pulls frames out of GStreamer's pipeline once all
> > previous decoded frames have been displayed. Thus when the video delay
> 
> 
> IIRC we used to pull when samples arrived but it was changed to this so 
> pending frames will be queued
> inside gstreamer and let it do throttling (or something similar)

I see. For reference that was commit 8835e757922c.
That's going to make it harder to get the correct decoding time :-(


> > +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;
> 
> 
> Is it possible to have a sample with no matching entry? how this sample 
> is unrefed in that case?

There's a gst_sample_unref(sample) missing somewhere.


[...]

> > @@ -429,10 +429,19 @@ sink_event_probe(GstPad *pad, GstPadProbeInfo *info, gpointer data)
[...]
> > +        /* 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;
> >    
> 
> 
> Isn't it done on attach_decoded_sample?

sink_event_probe() cannot call attach_decoded_sample() because all it 
has is a buffer. This is why it calls find_decoded_entry() instead which 
does not remove older entries.


> Also would be nice to update the comment above spice_gst_decoder_queue_frame
> with the current flow, will make it easier to follow.

Ok.


-- 
Francois Gouget <fgouget at codeweavers.com>