[vlc-devel] [PATCH] clock: fix false start with some SPU tracks
Thomas Guillem
thomas at gllm.fr
Fri Oct 11 09:23:47 CEST 2019
Reminder of the design: Any calls to vlc_clock_ConvertToSystem() or
vlc_clock_Update() will create the reference point (wait_sync_ref) if it is the
first call of all sub clocks. This point will be used as a reference by all
clocks until the master does its first vlc_clock_Update().
Creating a reference point from vlc_clock_ConvertToSystem() can be discussed.
You don't expect to modify the main_clock from a function with such name
("convert"). The pro of this design is that vlc_clock_ConvertToSystem() can't
fail and will always return a valid point. Returning a valid point is
mandatory for the actual design, let's see how the audio output is using it:
The aout will first call vlc_clock_ConvertToSystem(ts) to get the first system
date of the ts, this first system date will take into account the output
dejitter, the input dejitter and the pcr_delay int and will be very likely in
the future. Enough in the future to wait for any other tracks. When this system
date is reached by the aout plugin, the aout will call the first update() and
will drive all other clocks.
Now the problem: any outputs can create the first reference point, but when it
is the SPU output, it can lead to very high imprecision. For example, with the
following .srt file:
"""
1
00:00:00,00 --> 00:00:42,00
SRT
"""
If you seek to 20seconds, the SPU output will convert its first point, that is
00:00:00,00, this could lead to the creation of the reference point. This
reference point will be delayed by 20seconds and will cause all other outputs to
wait for 20seconds (approximately) before doing their first rendering.
To fix this issue, this commit adds a priority system. The clock with higher
priority will always be able to override an old reference point. The SPU clock
will always have the lowest priority.
---
src/audio_output/filters.c | 2 +-
src/clock/clock.c | 44 +++++++++++++++++++++++++++++++++-----
src/clock/clock.h | 8 ++++---
src/input/es_out.c | 1 +
4 files changed, 46 insertions(+), 9 deletions(-)
diff --git a/src/audio_output/filters.c b/src/audio_output/filters.c
index 6a37c8e8ab..6c7c7d5c74 100644
--- a/src/audio_output/filters.c
+++ b/src/audio_output/filters.c
@@ -501,7 +501,7 @@ aout_filters_t *aout_FiltersNewWithClock(vlc_object_t *obj, const vlc_clock_t *c
filters->count = 0;
if (clock)
{
- filters->clock = vlc_clock_CreateSlave(clock);
+ filters->clock = vlc_clock_CreateSlave(clock, AUDIO_ES);
if (!filters->clock)
goto error;
}
diff --git a/src/clock/clock.c b/src/clock/clock.c
index d1fa1848d0..9751a3c2de 100644
--- a/src/clock/clock.c
+++ b/src/clock/clock.c
@@ -24,6 +24,7 @@
#include <vlc_common.h>
#include <vlc_aout.h>
#include <assert.h>
+#include <limits.h>
#include "clock.h"
#include "clock_internal.h"
@@ -49,6 +50,7 @@ struct vlc_clock_main_t
vlc_tick_t pause_date;
+ unsigned wait_sync_ref_priority;
clock_point_t wait_sync_ref; /* When the master */
clock_point_t first_pcr;
vlc_tick_t output_dejitter; /* Delay used to absorb the output clock jitter */
@@ -69,6 +71,7 @@ struct vlc_clock_t
vlc_clock_main_t *owner;
vlc_tick_t delay;
+ unsigned priority;
const struct vlc_clock_cbs *cbs;
void *cbs_data;
@@ -90,6 +93,7 @@ static void vlc_clock_main_reset(vlc_clock_main_t *main_clock)
main_clock->rate = 1.0f;
main_clock->offset = VLC_TICK_INVALID;
+ main_clock->wait_sync_ref_priority = UINT_MAX;
main_clock->wait_sync_ref =
main_clock->last = clock_point_Create(VLC_TICK_INVALID, VLC_TICK_INVALID);
vlc_cond_broadcast(&main_clock->cond);
@@ -140,8 +144,11 @@ static vlc_tick_t vlc_clock_master_update(vlc_clock_t *clock,
}
}
else
+ {
+ main_clock->wait_sync_ref_priority = UINT_MAX;
main_clock->wait_sync_ref =
clock_point_Create(VLC_TICK_INVALID, VLC_TICK_INVALID);
+ }
main_clock->offset = system_now - ts * main_clock->coeff / rate;
@@ -230,7 +237,7 @@ vlc_clock_monotonic_to_system_locked(vlc_clock_t *clock, vlc_tick_t now,
{
vlc_clock_main_t *main_clock = clock->owner;
- if (main_clock->wait_sync_ref.system == VLC_TICK_INVALID)
+ if (clock->priority < main_clock->wait_sync_ref_priority)
{
/* XXX: This input_delay calculation is needed until we (finally) get
* ride of the input clock. This code is adapted from input_clock.c and
@@ -246,6 +253,7 @@ vlc_clock_monotonic_to_system_locked(vlc_clock_t *clock, vlc_tick_t now,
const vlc_tick_t delay =
__MAX(input_delay, main_clock->output_dejitter);
+ main_clock->wait_sync_ref_priority = clock->priority;
main_clock->wait_sync_ref = clock_point_Create(now + delay, ts);
}
return (ts - main_clock->wait_sync_ref.stream) / rate
@@ -311,6 +319,7 @@ static void vlc_clock_slave_reset(vlc_clock_t *clock)
{
vlc_clock_main_t *main_clock = clock->owner;
vlc_mutex_lock(&main_clock->lock);
+ main_clock->wait_sync_ref_priority = UINT_MAX;
main_clock->wait_sync_ref =
clock_point_Create(VLC_TICK_INVALID, VLC_TICK_INVALID);
@@ -382,6 +391,7 @@ vlc_clock_main_t *vlc_clock_main_New(void)
main_clock->first_pcr =
clock_point_Create(VLC_TICK_INVALID, VLC_TICK_INVALID);
+ main_clock->wait_sync_ref_priority = UINT_MAX;
main_clock->wait_sync_ref = main_clock->last =
clock_point_Create(VLC_TICK_INVALID, VLC_TICK_INVALID);
@@ -420,6 +430,7 @@ void vlc_clock_main_SetFirstPcr(vlc_clock_main_t *main_clock,
if (main_clock->first_pcr.system == VLC_TICK_INVALID)
{
main_clock->first_pcr = clock_point_Create(system_now, ts);
+ main_clock->wait_sync_ref_priority = UINT_MAX;
main_clock->wait_sync_ref =
clock_point_Create(VLC_TICK_INVALID, VLC_TICK_INVALID);
}
@@ -541,6 +552,7 @@ static void vlc_clock_set_slave_callbacks(vlc_clock_t *clock)
}
static vlc_clock_t *vlc_clock_main_Create(vlc_clock_main_t *main_clock,
+ unsigned priority,
const struct vlc_clock_cbs *cbs,
void *cbs_data)
{
@@ -552,6 +564,7 @@ static vlc_clock_t *vlc_clock_main_Create(vlc_clock_main_t *main_clock,
clock->delay = 0;
clock->cbs = cbs;
clock->cbs_data = cbs_data;
+ clock->priority = priority;
assert(!cbs || cbs->on_update);
return clock;
@@ -561,7 +574,8 @@ vlc_clock_t *vlc_clock_main_CreateMaster(vlc_clock_main_t *main_clock,
const struct vlc_clock_cbs *cbs,
void *cbs_data)
{
- vlc_clock_t *clock = vlc_clock_main_Create(main_clock, cbs, cbs_data);
+ /* The master has always the 0 priority */
+ vlc_clock_t *clock = vlc_clock_main_Create(main_clock, 0, cbs, cbs_data);
if (!clock)
return NULL;
@@ -580,10 +594,29 @@ vlc_clock_t *vlc_clock_main_CreateMaster(vlc_clock_main_t *main_clock,
}
vlc_clock_t *vlc_clock_main_CreateSlave(vlc_clock_main_t *main_clock,
+ enum es_format_category_e cat,
const struct vlc_clock_cbs *cbs,
void *cbs_data)
{
- vlc_clock_t *clock = vlc_clock_main_Create(main_clock, cbs, cbs_data);
+ /* SPU outputs should have lower priority than VIDEO outputs since they
+ * necessarily depend on a VIDEO output. This mean that a SPU reference
+ * point will always be overridden by AUDIO or VIDEO outputs. Cf.
+ * vlc_clock_monotonic_to_system_locked */
+ unsigned priority;
+ switch (cat)
+ {
+ case VIDEO_ES:
+ case AUDIO_ES:
+ priority = 1;
+ break;
+ case SPU_ES:
+ default:
+ priority = 2;
+ break;
+ }
+
+ vlc_clock_t *clock = vlc_clock_main_Create(main_clock, priority, cbs,
+ cbs_data);
if (!clock)
return NULL;
@@ -595,9 +628,10 @@ vlc_clock_t *vlc_clock_main_CreateSlave(vlc_clock_main_t *main_clock,
return clock;
}
-vlc_clock_t *vlc_clock_CreateSlave(const vlc_clock_t *clock)
+vlc_clock_t *vlc_clock_CreateSlave(const vlc_clock_t *clock,
+ enum es_format_category_e cat)
{
- return vlc_clock_main_CreateSlave(clock->owner, NULL, NULL);
+ return vlc_clock_main_CreateSlave(clock->owner, cat, NULL, NULL);
}
void vlc_clock_main_SetMaster(vlc_clock_main_t *main_clock, vlc_clock_t *clock)
diff --git a/src/clock/clock.h b/src/clock/clock.h
index 8e9e6eb681..fcb1f84975 100644
--- a/src/clock/clock.h
+++ b/src/clock/clock.h
@@ -106,15 +106,17 @@ vlc_clock_t *vlc_clock_main_CreateMaster(vlc_clock_main_t *main_clock,
* You must use vlc_clock_Delete to free it.
*/
vlc_clock_t *vlc_clock_main_CreateSlave(vlc_clock_main_t *main_clock,
- const struct vlc_clock_cbs *cbs,
- void *cbs_data);
+ enum es_format_category_e cat,
+ const struct vlc_clock_cbs *cbs,
+ void *cbs_data);
/**
* This function creates a new slave vlc_clock_t interface
*
* You must use vlc_clock_Delete to free it.
*/
-vlc_clock_t *vlc_clock_CreateSlave(const vlc_clock_t *clock);
+vlc_clock_t *vlc_clock_CreateSlave(const vlc_clock_t *clock,
+ enum es_format_category_e cat);
/**
* This function free the resources allocated by vlc_clock*Create*()
diff --git a/src/input/es_out.c b/src/input/es_out.c
index 810f243a70..3d67c35730 100644
--- a/src/input/es_out.c
+++ b/src/input/es_out.c
@@ -2038,6 +2038,7 @@ static void EsOutCreateDecoder( es_out_t *out, es_out_id_t *p_es )
{
p_es->master = false;
p_es->p_clock = vlc_clock_main_CreateSlave( p_es->p_pgrm->p_main_clock,
+ p_es->fmt.i_cat,
&clock_cbs, p_es );
}
--
2.20.1
More information about the vlc-devel
mailing list