[Spice-devel] [PATCH spice-gtk] channel-display-gst: Avoid PTS to decrease

[Snir Sheriber]

Hi,

On 1/31/19 5:43 PM, Frediano Ziglio wrote:
> GStreamer does not accept PTS to decrease and set the new PTS
> with the value of the previous one.
> This causes our code to not match output frames with input ones.
> Make sure PTS always increases (even if very slowly).
>
> Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> ---
>   src/channel-display-gst.c | 9 ++++++++-
>   1 file changed, 8 insertions(+), 1 deletion(-)
>
> This should avoid the issues the following patch is trying
> also to fix:
> https://patchwork.freedesktop.org/patch/277721/
>
> diff --git a/src/channel-display-gst.c b/src/channel-display-gst.c
> index 4272ade8..7d7b435d 100644
> --- a/src/channel-display-gst.c
> +++ b/src/channel-display-gst.c
> @@ -42,6 +42,8 @@ typedef struct SpiceGstDecoder {
>       GstAppSink *appsink;
>       GstElement *pipeline;
>       GstClock *clock;
> +    // minimum PTS attached to the next frame to avoid decreasing it
> +    GstClockTime next_minimum_pts;
>   
>       /* ---------- Decoding and display queues ---------- */
>   
> @@ -657,9 +659,14 @@ static gboolean spice_gst_decoder_queue_frame(VideoDecoder *video_decoder,
>                                                       frame->data, frame->size, 0, frame->size,
>                                                       frame, (GDestroyNotify) spice_frame_free);
>   
> +    GstClockTime pts = gst_clock_get_time(decoder->clock) -
> +                       gst_element_get_base_time(decoder->pipeline) +
> +                       ((uint64_t)MAX(0, latency)) * 1000 * 1000;
> +    pts = MAX(pts, decoder->next_minimum_pts);
> +    decoder->next_minimum_pts = pts + 1;
>       GST_BUFFER_DURATION(buffer) = GST_CLOCK_TIME_NONE;
>       GST_BUFFER_DTS(buffer) = GST_CLOCK_TIME_NONE;
> -    GST_BUFFER_PTS(buffer) = gst_clock_get_time(decoder->clock) - gst_element_get_base_time(decoder->pipeline) + ((uint64_t)MAX(0, latency)) * 1000 * 1000;
> +    GST_BUFFER_PTS(buffer) = pts;
>   
>       SpiceGstFrame *gst_frame = create_gst_frame(buffer, frame);
>       g_mutex_lock(&decoder->queues_mutex);




This should fix the issue, I am not sure what is the dropping policy of 
the different sinks, but such increasing may cause late
frames to be renderd now (I think they are usually skipped , needs to 
see how (& if) it affects, "feels" smoother or the opposite?)

BTW I personally still prefer to use GstReferenceTimestampMeta if 
possible and do not mess with the buffer's main time-stamp.


Snir.