From 92a9f11a0b4fda60c8880014be5920dcf3e95253 Mon Sep 17 00:00:00 2001 From: wm4 Date: Fri, 17 Jan 2014 23:13:09 +0100 Subject: sub: uglify sub decoder with locking The plan is to make the whole OSD thread-safe, and we start with this. We just put locks on all entry points (fortunately, dec_sub.c and all sd_*.c decoders are very closed off, and only the entry points in dec_sub.h let you access it). I think this is pretty ugly, but at least it's very simple. There's a special case with sub_get_bitmaps(): this function returns pointers to decoder data (specifically, libass images). There's no way to synchronize this internally, so expose sub_lock/sub_unlock functions. To make things simpler, and especially because the lock is sort-of exposed to the outside world, make the locks recursive. Although the only case where this is actually needed (although trivial) is sub_set_extradata(). One corner case are ASS subtitles: for some reason, we keep a single ASS_Renderer instance for subtitles around (probably to avoid rescanning fonts with ordered chapters), and this ASS_Renderer instance is not synchronized. Also, demux_libass.c loads ASS_Track objects, which are directly passed to sd_ass.c. These things are not synchronized (and would be hard to synchronize), and basically we're out of luck. But I think for now, accesses happen reasonably serialized, so there is no actual problem yet, even if we start to access OSD from other threads. --- sub/dec_sub.c | 77 ++++++++++++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 71 insertions(+), 6 deletions(-) (limited to 'sub/dec_sub.c') diff --git a/sub/dec_sub.c b/sub/dec_sub.c index 102a048da3..64d285f568 100644 --- a/sub/dec_sub.c +++ b/sub/dec_sub.c @@ -21,6 +21,7 @@ #include #include #include +#include #include "config.h" #include "demux/demux.h" @@ -57,6 +58,8 @@ static const struct sd_functions *sd_list[] = { #define MAX_NUM_SD 3 struct dec_sub { + pthread_mutex_t lock; + struct mp_log *log; struct MPOpts *opts; struct sd init_sd; @@ -73,11 +76,32 @@ struct packet_list { int num_packets; }; + +void sub_lock(struct dec_sub *sub) +{ + pthread_mutex_lock(&sub->lock); +} + +void sub_unlock(struct dec_sub *sub) +{ + pthread_mutex_unlock(&sub->lock); +} + +// Thread-safety of the returned object: all functions are thread-safe, +// except sub_get_bitmaps() and sub_get_text(). Decoder backends (sd_*) +// do not need to acquire locks. struct dec_sub *sub_create(struct mpv_global *global) { struct dec_sub *sub = talloc_zero(NULL, struct dec_sub); sub->log = mp_log_new(sub, global->log, "sub"); sub->opts = global->opts; + + pthread_mutexattr_t attr; + pthread_mutexattr_init(&attr); + pthread_mutexattr_settype(&attr, PTHREAD_MUTEX_RECURSIVE); + pthread_mutex_init(&sub->lock, &attr); + pthread_mutexattr_destroy(&attr); + return sub; } @@ -97,12 +121,16 @@ void sub_destroy(struct dec_sub *sub) if (!sub) return; sub_uninit(sub); + pthread_mutex_destroy(&sub->lock); talloc_free(sub); } bool sub_is_initialized(struct dec_sub *sub) { - return !!sub->num_sd; + pthread_mutex_lock(&sub->lock); + bool r = !!sub->num_sd; + pthread_mutex_unlock(&sub->lock); + return r; } static struct sd *sub_get_last_sd(struct dec_sub *sub) @@ -112,26 +140,34 @@ static struct sd *sub_get_last_sd(struct dec_sub *sub) void sub_set_video_res(struct dec_sub *sub, int w, int h) { + pthread_mutex_lock(&sub->lock); sub->init_sd.sub_video_w = w; sub->init_sd.sub_video_h = h; + pthread_mutex_unlock(&sub->lock); } void sub_set_video_fps(struct dec_sub *sub, double fps) { + pthread_mutex_lock(&sub->lock); sub->video_fps = fps; + pthread_mutex_unlock(&sub->lock); } void sub_set_extradata(struct dec_sub *sub, void *data, int data_len) { + pthread_mutex_lock(&sub->lock); sub->init_sd.extradata = data_len ? talloc_memdup(sub, data, data_len) : NULL; sub->init_sd.extradata_len = data_len; + pthread_mutex_unlock(&sub->lock); } void sub_set_ass_renderer(struct dec_sub *sub, struct ass_library *ass_library, struct ass_renderer *ass_renderer) { + pthread_mutex_lock(&sub->lock); sub->init_sd.ass_library = ass_library; sub->init_sd.ass_renderer = ass_renderer; + pthread_mutex_unlock(&sub->lock); } static void print_chain(struct dec_sub *sub) @@ -170,6 +206,8 @@ void sub_init_from_sh(struct dec_sub *sub, struct sh_stream *sh) assert(!sub->num_sd); assert(sh && sh->sub); + pthread_mutex_lock(&sub->lock); + if (sh->sub->extradata && !sub->init_sd.extradata) sub_set_extradata(sub, sh->sub->extradata, sh->sub->extradata_len); struct sd init_sd = sub->init_sd; @@ -189,6 +227,7 @@ void sub_init_from_sh(struct dec_sub *sub, struct sh_stream *sh) // Try adding new converters until a decoder is reached if (sd->driver->get_bitmaps || sd->driver->get_text) { print_chain(sub); + pthread_mutex_unlock(&sub->lock); return; } init_sd = (struct sd) { @@ -204,6 +243,7 @@ void sub_init_from_sh(struct dec_sub *sub, struct sh_stream *sh) sub_uninit(sub); MP_ERR(sub, "Could not find subtitle decoder for format '%s'.\n", sh->codec ? sh->codec : ""); + pthread_mutex_unlock(&sub->lock); } static struct demux_packet *get_decoded_packet(struct sd *sd) @@ -262,7 +302,9 @@ static void decode_chain_recode(struct dec_sub *sub, struct sd **sd, int num_sd, void sub_decode(struct dec_sub *sub, struct demux_packet *packet) { + pthread_mutex_lock(&sub->lock); decode_chain_recode(sub, sub->sd, sub->num_sd, packet); + pthread_mutex_unlock(&sub->lock); } static const char *guess_sub_cp(struct mp_log *log, struct packet_list *subs, @@ -373,8 +415,13 @@ bool sub_read_all_packets(struct dec_sub *sub, struct sh_stream *sh) assert(sh && sh->sub); struct MPOpts *opts = sub->opts; + pthread_mutex_lock(&sub->lock); + if (!sub_accept_packets_in_advance(sub) || sh->sub->track || sub->num_sd < 1) + { + pthread_mutex_unlock(&sub->lock); return false; + } struct packet_list *subs = talloc_zero(NULL, struct packet_list); @@ -446,17 +493,24 @@ bool sub_read_all_packets(struct dec_sub *sub, struct sh_stream *sh) add_sub_list(sub, preprocess, subs); + pthread_mutex_unlock(&sub->lock); talloc_free(subs); return true; } bool sub_accept_packets_in_advance(struct dec_sub *sub) { + pthread_mutex_lock(&sub->lock); // Converters are assumed to always accept packets in advance struct sd *sd = sub_get_last_sd(sub); - return sd && sd->driver->accept_packets_in_advance; + bool r = sd && sd->driver->accept_packets_in_advance; + pthread_mutex_unlock(&sub->lock); + return r; } +// You must call sub_lock/sub_unlock if more than 1 thread access sub. +// The issue is that *res will contain decoder allocated data, which might +// be deallocated on the next decoder access. void sub_get_bitmaps(struct dec_sub *sub, struct mp_osd_res dim, double pts, struct sub_bitmaps *res) { @@ -472,12 +526,17 @@ void sub_get_bitmaps(struct dec_sub *sub, struct mp_osd_res dim, double pts, bool sub_has_get_text(struct dec_sub *sub) { + pthread_mutex_lock(&sub->lock); struct sd *sd = sub_get_last_sd(sub); - return sd && sd->driver->get_text; + bool r = sd && sd->driver->get_text; + pthread_mutex_unlock(&sub->lock); + return r; } +// See sub_get_bitmaps() for locking requirements. char *sub_get_text(struct dec_sub *sub, double pts) { + pthread_mutex_lock(&sub->lock); struct MPOpts *opts = sub->opts; struct sd *sd = sub_get_last_sd(sub); char *text = NULL; @@ -485,27 +544,33 @@ char *sub_get_text(struct dec_sub *sub, double pts) if (sd->driver->get_text) text = sd->driver->get_text(sd, pts); } + pthread_mutex_unlock(&sub->lock); return text; } void sub_reset(struct dec_sub *sub) { + pthread_mutex_lock(&sub->lock); for (int n = 0; n < sub->num_sd; n++) { if (sub->sd[n]->driver->reset) sub->sd[n]->driver->reset(sub->sd[n]); } + pthread_mutex_unlock(&sub->lock); } int sub_control(struct dec_sub *sub, enum sd_ctrl cmd, void *arg) { + int r = CONTROL_UNKNOWN; + pthread_mutex_lock(&sub->lock); for (int n = 0; n < sub->num_sd; n++) { if (sub->sd[n]->driver->control) { - int r = sub->sd[n]->driver->control(sub->sd[n], cmd, arg); + r = sub->sd[n]->driver->control(sub->sd[n], cmd, arg); if (r != CONTROL_UNKNOWN) - return r; + break; } } - return CONTROL_UNKNOWN; + pthread_mutex_unlock(&sub->lock); + return r; } #define MAX_PACKETS 10 -- cgit v1.2.3