From 7c2302651559eda71833c291ddc17f4d590ad95a Mon Sep 17 00:00:00 2001 From: David Robillard Date: Sun, 31 Jul 2016 21:59:21 -0400 Subject: [PATCH] Support thread-safe LV2 state restoration The original LV2 state extension required that run() is suspended during restore(). Ardour violates this rule, which can lead to crashes and other issues. The state extension has been updated to allow restoring state in a thread-safe way by using the worker to enqueue state changes. This commit supports that new specification, i.e. supports dropout-free state restoration properly. However, the bug with old plugins that do not use this facility is still not fixed. --- libs/ardour/ardour/lv2_plugin.h | 6 ++- libs/ardour/ardour/worker.h | 36 ++++++++++++----- libs/ardour/lv2_plugin.cc | 68 +++++++++++++++++++++------------ libs/ardour/worker.cc | 30 ++++++++++----- 4 files changed, 95 insertions(+), 45 deletions(-) diff --git a/libs/ardour/ardour/lv2_plugin.h b/libs/ardour/ardour/lv2_plugin.h index efd4903e07..7f312459f5 100644 --- a/libs/ardour/ardour/lv2_plugin.h +++ b/libs/ardour/ardour/lv2_plugin.h @@ -20,6 +20,7 @@ #ifndef __ardour_lv2_plugin_h__ #define __ardour_lv2_plugin_h__ +#include #include #include #include @@ -163,7 +164,7 @@ class LIBARDOUR_API LV2Plugin : public ARDOUR::Plugin, public ARDOUR::Workee URIMap& uri_map() { return _uri_map; } const URIMap& uri_map() const { return _uri_map; } - int work(uint32_t size, const void* data); + int work(Worker& worker, uint32_t size, const void* data); int work_response(uint32_t size, const void* data); void set_property(uint32_t key, const Variant& value); @@ -177,6 +178,7 @@ class LIBARDOUR_API LV2Plugin : public ARDOUR::Plugin, public ARDOUR::Workee void* _module; LV2_Feature** _features; Worker* _worker; + Worker* _state_worker; framecnt_t _sample_rate; float* _control_data; float* _shadow_data; @@ -268,6 +270,8 @@ class LIBARDOUR_API LV2Plugin : public ARDOUR::Plugin, public ARDOUR::Workee RingBuffer* _to_ui; RingBuffer* _from_ui; + Glib::Threads::Mutex _work_mutex; + #ifdef LV2_EXTENDED const LV2_Inline_Display_Interface* _display_interface; #endif diff --git a/libs/ardour/ardour/worker.h b/libs/ardour/ardour/worker.h index c83f006f72..6e1a7c91f1 100644 --- a/libs/ardour/ardour/worker.h +++ b/libs/ardour/ardour/worker.h @@ -1,5 +1,5 @@ /* - Copyright (C) 2012 Paul Davis + Copyright (C) 2012-2016 Paul Davis Author: David Robillard This program is free software; you can redistribute it and/or modify @@ -31,6 +31,8 @@ namespace ARDOUR { +class Worker; + /** An object that needs to schedule non-RT work in the audio thread. */ @@ -41,7 +43,7 @@ public: /** Do some work in the worker thread. */ - virtual int work(uint32_t size, const void* data) = 0; + virtual int work(Worker& worker, uint32_t size, const void* data) = 0; /** Handle a response from the worker thread in the audio thread. @@ -50,12 +52,16 @@ public: }; /** - A worker thread for non-realtime tasks scheduled in the audio thread. + A worker for non-realtime tasks scheduled from another thread. + + A worker may be a separate thread that runs to execute scheduled work + asynchronously, or unthreaded, in which case work is executed immediately + upon scheduling by the calling thread. */ class LIBARDOUR_API Worker { public: - Worker(Workee* workee, uint32_t ring_size); + Worker(Workee* workee, uint32_t ring_size, bool threaded=true); ~Worker(); /** @@ -75,6 +81,16 @@ public: */ void emit_responses(); + /** + Enable or disable synchronous execution. + + If enabled, all work is performed immediately in schedule() regardless + of whether or not the worker is threaded. This is used for exporting, + where we want to temporarily execute all work synchronously but the + worker is typically used threaded for live rolling. + */ + void set_synchronous(bool synchronous) { _synchronous = synchronous; } + private: void run(); /** @@ -83,19 +99,19 @@ private: Handle the unlikley edge-case, if we're called in between the responder writing 'size' and 'data'. - @param rb the ringbuffer to check - @return true if the message is complete, false otherwise - */ + @param rb the ringbuffer to check + @return true if the message is complete, false otherwise + */ bool verify_message_completeness(RingBuffer* rb); Workee* _workee; RingBuffer* _requests; RingBuffer* _responses; uint8_t* _response; - PBD::Semaphore _sem; - bool _exit; + PBD::Semaphore _sem; Glib::Threads::Thread* _thread; - + bool _exit; + bool _synchronous; }; } // namespace ARDOUR diff --git a/libs/ardour/lv2_plugin.cc b/libs/ardour/lv2_plugin.cc index 7ecc0980ab..b0c67bf034 100644 --- a/libs/ardour/lv2_plugin.cc +++ b/libs/ardour/lv2_plugin.cc @@ -196,15 +196,9 @@ work_schedule(LV2_Worker_Schedule_Handle handle, uint32_t size, const void* data) { - LV2Plugin* plugin = (LV2Plugin*)handle; - if (plugin->session().engine().freewheeling()) { - // Freewheeling, do the work immediately in this (audio) thread - return (LV2_Worker_Status)plugin->work(size, data); - } else { - // Enqueue message for the worker thread - return plugin->worker()->schedule(size, data) ? - LV2_WORKER_SUCCESS : LV2_WORKER_ERR_UNKNOWN; - } + return (((Worker*)handle)->schedule(size, data) + ? LV2_WORKER_SUCCESS + : LV2_WORKER_ERR_UNKNOWN); } /** Called by the plugin to respond to non-RT work. */ @@ -213,15 +207,9 @@ work_respond(LV2_Worker_Respond_Handle handle, uint32_t size, const void* data) { - LV2Plugin* plugin = (LV2Plugin*)handle; - if (plugin->session().engine().freewheeling()) { - // Freewheeling, respond immediately in this (audio) thread - return (LV2_Worker_Status)plugin->work_response(size, data); - } else { - // Enqueue response for the worker - return plugin->worker()->respond(size, data) ? - LV2_WORKER_SUCCESS : LV2_WORKER_ERR_UNKNOWN; - } + return (((Worker*)handle)->respond(size, data) + ? LV2_WORKER_SUCCESS + : LV2_WORKER_ERR_UNKNOWN); } #ifdef LV2_EXTENDED @@ -328,6 +316,7 @@ LV2Plugin::LV2Plugin (AudioEngine& engine, , _impl(new Impl()) , _features(NULL) , _worker(NULL) + , _state_worker(NULL) , _insert_id("0") , _patch_port_in_index((uint32_t)-1) , _patch_port_out_index((uint32_t)-1) @@ -343,6 +332,7 @@ LV2Plugin::LV2Plugin (const LV2Plugin& other) , _impl(new Impl()) , _features(NULL) , _worker(NULL) + , _state_worker(NULL) , _insert_id(other._insert_id) , _patch_port_in_index((uint32_t)-1) , _patch_port_out_index((uint32_t)-1) @@ -477,19 +467,24 @@ LV2Plugin::init(const void* c_plugin, framecnt_t rate) log->vprintf = &log_vprintf; _log_feature.data = log; + const size_t ring_size = _session.engine().raw_buffer_size(DataType::MIDI) * NBUFS; LilvNode* worker_schedule = lilv_new_uri(_world.world, LV2_WORKER__schedule); if (lilv_plugin_has_feature(plugin, worker_schedule)) { LV2_Worker_Schedule* schedule = (LV2_Worker_Schedule*)malloc( sizeof(LV2_Worker_Schedule)); - size_t buf_size = _session.engine().raw_buffer_size(DataType::MIDI) * NBUFS; - _worker = new Worker(this, buf_size); - schedule->handle = this; + _worker = new Worker(this, ring_size); + schedule->handle = _worker; schedule->schedule_work = work_schedule; _work_schedule_feature.data = schedule; _features[n_features++] = &_work_schedule_feature; } lilv_node_free(worker_schedule); + if (_has_state_interface) { + // Create a non-threaded worker for use by state restore + _state_worker = new Worker(this, ring_size, false); + } + _impl->instance = lilv_plugin_instantiate(plugin, rate, _features); _impl->name = lilv_plugin_get_name(plugin); _impl->author = lilv_plugin_get_author_name(plugin); @@ -848,6 +843,7 @@ LV2Plugin::~LV2Plugin () delete _to_ui; delete _from_ui; delete _worker; + delete _state_worker; if (_atom_ev_buffers) { LV2_Evbuf** b = _atom_ev_buffers; @@ -1340,8 +1336,15 @@ LV2Plugin::load_preset(PresetRecord r) LilvNode* pset = lilv_new_uri(world, r.uri.c_str()); LilvState* state = lilv_state_new_from_world(world, _uri_map.urid_map(), pset); + const LV2_Feature* state_features[2] = { NULL, NULL }; + LV2_Worker_Schedule schedule = { _state_worker, work_schedule }; + const LV2_Feature state_sched_feature = { LV2_WORKER__schedule, &schedule }; + if (_state_worker) { + state_features[1] = &state_sched_feature; + } + if (state) { - lilv_state_restore(state, _impl->instance, set_port_value, this, 0, NULL); + lilv_state_restore(state, _impl->instance, set_port_value, this, 0, state_features); lilv_state_free(state); Plugin::load_preset(r); } @@ -1843,10 +1846,11 @@ LV2Plugin::emit_to_ui(void* controller, UIMessageSink sink) } int -LV2Plugin::work(uint32_t size, const void* data) +LV2Plugin::work(Worker& worker, uint32_t size, const void* data) { + Glib::Threads::Mutex::Lock lm(_work_mutex); return _impl->work_iface->work( - _impl->instance->lv2_handle, work_respond, this, size, data); + _impl->instance->lv2_handle, work_respond, &worker, size, data); } int @@ -2811,10 +2815,24 @@ LV2Plugin::run(pframes_t nframes) } } + if (_worker) { + // Execute work synchronously if we're freewheeling (export) + _worker->set_synchronous(session().engine().freewheeling()); + } + + // Run the plugin for this cycle lilv_instance_run(_impl->instance, nframes); - if (_impl->work_iface) { + // Emit any queued worker responses (calls a plugin callback) + if (_state_worker) { + _state_worker->emit_responses(); + } + if (_worker) { _worker->emit_responses(); + } + + // Notify the plugin that a work run cycle is complete + if (_impl->work_iface) { if (_impl->work_iface->end_run) { _impl->work_iface->end_run(_impl->instance->lv2_handle); } diff --git a/libs/ardour/worker.cc b/libs/ardour/worker.cc index 9d6dd09b55..f8a5e44aee 100644 --- a/libs/ardour/worker.cc +++ b/libs/ardour/worker.cc @@ -1,5 +1,5 @@ /* - Copyright (C) 2012 Paul Davis + Copyright (C) 2012-2016 Paul Davis Author: David Robillard This program is free software; you can redistribute it and/or modify @@ -27,26 +27,38 @@ namespace ARDOUR { -Worker::Worker(Workee* workee, uint32_t ring_size) +Worker::Worker(Workee* workee, uint32_t ring_size, bool threaded) : _workee(workee) - , _requests(new RingBuffer(ring_size)) + , _requests(threaded ? new RingBuffer(ring_size) : NULL) , _responses(new RingBuffer(ring_size)) , _response((uint8_t*)malloc(ring_size)) - , _sem ("worker_semaphore", 0) + , _sem("worker_semaphore", 0) + , _thread(NULL) , _exit(false) - , _thread (Glib::Threads::Thread::create(sigc::mem_fun(*this, &Worker::run))) -{} + , _synchronous(!threaded) +{ + if (threaded) { + _thread = Glib::Threads::Thread::create( + sigc::mem_fun(*this, &Worker::run)); + } +} Worker::~Worker() { _exit = true; _sem.signal(); - _thread->join(); + if (_thread) { + _thread->join(); + } } bool Worker::schedule(uint32_t size, const void* data) { + if (_synchronous || !_requests) { + _workee->work(*this, size, data); + return true; + } if (_requests->write_space() < size + sizeof(size)) { return false; } @@ -124,7 +136,7 @@ Worker::run() while (true) { _sem.wait(); if (_exit) { - if (buf) free(buf); + free(buf); return; } @@ -163,7 +175,7 @@ Worker::run() continue; // TODO: This is probably fatal } - _workee->work(size, buf); + _workee->work(*this, size, buf); } } -- 2.30.2