[Spice-devel] [PATCH spice-gtk] gstreamer: set timestamp in buffer's GstReferenceTimestampMeta

[Frediano Ziglio]

> 
> Currently we set timestamps as buffer's PTS, this value may be changed by
> the pipeline in some cases and cause an unexpected buffer warnings (when
> GstVideoOverlay is not used). Use GstReferenceTimestampMeta when
> synchronization is made by spice.
> 
> Signed-off-by: Snir Sheriber <ssheribe at redhat.com>
> ---
> 
> Currently it's causing to an error if you running with
> DISABLE_GSTVIDEOOVERLAY=1
> and start some audio playback in the guest.
> 

The patch make sense but looking at the commit message looks like that
to remove a warning you introduced an error.
It would be worth mentioning in which case(s) you have the warning and
which warning (I supposed "got an unexpected decoded buffer!" or
"the GStreamer pipeline dropped a frame").
Which error are you getting now?

> ---
>  src/channel-display-gst.c | 27 ++++++++++++++++++++++++---
>  1 file changed, 24 insertions(+), 3 deletions(-)
> 
> diff --git a/src/channel-display-gst.c b/src/channel-display-gst.c
> index 2c07f35..a00f53e 100644
> --- a/src/channel-display-gst.c
> +++ b/src/channel-display-gst.c
> @@ -33,6 +33,10 @@ typedef struct SpiceGstFrame SpiceGstFrame;
>  
>  /* GStreamer decoder implementation */
>  
> +#if GST_CHECK_VERSION(1,14,0)
> +static GstStaticCaps stream_reference =
> GST_STATIC_CAPS("timestamp/spice-stream");
> +#endif
> +
>  typedef struct SpiceGstDecoder {
>      VideoDecoder base;
>  
> @@ -88,7 +92,14 @@ struct SpiceGstFrame {
>  static SpiceGstFrame *create_gst_frame(GstBuffer *buffer, SpiceFrame *frame)
>  {
>      SpiceGstFrame *gstframe = g_new(SpiceGstFrame, 1);
> +#if GST_CHECK_VERSION(1,14,0)
> +    GstReferenceTimestampMeta *time_meta;
> +
> +    time_meta = gst_buffer_get_reference_timestamp_meta(buffer,
> gst_static_caps_get(&stream_reference));
> +    gstframe->timestamp = time_meta->timestamp;
> +#else
>      gstframe->timestamp = GST_BUFFER_PTS(buffer);
> +#endif
>      gstframe->frame = frame;
>      gstframe->sample = NULL;
>      return gstframe;
> @@ -213,6 +224,12 @@ static void fetch_pending_sample(SpiceGstDecoder
> *decoder)
>          decoder->pending_samples--;
>  
>          GstBuffer *buffer = gst_sample_get_buffer(sample);
> +        GstClockTime buffer_ts;
> +#if GST_CHECK_VERSION(1,14,0)
> +        buffer_ts = gst_buffer_get_reference_timestamp_meta(buffer,
> gst_static_caps_get(&stream_reference))->timestamp;
> +#else
> +        buffer_ts = GST_BUFFER_PTS(buffer);
> +#endif
>  
>          /* gst_app_sink_pull_sample() sometimes returns the same buffer
>          twice
>           * or buffers that have a modified, and thus unrecognizable, PTS.
> @@ -225,7 +242,7 @@ static void fetch_pending_sample(SpiceGstDecoder
> *decoder)
>          GList *l = g_queue_peek_head_link(decoder->decoding_queue);
>          while (l) {
>              gstframe = l->data;
> -            if (gstframe->timestamp == GST_BUFFER_PTS(buffer)) {
> +            if (gstframe->timestamp == buffer_ts) {
>                  /* The frame is now ready for display */
>                  gstframe->sample = sample;
>                  decoder->display_frame = gstframe;
> @@ -234,7 +251,7 @@ static void fetch_pending_sample(SpiceGstDecoder
> *decoder)
>                   * frames from the decoding queue.
>                   */
>                  while ((gstframe =
>                  g_queue_pop_head(decoder->decoding_queue))) {
> -                    if (gstframe->timestamp == GST_BUFFER_PTS(buffer)) {
> +                    if (gstframe->timestamp == buffer_ts) {
>                          break;
>                      }
>                      /* The GStreamer pipeline dropped the corresponding
> @@ -648,9 +665,13 @@ static gboolean
> spice_gst_decoder_queue_frame(VideoDecoder *video_decoder,
>                                                      frame->data,
>                                                      frame->size, 0,
>                                                      frame->size,
>                                                      frame->data_opaque,
>                                                      frame->unref_data);
>  
> +    GstClockTime pts = gst_clock_get_time(decoder->clock) -
> gst_element_get_base_time(decoder->pipeline) + ((uint64_t)MAX(0, latency)) *
> 1000 * 1000;
>      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;
> +#if GST_CHECK_VERSION(1,14,0)
> +    gst_buffer_add_reference_timestamp_meta(buffer, gst_static_caps_get
> (&stream_reference), pts, GST_CLOCK_TIME_NONE);
> +#endif
>  
>      if (decoder->appsink != NULL) {
>          SpiceGstFrame *gst_frame = create_gst_frame(buffer, frame);

Frediano