[vlc-commits] [Git][videolan/vlc][master] 3 commits: aout: change code order
Steve Lhomme (@robUx4)
gitlab at videolan.org
Fri Sep 9 08:22:09 UTC 2022
Steve Lhomme pushed to branch master at VideoLAN / VLC
Commits:
53a7968d by Thomas Guillem at 2022-09-09T07:49:08+00:00
aout: change code order
first_pts will be read directly from the timing callback. Therefore set
it before the first play (since timing are reported only after the first
play).
- - - - -
e9eb73ea by Thomas Guillem at 2022-09-09T07:49:08+00:00
aout: report timings directly
The current mechanism was made to limit the number of lock/wait from an
audio callback.
Nevertheless, it makes the input/decoder.c code more complex, preventing
the merge of the 2 mutexes, that could help fix #26915. Indeed, you
can't use the same lock for the audio callback and for calling
aout_stream functions.
This commit make the following change: vlc_clock_Update() will be called
directly from aout_TimingReport().
Here are the potential mutexes that can be held when calling
vlc_clock_Update():
- The clock mutex
- The player will likely listen to clock events, and thus, hold a
player_timer mutex (but not the player "global" mutex).
- A gui module or libVLC will likely listen to player timer events, and
send these events to the main thread asynchronously (likely one more
mutex)
All operations that are executed with theses mutexes held are fast
(there is no wait, I/0, big calculation).
- - - - -
75287e22 by Thomas Guillem at 2022-09-09T07:49:08+00:00
aout: rename internal struct
- - - - -
3 changed files:
- src/audio_output/aout_internal.h
- src/audio_output/dec.c
- src/input/decoder.c
Changes:
=====================================
src/audio_output/aout_internal.h
=====================================
@@ -139,8 +139,6 @@ struct vlc_aout_stream_cfg
struct vlc_clock_t *clock;
const char *str_id;
const audio_replay_gain_t *replay_gain;
- void (*notify_latency_cb)(void *data);
- void *notify_latency_data;
};
vlc_aout_stream *vlc_aout_stream_New(audio_output_t *p_aout,
@@ -153,7 +151,6 @@ void vlc_aout_stream_ChangeRate(vlc_aout_stream *stream, float rate);
void vlc_aout_stream_ChangeDelay(vlc_aout_stream *stream, vlc_tick_t delay);
void vlc_aout_stream_Flush(vlc_aout_stream *stream);
void vlc_aout_stream_Drain(vlc_aout_stream *stream);
-void vlc_aout_stream_UpdateLatency(vlc_aout_stream *stream);
/* Contrary to other vlc_aout_stream_*() functions, this function can be called from
* any threads */
bool vlc_aout_stream_IsDrained(vlc_aout_stream *stream);
=====================================
src/audio_output/dec.c
=====================================
@@ -39,13 +39,6 @@
#include "clock/clock.h"
#include "libvlc.h"
-#define MAX_TIMING_POINT 16
-struct timing_point
-{
- vlc_tick_t system_ts;
- vlc_tick_t audio_ts;
-};
-
struct vlc_aout_stream
{
aout_instance_t *instance;
@@ -70,26 +63,18 @@ struct vlc_aout_stream
struct
{
- vlc_mutex_t lock; /* Guard data, count and head */
+ vlc_mutex_t lock; /* Guard first_pts, last_drift, rate_system_ts and
+ rate_audio_ts */
- /* Circular array */
- struct timing_point data[MAX_TIMING_POINT];
- /* Number of points in the array */
- size_t count;
- /* Index of the next point to write */
- size_t head;
-
- bool running;
+ vlc_tick_t last_drift;
vlc_tick_t first_pts;
vlc_tick_t last_pts; /* Used for stream_TimeGet() emulation */
- struct timing_point rate_point;
+ vlc_tick_t rate_system_ts;
+ vlc_tick_t rate_audio_ts;
float rate;
-
- void (*notify_latency_cb)(void *data);
- void *notify_latency_data;
- } timing_points;
+ } timing;
const char *str_id;
@@ -135,7 +120,7 @@ static int stream_TimeGet(vlc_aout_stream *stream, vlc_tick_t *delay)
if (aout->time_get == NULL)
{
- if (stream->timing_points.last_pts == VLC_TICK_INVALID)
+ if (stream->timing.last_pts == VLC_TICK_INVALID)
return -1;
/* Interpolate the last updated point. */
@@ -143,7 +128,7 @@ static int stream_TimeGet(vlc_aout_stream *stream, vlc_tick_t *delay)
vlc_tick_t play_date =
vlc_clock_ConvertToSystem(stream->sync.clock, system_now,
- stream->timing_points.last_pts,
+ stream->timing.last_pts,
stream->sync.rate);
*delay = play_date - system_now;
return 0;
@@ -156,14 +141,11 @@ static void stream_Discontinuity(vlc_aout_stream *stream)
stream->sync.discontinuity = true;
stream->original_pts = VLC_TICK_INVALID;
- vlc_mutex_lock(&stream->timing_points.lock);
- stream->timing_points.head = stream->timing_points.count = 0;
- vlc_mutex_unlock(&stream->timing_points.lock);
-
- stream->timing_points.first_pts =
- stream->timing_points.last_pts = VLC_TICK_INVALID;
-
- stream->timing_points.running = false;
+ vlc_mutex_lock(&stream->timing.lock);
+ stream->timing.first_pts = VLC_TICK_INVALID;
+ stream->timing.last_drift = VLC_TICK_INVALID;
+ vlc_mutex_unlock(&stream->timing.lock);
+ stream->timing.last_pts = VLC_TICK_INVALID;
}
static void stream_Reset(vlc_aout_stream *stream)
@@ -197,8 +179,8 @@ static void stream_Reset(vlc_aout_stream *stream)
}
}
- stream->timing_points.rate_point.audio_ts = VLC_TICK_INVALID;
- stream->timing_points.rate = 1.0;
+ stream->timing.rate_audio_ts = VLC_TICK_INVALID;
+ stream->timing.rate = 1.0;
atomic_store_explicit(&stream->drained, false, memory_order_relaxed);
atomic_store_explicit(&stream->drain_deadline, VLC_TICK_INVALID,
@@ -262,12 +244,10 @@ vlc_aout_stream * vlc_aout_stream_New(audio_output_t *p_aout,
stream->sync.clock = cfg->clock;
stream->str_id = cfg->str_id;
- stream->timing_points.rate_point.audio_ts = VLC_TICK_INVALID;
- stream->timing_points.rate = 1.f;
+ stream->timing.rate_audio_ts = VLC_TICK_INVALID;
+ stream->timing.rate = 1.f;
- vlc_mutex_init(&stream->timing_points.lock);
- stream->timing_points.notify_latency_cb = cfg->notify_latency_cb;
- stream->timing_points.notify_latency_data = cfg->notify_latency_data;
+ vlc_mutex_init(&stream->timing.lock);
stream->sync.rate = 1.f;
stream->sync.resamp_type = AOUT_RESAMPLING_NONE;
@@ -637,99 +617,30 @@ static void stream_Synchronize(vlc_aout_stream *stream, vlc_tick_t system_now,
void vlc_aout_stream_NotifyTiming(vlc_aout_stream *stream, vlc_tick_t system_ts,
vlc_tick_t audio_ts)
{
- /* This function might be called from high priority audio threads (so, no
- * mutexes, allocation, IO, debug, wait...). That is why we use a circular
- * buffer of points. The vlc_aout_stream user will read these points and
- * update the clock from vlc_aout_stream_Play() and
- * vlc_aout_stream_UpdateLatency(). */
-
- /* VLC mutexes use atomic and the reader will only do very fast
- * operations (copy of the timing_point data). */
- vlc_mutex_lock(&stream->timing_points.lock);
-
- size_t write_idx;
- if (stream->timing_points.count == MAX_TIMING_POINT)
- {
- write_idx = stream->timing_points.head;
- stream->timing_points.head = (stream->timing_points.head + 1)
- % MAX_TIMING_POINT;
- }
- else
- write_idx = stream->timing_points.count++;
-
- struct timing_point *p = &stream->timing_points.data[write_idx];
- p->system_ts = system_ts;
- p->audio_ts = audio_ts;
-
- vlc_mutex_unlock(&stream->timing_points.lock);
-
- if (stream->timing_points.notify_latency_cb != NULL)
- stream->timing_points.notify_latency_cb(
- stream->timing_points.notify_latency_data);
-}
-
-static vlc_tick_t
-stream_ReadTimingPoints(vlc_aout_stream *stream)
-{
- struct timing_point points[MAX_TIMING_POINT];
- size_t count = 0;
-
- vlc_mutex_lock(&stream->timing_points.lock);
- assert(stream->timing_points.count <= MAX_TIMING_POINT
- && stream->timing_points.head < MAX_TIMING_POINT);
-
- size_t initial_read = stream->timing_points.head;
- for (size_t read = stream->timing_points.head;
- count < stream->timing_points.count; ++read, ++count)
- {
- points[count] = stream->timing_points.data[read % MAX_TIMING_POINT];
- points[count].audio_ts += stream->timing_points.first_pts;
- }
+ vlc_mutex_lock(&stream->timing.lock);
+ vlc_tick_t rate_audio_ts = stream->timing.rate_audio_ts;
+ vlc_tick_t rate_system_ts = stream->timing.rate_system_ts;
- stream->timing_points.count = stream->timing_points.head = 0;
- vlc_mutex_unlock(&stream->timing_points.lock);
+ audio_ts += stream->timing.first_pts;
- if (count == 0)
- return 0;
-
- if (initial_read > 0)
- {
- /* This is not critical to miss timing points, the more important is
- * to get the last ones. Log it anyway since it can help identifying
- * buggy audio outputs spamming timing points. */
-
- audio_output_t *aout = aout_stream_aout(stream);
- msg_Dbg(aout, "Missed %zu timing points", initial_read);
- }
- vlc_tick_t drift = 0; /* Use only the last updated drift */
-
- for (size_t i = 0; i < count; ++i)
+ if (rate_audio_ts != VLC_TICK_INVALID)
{
- struct timing_point *tp = &points[i];
- struct timing_point *rp = &stream->timing_points.rate_point;
-
- if (rp->audio_ts != VLC_TICK_INVALID)
+ /* Drop timing updates that comes before the rate change */
+ if (system_ts < rate_system_ts)
{
- /* Drop timing updates that comes before the rate change */
- if (tp->system_ts < rp->system_ts)
- continue;
-
- /* Fix the audio timestamp with the rate */
- tp->audio_ts = rp->audio_ts + (tp->system_ts - rp->system_ts)
- * stream->timing_points.rate;
+ vlc_mutex_unlock(&stream->timing.lock);
+ return;
}
- drift = vlc_clock_Update(stream->sync.clock, points[i].system_ts,
- points[i].audio_ts, stream->timing_points.rate);
+ /* Fix the audio timestamp with the rate */
+ audio_ts = rate_audio_ts + (system_ts - rate_system_ts)
+ * stream->timing.rate;
}
- return drift;
-}
-
-void vlc_aout_stream_UpdateLatency(vlc_aout_stream *stream)
-{
- if (stream->timing_points.running)
- stream_ReadTimingPoints(stream);
+ stream->timing.last_drift =
+ vlc_clock_Update(stream->sync.clock, system_ts,
+ audio_ts, stream->timing.rate);
+ vlc_mutex_unlock(&stream->timing.lock);
}
/*****************************************************************************
@@ -800,7 +711,10 @@ int vlc_aout_stream_Play(vlc_aout_stream *stream, block_t *block)
stream_Synchronize(stream, system_now, original_pts);
else
{
- vlc_tick_t drift = stream_ReadTimingPoints(stream);
+ vlc_mutex_lock(&stream->timing.lock);
+ vlc_tick_t drift = stream->timing.last_drift;
+ stream->timing.last_drift = VLC_TICK_INVALID;
+ vlc_mutex_unlock(&stream->timing.lock);
stream_HandleDrift(stream, drift, original_pts);
}
@@ -817,42 +731,35 @@ int vlc_aout_stream_Play(vlc_aout_stream *stream, block_t *block)
vlc_audio_meter_Process(&owner->meter, block, play_date);
- if (!stream->timing_points.running)
- {
- stream->timing_points.running = true;
-
- /* Assert that no timing points are updated between flush and play */
-#ifndef NDEBUG
- vlc_mutex_lock(&stream->timing_points.lock);
- assert(stream->timing_points.count == 0);
- vlc_mutex_unlock(&stream->timing_points.lock);
-#endif
- }
-
if (aout->time_get == NULL
- && stream->sync.rate != stream->timing_points.rate)
+ && stream->sync.rate != stream->timing.rate)
{
+ vlc_mutex_lock(&stream->timing.lock);
/* Save the first timing point seeing a rate change */
- stream->timing_points.rate_point = (struct timing_point) {
- .system_ts = play_date,
- .audio_ts = original_pts,
- };
- stream->timing_points.rate = stream->sync.rate;
+ stream->timing.rate_system_ts = play_date;
+ stream->timing.rate_audio_ts = original_pts;
+ stream->timing.rate = stream->sync.rate;
/* Update the clock immediately with the new rate, instead of waiting
* for a timing update that could come too late (after 1second). */
- vlc_clock_Update(stream->sync.clock, play_date,
- original_pts, stream->sync.rate);
+ stream->timing.last_drift =
+ vlc_clock_Update(stream->sync.clock, play_date, original_pts,
+ stream->sync.rate);
+ vlc_mutex_unlock(&stream->timing.lock);
}
+ if (stream->timing.first_pts == VLC_TICK_INVALID)
+ {
+ vlc_mutex_lock(&stream->timing.lock);
+ stream->timing.first_pts = original_pts;
+ vlc_mutex_unlock(&stream->timing.lock);
+ }
+ stream->timing.last_pts = original_pts;
+
/* Output */
stream->sync.discontinuity = false;
aout->play(aout, block, play_date);
- if (stream->timing_points.first_pts == VLC_TICK_INVALID)
- stream->timing_points.first_pts = original_pts;
- stream->timing_points.last_pts = original_pts;
-
atomic_fetch_add_explicit(&stream->buffers_played, 1, memory_order_relaxed);
return ret;
drop:
@@ -889,13 +796,13 @@ void vlc_aout_stream_ChangePause(vlc_aout_stream *stream, bool paused, vlc_tick_
/* Update the rate point after the pause */
if (aout->time_get == NULL && !paused
- && stream->timing_points.rate_point.audio_ts != VLC_TICK_INVALID)
+ && stream->timing.rate_audio_ts != VLC_TICK_INVALID)
{
vlc_tick_t play_date =
vlc_clock_ConvertToSystem(stream->sync.clock, date,
- stream->timing_points.rate_point.audio_ts,
+ stream->timing.rate_audio_ts,
stream->sync.rate);
- stream->timing_points.rate_point.system_ts = play_date;
+ stream->timing.rate_system_ts = play_date;
}
}
}
=====================================
src/input/decoder.c
=====================================
@@ -132,7 +132,6 @@ struct vlc_input_decoder_t
/* If p_aout is valid, then p_astream is valid too */
audio_output_t *p_aout;
vlc_aout_stream *p_astream;
- bool b_astream_update_latency;
vout_thread_t *p_vout;
bool vout_started;
@@ -271,7 +270,6 @@ static int DecoderThread_Reload( vlc_input_decoder_t *p_owner,
// no need to lock, the decoder and ModuleThread are dead
p_owner->p_aout = NULL;
p_owner->p_astream = NULL;
- p_owner->b_astream_update_latency = false;
if( p_aout )
{
assert( p_astream );
@@ -341,18 +339,6 @@ static bool aout_replaygain_changed( const audio_replay_gain_t *a,
return false;
}
-static void aout_stream_NotifyLatency(void *data)
-{
- /* Wake the DecoderThread in order to update the audio latency via
- * vlc_aout_stream_UpdateLatency() */
- vlc_input_decoder_t *owner = data;
-
- vlc_fifo_Lock(owner->p_fifo);
- owner->b_astream_update_latency = true;
- vlc_fifo_Signal(owner->p_fifo);
- vlc_fifo_Unlock(owner->p_fifo);
-}
-
static int ModuleThread_UpdateAudioFormat( decoder_t *p_dec )
{
vlc_input_decoder_t *p_owner = dec_get_owner( p_dec );
@@ -369,7 +355,6 @@ static int ModuleThread_UpdateAudioFormat( decoder_t *p_dec )
vlc_mutex_lock( &p_owner->lock );
p_owner->p_astream = NULL;
p_owner->p_aout = NULL; // the DecoderThread should not use the old aout anymore
- p_owner->b_astream_update_latency = false;
vlc_mutex_unlock( &p_owner->lock );
vlc_aout_stream_Delete( p_astream );
@@ -417,8 +402,6 @@ static int ModuleThread_UpdateAudioFormat( decoder_t *p_dec )
.clock = p_owner->p_clock,
.str_id = p_owner->psz_id,
.replay_gain = &p_dec->fmt_out.audio_replay_gain,
- .notify_latency_cb = aout_stream_NotifyLatency,
- .notify_latency_data = p_owner,
};
p_astream = vlc_aout_stream_New( p_aout, &cfg );
if( p_astream == NULL )
@@ -1628,20 +1611,6 @@ static void DecoderThread_ChangeRate( vlc_input_decoder_t *p_owner, float rate )
vlc_mutex_unlock( &p_owner->lock );
}
-static void DecoderThread_UpdateAudioLatency( vlc_input_decoder_t *p_owner )
-{
- decoder_t *p_dec = &p_owner->dec;
-
- /* Update the audio latency after a possible long wait in order to update
- * the audio clock as soon as possible. */
- assert( p_dec->fmt_in.i_cat == AUDIO_ES );
-
- vlc_mutex_lock( &p_owner->lock );
- if( p_owner->p_astream != NULL )
- vlc_aout_stream_UpdateLatency( p_owner->p_astream );
- vlc_mutex_unlock( &p_owner->lock );
-}
-
static void DecoderThread_ChangeDelay( vlc_input_decoder_t *p_owner, vlc_tick_t delay )
{
decoder_t *p_dec = &p_owner->dec;
@@ -1748,16 +1717,6 @@ static void *DecoderThread( void *p_data )
continue;
}
- if( p_owner->b_astream_update_latency )
- {
- vlc_fifo_Unlock( p_owner->p_fifo );
-
- DecoderThread_UpdateAudioLatency( p_owner );
-
- vlc_fifo_Lock( p_owner->p_fifo );
- p_owner->b_astream_update_latency = false;
- }
-
if( rate != p_owner->request_rate )
{
rate = p_owner->request_rate;
@@ -1902,7 +1861,6 @@ CreateDecoder( vlc_object_t *p_parent, const struct vlc_input_decoder_cfg *cfg )
p_owner->cbs_userdata = cfg->cbs_data;
p_owner->p_aout = NULL;
p_owner->p_astream = NULL;
- p_owner->b_astream_update_latency = false;
p_owner->p_vout = NULL;
p_owner->vout_started = false;
p_owner->i_spu_channel = VOUT_SPU_CHANNEL_INVALID;
View it on GitLab: https://code.videolan.org/videolan/vlc/-/compare/af785abd295bc9446eceaf127ccfc9d1759b64e9...75287e22c0c9b54d683837fe1815174807027d9a
--
View it on GitLab: https://code.videolan.org/videolan/vlc/-/compare/af785abd295bc9446eceaf127ccfc9d1759b64e9...75287e22c0c9b54d683837fe1815174807027d9a
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