Skip to content

Commit

Permalink
ALL: use new mp_thread abstraction
Browse files Browse the repository at this point in the history
  • Loading branch information
kasper93 authored and Dudemanguy committed Nov 5, 2023
1 parent 3a8b107 commit 174df99
Show file tree
Hide file tree
Showing 81 changed files with 1,252 additions and 1,299 deletions.
48 changes: 24 additions & 24 deletions DOCS/tech-overview.txt
Original file line number Diff line number Diff line change
Expand Up @@ -430,7 +430,7 @@ like a log file or the internal console.lua script.
Locking
-------

See generally available literature. In mpv, we use pthread for this.
See generally available literature. In mpv, we use mp_thread for this.

Always keep locking clean. Don't skip locking just because it will work "in
practice". (See undefined behavior section.) If your use case is simple, you may
Expand Down Expand Up @@ -555,13 +555,13 @@ Condition variables
-------------------

They're used whenever a thread needs to wait for something, without nonsense
like sleep calls or busy waiting. mpv uses the standard pthread API for this.
There's a lot of literature on it. Read it.
like sleep calls or busy waiting. mpv uses the mp_thread API for this.
There's a lot of literature on condition variables, threading in general. Read it.

For initial understanding, it may be helpful to know that condition variables
are not variables that signal a condition. pthread_cond_t does not have any
state per-se. Maybe pthread_cond_t would better be named pthread_interrupt_t,
because its sole purpose is to interrupt a thread waiting via pthread_cond_wait()
are not variables that signal a condition. mp_cond does not have any
state per-se. Maybe mp_cond would better be named mp_interrupt,
because its sole purpose is to interrupt a thread waiting via mp_cond_wait()
(or similar). The "something" in "waiting for something" can be called
predicate (to avoid confusing it with "condition"). Consult literature for the
proper terms.
Expand All @@ -570,68 +570,68 @@ The very short version is...

Shared declarations:

pthread_mutex_t lock;
pthread_cond_t cond_var;
mp_mutex lock;
mp_cond cond_var;
struct something state_var; // protected by lock, changes signaled by cond_var

Waiter thread:

pthread_mutex_lock(&lock);
mp_mutex_lock(&lock);

// Wait for a change in state_var. We want to wait until predicate_fulfilled()
// returns true.
// Must be a loop for 2 reasons:
// 1. cond_var may be associated with other conditions too
// 2. pthread_cond_wait() can have sporadic wakeups
// 2. mp_cond_wait() can have sporadic wakeups
while (!predicate_fulfilled(&state_var)) {
// This unlocks, waits for cond_var to be signaled, and then locks again.
// The _whole_ point of cond_var is that unlocking and waiting for the
// signal happens atomically.
pthread_cond_wait(&cond_var, &lock);
mp_cond_wait(&cond_var, &lock);
}

// Here you may react to the state change. The state cannot change
// asynchronously as long as you still hold the lock (and didn't release
// and reacquire it).
// ...

pthread_mutex_unlock(&lock);
mp_mutex_unlock(&lock);

Signaler thread:

pthread_mutex_lock(&lock);
mp_mutex_lock(&lock);

// Something changed. Update the shared variable with the new state.
update_state(&state_var);

// Notify that something changed. This will wake up the waiter thread if
// it's blocked in pthread_cond_wait(). If not, nothing happens.
pthread_cond_broadcast(&cond_var);
// it's blocked in mp_cond_wait(). If not, nothing happens.
mp_cond_broadcast(&cond_var);

// Fun fact: good implementations wake up the waiter only when the lock is
// released, to reduce kernel scheduling overhead.
pthread_mutex_unlock(&lock);
mp_mutex_unlock(&lock);

