Hi,
On 1/15/19 3:42 PM, Frediano Ziglio wrote:
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.
Before applying this patch you can reproduce the warnings by runing with
DISABLE_GSTVIDEOOVERLAY=1 and starting some audio playback in the guest.
Signed-off-by: Snir Sheriber <ssheribe@xxxxxxxxxx>
---
Changes from v1:
-Commit message
---
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 2f556fe..a90fa81 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;
@@ -86,7 +90,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;
@@ -211,6 +222,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;
This line crashes for me, gst_buffer_get_reference_timestamp_meta returns
NULL. It does not surprise me, you attach metadata to an input buffer
and you expect these metadata to be attached to output buffers.
Apparently this is not guaranteed.
IIUC It should:
"A GstBuffer is the object that is passed from an upstream element to a
downstream element and contains memory and metadata information."
Actually this is pretty similar to what we are currently doing- counting
on the PTS (which is may be
changed by the elements in oppose to GstReferenceTimestampMeta which
should be used as an
alternative timestamp to PTS which is not being touched)
Anyway this could be fixed with a fallback to PTS which i set in any
case but i want to understand if
there's some bug here, does it always happen to you? can you reproduce
it consistently?
Snir.
+#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.
@@ -223,7 +240,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;
@@ -232,7 +249,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
@@ -626,9 +643,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
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel