On 1/3/19 10:43 AM, Frediano Ziglio wrote:
If playback is not active and it's streaming mode set latency to 0
so frames will not be synchronized with mm time.
Signed-off-by: Snir Sheriber <ssheribe@xxxxxxxxxx>
---
This patch is a suggestion to improve current state.
Latency in display channel is the difference between multimedia time (mm)
at frame's arrival time and its timestamp, this latency is added to current
gstreamer's clock and attached to a gst buffer that is pushed into
gstreamer's
so it will be played in sync at the right time.
Currently the mm time is being set when session begins, by the server with
400ms delay for buffering so as a result in streaming mode you feel at least
400ms latency.
But when playback starts mm time is set according to playback's timestamp at
its
arrival time (so no 400ms delay anymore (a bug? maybe), there is another
delay
which is usually smaller)
Yes, kind of a bug, really weird behaviour causing some issue on
bandwidth management on the server.
And possibly this patch will make thing worse as the stream_report is
using this "latency" you are setting, would be better to save the computed
value before resetting it and passing the not 0 one to display_update_stream_report
(after queue_frame call on the same function).
Fix attached.
With this patch, when there is no playback, latency is 0 so buffer's
timestamp
is set to only gst clock, means, it won't be synced.
Theoretically it should be safer to set buffer's timestamp to
GST_CLOCK_TIME_NONE
if there is no playback but i tried it and it feels faster and simpler this
way
and there were no issues by now.
---
src/channel-display.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/src/channel-display.c b/src/channel-display.c
index 7c663cb..e31a19c 100644
--- a/src/channel-display.c
+++ b/src/channel-display.c
@@ -1561,7 +1561,15 @@ static void display_handle_stream_data(SpiceChannel
*channel, SpiceMsgIn *in)
st->cur_drops_seq_stats.len++;
st->playback_sync_drops_seq_len++;
} else {
- CHANNEL_DEBUG(channel, "video latency: %d", latency);
+ SpiceSession *s = spice_channel_get_session(channel);
+
+ if (st->surface->streaming_mode &&
!spice_session_is_playback_active(s)) {
Why also checking for streaming mode? I would just check the playback.
This is not a big deal but currently if you ignore the latency, frames
will be presented
according to their arrival time, so it may mismatch the stream framerate
a bit.
So the reason behind this was that if video stream is just part of the
screen
you won't feel the latency as much as in streaming mode, so just let it
sync, although
there's no audio, so it will play the video frames in timestamps that
match the framerate.
Snir.
+ CHANNEL_DEBUG(channel, "video latency: %d, set to 0 since there
is no playback", latency);
+ latency = 0;
+ } else {
+ CHANNEL_DEBUG(channel, "video latency: %d", latency);
+ }
+
if (st->cur_drops_seq_stats.len) {
st->cur_drops_seq_stats.duration = op->multi_media_time -
st->cur_drops_seq_stats.start_mm_time;
Frediano
>From 3fd8d7d2d38f8fcb6d4db848d08bb4a470c29e82 Mon Sep 17 00:00:00 2001
From: Snir Sheriber <ssheribe@xxxxxxxxxx>
Date: Mon, 31 Dec 2018 11:41:07 +0200
Subject: [PATCH] channel-display: set 0 latency if there is no playback
If playback is not active and it's streaming mode set latency to 0
so frames will not be synchronized with mm time.
Signed-off-by: Snir Sheriber <ssheribe@xxxxxxxxxx>
---
src/channel-display.c | 16 ++++++++++++----
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/src/channel-display.c b/src/channel-display.c
index 7c663cb..3deebdd 100644
--- a/src/channel-display.c
+++ b/src/channel-display.c
@@ -1525,7 +1525,7 @@ static void display_handle_stream_data(SpiceChannel *channel, SpiceMsgIn *in)
SpiceStreamDataHeader *op = spice_msg_in_parsed(in);
display_stream *st = get_stream_by_id(channel, op->id);
guint32 mmtime;
- int32_t latency;
+ int32_t latency, latency_report;
SpiceFrame *frame;
g_return_if_fail(st != NULL);
@@ -1545,7 +1545,7 @@ static void display_handle_stream_data(SpiceChannel *channel, SpiceMsgIn *in)
}
st->num_input_frames++;
- latency = op->multi_media_time - mmtime;
+ latency = latency_report = op->multi_media_time - mmtime;
if (latency < 0) {
CHANNEL_DEBUG(channel, "stream data too late by %u ms (ts: %u, mmtime: %u)",
mmtime - op->multi_media_time, op->multi_media_time, mmtime);
@@ -1561,7 +1561,15 @@ static void display_handle_stream_data(SpiceChannel *channel, SpiceMsgIn *in)
st->cur_drops_seq_stats.len++;
st->playback_sync_drops_seq_len++;
} else {
- CHANNEL_DEBUG(channel, "video latency: %d", latency);
+ SpiceSession *s = spice_channel_get_session(channel);
+
+ if (st->surface->streaming_mode && !spice_session_is_playback_active(s)) {
+ CHANNEL_DEBUG(channel, "video latency: %d, set to 0 since there is no playback", latency);
+ latency = 0;
+ } else {
+ CHANNEL_DEBUG(channel, "video latency: %d", latency);
+ }
+
if (st->cur_drops_seq_stats.len) {
st->cur_drops_seq_stats.duration = op->multi_media_time -
st->cur_drops_seq_stats.start_mm_time;
@@ -1592,7 +1600,7 @@ static void display_handle_stream_data(SpiceChannel *channel, SpiceMsgIn *in)
if (c->enable_adaptive_streaming) {
display_update_stream_report(SPICE_DISPLAY_CHANNEL(channel), op->id,
- op->multi_media_time, latency);
+ op->multi_media_time, latency_report);
if (st->playback_sync_drops_seq_len >= STREAM_PLAYBACK_SYNC_DROP_SEQ_LEN_LIMIT) {
spice_session_sync_playback_latency(spice_channel_get_session(channel));
st->playback_sync_drops_seq_len = 0;
--
2.19.1
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel