From 5b3724c8ecda6879c8217038adb7e0992be7690a Mon Sep 17 00:00:00 2001
From: Alexandre Janniaux <ajanni@videolabs.io>
Date: Mon, 7 Oct 2024 10:18:11 +0200
Subject: [PATCH 01/28] clock: remove extra line
---
src/clock/clock.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/src/clock/clock.c b/src/clock/clock.c
index c1fdc02d5ae5..c30ac6cfdc02 100644
--- a/src/clock/clock.c
+++ b/src/clock/clock.c
@@ -373,7 +373,6 @@ static inline void vlc_clock_on_update(vlc_clock_t *clock,
system_now, ts, drift, clock->context ? clock->context->clock_id : 0);
}
-
static void vlc_clock_master_update_coeff(
vlc_clock_t *clock, struct vlc_clock_context *ctx,
vlc_tick_t system_now, vlc_tick_t ts, double rate)
--
GitLab
From ea2750ffdace5dd57a7859374d9592b9dc988247 Mon Sep 17 00:00:00 2001
From: Alexandre Janniaux <ajanni@videolabs.io>
Date: Mon, 22 Jul 2024 15:25:06 +0200
Subject: [PATCH 02/28] test: clock: specify start date for run tests
Make sure the start date is always defined for CLOCK_SCENARIO_RUN tests
so that the proper first PCR is defined.
---
test/src/clock/clock.c | 15 +++++++++++++--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/test/src/clock/clock.c b/test/src/clock/clock.c
index 349ff02f9a93..aa7e6eee7577 100644
--- a/test/src/clock/clock.c
+++ b/test/src/clock/clock.c
@@ -57,10 +57,11 @@ struct clock_scenario
CLOCK_SCENARIO_RUN,
} type;
+ vlc_tick_t stream_start;
+ vlc_tick_t system_start; /* VLC_TICK_INVALID for vlc_tick_now() */
+
union {
struct {
- vlc_tick_t stream_start;
- vlc_tick_t system_start; /* VLC_TICK_INVALID for vlc_tick_now() */
vlc_tick_t duration;
vlc_tick_t stream_increment; /* VLC_TICK_INVALID for manual increment */
unsigned video_fps;
@@ -894,6 +895,8 @@ static struct clock_scenario clock_scenarios[] = {
.desc = "pause + resume is delaying the next conversion",
.type = CLOCK_SCENARIO_RUN,
.run = master_pause_run,
+ .system_start = VLC_TICK_FROM_MS(100),
+ .stream_start = VLC_TICK_FROM_MS(1000),
},
{
.name = "monotonic_pause",
@@ -901,12 +904,16 @@ static struct clock_scenario clock_scenarios[] = {
.type = CLOCK_SCENARIO_RUN,
.run = monotonic_pause_run,
.disable_jitter = true,
+ .system_start = VLC_TICK_FROM_MS(100),
+ .stream_start = VLC_TICK_FROM_MS(1000),
},
{
.name = "master_convert_paused",
.desc = "it is possible to convert ts while paused",
.type = CLOCK_SCENARIO_RUN,
.run = master_convert_paused_run,
+ .system_start = VLC_TICK_FROM_MS(100),
+ .stream_start = VLC_TICK_FROM_MS(1000),
},
{
.name = "monotonic_convert_paused",
@@ -914,6 +921,8 @@ static struct clock_scenario clock_scenarios[] = {
.type = CLOCK_SCENARIO_RUN,
.run = monotonic_convert_paused_run,
.disable_jitter = true,
+ .system_start = VLC_TICK_FROM_MS(100),
+ .stream_start = VLC_TICK_FROM_MS(1000),
},
{
.name = "contexts",
@@ -921,6 +930,8 @@ static struct clock_scenario clock_scenarios[] = {
.type = CLOCK_SCENARIO_RUN,
.run = contexts_run,
.disable_jitter = true,
+ .system_start = VLC_TICK_FROM_MS(100),
+ .stream_start = VLC_TICK_FROM_MS(1000),
},
};
--
GitLab
From 5fa84df1aa37af22a9a09b9510a2717f88cba586 Mon Sep 17 00:00:00 2001
From: Alexandre Janniaux <ajanni@videolabs.io>
Date: Thu, 18 Jul 2024 10:47:40 +0200
Subject: [PATCH 03/28] test: store input vlc_clock
The input vlc_clock will be used to define start_date for the playback
timeline, when vlc_clock_Start() operation will be introduced.
---
test/src/clock/clock.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/test/src/clock/clock.c b/test/src/clock/clock.c
index aa7e6eee7577..c71949bccc54 100644
--- a/test/src/clock/clock.c
+++ b/test/src/clock/clock.c
@@ -85,6 +85,7 @@ struct clock_ctx
vlc_clock_main_t *mainclk;
vlc_clock_t *master;
vlc_clock_t *slave;
+ vlc_clock_t *input;
vlc_tick_t system_start;
vlc_tick_t stream_start;
@@ -329,6 +330,9 @@ static void play_scenario(libvlc_int_t *vlc, struct vlc_tracer *tracer,
assert(mainclk != NULL);
vlc_clock_main_Lock(mainclk);
+ vlc_clock_t *input = vlc_clock_main_CreateInputSlave(mainclk);
+ assert(input != NULL);
+
vlc_clock_t *master = vlc_clock_main_CreateMaster(mainclk, scenario->name,
NULL, NULL);
assert(master != NULL);
@@ -357,6 +361,7 @@ static void play_scenario(libvlc_int_t *vlc, struct vlc_tracer *tracer,
const struct clock_ctx ctx = {
.mainclk = mainclk,
+ .input = input,
.master = master,
.slave = slave,
.scenario = scenario,
@@ -421,6 +426,7 @@ static void play_scenario(libvlc_int_t *vlc, struct vlc_tracer *tracer,
end:
vlc_clock_Delete(master);
vlc_clock_Delete(slave);
+ vlc_clock_Delete(input);
free(slave_name);
vlc_clock_main_Delete(mainclk);
}
--
GitLab
From 02e7a863ea7b1e84f8629f98712bd5dc118e7a12 Mon Sep 17 00:00:00 2001
From: Alexandre Janniaux <ajanni@videolabs.io>
Date: Wed, 9 Oct 2024 12:01:16 +0200
Subject: [PATCH 04/28] test: clock: increase time difference
Using 100 ticks instead of 1 tick allows differentiating
off-by-VLC_TICK_0 errors from pause not working correctly.
---
test/src/clock/clock.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/test/src/clock/clock.c b/test/src/clock/clock.c
index c71949bccc54..db78405bda59 100644
--- a/test/src/clock/clock.c
+++ b/test/src/clock/clock.c
@@ -738,7 +738,7 @@ static void convert_paused_common(const struct clock_ctx *ctx, vlc_clock_t *upda
vlc_clock_main_Lock(ctx->mainclk);
vlc_clock_main_ChangePause(ctx->mainclk, system, true);
vlc_clock_main_Unlock(ctx->mainclk);
- system += 1;
+ system += 100;
vlc_clock_Lock(ctx->slave);
vlc_tick_t converted = vlc_clock_ConvertToSystem(ctx->slave, system, ctx->stream_start, 1.0f, NULL);
--
GitLab
From eabf767ad771562bd1e715647ed12b4466a91545 Mon Sep 17 00:00:00 2001
From: Alexandre Janniaux <ajanni@videolabs.io>
Date: Tue, 8 Oct 2024 19:42:53 +0200
Subject: [PATCH 05/28] test: clock: add logging
---
test/src/clock/clock.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/test/src/clock/clock.c b/test/src/clock/clock.c
index db78405bda59..8652cd47ba60 100644
--- a/test/src/clock/clock.c
+++ b/test/src/clock/clock.c
@@ -570,6 +570,8 @@ static void normal_check(const struct clock_ctx *ctx, size_t update_count,
vlc_clock_ConvertToSystem(ctx->slave, expected_system_end,
stream_end, 1.0f, NULL);
vlc_clock_Unlock(ctx->slave);
+ fprintf(stderr, "%s:%d: converted=%" PRId64 " == expected_system_end=%" PRId64 "\n",
+ __func__, __LINE__, converted, expected_system_end);
assert(converted == expected_system_end);
}
@@ -743,6 +745,9 @@ static void convert_paused_common(const struct clock_ctx *ctx, vlc_clock_t *upda
vlc_clock_Lock(ctx->slave);
vlc_tick_t converted = vlc_clock_ConvertToSystem(ctx->slave, system, ctx->stream_start, 1.0f, NULL);
vlc_clock_Unlock(ctx->slave);
+
+ fprintf(stderr, "%s:%d converted=%" PRId64 " == system_start=%" PRId64 "\n",
+ __func__, __LINE__, converted, system_start);
assert(converted == system_start);
}
@@ -771,7 +776,10 @@ static void contexts_run(const struct clock_ctx *ctx)
/* Check that the converted point is valid */
converted = vlc_clock_ConvertToSystem(ctx->slave, system, stream_context0,
1.0f, &clock_id);
+ fprintf(stderr, "%s:%d: clock_id=%" PRIu32 " == 0\n", __func__, __LINE__, clock_id);
assert(clock_id == 0);
+ fprintf(stderr, "%s:%d: converted=%"PRId64 " == system=%" PRId64 "\n",
+ __func__, __LINE__, converted, system);
assert(converted == system);
vlc_clock_Update(ctx->slave, system, stream_context0, 1.0f);
@@ -784,7 +792,9 @@ static void contexts_run(const struct clock_ctx *ctx)
/* Check that we can use the new context (or new origin) */
converted = vlc_clock_ConvertToSystem(ctx->slave, system, stream_context1,
1.0f, &clock_id);
+ fprintf(stderr, "%s:%d: clock_id=%" PRIu32 " == 1\n", __func__, __LINE__, clock_id);
assert(clock_id == 1);
+ fprintf(stderr, "%s:%d: converted=%"PRId64 " == system=%" PRId64 "\n", __func__, __LINE__, converted, system);
assert(converted == system);
/* Check that we can still use the old context when converting a point
@@ -792,7 +802,10 @@ static void contexts_run(const struct clock_ctx *ctx)
converted = vlc_clock_ConvertToSystem(ctx->slave, system,
VLC_TICK_FROM_MS(10) + stream_context0,
1.0f, &clock_id);
+ fprintf(stderr, "%s:%d: clock_id=%" PRIu32 " == 0\n", __func__, __LINE__, clock_id);
assert(clock_id == 0);
+ fprintf(stderr, "%s:%d: converted=%"PRId64 " == system=%" PRId64 "\n",
+ __func__, __LINE__, converted, system_context0 + VLC_TICK_FROM_MS(10));
assert(converted == system_context0 + VLC_TICK_FROM_MS(10));
/* Update on the newest context will cause previous contexts to be removed */
@@ -806,7 +819,10 @@ static void contexts_run(const struct clock_ctx *ctx)
/* Check that we can use the new context (or new origin) */
converted = vlc_clock_ConvertToSystem(ctx->slave, system, stream_context2,
1.0f, &clock_id);
+ fprintf(stderr, "%s:%d: clock_id=%" PRIu32 " == 2\n", __func__, __LINE__, clock_id);
assert(clock_id == 2);
+ fprintf(stderr, "%s:%d: converted=%"PRId64 " == system=%" PRId64 "\n",
+ __func__, __LINE__, converted, system);
assert(converted == system);
/* Update on the newest context will cause previous contexts to be removed */
vlc_clock_Update(ctx->slave, system, stream_context2, 1.0f);
@@ -816,7 +832,10 @@ static void contexts_run(const struct clock_ctx *ctx)
system += VLC_TICK_FROM_MS(100);
converted = vlc_clock_ConvertToSystem(ctx->slave, system, stream_context1, 1.0f,
&clock_id);
+ fprintf(stderr, "%s:%d: clock_id=%" PRIu32 " == 2\n", __func__, __LINE__, clock_id);
assert(clock_id == 2);
+ fprintf(stderr, "%s:%d: converted=%"PRId64 " == system=%" PRId64 "\n",
+ __func__, __LINE__, converted, system_context0);
assert(converted != system_context0);
vlc_clock_main_Unlock(ctx->mainclk);
--
GitLab
From 7ed71b86cff329d6f77c6b947742ec0e8640b900 Mon Sep 17 00:00:00 2001
From: Alexandre Janniaux <ajanni@videolabs.io>
Date: Tue, 6 Feb 2024 20:27:52 +0100
Subject: [PATCH 06/28] es_out: remove wake-up delay
The delay is added to specify some margin for the start but we actually
want to register the real startup date, since it will be accounted to
startup the monotonic clock and prepare the playback offset from the
input clock.
Co-authored-by: Thomas Guillem <thomas@gllm.fr>
---
src/input/es_out.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/src/input/es_out.c b/src/input/es_out.c
index 6ab2433d1889..f2fe3940f43a 100644
--- a/src/input/es_out.c
+++ b/src/input/es_out.c
@@ -1070,10 +1070,9 @@ static void EsOutDecodersStopBuffering(es_out_sys_t *p_sys, bool b_forced)
EsOutStopFreeVout(p_sys);
/* */
- const vlc_tick_t i_wakeup_delay = VLC_TICK_FROM_MS(10); /* FIXME CLEANUP thread wake up time*/
const vlc_tick_t i_current_date = p_sys->b_paused ? p_sys->i_pause_date : vlc_tick_now();
- const vlc_tick_t update = i_current_date + i_wakeup_delay - i_buffering_duration;
+ const vlc_tick_t update = i_current_date - i_buffering_duration;
/* The call order of these 3 input_clock_t/vlc_clock_main_t functions is
* important:
--
GitLab
From f0e427894b8c5549104753844c8df5b616243ad3 Mon Sep 17 00:00:00 2001
From: Alexandre Janniaux <ajanni@videolabs.io>
Date: Thu, 18 Jul 2024 17:28:29 +0200
Subject: [PATCH 07/28] test_log: move to stderr
Logging to stdout makes weird log interlacing with console logger which
outputs to stderr, and makes it less easy to track synchronize in the
test log file.
---
test/libvlc/test.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/test/libvlc/test.h b/test/libvlc/test.h
index 7ffadf4ba867..aba752177324 100644
--- a/test/libvlc/test.h
+++ b/test/libvlc/test.h
@@ -61,7 +61,7 @@ static const char test_default_sample[] = "mock://";
* Some useful common functions
*/
-#define test_log( ... ) printf( "testapi: " __VA_ARGS__ );
+#define test_log( ... ) fprintf(stderr, "testapi: " __VA_ARGS__);
static inline void test_setup(void)
{
--
GitLab
From 6b4f88374e22d957dba6c68ddb1956edc4025a16 Mon Sep 17 00:00:00 2001
From: Alexandre Janniaux <ajanni@videolabs.io>
Date: Sun, 10 Mar 2024 18:36:47 +0100
Subject: [PATCH 08/28] es_out: reset main clock with input clock
Currently, the main clock is reset once the new buffering is done. This
buffering step will happen at the following locations:
- at the beginning of a playback
- when a seek is triggered by the user
- when the demux needs to trigger RESET_PCR
But the input clock was already reset at this location and will start to
provide new points. Resetting the clock there as well as when the
buffering is done currently changes nothing because we either have
already flushed everything (seek or RESET_PCR) or we are not running
through a clock (frame-by-frame mode).
However, future patches will remove the clock reset at the end of the
buffering and expect the clock to be already reset, to introduce a start
function for the clock.
Co-authored-by: Thomas Guillem <thomas@gllm.fr>
---
src/input/es_out.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/src/input/es_out.c b/src/input/es_out.c
index f2fe3940f43a..36780e4fbe1e 100644
--- a/src/input/es_out.c
+++ b/src/input/es_out.c
@@ -960,7 +960,10 @@ static void EsOutChangePosition(es_out_sys_t *p_sys, bool b_flush,
vlc_list_foreach(pgrm, &p_sys->programs, node)
{
input_clock_Reset(pgrm->p_input_clock);
+ vlc_clock_main_Lock(pgrm->clocks.main);
pgrm->i_last_pcr = VLC_TICK_INVALID;
+ vlc_clock_main_Reset(pgrm->clocks.main);
+ vlc_clock_main_Unlock(pgrm->clocks.main);
}
p_sys->b_buffering = true;
--
GitLab
From 2682cfca1036d065b01c83f4886ae5857b9e6669 Mon Sep 17 00:00:00 2001
From: Alexandre Janniaux <ajanni@videolabs.io>
Date: Thu, 3 Oct 2024 10:32:09 +0200
Subject: [PATCH 09/28] clock: factor the offset computation
We'll be changing the way the offset is computed, and reproduce the
computation at multiple locations. Factoring the computation in a
function will allow to simplify the rest of the changes.
---
src/clock/clock.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/src/clock/clock.c b/src/clock/clock.c
index c30ac6cfdc02..d14547b80642 100644
--- a/src/clock/clock.c
+++ b/src/clock/clock.c
@@ -201,6 +201,17 @@ static inline void TraceRender(struct vlc_tracer *tracer, const char *type,
VLC_TRACE_END);
}
+static vlc_tick_t ComputeOffset(vlc_clock_main_t *main,
+ struct vlc_clock_context *ctx,
+ vlc_tick_t system_now,
+ vlc_tick_t ts,
+ double rate)
+{
+ (void)main;
+
+ return system_now - ((vlc_tick_t) (ts * ctx->coeff / rate));
+}
+
static void context_reset(struct vlc_clock_context *ctx)
{
ctx->coeff = 1.0f;
@@ -437,7 +448,7 @@ static void vlc_clock_master_update_coeff(
vlc_clock_SendEvent(main_clock, discontinuity);
}
- ctx->offset = system_now - ((vlc_tick_t) (ts * ctx->coeff / rate));
+ ctx->offset = ComputeOffset(main_clock, ctx, system_now, ts, rate);
if (main_clock->tracer != NULL && clock->track_str_id != NULL)
vlc_tracer_Trace(main_clock->tracer,
--
GitLab
From 47e7a4d25a381804ad6f2e4659891836447ded67 Mon Sep 17 00:00:00 2001
From: Alexandre Janniaux <ajanni@videolabs.io>
Date: Mon, 15 Jul 2024 11:22:11 +0200
Subject: [PATCH 10/28] clock: implement vlc_clock_Start
This commit creates a new way in the clock system to intercept the
behaviour of the "startup" depending on whether the clock is for the
input or the output and whether it's a reference clock or a derived one.
Allowing to specify this behaviour contributes to unify the usage of the
clock, so that it's easier to use in the future (think start() with the
current time and startup PCR as planned vs SetFirstPCR() with a
corrected startup time and the last PCR received). It also contributes
to improve the cases where the input is a reference clock and paced by
the source.
The es_out is handling the current startup sequence for the clock by
resetting the clock, shifting the input clock and setting the startup
time again. Those functions are mixing three different components and
behaviour and makes it hard to test the startup scenarios.
vlc_clock_Start() on the input vlc_clock_t will replace those calls by
providing the correct offset on the output points instead of
transforming the input points.
In addition, vlc_clock_Start() will be provided to the output clocks so
that they can provide the wait_sync reference point, instead of relying
on the first conversion done, and specify the exact startup time for the
output clocks by computing the additional delay at the moment the
playback really starts.
Like first_pcr, start_time must be shifted when pause is set. It would
also be possible to "reset" the start_time when un-pausing to the
un-pause date.
Note that this commit doesn't change the behaviour yet, and the
consequences it brings will be seen when SetFirstPCR is effectively
removed. Current case coverage is also quite low in tests, so failures
would happen without SetFirstPCR in a random way.
Co-authored-by: Thomas Guillem <thomas@gllm.fr>
---
src/clock/clock.c | 113 +++++++++++++++++++++++++++++++++++++++++++++-
src/clock/clock.h | 12 +++++
2 files changed, 123 insertions(+), 2 deletions(-)
diff --git a/src/clock/clock.c b/src/clock/clock.c
index d14547b80642..b91ade8f6d52 100644
--- a/src/clock/clock.c
+++ b/src/clock/clock.c
@@ -55,6 +55,12 @@ struct vlc_clock_context
clock_point_t last;
clock_point_t wait_sync_ref;
+ /**
+ * Start point emitted by the buffering to indicate when we supposedly
+ * started the playback. It does not account for latency of the output
+ * when one of them is driving the main_clock. */
+ clock_point_t start_time;
+
struct vlc_list node;
struct vlc_list using_clocks;
};
@@ -83,6 +89,7 @@ struct vlc_clock_main_t
unsigned wait_sync_ref_priority;
clock_point_t first_pcr;
+
vlc_tick_t output_dejitter; /* Delay used to absorb the output clock jitter */
vlc_tick_t input_dejitter; /* Delay used to absorb the input jitter */
@@ -99,6 +106,21 @@ struct vlc_clock_ops
vlc_tick_t (*set_delay)(vlc_clock_t *clock, vlc_tick_t delay);
vlc_tick_t (*to_system)(vlc_clock_t *clock, struct vlc_clock_context *ctx,
vlc_tick_t system_now, vlc_tick_t ts, double rate);
+
+ /**
+ * Signal the clock bus that the given clock will start a new timeline.
+ *
+ * When starting an input clock, a whole new context is created to track
+ * the parameters for the new timeline.
+ *
+ * When starting an output clock, the given clock will track the new context
+ * to register the progress and convert the timestamps.
+ *
+ * \param clock the clock which is ready to start
+ * \param system_now the system time for the start point
+ * \param ts the time for the start point
+ **/
+ void (*start)(vlc_clock_t *clock, vlc_tick_t system_now, vlc_tick_t ts);
};
struct vlc_clock_t
@@ -219,6 +241,7 @@ static void context_reset(struct vlc_clock_context *ctx)
ctx->offset = VLC_TICK_INVALID;
ctx->wait_sync_ref = clock_point_Create(VLC_TICK_INVALID, VLC_TICK_INVALID);
ctx->last = clock_point_Create(VLC_TICK_INVALID, VLC_TICK_INVALID);
+ ctx->start_time = clock_point_Create(VLC_TICK_INVALID, VLC_TICK_INVALID);
}
static void context_init(struct vlc_clock_context *ctx)
@@ -557,6 +580,39 @@ static vlc_tick_t vlc_clock_master_set_delay(vlc_clock_t *clock, vlc_tick_t dela
return delta;
}
+static void
+vlc_clock_input_start(vlc_clock_t *clock,
+ vlc_tick_t start_date,
+ vlc_tick_t first_ts)
+{
+ vlc_clock_main_t *main_clock = clock->owner;
+ vlc_mutex_assert(&main_clock->lock);
+
+ struct vlc_clock_context *context = main_clock->context;
+ if (main_clock->first_pcr.system != VLC_TICK_INVALID)
+ return;
+
+ if (context->start_time.system != VLC_TICK_INVALID
+ && (context->last.system != VLC_TICK_INVALID ||
+ context->wait_sync_ref.stream != VLC_TICK_INVALID))
+ {
+ assert(context->start_time.stream != VLC_TICK_INVALID);
+ struct vlc_clock_context *new_context = malloc(sizeof(*new_context));
+ if (new_context != NULL)
+ {
+ *new_context = *context;
+ vlc_list_init(&new_context->using_clocks);
+ vlc_list_append(&new_context->node, &main_clock->prev_contexts);
+ }
+ context->clock_id++;
+ }
+
+ context_reset(context);
+ context->start_time = clock_point_Create(start_date, first_ts);
+ context->offset = ComputeOffset(main_clock, context, start_date, first_ts, 1.f);
+ main_clock->wait_sync_ref_priority = UINT_MAX;
+}
+
static vlc_tick_t
vlc_clock_monotonic_to_system(vlc_clock_t *clock, struct vlc_clock_context *ctx,
vlc_tick_t now, vlc_tick_t ts, double rate)
@@ -680,6 +736,29 @@ static vlc_tick_t vlc_clock_slave_set_delay(vlc_clock_t *clock, vlc_tick_t delay
return 0;
}
+static void
+vlc_clock_output_start(vlc_clock_t *clock,
+ vlc_tick_t start_date,
+ vlc_tick_t first_ts)
+{
+ (void)start_date; (void)first_ts;
+
+ vlc_clock_main_t *main_clock = clock->owner;
+ vlc_mutex_assert(&main_clock->lock);
+
+ struct vlc_clock_context *context = main_clock->context;
+#if 0
+ /* Disabled for now, the handling will be done later. */
+ /* vlc_clock_Start must have already been called. */
+ assert(context->start_time.system != VLC_TICK_INVALID);
+ assert(context->start_time.stream != VLC_TICK_INVALID);
+#endif
+ if (context->start_time.system == VLC_TICK_INVALID)
+ return;
+
+ /* Do nothing for now. */
+}
+
void vlc_clock_Lock(vlc_clock_t *clock)
{
vlc_clock_main_t *main_clock = clock->owner;
@@ -750,6 +829,9 @@ vlc_clock_main_t *vlc_clock_main_New(struct vlc_logger *parent_logger, struct vl
clock_point_Create(VLC_TICK_INVALID, VLC_TICK_INVALID);
main_clock->wait_sync_ref_priority = UINT_MAX;
+ ctx->start_time =
+ clock_point_Create(VLC_TICK_INVALID, VLC_TICK_INVALID);
+
main_clock->pause_date = VLC_TICK_INVALID;
main_clock->input_dejitter = DEFAULT_PTS_DELAY;
main_clock->output_dejitter = AOUT_MAX_PTS_ADVANCE * 2;
@@ -845,6 +927,8 @@ void vlc_clock_main_ChangePause(vlc_clock_main_t *main_clock, vlc_tick_t now,
}
if (main_clock->first_pcr.system != VLC_TICK_INVALID)
main_clock->first_pcr.system += delay;
+ if (ctx->start_time.system != VLC_TICK_INVALID)
+ ctx->start_time.system += delay;
if (ctx->wait_sync_ref.system != VLC_TICK_INVALID)
ctx->wait_sync_ref.system += delay;
main_clock->pause_date = VLC_TICK_INVALID;
@@ -933,6 +1017,7 @@ static const struct vlc_clock_ops master_ops = {
.reset = vlc_clock_master_reset,
.set_delay = vlc_clock_master_set_delay,
.to_system = vlc_clock_master_to_system,
+ .start = vlc_clock_output_start,
};
static const struct vlc_clock_ops slave_ops = {
@@ -940,6 +1025,23 @@ static const struct vlc_clock_ops slave_ops = {
.reset = vlc_clock_slave_reset,
.set_delay = vlc_clock_slave_set_delay,
.to_system = vlc_clock_slave_to_system,
+ .start = vlc_clock_output_start,
+};
+
+static const struct vlc_clock_ops input_master_ops = {
+ .update = vlc_clock_master_update,
+ .reset = vlc_clock_master_reset,
+ .set_delay = vlc_clock_master_set_delay,
+ .to_system = vlc_clock_master_to_system,
+ .start = vlc_clock_input_start,
+};
+
+static const struct vlc_clock_ops input_slave_ops = {
+ .update = vlc_clock_slave_update,
+ .reset = vlc_clock_slave_reset,
+ .set_delay = vlc_clock_slave_set_delay,
+ .to_system = vlc_clock_slave_to_system,
+ .start = vlc_clock_input_start,
};
static vlc_clock_t *vlc_clock_main_Create(vlc_clock_main_t *main_clock,
@@ -1024,7 +1126,7 @@ vlc_clock_t *vlc_clock_main_CreateInputMaster(vlc_clock_main_t *main_clock)
if (main_clock->master != NULL)
main_clock->master->ops = &slave_ops;
- clock->ops = &master_ops;
+ clock->ops = &input_master_ops;
main_clock->input_master = clock;
main_clock->rc++;
@@ -1041,7 +1143,7 @@ vlc_clock_t *vlc_clock_main_CreateInputSlave(vlc_clock_main_t *main_clock)
if (!clock)
return NULL;
- clock->ops = &slave_ops;
+ clock->ops = &input_slave_ops;
main_clock->rc++;
return clock;
@@ -1114,3 +1216,10 @@ void vlc_clock_Delete(vlc_clock_t *clock)
vlc_mutex_unlock(&main_clock->lock);
free(clock);
}
+
+void vlc_clock_Start(vlc_clock_t *clock,
+ vlc_tick_t start_date,
+ vlc_tick_t first_ts)
+{
+ clock->ops->start(clock, start_date, first_ts);
+}
diff --git a/src/clock/clock.h b/src/clock/clock.h
index ddc9c0e8b29f..7b0557096fde 100644
--- a/src/clock/clock.h
+++ b/src/clock/clock.h
@@ -330,4 +330,16 @@ vlc_tick_t vlc_clock_ConvertToSystem(vlc_clock_t *clock,
vlc_tick_t system_now, vlc_tick_t ts,
double rate, uint32_t *clock_id);
+/**
+ * Starts a new clock based on the given clock point, accounting for
+ * previous updates.
+ *
+ * @param clock an input or output clock that is being started
+ * @param start_date the system time at which the first update starts
+ * @param first_ts the media time origin
+ **/
+void vlc_clock_Start(vlc_clock_t *clock,
+ vlc_tick_t start_date,
+ vlc_tick_t first_ts);
+
#endif /*CLOCK_H*/
--
GitLab
From 90a94a03895d7f9b7a3cdf0f099452652e868b46 Mon Sep 17 00:00:00 2001
From: Alexandre Janniaux <ajanni@videolabs.io>
Date: Thu, 3 Oct 2024 10:32:09 +0200
Subject: [PATCH 11/28] clock: reset by computing offset from coeff
Resetting the main_clock breaks the clock context information,
especially regarding startup and monotonic synchronization. We only want
to reset the coefficient to 1.f, and recompute the offset accordingly.
---
src/clock/clock.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/clock/clock.c b/src/clock/clock.c
index b91ade8f6d52..1be83aa362ed 100644
--- a/src/clock/clock.c
+++ b/src/clock/clock.c
@@ -453,7 +453,9 @@ static void vlc_clock_master_update_coeff(
/* Reset and continue (calculate the offset from the
* current point) */
- vlc_clock_main_reset(main_clock);
+ ctx->coeff = 1.f;
+ AvgResetAndFill(&main_clock->coeff_avg, 1.);
+ ctx->offset = ComputeOffset(main_clock, ctx, system_now, ts, rate);
}
else
{
--
GitLab
From 67ec80ac507ea9c2ad743f8bfb2a57466d61fd7c Mon Sep 17 00:00:00 2001
From: Alexandre Janniaux <ajanni@videolabs.io>
Date: Fri, 19 Jul 2024 12:09:41 +0200
Subject: [PATCH 12/28] clock: use 0 instead of VLC_TICK_INVALID for offset
Offset is a time difference, so it should really be initialized to 0 and
not VLC_TICK_INVALID.
---
src/clock/clock.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/clock/clock.c b/src/clock/clock.c
index 1be83aa362ed..c654aa3d86b8 100644
--- a/src/clock/clock.c
+++ b/src/clock/clock.c
@@ -238,7 +238,7 @@ static void context_reset(struct vlc_clock_context *ctx)
{
ctx->coeff = 1.0f;
ctx->rate = 1.0f;
- ctx->offset = VLC_TICK_INVALID;
+ ctx->offset = 0;
ctx->wait_sync_ref = clock_point_Create(VLC_TICK_INVALID, VLC_TICK_INVALID);
ctx->last = clock_point_Create(VLC_TICK_INVALID, VLC_TICK_INVALID);
ctx->start_time = clock_point_Create(VLC_TICK_INVALID, VLC_TICK_INVALID);
@@ -1122,7 +1122,7 @@ vlc_clock_t *vlc_clock_main_CreateInputMaster(vlc_clock_main_t *main_clock)
/* Even if the master ES clock has already been created, it should not
* have updated any points */
- assert(ctx->offset == VLC_TICK_INVALID); (void) ctx;
+ assert(ctx->offset == 0); (void) ctx;
/* Override the master ES clock if it exists */
if (main_clock->master != NULL)
--
GitLab
From 0ec64c5b2db75fe1d4f4f3fb08b4f31bebf0e0b1 Mon Sep 17 00:00:00 2001
From: Alexandre Janniaux <ajanni@videolabs.io>
Date: Thu, 30 Jan 2025 12:01:52 +0100
Subject: [PATCH 13/28] clock: clock_master: report 0 as drift
...instead of VLC_TICK_INVALID. Drift is a time difference and a
reference clock is never drifting.
---
src/clock/clock.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/clock/clock.c b/src/clock/clock.c
index c654aa3d86b8..2a2da8c9ff1c 100644
--- a/src/clock/clock.c
+++ b/src/clock/clock.c
@@ -515,7 +515,7 @@ static vlc_tick_t vlc_clock_master_update(vlc_clock_t *clock,
if (clock->delay > 0 && main_clock->delay < 0 && ts > -main_clock->delay)
ts += main_clock->delay;
- vlc_tick_t drift = VLC_TICK_INVALID;
+ vlc_tick_t drift = 0;
vlc_clock_on_update(clock, system_now, ts, drift,
rate, frame_rate, frame_rate_base);
return drift;
--
GitLab
From e583c6324d280851ba2d30a6a226ff278ecaff19 Mon Sep 17 00:00:00 2001
From: Alexandre Janniaux <ajanni@videolabs.io>
Date: Tue, 16 Jul 2024 17:54:22 +0200
Subject: [PATCH 14/28] clock: compute offset from start_time
Previously, the clock was driven by two values: coeff and offset. The
idea was that since the clock is supposed to look like an affine line,
we could extrapolate any `ts` media point into `ts * coeff + offset` in
system time.
It meant that offset could take any value depending on the difference
between system time and media time, reducing the interpretability of the
values.
After this patch, offset is computed from the start of the stream, so
it will basically only include the difference between the timestamp
computation and the real clock.
Co-authored-by: Thomas Guillem <thomas@gllm.fr>
---
src/clock/clock.c | 30 +++++++++++++++++++++---------
1 file changed, 21 insertions(+), 9 deletions(-)
diff --git a/src/clock/clock.c b/src/clock/clock.c
index 2a2da8c9ff1c..d9aa4a374e72 100644
--- a/src/clock/clock.c
+++ b/src/clock/clock.c
@@ -229,9 +229,14 @@ static vlc_tick_t ComputeOffset(vlc_clock_main_t *main,
vlc_tick_t ts,
double rate)
{
- (void)main;
-
- return system_now - ((vlc_tick_t) (ts * ctx->coeff / rate));
+ vlc_tick_t start_stream = ctx->start_time.stream - VLC_TICK_0;
+ vlc_tick_t start_system = ctx->start_time.system - VLC_TICK_0;
+ if (main->first_pcr.system != VLC_TICK_INVALID)
+ start_stream = start_system = 0;
+
+ return system_now
+ - ((vlc_tick_t) ((ts - start_stream) * ctx->coeff / rate))
+ - start_system;
}
static void context_reset(struct vlc_clock_context *ctx)
@@ -265,7 +270,10 @@ static vlc_tick_t context_stream_to_system(const struct vlc_clock_context *ctx,
{
if (ctx->last.system == VLC_TICK_INVALID)
return VLC_TICK_INVALID;
- return ((vlc_tick_t) (ts * ctx->coeff / ctx->rate)) + ctx->offset;
+
+ return ((vlc_tick_t) ((ts - ctx->start_time.stream) * ctx->coeff / ctx->rate))
+ + ctx->offset
+ + ctx->start_time.system;
}
static void
@@ -628,10 +636,13 @@ vlc_clock_monotonic_to_system(vlc_clock_t *clock, struct vlc_clock_context *ctx,
* ride of the input clock. This code is adapted from input_clock.c and
* is used to introduce the same delay than the input clock (first PTS
* - first PCR). */
- vlc_tick_t pcr_delay =
- main_clock->first_pcr.system == VLC_TICK_INVALID ? 0 :
- (ts - main_clock->first_pcr.stream) / rate +
- main_clock->first_pcr.system - now;
+ vlc_tick_t pcr_delay = 0;
+ if (main_clock->first_pcr.system != VLC_TICK_INVALID)
+ pcr_delay = (ts - main_clock->first_pcr.stream) / rate +
+ main_clock->first_pcr.system - now;
+ else if (ctx->start_time.system != VLC_TICK_INVALID)
+ pcr_delay = (ts - ctx->start_time.stream) / rate +
+ ctx->start_time.system - now;
if (pcr_delay > MAX_PCR_DELAY)
{
@@ -925,7 +936,8 @@ void vlc_clock_main_ChangePause(vlc_clock_main_t *main_clock, vlc_tick_t now,
if (ctx->last.system != VLC_TICK_INVALID)
{
ctx->last.system += delay;
- ctx->offset += delay;
+ if (ctx->start_time.system == VLC_TICK_INVALID)
+ ctx->offset += delay;
}
if (main_clock->first_pcr.system != VLC_TICK_INVALID)
main_clock->first_pcr.system += delay;
--
GitLab
From 324d1a551def164a107583a62d6cb18250b748ad Mon Sep 17 00:00:00 2001
From: Alexandre Janniaux <ajanni@videolabs.io>
Date: Mon, 26 Feb 2024 19:42:12 +0100
Subject: [PATCH 15/28] test: add clock_start test
The test is extending the current clock.c test but is specifically
designed to check the clock startup scenarios. Those tests are also
matching with the "SCENARIO_RUN" case in the clock.c test.
It doesn't need the tracer infrastructure to run.
Co-authored-by: Thomas Guillem <thomas@gllm.fr>
---
test/Makefile.am | 6 +
test/src/clock/clock_start.c | 462 +++++++++++++++++++++++++++++++++++
test/src/meson.build | 10 +
3 files changed, 478 insertions(+)
create mode 100644 test/src/clock/clock_start.c
diff --git a/test/Makefile.am b/test/Makefile.am
index 0ae5a58814b1..17fbb3a3fdbc 100644
--- a/test/Makefile.am
+++ b/test/Makefile.am
@@ -25,6 +25,7 @@ check_PROGRAMS = \
test_libvlc_slaves \
test_src_config_chain \
test_src_clock_clock \
+ test_src_clock_start \
test_src_misc_ancillary \
test_src_misc_variables \
test_src_input_stream \
@@ -166,6 +167,11 @@ test_src_clock_clock_SOURCES = src/clock/clock.c \
../src/clock/clock.c \
../src/clock/clock_internal.c
test_src_clock_clock_LDADD = $(LIBVLCCORE) $(LIBVLC)
+test_src_clock_start_SOURCES = src/clock/clock_start.c \
+ ../src/clock/clock.c \
+ ../src/clock/clock_internal.c
+test_src_clock_start_LDADD = $(LIBVLCCORE) $(LIBVLC)
+
test_src_misc_ancillary_SOURCES = src/misc/ancillary.c
test_src_misc_ancillary_LDADD = $(LIBVLCCORE) $(LIBVLC)
test_src_misc_variables_SOURCES = src/misc/variables.c
diff --git a/test/src/clock/clock_start.c b/test/src/clock/clock_start.c
new file mode 100644
index 000000000000..efdd39c31366
--- /dev/null
+++ b/test/src/clock/clock_start.c
@@ -0,0 +1,462 @@
+/*****************************************************************************
+ * clock/clock_start.c: test for the vlc clock
+ *****************************************************************************
+ * Copyright (C) 2024 Videolabs
+ *
+ * Authors: Alexandre Janniaux <ajanni@videolabs.io>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU Lesser General Public License as published by
+ * the Free Software Foundation; either version 2.1 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston MA 02110-1301, USA.
+ *****************************************************************************/
+
+#ifdef HAVE_CONFIG_H
+# include "config.h"
+#endif
+
+#include <vlc_common.h>
+#include <vlc_tick.h>
+#include <vlc_es.h>
+#include <vlc_tracer.h>
+
+#include "../../../src/clock/clock.h"
+
+#include <vlc/vlc.h>
+#include "../../libvlc/test.h"
+#include "../../../lib/libvlc_internal.h"
+
+#define MODULE_NAME test_clock_clock
+#undef VLC_DYNAMIC_PLUGIN
+
+#include <vlc_plugin.h>
+#include <vlc_vector.h>
+
+/* Define a builtin module for mocked parts */
+const char vlc_module_name[] = MODULE_STRING;
+
+/**
+ * Fixture structure containing the clocks used for each test.
+ */
+struct clock_fixture_simple {
+ vlc_clock_main_t *main;
+ vlc_clock_t *input;
+ vlc_clock_t *master;
+ vlc_clock_t *slave;
+};
+
+struct clock_point
+{
+ vlc_tick_t system;
+ vlc_tick_t stream;
+};
+
+/**
+ * Initialize the fixture.
+ *
+ * It must be released using \ref destroy_fixture_simple.
+ */
+static struct clock_fixture_simple
+init_fixture_simple(struct vlc_logger *logger,
+ bool is_input_master,
+ vlc_tick_t input_dejitter,
+ vlc_tick_t output_dejitter)
+{
+ struct clock_fixture_simple f = {
+ .main = vlc_clock_main_New(logger, NULL),
+ };
+ assert(f.main != NULL);
+
+ vlc_clock_main_Lock(f.main);
+ vlc_clock_main_SetInputDejitter(f.main, input_dejitter);
+ vlc_clock_main_SetDejitter(f.main, output_dejitter);
+
+ if (is_input_master)
+ {
+ f.input = vlc_clock_main_CreateInputMaster(f.main);
+ f.master = f.input;
+ }
+ else
+ {
+ f.input = vlc_clock_main_CreateInputSlave(f.main);
+ f.master = vlc_clock_main_CreateMaster(f.main, "Driving", NULL, NULL);
+ }
+ assert(f.input != NULL);
+ assert(f.master != NULL);
+
+ f.slave = vlc_clock_main_CreateSlave(f.main, "Output", AUDIO_ES,
+ NULL, NULL);
+ assert(f.slave != NULL);
+ vlc_clock_main_Unlock(f.main);
+
+ return f;
+}
+
+static void destroy_fixture_simple(struct clock_fixture_simple *f)
+{
+ vlc_clock_Delete(f->slave);
+ if (f->master != f->input)
+ vlc_clock_Delete(f->master);
+ vlc_clock_Delete(f->input);
+ vlc_clock_main_Delete(f->main);
+}
+
+/**
+ * Check that conversion doesn't work as long as clock has not
+ * been started.
+ **/
+#if 0
+static void test_clock_without_start(struct vlc_logger *logger)
+{
+ struct clock_fixture_simple f = init_fixture_simple(logger, true, 0, 0);
+
+ vlc_clock_main_Lock(f.main);
+ const vlc_tick_t now = VLC_TICK_FROM_MS(1000);
+ {
+ vlc_clock_Update(f.master, now, now * 1000, 1.);
+ vlc_tick_t result = vlc_clock_ConvertToSystem(f.slave, now, now * 1000, 1., NULL);
+ //assert(result == VLC_TICK_INVALID);
+ }
+
+ vlc_clock_Start(f.slave, now, now * 1000);
+
+ {
+ vlc_tick_t result = vlc_clock_ConvertToSystem(f.slave, now, now * 1000, 1., NULL);
+ assert(result == VLC_TICK_INVALID);
+ }
+ vlc_clock_main_Unlock(f.main);
+
+ destroy_fixture_simple(&f);
+}
+#endif
+
+/**
+ * Check that conversion works and is valid after the clock start.
+ **/
+static void test_clock_with_start(struct vlc_logger *logger)
+{
+ fprintf(stderr, "%s:\n", __func__);
+ struct clock_fixture_simple f = init_fixture_simple(logger, true, 0, 0);
+ const vlc_tick_t now = VLC_TICK_FROM_MS(1000);
+
+ vlc_clock_main_Lock(f.main);
+ vlc_clock_Update(f.master, now, now * 1000, 1.);
+ vlc_clock_Start(f.input, now, now * 1000);
+ vlc_clock_Start(f.slave, now, now * 1000);
+
+ vlc_tick_t result = vlc_clock_ConvertToSystem(f.slave, now, now * 1000, 1., NULL);
+ assert(result != VLC_TICK_INVALID);
+ assert(result == now);
+ vlc_clock_main_Unlock(f.main);
+
+ destroy_fixture_simple(&f);
+}
+
+/**
+ * Check that master clock can start.
+ **/
+static void test_clock_with_start_master(struct vlc_logger *logger)
+{
+ fprintf(stderr, "%s:\n", __func__);
+ struct clock_fixture_simple f = init_fixture_simple(logger, false, 0, 0);
+ const vlc_tick_t now = VLC_TICK_FROM_MS(1000);
+
+ vlc_clock_main_Lock(f.main);
+ vlc_clock_Start(f.input, now, now * 1000);
+ vlc_clock_Start(f.master, now, now * 1000);
+ vlc_clock_Start(f.slave, now, now * 1000);
+ vlc_clock_Update(f.master, now, now * 1000, 1.);
+
+ vlc_tick_t result = vlc_clock_ConvertToSystem(f.slave, now, now * 1000, 1., NULL);
+ assert(result != VLC_TICK_INVALID);
+ assert(result == now);
+ vlc_clock_main_Unlock(f.main);
+
+ destroy_fixture_simple(&f);
+}
+
+/**
+ * Check that conversion works and is valid after the clock start,
+ * when start is done later.
+ **/
+static void test_clock_with_output_dejitter(struct vlc_logger *logger)
+{
+ fprintf(stderr, "%s:\n", __func__);
+ struct clock_fixture_simple f = init_fixture_simple(logger, true, 0, 0);
+ vlc_clock_main_Lock(f.main);
+
+ const vlc_tick_t now = VLC_TICK_FROM_MS(1000);
+ vlc_clock_Update(f.master, now, now * 1000, 1.);
+ vlc_clock_Start(f.input, 2 * now, now * 1000);
+ vlc_clock_Start(f.slave, 2 * now, now * 1000);
+
+ vlc_tick_t result = vlc_clock_ConvertToSystem(f.slave, now, now * 1000, 1., NULL);
+ fprintf(stderr, "%s:%d: now=%" PRId64 " converted=%" PRId64 "\n",
+ __func__, __LINE__, now, result);
+ assert(result != VLC_TICK_INVALID);
+ assert(result == 2 * now);
+ vlc_clock_main_Unlock(f.main);
+
+ destroy_fixture_simple(&f);
+}
+
+/**
+ * Check that clock start is working correctly with an output driving
+ * the clock bus.
+ **/
+static void test_clock_with_output_clock_start(struct vlc_logger *logger)
+{
+ fprintf(stderr, "%s:\n", __func__);
+ struct clock_fixture_simple f = init_fixture_simple(logger, false, 0, 0);
+ vlc_clock_main_Lock(f.main);
+
+ /* When the input is not driving the clock bus, we should be able to start
+ * without any reference point since the output will provide it later. */
+ const vlc_tick_t now = VLC_TICK_FROM_MS(1000);
+ vlc_clock_Start(f.input, now, now * 1000);
+ vlc_clock_Start(f.slave, now, now * 1000);
+
+ /* We provide the clock point to the clock bus. The system time should
+ * approximately match the start date. */
+ vlc_clock_Update(f.master, now, now * 1000, 1.);
+
+ vlc_tick_t result = vlc_clock_ConvertToSystem(f.slave, now, now * 1000, 1., NULL);
+ assert(result != VLC_TICK_INVALID);
+ assert(result == now);
+ vlc_clock_main_Unlock(f.main);
+
+ destroy_fixture_simple(&f);
+}
+
+/**
+ * Check that monotonic clock start is working correctly.
+ **/
+static void test_clock_with_monotonic_clock(struct vlc_logger *logger)
+{
+ fprintf(stderr, "%s:\n", __func__);
+ struct clock_fixture_simple f = init_fixture_simple(logger, false, 0, 0);
+ vlc_clock_main_Lock(f.main);
+
+ /* When the input is not driving the clock bus, we should be able to start
+ * without any reference point since the output will provide it later. */
+ const vlc_tick_t now = VLC_TICK_FROM_MS(1000);
+ vlc_clock_Start(f.input, now, now * 1000);
+
+ /* The output wants to start before a reference point is found. */
+ vlc_clock_Start(f.slave, now, now * 1000);
+
+ // TODO
+ vlc_tick_t result = vlc_clock_ConvertToSystem(f.slave, now, now * 1000, 1., NULL);
+ assert(result != VLC_TICK_INVALID);
+ assert(result == now);
+ vlc_clock_main_Unlock(f.main);
+
+
+ destroy_fixture_simple(&f);
+}
+
+/**
+ * Check that monotonic clock start accounts for the start date.
+ **/
+static vlc_tick_t test_clock_with_monotonic_clock_start(
+ struct vlc_logger *logger,
+ vlc_tick_t input_dejitter,
+ vlc_tick_t output_dejitter,
+ struct clock_point start_point,
+ struct clock_point output_point,
+ struct clock_point convert_point
+){
+ fprintf(stderr, "%s:\n", __func__);
+ struct clock_fixture_simple f = init_fixture_simple(logger, false, input_dejitter, output_dejitter);
+ vlc_clock_main_Lock(f.main);
+
+ /* When the input is not driving the clock bus, we should be able to start
+ * without any reference point since the output will provide it later. */
+ vlc_clock_Start(f.input, start_point.system, start_point.stream);
+
+ /* The output wants to start before a reference point is found. */
+ vlc_clock_Start(f.slave, output_point.system, output_point.stream);
+
+ vlc_tick_t result = vlc_clock_ConvertToSystem(f.slave, convert_point.system, convert_point.stream, 1., NULL);
+ vlc_clock_main_Unlock(f.main);
+
+ destroy_fixture_simple(&f);
+ return result;
+}
+
+static vlc_tick_t test_clock_slave_update(
+ struct vlc_logger *logger,
+ struct clock_point start_point,
+ struct clock_point output_point
+){
+ fprintf(stderr, "%s:\n", __func__);
+ struct clock_fixture_simple f = init_fixture_simple(logger, false, 0, 0);
+ vlc_clock_main_Lock(f.main);
+
+ /* When the input is not driving the clock bus, we should be able to start
+ * without any reference point since the output will provide it later. */
+ vlc_clock_Start(f.input, start_point.system, start_point.stream);
+ vlc_clock_Update(f.master, start_point.system, start_point.stream, 1.);
+
+ /* The output wants to start before a reference point is found. */
+ vlc_clock_Start(f.slave, output_point.system, output_point.stream);
+
+ vlc_tick_t result = vlc_clock_Update(f.slave, output_point.system, output_point.stream, 1.);
+ vlc_clock_main_Unlock(f.main);
+
+ destroy_fixture_simple(&f);
+ return result;
+}
+
+static void test_clock_start_reset(
+ struct vlc_logger *logger
+){
+ fprintf(stderr, "%s:\n", __func__);
+ struct clock_fixture_simple f = init_fixture_simple(logger, false, 0, 0);
+ vlc_clock_main_Lock(f.main);
+
+ const vlc_tick_t now = VLC_TICK_FROM_MS(1000);
+ struct clock_point start_point;
+ start_point = (struct clock_point){ now, now * 1000 };
+
+ /* When the input is not driving the clock bus, we should be able to start
+ * without any reference point since the output will provide it later. */
+ vlc_clock_Start(f.input, start_point.system, start_point.stream);
+ vlc_clock_Update(f.master, start_point.system, start_point.stream, 1.);
+ vlc_clock_main_Reset(f.main);
+
+ vlc_tick_t result;
+ result = vlc_clock_ConvertToSystem(f.slave, start_point.system, start_point.stream, 1., NULL);
+ //assert(result == VLC_TICK_INVALID);
+
+ start_point.system *= 2;
+ vlc_clock_Start(f.input, start_point.system, start_point.stream);
+ vlc_clock_Update(f.master, start_point.system, start_point.stream, 1.);
+ result = vlc_clock_ConvertToSystem(f.slave, start_point.system, start_point.stream, 1., NULL);
+ assert(result == start_point.system);
+ vlc_clock_main_Unlock(f.main);
+
+ destroy_fixture_simple(&f);
+}
+
+static void test_clock_slave_only(
+ struct vlc_logger *logger
+){
+ fprintf(stderr, "%s:\n", __func__);
+ struct clock_fixture_simple f = init_fixture_simple(logger, false, 0, 0);
+ vlc_clock_main_Lock(f.main);
+
+ const vlc_tick_t now = VLC_TICK_FROM_MS(1000);
+ struct clock_point start_point = { now, now * 1000 };
+
+ /* When the input is not driving the clock bus, we should be able to start
+ * without any reference point since the output will provide it later. */
+ vlc_clock_Start(f.input, start_point.system, start_point.stream);
+
+ vlc_clock_Start(f.slave, start_point.system, start_point.stream);
+ vlc_tick_t result;
+ result = vlc_clock_ConvertToSystem(f.slave, start_point.system, start_point.stream, 1., NULL);
+ assert(result == start_point.system);
+
+ result = vlc_clock_ConvertToSystem(f.slave, start_point.system, start_point.stream + 1000, 1., NULL);
+ assert(result == start_point.system + 1000);
+ vlc_clock_main_Unlock(f.main);
+
+ destroy_fixture_simple(&f);
+}
+int main(void)
+{
+ libvlc_instance_t *libvlc = libvlc_new(0, NULL);
+ assert(libvlc != NULL);
+
+ libvlc_int_t *vlc = libvlc->p_libvlc_int;
+ struct vlc_logger *logger = vlc->obj.logger;
+
+ const vlc_tick_t now = VLC_TICK_FROM_MS(1000);
+
+ //test_clock_without_start(logger);
+ test_clock_with_start(logger);
+ test_clock_with_start_master(logger);
+ test_clock_with_output_dejitter(logger);
+ test_clock_with_output_clock_start(logger);
+ test_clock_with_monotonic_clock(logger);
+
+ struct clock_point start_point, output_point;
+ start_point = (struct clock_point){ now, now * 1000 };
+ output_point = (struct clock_point){ now, now * 1000 };
+
+ vlc_tick_t result = test_clock_with_monotonic_clock_start(
+ logger, 0, 0, start_point, start_point, start_point);
+ fprintf(stderr, "%" PRId64 " = %" PRId64 "\n", result, start_point.system);
+ assert(result == start_point.system);
+
+ /* Monotonic clock must start at the start point. */
+ start_point = (struct clock_point){ 2 * now, now * 1000 };
+ result = test_clock_with_monotonic_clock_start(
+ logger, 0, 0, start_point, output_point, output_point);
+ fprintf(stderr, "%" PRId64 " = %" PRId64 "\n", result, start_point.system);
+ assert(result == start_point.system);
+
+ /* A point later in the future will be started later also. */
+ start_point = (struct clock_point){ now, now * 1000 };
+ output_point = (struct clock_point){ now, now * 1000 + 100};
+ result = test_clock_with_monotonic_clock_start(
+ logger, 0, 0, start_point, output_point, output_point);
+ fprintf(stderr, "result(%" PRId64 ") = start_point.system(%" PRId64 ")\n", result, start_point.system + 100);
+ assert(result == start_point.system + 100);
+
+ /* Input dejitter and PCR delay are accounted together. */
+ result = test_clock_with_monotonic_clock_start(
+ logger, 30, 0, start_point, output_point, output_point);
+ fprintf(stderr, "%" PRId64 " = %" PRId64 "\n", result, start_point.system + 130);
+ assert(result == start_point.system + 130);
+
+ /* Output dejitter will not account the current PCR delay. */
+ result = test_clock_with_monotonic_clock_start(
+ logger, 0, 100, start_point, output_point, output_point);
+ fprintf(stderr, "%" PRId64 " = %" PRId64 "\n", result, start_point.system + 100);
+ assert(result == start_point.system + 100);
+
+ /* The highest dejitter value is used. */
+ result = test_clock_with_monotonic_clock_start(
+ logger, 33, 55, start_point, output_point, output_point);
+ fprintf(stderr, "%" PRId64 " = %" PRId64 "\n", result, start_point.system + 133);
+ assert(result == start_point.system + 133);
+
+ /* The highest dejitter value is used. */
+ result = test_clock_with_monotonic_clock_start(
+ logger, 33, 555, start_point, output_point, output_point);
+ fprintf(stderr, "%" PRId64 " = %" PRId64 "\n", result, start_point.system + 555);
+ assert(result == start_point.system + 555);
+
+ start_point = (struct clock_point){ now, now * 1000 };
+ output_point = (struct clock_point){ now, now * 1000 };
+ result = test_clock_slave_update(logger, start_point, output_point);
+ fprintf(stderr, "%" PRId64 " = %" PRId64 "\n", result, (vlc_tick_t)0);
+ assert(result == 0);
+
+ output_point = (struct clock_point){ now + 10, now * 1000 };
+ result = test_clock_slave_update(logger, start_point, output_point);
+ fprintf(stderr, "%" PRId64 " = %" PRId64 "\n", result, (vlc_tick_t)-10);
+ assert(result == -10);
+
+ output_point = (struct clock_point){ now, now * 1000 + 10 };
+ result = test_clock_slave_update(logger, start_point, output_point);
+ fprintf(stderr, "%" PRId64 " = %" PRId64 "\n", result, (vlc_tick_t)10);
+ assert(result == 10);
+
+ test_clock_start_reset(logger);
+ test_clock_slave_only(logger);
+
+ libvlc_release(libvlc);
+}
+
diff --git a/test/src/meson.build b/test/src/meson.build
index 5eac3cab553d..ecf5c825a1aa 100644
--- a/test/src/meson.build
+++ b/test/src/meson.build
@@ -35,6 +35,16 @@ vlc_tests += {
'link_with' : [libvlc, libvlccore],
}
+vlc_tests += {
+ 'name' : 'test_src_clock_start',
+ 'sources' : files(
+ 'clock/clock_start.c',
+ '../../src/clock/clock.c',
+ '../../src/clock/clock_internal.c'),
+ 'suite' : ['src', 'test_src'],
+ 'link_with' : [libvlc, libvlccore],
+}
+
vlc_tests += {
'name' : 'test_src_misc_variables',
'sources' : files('misc/variables.c'),
--
GitLab
From 27ace9d2875c36c6030781d96861f64b42e17c0e Mon Sep 17 00:00:00 2001
From: Alexandre Janniaux <ajanni@videolabs.io>
Date: Mon, 15 Jul 2024 12:10:29 +0200
Subject: [PATCH 16/28] clock: setup monotonic wait_sync_ref start in
vlc_clock_Start()
Previously, calling vlc_clock_ConvertToSystem was the only way to
initialize the monotonic wait_sync_ref point which will be used to start
the outputs. The rationale for that is written in commit:
commit 491b48a6ca5a9b69f8aed25455f8e11574f32a7b
Author: Thomas Guillem <thomas@gllm.fr>
Date: Thu Oct 10 17:25:51 2019 +0200
clock: fix false start with some SPU tracks
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.
...
Nowadays, two startup modes can be highlighted for the clock:
- Either the input clock drives the clock bus and we have a reference
point from the start of the stream in the next patches, so we don't
use the monotonic start to setup the first clock points.
- Or one of the outputs will drive the clock bus and we setup the first
time conversion for those output depending on the input start, the
clock priorities (added in 491b48a6ca5a9b69f8aed25455f8e11574f32a7b).
In the second point, it can only happen at the beginning of the
elementary stream, or after a proper flush/seek. By calling a
vlc_clock_Start function instead, we make it clearer that some setup
will be done, and we can log those points to trace the startup
conditions.
The code is extracted from the current vlc_clock_ConvertToSystem startup
mechanism but has been heavily documented to replace the "Fixup: remove"
comment.
Co-authored-by: Thomas Guillem <thomas@gllm.fr>
---
src/clock/clock.c | 75 ++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 74 insertions(+), 1 deletion(-)
diff --git a/src/clock/clock.c b/src/clock/clock.c
index d9aa4a374e72..2a604fb2be48 100644
--- a/src/clock/clock.c
+++ b/src/clock/clock.c
@@ -769,7 +769,80 @@ vlc_clock_output_start(vlc_clock_t *clock,
if (context->start_time.system == VLC_TICK_INVALID)
return;
- /* Do nothing for now. */
+ if (clock->priority >= main_clock->wait_sync_ref_priority)
+ return;
+
+ /**
+ * The clock should have already been started, so we have a valid
+ * start_time which links a PCR value to a time where the PCR is being
+ * "used". Any PTS from the output will necessarily be converted after
+ * this start_time, and will necessarily have bigger PTS than the
+ * registered PCR, so we can use the difference to find how much delta
+ * there is between the first PTS and the first PCR.
+ *
+ * When setting up the outputs, some amount of wait will already be
+ * consumed, for instance when playing live stream, we consume some time
+ * between the first decoded packet and the end of the buffering. The
+ * core currently handles this by substracting the buffering duration to
+ * the start time, and we re-inject this duration through the pts-delay,
+ * called input_dejitter in the clock.
+ *
+ * | Start End of Decoders ready
+ * | buffering buffering (DecoderWaitUnblock)
+ * | v v v
+ * | x-----------------------x-----------x---> system time
+ * |
+ * | Preroll and
+ * | pts delay decoder delay
+ * | |----------------------->|---------->
+ * | <-----------------------|
+ * | Buffering duration
+ * | (media time)
+ * |
+ * | Fig.1: System time representation of the clock startup
+ *
+ * The defined start_time from the input is set according to the PCR
+ * but the first PTS received by an output is likely later than this
+ * clock time, so the real output start_time must be shifted to keep
+ * intra-media synchronization in acceptable levels.
+ *
+ * | Start First audio First video
+ * | time packet PTS packet PTS
+ * | v v v
+ * | x------------x------------x----------> media time
+ * |
+ * | |------------>
+ * | Audio PCR delay
+ * |
+ * | |------------------------->
+ * | Video PCR delay
+ * |
+ * | Fig.2: Media time representation of the clock startup
+ *
+ * If the pts-delay is very low, and if the PCR delay is very small,
+ * an additional output_dejitter is used to offset the beginning of
+ * the playback in a uniform manner, to let some time for the outputs
+ * to start. It only makes sense when one of the output will drive
+ * the bus clock.
+ **/
+
+ vlc_tick_t pcr_delay = (first_ts - context->start_time.stream) / context->rate
+ + context->start_time.system - start_date;
+
+ if (pcr_delay > MAX_PCR_DELAY)
+ {
+ if (main_clock->logger != NULL)
+ vlc_error(main_clock->logger, "Invalid PCR delay ! Ignoring it...");
+ pcr_delay = 0;
+ }
+
+ const vlc_tick_t input_delay = main_clock->input_dejitter + pcr_delay;
+
+ const vlc_tick_t delay =
+ __MAX(input_delay, main_clock->output_dejitter);
+
+ main_clock->wait_sync_ref_priority = clock->priority;
+ context->wait_sync_ref = clock_point_Create(start_date + delay, first_ts);
}
void vlc_clock_Lock(vlc_clock_t *clock)
--
GitLab
From b48fc809e6931180ddacbab9d0273b63c81bb996 Mon Sep 17 00:00:00 2001
From: Alexandre Janniaux <ajanni@videolabs.io>
Date: Tue, 15 Oct 2024 16:00:03 +0200
Subject: [PATCH 17/28] test: clock: call SetFirstPCR in convert_paused_common
The initialization of the clock was missing in the test, which was not
an issue before but will be in the next commits.
---
test/src/clock/clock.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/test/src/clock/clock.c b/test/src/clock/clock.c
index 8652cd47ba60..172da042cc8e 100644
--- a/test/src/clock/clock.c
+++ b/test/src/clock/clock.c
@@ -731,6 +731,10 @@ static void convert_paused_common(const struct clock_ctx *ctx, vlc_clock_t *upda
const vlc_tick_t system_start = ctx->system_start;
vlc_tick_t system = system_start;
+ vlc_clock_main_Lock(ctx->mainclk);
+ vlc_clock_main_SetFirstPcr(ctx->mainclk, ctx->system_start, ctx->stream_start);
+ vlc_clock_main_Unlock(ctx->mainclk);
+
vlc_clock_Lock(updater);
vlc_clock_Update(updater, ctx->system_start, ctx->stream_start, 1.0f);
vlc_clock_Unlock(updater);
--
GitLab
From 7a9cc359d88ce0cf1d595637bde390904460dbed Mon Sep 17 00:00:00 2001
From: Thomas Guillem <thomas@gllm.fr>
Date: Thu, 1 Aug 2024 16:34:13 +0200
Subject: [PATCH 18/28] clock: don't create wait_sync_ref from slave updates
We want every outputs to update their points, but they might be forced
(like the first video picture). Since a valid update follow a convert,
we prefer to only create wait_sync_ref from convert.
This will be clear with the upcoming vlc_clock_Start rework (the
wait_sync_ref point will be always created from this upcoming function).
Co-authored-by: Alexandre Janniaux <ajanni@videolabs.io>
---
src/clock/clock.c | 16 ++++++++++++++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/src/clock/clock.c b/src/clock/clock.c
index 2a604fb2be48..ad354d5c9556 100644
--- a/src/clock/clock.c
+++ b/src/clock/clock.c
@@ -706,6 +706,8 @@ static vlc_tick_t vlc_clock_slave_update(vlc_clock_t *clock,
unsigned frame_rate,
unsigned frame_rate_base)
{
+ vlc_clock_main_t *main_clock = clock->owner;
+
if (system_now == VLC_TICK_MAX)
{
/* If system_now is VLC_TICK_MAX, the update is forced, don't modify
@@ -715,9 +717,19 @@ static vlc_tick_t vlc_clock_slave_update(vlc_clock_t *clock,
return VLC_TICK_MAX;
}
- vlc_tick_t computed = clock->ops->to_system(clock, ctx, system_now, ts, rate);
+ vlc_tick_t drift;
+ vlc_tick_t computed;
+ if (main_clock->first_pcr.system != VLC_TICK_INVALID || ctx->wait_sync_ref.stream != VLC_TICK_INVALID)
+ {
+ computed = clock->ops->to_system(clock, ctx, system_now, ts, rate);
+ drift = computed - system_now;
+ }
+ else
+ {
+ computed = system_now;
+ drift = 0;
+ }
- vlc_tick_t drift = computed - system_now;
vlc_clock_on_update(clock, computed, ts, drift, rate,
frame_rate, frame_rate_base);
return drift;
--
GitLab
From 489a2031235a13a1f4e6985df2763ab051fd488a Mon Sep 17 00:00:00 2001
From: Alexandre Janniaux <ajanni@videolabs.io>
Date: Thu, 10 Oct 2024 18:28:15 +0200
Subject: [PATCH 19/28] clock: remove wait_sync_ref assignment in slave_reset
wait_sync_ref should not be reset when resetting a derived clock. The
clock bus should instead accept multiple possible time and choose the
best start time for the given context, and reset() should only allow the
clock to switch context.
Co-authored-by: Thomas Guillem <thomas@gllm.fr>
---
src/clock/clock.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/src/clock/clock.c b/src/clock/clock.c
index ad354d5c9556..4e4f5214630a 100644
--- a/src/clock/clock.c
+++ b/src/clock/clock.c
@@ -744,9 +744,6 @@ static void vlc_clock_slave_reset(vlc_clock_t *clock)
if (clock->context != NULL && clock->context != ctx)
vlc_clock_switch_context(clock, ctx);
- main_clock->wait_sync_ref_priority = UINT_MAX;
- ctx->wait_sync_ref = clock_point_Create(VLC_TICK_INVALID, VLC_TICK_INVALID);
-
vlc_clock_on_update(clock, VLC_TICK_INVALID, VLC_TICK_INVALID,
VLC_TICK_INVALID, 1.0f, 0, 0);
}
--
GitLab
From 7c4157a664d14f59df7b7ec65bec0e34f08eac22 Mon Sep 17 00:00:00 2001
From: Alexandre Janniaux <ajanni@videolabs.io>
Date: Fri, 11 Oct 2024 18:00:33 +0200
Subject: [PATCH 20/28] input: es_out: use specific clock reset instead of
clock-bus
vlc_clock_main_Reset() breaks the information inside clock context and
it makes tracking the context switching harder. By using the
clock-specific reset function, we'll be able to setup the behaviour of
the input clock being reset and handle the creation of a new context
instead.
---
src/input/es_out.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/input/es_out.c b/src/input/es_out.c
index 36780e4fbe1e..cdf56ce699aa 100644
--- a/src/input/es_out.c
+++ b/src/input/es_out.c
@@ -962,7 +962,7 @@ static void EsOutChangePosition(es_out_sys_t *p_sys, bool b_flush,
input_clock_Reset(pgrm->p_input_clock);
vlc_clock_main_Lock(pgrm->clocks.main);
pgrm->i_last_pcr = VLC_TICK_INVALID;
- vlc_clock_main_Reset(pgrm->clocks.main);
+ vlc_clock_Reset(pgrm->clocks.input);
vlc_clock_main_Unlock(pgrm->clocks.main);
}
--
GitLab
From 3614593fc520ad518d724e94bd169225ebf4cff3 Mon Sep 17 00:00:00 2001
From: Alexandre Janniaux <ajanni@videolabs.io>
Date: Tue, 15 Oct 2024 17:11:36 +0200
Subject: [PATCH 21/28] clock: setup input-specific clock reset operations
No functional changes for now. The function will be implemented
differently for the input vlc_clock, so that it always a source of new
clock context, while the reset of the output clocks will allow selecting
a newer clock context insteand.
---
src/clock/clock.c | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/src/clock/clock.c b/src/clock/clock.c
index 4e4f5214630a..652cb28fa624 100644
--- a/src/clock/clock.c
+++ b/src/clock/clock.c
@@ -623,6 +623,16 @@ vlc_clock_input_start(vlc_clock_t *clock,
main_clock->wait_sync_ref_priority = UINT_MAX;
}
+static void vlc_clock_slave_reset(vlc_clock_t *clock);
+static void vlc_clock_input_reset(vlc_clock_t *clock)
+{
+ vlc_clock_main_t *main_clock = clock->owner;
+ if (main_clock->master == clock)
+ vlc_clock_master_reset(clock);
+ else
+ vlc_clock_slave_reset(clock);
+}
+
static vlc_tick_t
vlc_clock_monotonic_to_system(vlc_clock_t *clock, struct vlc_clock_context *ctx,
vlc_tick_t now, vlc_tick_t ts, double rate)
@@ -1126,7 +1136,7 @@ static const struct vlc_clock_ops slave_ops = {
static const struct vlc_clock_ops input_master_ops = {
.update = vlc_clock_master_update,
- .reset = vlc_clock_master_reset,
+ .reset = vlc_clock_input_reset,
.set_delay = vlc_clock_master_set_delay,
.to_system = vlc_clock_master_to_system,
.start = vlc_clock_input_start,
@@ -1134,7 +1144,7 @@ static const struct vlc_clock_ops input_master_ops = {
static const struct vlc_clock_ops input_slave_ops = {
.update = vlc_clock_slave_update,
- .reset = vlc_clock_slave_reset,
+ .reset = vlc_clock_input_reset,
.set_delay = vlc_clock_slave_set_delay,
.to_system = vlc_clock_slave_to_system,
.start = vlc_clock_input_start,
--
GitLab
From 20caec57d508acef7f45c422545ca7b1d54b0978 Mon Sep 17 00:00:00 2001
From: Alexandre Janniaux <ajanni@videolabs.io>
Date: Mon, 15 Jul 2024 14:12:44 +0200
Subject: [PATCH 22/28] clock: setup new clock_context selection
Ensure that every clocks (even input) has a valid clock_context, and
specifically select them during vlc_clock_Start(), and un-select them
during vlc_clock_Reset().
The previous mode is still present in the code to avoid breaking the
current code until all the necessary changes are introduced, and is
selected by whether the main_clock->first_pcr value has been set or not.
Now that vlc_clock_main_Reset() is not called anymore at this location,
the points registered from the input when it is driving the clock bus
will not be removed.
/!\ As a consequence, --clock-master=input now starts from the input
/!\ clock points instead of starting from a monotonic clock point.
This indirectly fixes an issue where the output_dejitter is accounted in
computing the wait_sync_ref date, but also making all the outputs late
compared to the input clock, leading to "Deferring start" messages from
the audio output, and then "Starting late" message.
Co-authored-by: Thomas Guillem <thomas@gllm.fr>
---
src/clock/clock.c | 118 ++++++++++++++++++++++++++++++++--------------
1 file changed, 83 insertions(+), 35 deletions(-)
diff --git a/src/clock/clock.c b/src/clock/clock.c
index 652cb28fa624..53f9fc00ef19 100644
--- a/src/clock/clock.c
+++ b/src/clock/clock.c
@@ -542,7 +542,6 @@ static void vlc_clock_master_reset(vlc_clock_t *clock)
if (clock->context != NULL && clock->context != ctx)
vlc_clock_switch_context(clock, ctx);
- vlc_clock_main_reset(main_clock);
assert(main_clock->delay <= 0);
assert(clock->delay >= 0);
@@ -597,40 +596,70 @@ vlc_clock_input_start(vlc_clock_t *clock,
{
vlc_clock_main_t *main_clock = clock->owner;
vlc_mutex_assert(&main_clock->lock);
+ assert(main_clock->context != NULL);
- struct vlc_clock_context *context = main_clock->context;
if (main_clock->first_pcr.system != VLC_TICK_INVALID)
return;
- if (context->start_time.system != VLC_TICK_INVALID
- && (context->last.system != VLC_TICK_INVALID ||
- context->wait_sync_ref.stream != VLC_TICK_INVALID))
- {
- assert(context->start_time.stream != VLC_TICK_INVALID);
- struct vlc_clock_context *new_context = malloc(sizeof(*new_context));
- if (new_context != NULL)
- {
- *new_context = *context;
- vlc_list_init(&new_context->using_clocks);
- vlc_list_append(&new_context->node, &main_clock->prev_contexts);
- }
- context->clock_id++;
- }
+ struct vlc_clock_context *context
+ = main_clock->context;
+
+ struct vlc_clock_context *last_context =
+ vlc_list_last_entry_or_null(&main_clock->prev_contexts, struct vlc_clock_context, node);
- context_reset(context);
+ if (last_context != NULL)
+ context->clock_id = last_context->clock_id + 1;
+
+ // Note: should not trigger when SetFirstPCR is called
context->start_time = clock_point_Create(start_date, first_ts);
context->offset = ComputeOffset(main_clock, context, start_date, first_ts, 1.f);
main_clock->wait_sync_ref_priority = UINT_MAX;
+
+ vlc_clock_switch_context(clock, context);
}
static void vlc_clock_slave_reset(vlc_clock_t *clock);
static void vlc_clock_input_reset(vlc_clock_t *clock)
{
vlc_clock_main_t *main_clock = clock->owner;
- if (main_clock->master == clock)
- vlc_clock_master_reset(clock);
- else
- vlc_clock_slave_reset(clock);
+
+ if (main_clock->tracer != NULL && clock->track_str_id != NULL)
+ vlc_tracer_TraceEvent(main_clock->tracer, "RENDER", clock->track_str_id,
+ "reset_user");
+
+ if (main_clock->first_pcr.system != VLC_TICK_INVALID)
+ {
+ (main_clock->master == clock ? vlc_clock_master_reset : vlc_clock_slave_reset)(clock);
+ return;
+ }
+
+ if (clock->context == NULL || clock->context->start_time.system == VLC_TICK_INVALID)
+ return;
+
+ struct vlc_clock_context *context = main_clock->context;
+
+ bool has_other_clock = false;
+ vlc_clock_t *clock_iterator;
+ vlc_list_foreach(clock_iterator, &context->using_clocks, node)
+ {
+ if (clock_iterator == clock)
+ continue;
+
+ has_other_clock = true;
+ }
+
+ if (!has_other_clock)
+ {
+ context_reset(context);
+ return;
+ }
+
+ assert(context->start_time.stream != VLC_TICK_INVALID);
+ vlc_list_append(&context->node, &main_clock->prev_contexts);
+ main_clock->context = context_new();
+
+ if (main_clock->context == NULL)
+ main_clock->context = context; /* TODO: It fallbacks to previous context */
}
static vlc_tick_t
@@ -778,7 +807,11 @@ vlc_clock_output_start(vlc_clock_t *clock,
vlc_clock_main_t *main_clock = clock->owner;
vlc_mutex_assert(&main_clock->lock);
- struct vlc_clock_context *context = main_clock->context;
+
+ /* Attach to the correct context in case of reset */
+ struct vlc_clock_context *context
+ = vlc_clock_get_context(clock, start_date, first_ts, true);
+
#if 0
/* Disabled for now, the handling will be done later. */
/* vlc_clock_Start must have already been called. */
@@ -788,6 +821,9 @@ vlc_clock_output_start(vlc_clock_t *clock,
if (context->start_time.system == VLC_TICK_INVALID)
return;
+ /* Attach to the correct context in case of reset */
+ clock->context = context;
+
if (clock->priority >= main_clock->wait_sync_ref_priority)
return;
@@ -1073,8 +1109,14 @@ vlc_tick_t vlc_clock_Update(vlc_clock_t *clock, vlc_tick_t system_now,
vlc_tick_t ts, double rate)
{
AssertLocked(clock);
- struct vlc_clock_context *ctx =
- vlc_clock_get_context(clock, system_now, ts, true);
+
+ vlc_clock_main_t *main_clock = clock->owner;
+ struct vlc_clock_context *ctx;
+
+ if (main_clock->first_pcr.system != VLC_TICK_INVALID)
+ ctx = vlc_clock_get_context(clock, system_now, ts, true);
+ else
+ ctx = clock->context;
return clock->ops->update(clock, ctx, system_now, ts, rate, 0, 0);
}
@@ -1084,8 +1126,13 @@ vlc_tick_t vlc_clock_UpdateVideo(vlc_clock_t *clock, vlc_tick_t system_now,
unsigned frame_rate, unsigned frame_rate_base)
{
AssertLocked(clock);
- struct vlc_clock_context *ctx =
- vlc_clock_get_context(clock, system_now, ts, true);
+ vlc_clock_main_t *main_clock = clock->owner;
+ struct vlc_clock_context *ctx;
+
+ if (main_clock->first_pcr.system != VLC_TICK_INVALID)
+ ctx = vlc_clock_get_context(clock, system_now, ts, true);
+ else
+ ctx = clock->context;
return clock->ops->update(clock, ctx, system_now, ts, rate,
frame_rate, frame_rate_base);
@@ -1170,18 +1217,19 @@ static vlc_clock_t *vlc_clock_main_Create(vlc_clock_main_t *main_clock,
clock->last_conversion = 0;
if (input)
- clock->context = NULL; /* Always use the main one */
- else
{
- /* Attach the clock to the first context or the main one */
- struct vlc_clock_context *ctx =
- vlc_list_first_entry_or_null(&main_clock->prev_contexts,
- struct vlc_clock_context, node);
- if (likely(ctx == NULL))
- ctx = main_clock->context;
- vlc_clock_attach_context(clock, ctx);
+ /* Always use the main one */
+ vlc_clock_attach_context(clock, main_clock->context);
+ return clock;
}
+ /* Attach the clock to the first context or the main one */
+ struct vlc_clock_context *ctx =
+ vlc_list_first_entry_or_null(&main_clock->prev_contexts,
+ struct vlc_clock_context, node);
+ if (likely(ctx == NULL))
+ ctx = main_clock->context;
+ vlc_clock_attach_context(clock, ctx);
return clock;
}
--
GitLab
From eb4809958f37ef539cc3213a702675a3f335c14a Mon Sep 17 00:00:00 2001
From: Alexandre Janniaux <ajanni@videolabs.io>
Date: Fri, 11 Oct 2024 18:19:38 +0200
Subject: [PATCH 23/28] clock: store start date in last conversion
This fixes a lipsync issue when a context is created from the beginning
of the stream.
---
src/clock/clock.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/clock/clock.c b/src/clock/clock.c
index 53f9fc00ef19..43cc1cb63870 100644
--- a/src/clock/clock.c
+++ b/src/clock/clock.c
@@ -807,6 +807,8 @@ vlc_clock_output_start(vlc_clock_t *clock,
vlc_clock_main_t *main_clock = clock->owner;
vlc_mutex_assert(&main_clock->lock);
+ if (clock->last_conversion == VLC_TICK_INVALID)
+ clock->last_conversion = start_date;
/* Attach to the correct context in case of reset */
struct vlc_clock_context *context
@@ -1214,7 +1216,7 @@ static vlc_clock_t *vlc_clock_main_Create(vlc_clock_main_t *main_clock,
clock->cbs_data = cbs_data;
clock->priority = priority;
assert(!cbs || cbs->on_update);
- clock->last_conversion = 0;
+ clock->last_conversion = VLC_TICK_INVALID;
if (input)
{
--
GitLab
From 95a5e14cdaf233f1a2230ee0d48aaa8d4e08edae Mon Sep 17 00:00:00 2001
From: Thomas Guillem <thomas@gllm.fr>
Date: Tue, 30 Jul 2024 17:55:18 +0200
Subject: [PATCH 24/28] decoder: start clock only once and even when not
buffered
Not buffered can happen if a decoder output a first frame late (after buffering)
---
src/input/decoder.c | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/src/input/decoder.c b/src/input/decoder.c
index 9ba55bed3fa6..21901b682aad 100644
--- a/src/input/decoder.c
+++ b/src/input/decoder.c
@@ -209,6 +209,7 @@ struct vlc_input_decoder_t
bool b_waiting;
bool b_first;
bool b_has_data;
+ bool out_started;
/* Flushing */
bool flushing;
@@ -1036,16 +1037,15 @@ static int DecoderWaitUnblock( vlc_input_decoder_t *p_owner )
vlc_cond_signal( &p_owner->wait_acknowledge );
}
- bool did_wait = false;
while (p_owner->b_waiting && p_owner->b_has_data && !p_owner->flushing)
- {
vlc_fifo_WaitCond(p_owner->p_fifo, &p_owner->wait_request);
- /* We should not start the clock if we ended up flushing. */
- did_wait = !p_owner->flushing;
- }
- if (tracer != NULL && did_wait)
- vlc_tracer_TraceEvent(tracer, "DEC", p_owner->psz_id, "stop wait");
+ if (!p_owner->out_started)
+ {
+ p_owner->out_started = true;
+ if (tracer != NULL)
+ vlc_tracer_TraceEvent(tracer, "DEC", p_owner->psz_id, "stop wait");
+ }
if (p_owner->flushing)
{
@@ -1813,6 +1813,7 @@ static void *DecoderThread( void *p_data )
* is called again. This will avoid a second useless flush (but
* harmless). */
p_owner->flushing = false;
+ p_owner->out_started = false;
p_owner->i_preroll_end = PREROLL_NONE;
continue;
}
@@ -1976,6 +1977,7 @@ CreateDecoder( vlc_object_t *p_parent, const struct vlc_input_decoder_cfg *cfg )
p_owner->b_waiting = false;
p_owner->b_first = true;
p_owner->b_has_data = false;
+ p_owner->out_started = false;
p_owner->error = false;
--
GitLab
From 56327ed9963dac621c2e17dfc925555dbe26376c Mon Sep 17 00:00:00 2001
From: Alexandre Janniaux <ajanni@videolabs.io>
Date: Wed, 20 Mar 2024 08:30:15 +0100
Subject: [PATCH 25/28] input: decoder: start clock when decoders are ready
DecoderWaitUnblock is waiting for the vlc_input_decoder_StopWait which
will be called by the es_out when every decoders are ready to start
playing.
When DecoderWaitUnblock is called and almost ready to return, it means
that we already have a frame in the output and that playback has
started. We can compute the monotonic start by calling vlc_clock_Start.
Some care must be taken given that other reasons might unblock the
decoders, like flushing or closing the decoder.
Currently, vlc_clock_Start() does nothing as long as we didn't start the
input vlc_clock_t also, but all of the code will be enabled after.
Co-authored-by: Thomas Guillem <thomas@gllm.fr>
---
src/input/decoder.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/src/input/decoder.c b/src/input/decoder.c
index 21901b682aad..beb714f5ae8f 100644
--- a/src/input/decoder.c
+++ b/src/input/decoder.c
@@ -1024,7 +1024,7 @@ static void RequestReload( vlc_input_decoder_t *p_owner )
atomic_compare_exchange_strong( &p_owner->reload, &expected, RELOAD_DECODER );
}
-static int DecoderWaitUnblock( vlc_input_decoder_t *p_owner )
+static int DecoderWaitUnblock(vlc_input_decoder_t *p_owner, vlc_tick_t date)
{
struct vlc_tracer *tracer = vlc_object_get_tracer(VLC_OBJECT(&p_owner->dec));
vlc_fifo_Assert(p_owner->p_fifo);
@@ -1045,6 +1045,10 @@ static int DecoderWaitUnblock( vlc_input_decoder_t *p_owner )
p_owner->out_started = true;
if (tracer != NULL)
vlc_tracer_TraceEvent(tracer, "DEC", p_owner->psz_id, "stop wait");
+
+ vlc_clock_Lock(p_owner->p_clock);
+ vlc_clock_Start(p_owner->p_clock, vlc_tick_now(), date);
+ vlc_clock_Unlock(p_owner->p_clock);
}
if (p_owner->flushing)
@@ -1364,12 +1368,13 @@ static int ModuleThread_PlayVideo( vlc_input_decoder_t *p_owner, picture_t *p_pi
}
else
{
- int ret = DecoderWaitUnblock(p_owner);
+ int ret = DecoderWaitUnblock(p_owner, p_picture->date);
if (ret != VLC_SUCCESS)
{
picture_Release(p_picture);
return ret;
}
+
}
if( unlikely(p_owner->paused) && likely(p_owner->frames_countdown > 0) )
@@ -1506,7 +1511,7 @@ static int ModuleThread_PlayAudio( vlc_input_decoder_t *p_owner, vlc_frame_t *p_
vlc_aout_stream_Flush( p_astream );
}
- int ret = DecoderWaitUnblock(p_owner);
+ int ret = DecoderWaitUnblock(p_owner, p_audio->i_pts);
if (ret != VLC_SUCCESS)
{
block_Release(p_audio);
@@ -1573,7 +1578,7 @@ static void ModuleThread_PlaySpu( vlc_input_decoder_t *p_owner, subpicture_t *p_
}
/* */
- int ret = DecoderWaitUnblock(p_owner);
+ int ret = DecoderWaitUnblock(p_owner, p_subpic->i_start);
if (ret != VLC_SUCCESS || p_subpic->i_start == VLC_TICK_INVALID)
{
--
GitLab
From 4d1fcf56148eae0e74c50310d799a1e991e5a89b Mon Sep 17 00:00:00 2001
From: Alexandre Janniaux <ajanni@videolabs.io>
Date: Tue, 19 Mar 2024 19:19:53 +0100
Subject: [PATCH 26/28] test: clock: adapt after previous changes
Clock must now be started from a dedicated input vlc_clock. The start is
not yet mandatory but is available and will be mandatory after next
patches.
Previously, the clock was driven by two values: coeff and offset. The
idea was that since the clock is supposed to look like an affine line,
we could extrapolate any `ts` media point into `ts * coeff + offset` in
system time.
It meant that offset could take any value depending on the difference
between system time and media time, reducing the interpretability of the
values.
After this patch, offset is computed from the start of the stream in the
test, so it will basically only include the difference between the
timestamp computation and the real clock.
Co-authored-by: Thomas Guillem <thomas@gllm.fr>
---
test/src/clock/clock.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/test/src/clock/clock.c b/test/src/clock/clock.c
index 172da042cc8e..d1c9644bcd6b 100644
--- a/test/src/clock/clock.c
+++ b/test/src/clock/clock.c
@@ -357,6 +357,11 @@ static void play_scenario(libvlc_int_t *vlc, struct vlc_tracer *tracer,
system_start = scenario->system_start;
stream_start = scenario->stream_start;
}
+
+ vlc_clock_Start(input, system_start, stream_start);
+
+ if (scenario->type == CLOCK_SCENARIO_UPDATE)
+ vlc_clock_Start(master, system_start, stream_start);
vlc_clock_main_Unlock(mainclk);
const struct clock_ctx ctx = {
@@ -552,8 +557,7 @@ static void normal_check(const struct clock_ctx *ctx, size_t update_count,
{
case TRACER_EVENT_TYPE_UPDATE:
assert(event.update.coeff == 1.0f);
- assert(event.update.offset ==
- scenario->system_start - scenario->stream_start);
+ assert(event.update.offset == 0);
break;
case TRACER_EVENT_TYPE_RENDER_VIDEO:
if (last_video_date != VLC_TICK_INVALID)
@@ -687,6 +691,7 @@ static void pause_common(const struct clock_ctx *ctx, vlc_clock_t *updater)
vlc_tick_t system = system_start;
vlc_clock_Lock(updater);
+ vlc_clock_Start(updater, ctx->system_start, ctx->stream_start);
vlc_clock_Update(updater, system, ctx->stream_start, 1.0f);
vlc_clock_Unlock(updater);
@@ -736,6 +741,7 @@ static void convert_paused_common(const struct clock_ctx *ctx, vlc_clock_t *upda
vlc_clock_main_Unlock(ctx->mainclk);
vlc_clock_Lock(updater);
+ vlc_clock_Start(updater, ctx->system_start, ctx->stream_start);
vlc_clock_Update(updater, ctx->system_start, ctx->stream_start, 1.0f);
vlc_clock_Unlock(updater);
--
GitLab
From b295062ded39325481fd06d49c146ece57c49206 Mon Sep 17 00:00:00 2001
From: Alexandre Janniaux <ajanni@videolabs.io>
Date: Tue, 15 Oct 2024 16:41:34 +0200
Subject: [PATCH 27/28] clock: don't generate wait_sync_ref with clock-start
The conversion function might not have wait_sync_ref defined in the
next steps as it won't generate the wait_sync_ref itself, but choosing
the start_date requires transforming the current point.
The assertion on wait_sync_ref is removed given that it would be NULL
when using start_time.
Co-authored-by: Thomas Guillem <thomas@gllm.fr>
---
src/clock/clock.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/clock/clock.c b/src/clock/clock.c
index 43cc1cb63870..3d7ce184835b 100644
--- a/src/clock/clock.c
+++ b/src/clock/clock.c
@@ -669,7 +669,8 @@ vlc_clock_monotonic_to_system(vlc_clock_t *clock, struct vlc_clock_context *ctx,
vlc_clock_main_t *main_clock = clock->owner;
if (clock->priority < main_clock->wait_sync_ref_priority
- && ctx == main_clock->context)
+ && ctx == main_clock->context
+ && main_clock->first_pcr.system != VLC_TICK_INVALID)
{
/* 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
@@ -699,7 +700,6 @@ vlc_clock_monotonic_to_system(vlc_clock_t *clock, struct vlc_clock_context *ctx,
ctx->wait_sync_ref = clock_point_Create(now + delay, ts);
}
- assert(ctx->wait_sync_ref.stream != VLC_TICK_INVALID);
return (ts - ctx->wait_sync_ref.stream) / rate + ctx->wait_sync_ref.system;
}
--
GitLab
From ae286927a6eab3436fd5110f1f51ee4c15704bcf Mon Sep 17 00:00:00 2001
From: Alexandre Janniaux <ajanni@videolabs.io>
Date: Tue, 15 Oct 2024 12:01:47 +0200
Subject: [PATCH 28/28] clock: prefer start_time over first_pcr
This prioritize using the start_time path if start_time has been defined
regardless of whether vlc_clock_SetFirstPcr has been called or not,
instead of using the first_pcr path as long as first_pcr has been
defined.
---
src/clock/clock.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/clock/clock.c b/src/clock/clock.c
index 3d7ce184835b..891634401eb0 100644
--- a/src/clock/clock.c
+++ b/src/clock/clock.c
@@ -677,12 +677,12 @@ vlc_clock_monotonic_to_system(vlc_clock_t *clock, struct vlc_clock_context *ctx,
* is used to introduce the same delay than the input clock (first PTS
* - first PCR). */
vlc_tick_t pcr_delay = 0;
- if (main_clock->first_pcr.system != VLC_TICK_INVALID)
- pcr_delay = (ts - main_clock->first_pcr.stream) / rate +
- main_clock->first_pcr.system - now;
- else if (ctx->start_time.system != VLC_TICK_INVALID)
+ if (ctx->start_time.system != VLC_TICK_INVALID)
pcr_delay = (ts - ctx->start_time.stream) / rate +
ctx->start_time.system - now;
+ else if (main_clock->first_pcr.system != VLC_TICK_INVALID)
+ pcr_delay = (ts - main_clock->first_pcr.stream) / rate +
+ main_clock->first_pcr.system - now;
if (pcr_delay > MAX_PCR_DELAY)
{
--
GitLab