Some basic rules:
1. Always access your state under proper locking
2. Always check your predicate before every call to pthread_cond_wait()
(And don't call pthread_cond_wait() if the predicate is fulfilled.)
3. Always call pthread_cond_wait() in a loop
2. Always check your predicate before every call to mp_cond_wait()
(And don't call mp_cond_wait() if the predicate is fulfilled.)
3. Always call mp_cond_wait() in a loop
(And only if your predicate failed without releasing the lock..)
4. Always call pthread_cond_broadcast()/_signal() inside of its associated
4. Always call mp_cond_broadcast()/_signal() inside of its associated
lock

mpv sometimes violates rule 3, and leaves "retrying" (i.e. looping) to the
caller.

Common pitfalls:
- Thinking that pthread_cond_t is some kind of semaphore, or holds any
- Thinking that mp_cond is some kind of semaphore, or holds any
application state or the user predicate (it _only_ wakes up threads
that are at the same time blocking on pthread_cond_wait() and friends,
that are at the same time blocking on mp_cond_wait() and friends,
nothing else)
- Changing the predicate, but not updating all pthread_cond_broadcast()/
- Changing the predicate, but not updating all mp_cond_broadcast()/
_signal() calls correctly
- Forgetting that pthread_cond_wait() unlocks the lock (other threads can
- Forgetting that mp_cond_wait() unlocks the lock (other threads can
and must acquire the lock)
- Holding multiple nested locks while trying to wait (=> deadlock, violates
the lock order anyway)
Expand Down
39 changes: 19 additions & 20 deletions audio/out/ao_audiotrack.c
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,9 @@ struct priv {

bool thread_terminate;
bool thread_created;
pthread_t thread;
pthread_mutex_t lock;
pthread_cond_t wakeup;
mp_thread thread;
mp_mutex lock;
mp_cond wakeup;
};

struct JNIByteBuffer {
Expand Down Expand Up @@ -549,13 +549,13 @@ static int init_jni(struct ao *ao)
return 0;
}

static void *playthread(void *arg)
static MP_THREAD_VOID playthread(void *arg)
{
struct ao *ao = arg;
struct priv *p = ao->priv;
JNIEnv *env = MP_JNI_GET_ENV(ao);
mpthread_set_name("ao/audiotrack");
pthread_mutex_lock(&p->lock);
mp_thread_set_name("ao/audiotrack");
mp_mutex_lock(&p->lock);
while (!p->thread_terminate) {
int state = AudioTrack.PLAYSTATE_PAUSED;
if (p->audiotrack) {
Expand All @@ -579,12 +579,11 @@ static void *playthread(void *arg)
MP_ERR(ao, "AudioTrack.write failed with %d\n", ret);
}
} else {
struct timespec wait = mp_rel_time_to_timespec(0.300);
pthread_cond_timedwait(&p->wakeup, &p->lock, &wait);
mp_cond_timedwait(&p->wakeup, &p->lock, MP_TIME_MS_TO_NS(300));
}
}
pthread_mutex_unlock(&p->lock);
return NULL;
mp_mutex_unlock(&p->lock);
MP_THREAD_RETURN();
}

static void uninit(struct ao *ao)
Expand All @@ -598,13 +597,13 @@ static void uninit(struct ao *ao)
MP_JNI_EXCEPTION_LOG(ao);
}

pthread_mutex_lock(&p->lock);
mp_mutex_lock(&p->lock);
p->thread_terminate = true;
pthread_cond_signal(&p->wakeup);
pthread_mutex_unlock(&p->lock);
mp_cond_signal(&p->wakeup);
mp_mutex_unlock(&p->lock);

if (p->thread_created)
pthread_join(p->thread, NULL);
mp_thread_join(p->thread);

if (p->audiotrack) {
MP_JNI_CALL_VOID(p->audiotrack, AudioTrack.release);
Expand Down Expand Up @@ -638,8 +637,8 @@ static void uninit(struct ao *ao)
p->timestamp = NULL;
}

pthread_cond_destroy(&p->wakeup);
pthread_mutex_destroy(&p->lock);
mp_cond_destroy(&p->wakeup);
mp_mutex_destroy(&p->lock);

uninit_jni(ao);
}
Expand All @@ -651,8 +650,8 @@ static int init(struct ao *ao)
if (!env)
return -1;

pthread_mutex_init(&p->lock, NULL);
pthread_cond_init(&p->wakeup, NULL);
mp_mutex_init(&p->lock);
mp_cond_init(&p->wakeup);

if (init_jni(ao) < 0)
return -1;
Expand Down Expand Up @@ -781,7 +780,7 @@ static int init(struct ao *ao)
goto error;
}

if (pthread_create(&p->thread, NULL, playthread, ao)) {
if (mp_thread_create(&p->thread, playthread, ao)) {
MP_ERR(ao, "pthread creation failed\n");
goto error;
}
Expand Down Expand Up @@ -828,7 +827,7 @@ static void start(struct ao *ao)
MP_JNI_CALL_VOID(p->audiotrack, AudioTrack.play);
MP_JNI_EXCEPTION_LOG(ao);

pthread_cond_signal(&p->wakeup);
mp_cond_signal(&p->wakeup);
}

