[vlc-commits] [Git][videolan/vlc][master] 8 commits: clock: split vlc_clock_master_update
Steve Lhomme (@robUx4)
gitlab at videolan.org
Thu May 30 06:15:29 UTC 2024
Steve Lhomme pushed to branch master at VideoLAN / VLC
Commits:
09a4fec7 by Alexandre Janniaux at 2024-05-30T05:56:36+00:00
clock: split vlc_clock_master_update
The function computing offset and coefficient has become quite complex,
and adding new conditions to this computation leads to a lot of nesting.
Co-authored-by: Thomas Guillem <thomas at gllm.fr>
- - - - -
9f337097 by Alexandre Janniaux at 2024-05-30T05:56:36+00:00
clock: early return in vlc_clock_main_ChangePause
The case where pause is set is much simpler than the other case, and
there is no common code afterwards so we can just early return this case
to simplify the other one.
Co-authored-by: Thomas Guillem <thomas at gllm.fr>
- - - - -
8b57bd3d by Alexandre Janniaux at 2024-05-30T05:56:36+00:00
clock: check clock_main->last instead of offset
Offset could very well be 0, which is currently equal to
VLC_TICK_INVALID. Checking that clock_main->last has been set instead
provides a safer alternative.
Co-authored-by: Thomas Guillem <thomas at gllm.fr>
- - - - -
8963523e by Alexandre Janniaux at 2024-05-30T05:56:36+00:00
test: clock: option to disable jitter
- - - - -
1872f8b2 by Alexandre Janniaux at 2024-05-30T05:56:36+00:00
test: clock: fix typos
- - - - -
922b6f69 by Alexandre Janniaux at 2024-05-30T05:56:36+00:00
test: clock: convert from deterministic values
Using vlc_tick_now() makes the test depend on when it is run. Instead,
provide fixed values to ensure the same inputs in the test.
It can be noted that using VLC_TICK_0 instead of VLC_TICK_0 + 15000ms
makes the test fail though, because conversion in sent to the future.
Co-authored-by: Thomas Guillem <thomas at gllm.fr>
- - - - -
327ef6e7 by Alexandre Janniaux at 2024-05-30T05:56:36+00:00
test: clock: convert early to start the clock
Before the next patches, the monotonic clock is only started after
vlc_clock_ConvertToSystem is called.
Co-authored-by: Thomas Guillem <thomas at gllm.fr>
- - - - -
f8466197 by Alexandre Janniaux at 2024-05-30T05:56:36+00:00
test: clock: fix infinite loop on trace
When the trace is not matching one of the expected field, continue was
called before the iterator was incremented, leading to infinitely
calling continue.
Co-authored-by: Thomas Guillem <thomas at gllm.fr>
- - - - -
2 changed files:
- src/clock/clock.c
- test/src/clock/clock.c
Changes:
=====================================
src/clock/clock.c
=====================================
@@ -2,6 +2,10 @@
* clock.c: Output modules synchronisation clock
*****************************************************************************
* Copyright (C) 2018-2019 VLC authors, VideoLAN and Videolabs SAS
+ * Copyright (C) 2024 Videolabs
+ *
+ * Authors: Thomas Guillem <thomas at gllm.fr>
+ * Alexandre Janniaux <ajanni at 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
@@ -178,7 +182,7 @@ static inline void TraceRender(struct vlc_tracer *tracer, const char *type,
static vlc_tick_t main_stream_to_system(vlc_clock_main_t *main_clock,
vlc_tick_t ts)
{
- if (main_clock->offset == VLC_TICK_INVALID)
+ if (main_clock->last.system == VLC_TICK_INVALID)
return VLC_TICK_INVALID;
return ((vlc_tick_t) (ts * main_clock->coeff / main_clock->rate))
+ main_clock->offset;
@@ -216,93 +220,102 @@ static inline void vlc_clock_on_update(vlc_clock_t *clock,
system_now, ts, drift);
}
-static vlc_tick_t vlc_clock_master_update(vlc_clock_t *clock,
- vlc_tick_t system_now,
- vlc_tick_t ts, double rate,
- unsigned frame_rate,
- unsigned frame_rate_base)
+
+static void vlc_clock_master_update_coeff(
+ vlc_clock_t *clock, vlc_tick_t system_now, vlc_tick_t ts, double rate)
{
vlc_clock_main_t *main_clock = clock->owner;
+ vlc_mutex_assert(&main_clock->lock);
- if (unlikely(ts == VLC_TICK_INVALID || system_now == VLC_TICK_INVALID))
- return VLC_TICK_INVALID;
-
- /* If system_now is VLC_TICK_MAX, the update is forced, don't modify
- * anything but only notify the new clock point. */
- if (system_now != VLC_TICK_MAX)
+ if (main_clock->last.system != VLC_TICK_INVALID
+ && ts != main_clock->last.stream)
{
- if (main_clock->offset != VLC_TICK_INVALID
- && ts != main_clock->last.stream)
+ if (rate == main_clock->rate)
{
- if (rate == main_clock->rate)
- {
- /* We have a reference so we can update coeff */
- vlc_tick_t system_diff = system_now - main_clock->last.system;
- vlc_tick_t stream_diff = ts - main_clock->last.stream;
+ /* We have a reference so we can update coeff */
+ vlc_tick_t system_diff = system_now - main_clock->last.system;
+ vlc_tick_t stream_diff = ts - main_clock->last.stream;
- double instant_coeff = system_diff / (double) stream_diff * rate;
+ double instant_coeff = system_diff / (double) stream_diff * rate;
- /* System and stream ts should be incrementing */
- bool decreasing_ts = system_diff < 0 || stream_diff < 0;
- /* The instant coeff should always be around 1.0 */
- bool coefficient_unstable = instant_coeff > 1.0 + COEFF_THRESHOLD
- || instant_coeff < 1.0 - COEFF_THRESHOLD;
+ /* System and stream ts should be incrementing */
+ bool decreasing_ts = system_diff < 0 || stream_diff < 0;
+ /* The instant coeff should always be around 1.0 */
+ bool coefficient_unstable = instant_coeff > 1.0 + COEFF_THRESHOLD
+ || instant_coeff < 1.0 - COEFF_THRESHOLD;
- if (decreasing_ts || coefficient_unstable)
- {
- if (main_clock->logger != NULL)
- {
- if (decreasing_ts)
- vlc_warning(main_clock->logger, "resetting master clock: "
- "decreasing ts: system: %"PRId64 ", stream: %" PRId64,
- system_diff, stream_diff);
- else
- vlc_warning(main_clock->logger, "resetting master clock: "
- "coefficient too unstable: %f", instant_coeff);
- }
-
- if (main_clock->tracer != NULL && clock->track_str_id != NULL)
- vlc_tracer_TraceEvent(main_clock->tracer, "RENDER",
- clock->track_str_id,
- "reset_bad_source");
-
- vlc_clock_SendEvent(main_clock, discontinuity);
-
- /* Reset and continue (calculate the offset from the
- * current point) */
- vlc_clock_main_reset(main_clock);
- }
- else
+ if (decreasing_ts || coefficient_unstable)
+ {
+ if (main_clock->logger != NULL)
{
- AvgUpdate(&main_clock->coeff_avg, instant_coeff);
- main_clock->coeff = AvgGet(&main_clock->coeff_avg);
+ if (decreasing_ts)
+ vlc_warning(main_clock->logger, "resetting master clock: "
+ "decreasing ts: system: %"PRId64 ", stream: %" PRId64,
+ system_diff, stream_diff);
+ else
+ vlc_warning(main_clock->logger, "resetting master clock: "
+ "coefficient too unstable: %f", instant_coeff);
}
+
+ if (main_clock->tracer != NULL && clock->track_str_id != NULL)
+ vlc_tracer_TraceEvent(main_clock->tracer, "RENDER",
+ clock->track_str_id,
+ "reset_bad_source");
+
+ vlc_clock_SendEvent(main_clock, discontinuity);
+
+ /* Reset and continue (calculate the offset from the
+ * current point) */
+ vlc_clock_main_reset(main_clock);
+ }
+ else
+ {
+ AvgUpdate(&main_clock->coeff_avg, instant_coeff);
+ main_clock->coeff = AvgGet(&main_clock->coeff_avg);
}
}
- else
- {
- main_clock->wait_sync_ref_priority = UINT_MAX;
- main_clock->wait_sync_ref =
- clock_point_Create(VLC_TICK_INVALID, VLC_TICK_INVALID);
+ }
+ else
+ {
+ main_clock->wait_sync_ref_priority = UINT_MAX;
+ main_clock->wait_sync_ref =
+ clock_point_Create(VLC_TICK_INVALID, VLC_TICK_INVALID);
- vlc_clock_SendEvent(main_clock, discontinuity);
- }
+ vlc_clock_SendEvent(main_clock, discontinuity);
+ }
- main_clock->offset =
- system_now - ((vlc_tick_t) (ts * main_clock->coeff / rate));
+ main_clock->offset =
+ system_now - ((vlc_tick_t) (ts * main_clock->coeff / rate));
- if (main_clock->tracer != NULL && clock->track_str_id != NULL)
- vlc_tracer_Trace(main_clock->tracer,
- VLC_TRACE("type", "RENDER"),
- VLC_TRACE("id", clock->track_str_id),
- VLC_TRACE_TICK_NS("offset", main_clock->offset),
- VLC_TRACE("coeff", main_clock->coeff),
- VLC_TRACE_END);
+ if (main_clock->tracer != NULL && clock->track_str_id != NULL)
+ vlc_tracer_Trace(main_clock->tracer,
+ VLC_TRACE("type", "RENDER"),
+ VLC_TRACE("id", clock->track_str_id),
+ VLC_TRACE_TICK_NS("offset", main_clock->offset),
+ VLC_TRACE("coeff", main_clock->coeff),
+ VLC_TRACE_END);
+
+ main_clock->rate = rate;
+ vlc_cond_broadcast(&main_clock->cond);
+}
- main_clock->last = clock_point_Create(system_now, ts);
+static vlc_tick_t vlc_clock_master_update(vlc_clock_t *clock,
+ vlc_tick_t system_now,
+ vlc_tick_t ts, double rate,
+ unsigned frame_rate,
+ unsigned frame_rate_base)
+{
+ vlc_clock_main_t *main_clock = clock->owner;
+
+ if (unlikely(ts == VLC_TICK_INVALID || system_now == VLC_TICK_INVALID))
+ return VLC_TICK_INVALID;
- main_clock->rate = rate;
- vlc_cond_broadcast(&main_clock->cond);
+ /* If system_now is VLC_TICK_MAX, the update is forced, don't modify
+ * anything but only notify the new clock point. */
+ if (system_now != VLC_TICK_MAX)
+ {
+ vlc_clock_master_update_coeff(clock, system_now, ts, rate);
+ main_clock->last = clock_point_Create(system_now, ts);
}
/* Fix the reported ts if both master and slaves source are delayed. This
@@ -611,26 +624,27 @@ void vlc_clock_main_ChangePause(vlc_clock_main_t *main_clock, vlc_tick_t now,
assert(paused == (main_clock->pause_date == VLC_TICK_INVALID));
if (paused)
+ {
main_clock->pause_date = now;
- else
+ return;
+ }
+
+ /**
+ * Only apply a delay if the clock has a reference point to avoid
+ * messing up the timings if the stream was paused then seeked
+ */
+ const vlc_tick_t delay = now - main_clock->pause_date;
+ if (main_clock->last.system != VLC_TICK_INVALID)
{
- /**
- * Only apply a delay if the clock has a reference point to avoid
- * messing up the timings if the stream was paused then seeked
- */
- const vlc_tick_t delay = now - main_clock->pause_date;
- if (main_clock->offset != VLC_TICK_INVALID)
- {
- main_clock->last.system += delay;
- main_clock->offset += delay;
- }
- if (main_clock->first_pcr.system != VLC_TICK_INVALID)
- main_clock->first_pcr.system += delay;
- if (main_clock->wait_sync_ref.system != VLC_TICK_INVALID)
- main_clock->wait_sync_ref.system += delay;
- main_clock->pause_date = VLC_TICK_INVALID;
- vlc_cond_broadcast(&main_clock->cond);
+ main_clock->last.system += delay;
+ main_clock->offset += delay;
}
+ if (main_clock->first_pcr.system != VLC_TICK_INVALID)
+ main_clock->first_pcr.system += delay;
+ if (main_clock->wait_sync_ref.system != VLC_TICK_INVALID)
+ main_clock->wait_sync_ref.system += delay;
+ main_clock->pause_date = VLC_TICK_INVALID;
+ vlc_cond_broadcast(&main_clock->cond);
}
void vlc_clock_main_Delete(vlc_clock_main_t *main_clock)
=====================================
test/src/clock/clock.c
=====================================
@@ -2,6 +2,10 @@
* clock/clock.c: test for the vlc clock
*****************************************************************************
* Copyright (C) 2023 VLC authors, VideoLAN and Videolabs SAS
+ * Copyright (C) 2024 Videolabs
+ *
+ * Authors: Thomas Guillem <thomas at gllm.fr>
+ * Alexandre Janniaux <ajanni at 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
@@ -68,7 +72,10 @@ struct clock_scenario
void (*check)(const struct clock_ctx *ctx, size_t update_count,
vlc_tick_t expected_system_end, vlc_tick_t stream_end);
};
- void (*run)(const struct clock_ctx *ctx);
+ struct {
+ void (*run)(const struct clock_ctx *ctx);
+ bool disable_jitter;
+ };
};
};
@@ -78,6 +85,9 @@ struct clock_ctx
vlc_clock_t *master;
vlc_clock_t *slave;
+ vlc_tick_t system_start;
+ vlc_tick_t stream_start;
+
const struct clock_scenario *scenario;
};
@@ -181,7 +191,7 @@ static void TracerTrace(void *opaque, vlc_tick_t ts,
{
struct vlc_tracer *libvlc_tracer = opaque;
- const struct vlc_tracer_entry *entry = trace->entries;
+ const struct vlc_tracer_entry *cursor = trace->entries;
bool is_render = false, is_render_video = false, is_status = false;
unsigned nb_update = 0;
@@ -190,8 +200,11 @@ static void TracerTrace(void *opaque, vlc_tick_t ts,
vlc_tick_t render_video_pts = VLC_TICK_INVALID;
enum tracer_event_status status = 0;
- while (entry->key != NULL)
+ while (cursor->key != NULL)
{
+ const struct vlc_tracer_entry *entry = cursor;
+ cursor++;
+
switch (entry->type)
{
case VLC_TRACER_INT:
@@ -247,7 +260,6 @@ static void TracerTrace(void *opaque, vlc_tick_t ts,
break;
default: vlc_assert_unreachable();
}
- entry++;
}
if (libvlc_tracer != NULL)
@@ -326,6 +338,20 @@ static void play_scenario(libvlc_int_t *vlc, struct vlc_tracer *tracer,
vlc_clock_t *slave = vlc_clock_main_CreateSlave(mainclk, slave_name, VIDEO_ES,
NULL, NULL);
assert(slave != NULL);
+ if (scenario->type == CLOCK_SCENARIO_RUN && scenario->disable_jitter)
+ {
+ /* Don't add any delay for the first monotonic ref point */
+ vlc_clock_main_SetInputDejitter(mainclk, 0);
+ vlc_clock_main_SetDejitter(mainclk, 0);
+ }
+
+ vlc_tick_t system_start = VLC_TICK_0 + VLC_TICK_FROM_MS(15000);
+ vlc_tick_t stream_start = VLC_TICK_0 + VLC_TICK_FROM_MS(15000);
+ if (scenario->type == CLOCK_SCENARIO_UPDATE)
+ {
+ system_start = scenario->system_start;
+ stream_start = scenario->stream_start;
+ }
vlc_clock_main_Unlock(mainclk);
const struct clock_ctx ctx = {
@@ -333,6 +359,8 @@ static void play_scenario(libvlc_int_t *vlc, struct vlc_tracer *tracer,
.master = master,
.slave = slave,
.scenario = scenario,
+ .system_start = system_start,
+ .stream_start = stream_start,
};
if (scenario->type == CLOCK_SCENARIO_RUN)
@@ -645,14 +673,21 @@ static void drift_sudden_update(const struct clock_ctx *ctx, size_t index,
static void pause_common(const struct clock_ctx *ctx, vlc_clock_t *updater)
{
- const vlc_tick_t system_start = vlc_tick_now();
+ const vlc_tick_t system_start = ctx->system_start;
const vlc_tick_t pause_duration = VLC_TICK_FROM_MS(20);
vlc_tick_t system = system_start;
vlc_clock_Lock(updater);
- vlc_clock_Update(updater, system, 1, 1.0f);
+ vlc_clock_Update(updater, system, ctx->stream_start, 1.0f);
vlc_clock_Unlock(updater);
+ {
+ vlc_clock_Lock(ctx->slave);
+ vlc_tick_t converted = vlc_clock_ConvertToSystem(ctx->slave, system, ctx->stream_start, 1.0f);
+ assert(converted == system);
+ vlc_clock_Unlock(ctx->slave);
+ }
+
system += VLC_TICK_FROM_MS(10);
vlc_clock_main_Lock(ctx->mainclk);
@@ -667,7 +702,7 @@ static void pause_common(const struct clock_ctx *ctx, vlc_clock_t *updater)
system += 1;
vlc_clock_Lock(ctx->slave);
- vlc_tick_t converted = vlc_clock_ConvertToSystem(ctx->slave, system, 1, 1.0f);
+ vlc_tick_t converted = vlc_clock_ConvertToSystem(ctx->slave, system, ctx->stream_start, 1.0f);
vlc_clock_Unlock(ctx->slave);
assert(converted == system_start + pause_duration);
}
@@ -679,22 +714,16 @@ static void master_pause_run(const struct clock_ctx *ctx)
static void monotonic_pause_run(const struct clock_ctx *ctx)
{
- /* Don't add any delay for the first monotonic ref point */
- vlc_clock_main_Lock(ctx->mainclk);
- vlc_clock_main_SetInputDejitter(ctx->mainclk, 0);
- vlc_clock_main_SetDejitter(ctx->mainclk, 0);
- vlc_clock_main_Unlock(ctx->mainclk);
-
pause_common(ctx, ctx->slave);
}
static void convert_paused_common(const struct clock_ctx *ctx, vlc_clock_t *updater)
{
- const vlc_tick_t system_start = vlc_tick_now();
+ const vlc_tick_t system_start = ctx->system_start;
vlc_tick_t system = system_start;
vlc_clock_Lock(updater);
- vlc_clock_Update(updater, system_start, 1, 1.0f);
+ vlc_clock_Update(updater, ctx->system_start, ctx->stream_start, 1.0f);
vlc_clock_Unlock(updater);
system += VLC_TICK_FROM_MS(10);
@@ -705,7 +734,7 @@ static void convert_paused_common(const struct clock_ctx *ctx, vlc_clock_t *upda
system += 1;
vlc_clock_Lock(ctx->slave);
- vlc_tick_t converted = vlc_clock_ConvertToSystem(ctx->slave, system, 1, 1.0f);
+ vlc_tick_t converted = vlc_clock_ConvertToSystem(ctx->slave, system, ctx->stream_start, 1.0f);
vlc_clock_Unlock(ctx->slave);
assert(converted == system_start);
}
@@ -717,12 +746,6 @@ static void master_convert_paused_run(const struct clock_ctx *ctx)
static void monotonic_convert_paused_run(const struct clock_ctx *ctx)
{
- /* Don't add any delay for the first monotonic ref point */
- vlc_clock_main_Lock(ctx->mainclk);
- vlc_clock_main_SetInputDejitter(ctx->mainclk, 0);
- vlc_clock_main_SetDejitter(ctx->mainclk, 0);
- vlc_clock_main_Unlock(ctx->mainclk);
-
convert_paused_common(ctx, ctx->slave);
}
@@ -732,7 +755,7 @@ static void monotonic_convert_paused_run(const struct clock_ctx *ctx)
#define INIT_SYSTEM_STREAM_TIMING(duration_, increment_, video_fps_) \
.stream_start = VLC_TICK_0 + VLC_TICK_FROM_MS(31000000), \
- .system_start = VLC_TICK_INVALID, \
+ .system_start = VLC_TICK_0, \
.duration = duration_, \
.stream_increment = increment_, \
.video_fps = video_fps_
@@ -811,18 +834,20 @@ static struct clock_scenario clock_scenarios[] = {
.desc = "pause + resume is delaying the next conversion",
.type = CLOCK_SCENARIO_RUN,
.run = monotonic_pause_run,
+ .disable_jitter = true,
},
{
.name = "master_convert_paused",
- .desc = "it is possible de convert ts while paused",
+ .desc = "it is possible to convert ts while paused",
.type = CLOCK_SCENARIO_RUN,
.run = master_convert_paused_run,
},
{
.name = "monotonic_convert_paused",
- .desc = "it is possible de convert ts while paused",
+ .desc = "it is possible to convert ts while paused",
.type = CLOCK_SCENARIO_RUN,
.run = monotonic_convert_paused_run,
+ .disable_jitter = true,
},
};
View it on GitLab: https://code.videolan.org/videolan/vlc/-/compare/9b3a9076afc7e77c7bc3b9d1babe36de210cfb25...f84661975a8aea699d1b433ef80d92750e933763
--
This project does not include diff previews in email notifications.
View it on GitLab: https://code.videolan.org/videolan/vlc/-/compare/9b3a9076afc7e77c7bc3b9d1babe36de210cfb25...f84661975a8aea699d1b433ef80d92750e933763
You're receiving this email because of your account on code.videolan.org.
VideoLAN code repository instance
More information about the vlc-commits
mailing list