#define OPT_BASE_STRUCT struct priv
Expand Down
6 changes: 3 additions & 3 deletions audio/out/ao_lavc.c
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ static int init(struct ao *ao)
return 0;

fail:
pthread_mutex_unlock(&ao->encode_lavc_ctx->lock);
mp_mutex_unlock(&ao->encode_lavc_ctx->lock);
ac->shutdown = true;
return -1;
}
Expand Down Expand Up @@ -261,7 +261,7 @@ static bool audio_write(struct ao *ao, void **data, int samples)
double outpts = pts;

// for ectx PTS fields
pthread_mutex_lock(&ectx->lock);
mp_mutex_lock(&ectx->lock);

if (!ectx->options->rawts) {
// Fix and apply the discontinuity pts offset.
Expand Down Expand Up @@ -290,7 +290,7 @@ static bool audio_write(struct ao *ao, void **data, int samples)
ectx->next_in_pts = nextpts;
}

pthread_mutex_unlock(&ectx->lock);
mp_mutex_unlock(&ectx->lock);

mp_aframe_set_pts(af, outpts);

Expand Down
13 changes: 6 additions & 7 deletions audio/out/ao_opensles.c
Original file line number Diff line number Diff line change
Expand Up @@ -23,21 +23,20 @@
#include "common/msg.h"
#include "audio/format.h"
#include "options/m_option.h"
#include "osdep/threads.h"
#include "osdep/timer.h"

#include <SLES/OpenSLES.h>
#include <SLES/OpenSLES_Android.h>

#include <pthread.h>

struct priv {
SLObjectItf sl, output_mix, player;
SLBufferQueueItf buffer_queue;
SLEngineItf engine;
SLPlayItf play;
void *buf;
int bytes_per_enqueue;
pthread_mutex_t buffer_lock;
mp_mutex buffer_lock;
double audio_latency;

int frames_per_enqueue;
Expand All @@ -62,7 +61,7 @@ static void uninit(struct ao *ao)
p->engine = NULL;
p->play = NULL;

pthread_mutex_destroy(&p->buffer_lock);
mp_mutex_destroy(&p->buffer_lock);

free(p->buf);
p->buf = NULL;
Expand All @@ -77,7 +76,7 @@ static void buffer_callback(SLBufferQueueItf buffer_queue, void *context)
SLresult res;
double delay;

pthread_mutex_lock(&p->buffer_lock);
mp_mutex_lock(&p->buffer_lock);

delay = p->frames_per_enqueue / (double)ao->samplerate;
delay += p->audio_latency;
Expand All @@ -88,7 +87,7 @@ static void buffer_callback(SLBufferQueueItf buffer_queue, void *context)
if (res != SL_RESULT_SUCCESS)
MP_ERR(ao, "Failed to Enqueue: %d\n", res);

pthread_mutex_unlock(&p->buffer_lock);
mp_mutex_unlock(&p->buffer_lock);
}

#define CHK(stmt) \
Expand Down Expand Up @@ -170,7 +169,7 @@ static int init(struct ao *ao)
goto error;
}

int r = pthread_mutex_init(&p->buffer_lock, NULL);
int r = mp_mutex_init(&p->buffer_lock);
if (r) {
MP_ERR(ao, "Failed to initialize the mutex: %d\n", r);
goto error;
Expand Down
1 change: 0 additions & 1 deletion audio/out/ao_pulse.c
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
#include <string.h>
#include <stdint.h>
#include <math.h>
#include <pthread.h>

#include <pulse/pulseaudio.h>

Expand Down
2 changes: 1 addition & 1 deletion audio/out/ao_wasapi.c
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ static DWORD __stdcall AudioThread(void *lpParameter)
{
struct ao *ao = lpParameter;
struct wasapi_state *state = ao->priv;
mpthread_set_name("ao/wasapi");
mp_thread_set_name("ao/wasapi");
CoInitializeEx(NULL, COINIT_APARTMENTTHREADED);

state->init_ok = wasapi_thread_init(ao);
Expand Down
Loading

0 comments on commit 174df99

Please sign in to comment.