From bf71b4be9f36f9d7c619053d97ab6f98bb98dc21 Mon Sep 17 00:00:00 2001 From: David Robillard Date: Wed, 14 Dec 2016 14:44:32 -0500 Subject: Fix real-time safety of control bindings --- src/server/Buffer.cpp | 24 +++++++ src/server/Buffer.hpp | 3 + src/server/ControlBindings.cpp | 138 ++++++++++++++++++----------------------- src/server/ControlBindings.hpp | 83 +++++++++++++++---------- src/server/events/Delete.cpp | 11 +++- src/server/events/Delete.hpp | 4 +- src/server/events/Delta.cpp | 54 +++++++++------- src/server/events/Delta.hpp | 32 +++++----- 8 files changed, 197 insertions(+), 152 deletions(-) (limited to 'src') diff --git a/src/server/Buffer.cpp b/src/server/Buffer.cpp index 71dd320a..cf313b67 100644 --- a/src/server/Buffer.cpp +++ b/src/server/Buffer.cpp @@ -341,6 +341,30 @@ Buffer::append_event(int64_t frames, const LV2_Atom* body) return append_event(frames, body->size, body->type, (const uint8_t*)(body + 1)); } +bool +Buffer::append_event_buffer(const Buffer* buf) +{ + LV2_Atom_Sequence* seq = (LV2_Atom_Sequence*)get(); + LV2_Atom_Sequence* bseq = (LV2_Atom_Sequence*)buf->get(); + if (seq->atom.type == _factory.uris().atom_Chunk) { + clear(); // Chunk initialized with prepare_output_write(), clear + } + + const uint32_t total_size = lv2_atom_total_size(&seq->atom); + uint8_t* const end = (uint8_t*)seq + total_size; + const uint32_t n_bytes = bseq->atom.size - sizeof(bseq->body); + if (sizeof(LV2_Atom) + total_size + n_bytes >= _capacity) { + return false; // Not enough space + } + + memcpy(end, bseq + 1, n_bytes); + seq->atom.size += n_bytes; + + _latest_event = std::max(_latest_event, buf->_latest_event); + + return true; +} + SampleCount Buffer::next_value_offset(SampleCount offset, SampleCount end) const { diff --git a/src/server/Buffer.hpp b/src/server/Buffer.hpp index 6404479b..07ec5ffb 100644 --- a/src/server/Buffer.hpp +++ b/src/server/Buffer.hpp @@ -176,6 +176,9 @@ public: /// Sequence buffers only bool append_event(int64_t frames, const LV2_Atom* body); + /// Sequence buffers only + bool append_event_buffer(const Buffer* buf); + /// Value buffer for numeric sequences BufferRef value_buffer() { return _value_buffer; } diff --git a/src/server/ControlBindings.cpp b/src/server/ControlBindings.cpp index fc3ac7de..2c6d3acf 100644 --- a/src/server/ControlBindings.cpp +++ b/src/server/ControlBindings.cpp @@ -38,7 +38,7 @@ namespace Server { ControlBindings::ControlBindings(Engine& engine) : _engine(engine) - , _learn_port(NULL) + , _learn_binding(NULL) , _bindings(new Bindings()) , _feedback(new Buffer(*_engine.buffer_factory(), engine.world()->uris().atom_Sequence, @@ -52,14 +52,15 @@ ControlBindings::ControlBindings(Engine& engine) ControlBindings::~ControlBindings() { _feedback.reset(); + delete _learn_binding.load(); } ControlBindings::Key ControlBindings::port_binding(PortImpl* port) const { ThreadManager::assert_thread(THREAD_PRE_PROCESS); - const Ingen::URIs& uris = _engine.world()->uris(); - const Atom& binding = port->get_property(uris.midi_binding); + const Ingen::URIs& uris = _engine.world()->uris(); + const Atom& binding = port->get_property(uris.midi_binding); return binding_key(binding); } @@ -124,14 +125,20 @@ ControlBindings::midi_event_key(uint16_t size, const uint8_t* buf, uint16_t& val } } -void -ControlBindings::port_binding_changed(RunContext& context, - PortImpl* port, - const Atom& binding) +bool +ControlBindings::set_port_binding(RunContext& context, + PortImpl* port, + Binding* binding, + const Atom& value) { - const Key key = binding_key(binding); + const Key key = binding_key(value); if (key) { - _bindings->insert(make_pair(key, port)); + binding->key = key; + binding->port = port; + _bindings->insert(*binding); + return true; + } else { + return false; } } @@ -180,16 +187,21 @@ ControlBindings::port_value_changed(RunContext& context, break; } if (size > 0) { - _feedback->append_event(0, size, (LV2_URID)uris.midi_MidiEvent, buf); + _feedback->append_event(context.nframes() - 1, size, (LV2_URID)uris.midi_MidiEvent, buf); } } } void -ControlBindings::learn(PortImpl* port) +ControlBindings::start_learn(PortImpl* port) { ThreadManager::assert_thread(THREAD_PRE_PROCESS); - _learn_port = port; + Binding* b = _learn_binding.load(); + if (!b) { + _learn_binding = new Binding(Type::NULL_CONTROL, port); + } else { + b->port = port; + } } static void @@ -203,7 +215,7 @@ get_range(RunContext& context, const PortImpl* port, float* min, float* max) } } -Atom +float ControlBindings::control_to_port_value(RunContext& context, const PortImpl* port, Type type, @@ -232,7 +244,7 @@ ControlBindings::control_to_port_value(RunContext& context, float min, max; get_range(context, port, &min, &max); - return _engine.world()->forge().make(normal * (max - min) + min); + return normal * (max - min) + min; } int16_t @@ -315,28 +327,27 @@ ControlBindings::set_port_value(RunContext& context, float min, max; get_range(context, port, &min, &max); - const Atom port_value(control_to_port_value(context, port, type, value)); + const float val = control_to_port_value(context, port, type, value); - assert(port_value.type() == port->bufs().forge().Float); - port->set_value(port_value); // FIXME: not thread safe - port->set_control_value(context, context.start(), port_value.get()); + // TODO: Set port value property so it is saved + port->set_control_value(context, context.start(), val); URIs& uris = context.engine().world()->uris(); context.notify(uris.ingen_value, context.start(), port, - port_value.size(), port_value.type(), port_value.get_body()); + sizeof(float), _forge.Float, &val); } bool -ControlBindings::bind(RunContext& context, Key key) +ControlBindings::finish_learn(RunContext& context, Key key) { - const Ingen::URIs& uris = context.engine().world()->uris(); - assert(_learn_port); - if (key.type == Type::MIDI_NOTE) { - if (!_learn_port->is_toggled()) - return false; + const Ingen::URIs& uris = context.engine().world()->uris(); + Binding* binding = _learn_binding.exchange(NULL); + if (!binding || (key.type == Type::MIDI_NOTE && !binding->port->is_toggled())) { + return false; } - _bindings->insert(make_pair(key, _learn_port)); + binding->key = key; + _bindings->insert(*binding); uint8_t buf[128]; memset(buf, 0, sizeof(buf)); @@ -345,70 +356,42 @@ ControlBindings::bind(RunContext& context, Key key) const LV2_Atom* atom = (const LV2_Atom*)buf; context.notify(uris.midi_binding, context.start(), - _learn_port, + binding->port, atom->size, atom->type, LV2_ATOM_BODY_CONST(atom)); - _learn_port = NULL; return true; } -SPtr -ControlBindings::remove(const Raul::Path& path) +void +ControlBindings::get_all(const Raul::Path& path, std::vector& bindings) { ThreadManager::assert_thread(THREAD_PRE_PROCESS); - SPtr old_bindings(_bindings); - SPtr copy(new Bindings(*_bindings.get())); - - for (Bindings::iterator i = copy->begin(); i != copy->end();) { - Bindings::iterator next = i; - ++next; - - if (i->second->path() == path || i->second->path().is_child_of(path)) - copy->erase(i); - - i = next; + for (Binding& b : *_bindings) { + if (b.port->path() == path || b.port->path().is_child_of(path)) { + bindings.push_back(&b); + } } - - _bindings = copy; - return old_bindings; } -SPtr -ControlBindings::remove(PortImpl* port) +void +ControlBindings::remove(RunContext& ctx, const std::vector& bindings) { - ThreadManager::assert_thread(THREAD_PRE_PROCESS); - - SPtr old_bindings(_bindings); - SPtr copy(new Bindings(*_bindings.get())); - - for (Bindings::iterator i = copy->begin(); i != copy->end();) { - Bindings::iterator next = i; - ++next; - - if (i->second == port) - copy->erase(i); - - i = next; + for (Binding* b : bindings) { + _bindings->erase(*b); } - - _bindings = copy; - return old_bindings; } void ControlBindings::pre_process(RunContext& context, Buffer* buffer) { - uint16_t value = 0; - Bindings* bindings = _bindings.get(); - _feedback->clear(); - + uint16_t value = 0; Ingen::World* world = context.engine().world(); const Ingen::URIs& uris = world->uris(); - if (!_learn_port && bindings->empty()) { - // Don't bother reading input - return; + _feedback->clear(); + if (!_learn_binding && _bindings->empty()) { + return; // Don't bother reading input } LV2_Atom_Sequence* seq = buffer->get(); @@ -416,13 +399,17 @@ ControlBindings::pre_process(RunContext& context, Buffer* buffer) if (ev->body.type == uris.midi_MidiEvent) { const uint8_t* buf = (const uint8_t*)LV2_ATOM_BODY(&ev->body); const Key key = midi_event_key(ev->body.size, buf, value); - if (_learn_port && key) { - bind(context, key); + + if (_learn_binding && key) { + finish_learn(context, key); // Learn new binding } - Bindings::const_iterator i = bindings->find(key); - if (i != bindings->end()) { - set_port_value(context, i->second, key.type, value); + // Set all controls bound to this key + const Binding k = {key, NULL}; + for (Bindings::const_iterator i = _bindings->find(k); + i != _bindings->end() && i->key == key; + ++i) { + set_port_value(context, i->port, key.type, value); } } } @@ -431,8 +418,7 @@ ControlBindings::pre_process(RunContext& context, Buffer* buffer) void ControlBindings::post_process(RunContext& context, Buffer* buffer) { - // TODO: merge buffer's existing contents (anything send to it in the graph) - buffer->copy(context, _feedback.get()); + buffer->append_event_buffer(_feedback.get()); } } // namespace Server diff --git a/src/server/ControlBindings.hpp b/src/server/ControlBindings.hpp index 532ee4d8..8a44cd51 100644 --- a/src/server/ControlBindings.hpp +++ b/src/server/ControlBindings.hpp @@ -17,12 +17,15 @@ #ifndef INGEN_ENGINE_CONTROLBINDINGS_HPP #define INGEN_ENGINE_CONTROLBINDINGS_HPP -#include +#include +#include +#include #include #include "ingen/Atom.hpp" #include "ingen/types.hpp" #include "lv2/lv2plug.in/ns/ext/atom/forge.h" +#include "raul/Maid.hpp" #include "raul/Path.hpp" #include "BufferFactory.hpp" @@ -36,7 +39,7 @@ class PortImpl; class ControlBindings { public: - enum class Type { + enum class Type : uint16_t { NULL_CONTROL, MIDI_BENDER, MIDI_CC, @@ -56,7 +59,23 @@ public: int16_t num; }; - typedef std::map Bindings; + /** One binding of a controller to a port. */ + struct Binding : public boost::intrusive::set_base_hook<>, + public Raul::Maid::Disposable { + Binding(Key k=Key(), PortImpl* p=NULL) : key(k), port(p) {} + + inline bool operator<(const Binding& rhs) const { return key < rhs.key; } + + Key key; + PortImpl* port; + }; + + /** Comparator for bindings by key. */ + struct BindingLess { + bool operator()(const Binding& lhs, const Binding& rhs) const { + return lhs.key < rhs.key; + } + }; explicit ControlBindings(Engine& engine); ~ControlBindings(); @@ -64,57 +83,57 @@ public: Key port_binding(PortImpl* port) const; Key binding_key(const Atom& binding) const; - void learn(PortImpl* port); + void start_learn(PortImpl* port); - void port_binding_changed(RunContext& context, - PortImpl* port, - const Atom& binding); + /** Set the binding for `port` to `binding` and take ownership of it. */ + bool set_port_binding(RunContext& ctx, + PortImpl* port, + Binding* binding, + const Atom& value); - void port_value_changed(RunContext& context, + void port_value_changed(RunContext& ctx, PortImpl* port, Key key, const Atom& value); - void pre_process(RunContext& context, Buffer* control_in); - void post_process(RunContext& context, Buffer* control_out); + void pre_process(RunContext& ctx, Buffer* control_in); + void post_process(RunContext& ctx, Buffer* control_out); - /** Remove all bindings for `path` or children of `path`. - * The caller must safely drop the returned reference in the - * post-processing thread after at least one process thread has run. - */ - SPtr remove(const Raul::Path& path); + /** Get all bindings for `path` or children of `path`. */ + void get_all(const Raul::Path& path, std::vector& bindings); - /** Remove binding for a particular port. - * The caller must safely drop the returned reference in the - * post-processing thread after at least one process thread has run. - */ - SPtr remove(PortImpl* port); + /** Remove a set of bindings from an earlier call to get_all(). */ + void remove(RunContext& ctx, const std::vector& bindings); private: + typedef boost::intrusive::multiset< + Binding, + boost::intrusive::compare > Bindings; + Key midi_event_key(uint16_t size, const uint8_t* buf, uint16_t& value); - void set_port_value(RunContext& context, + void set_port_value(RunContext& ctx, PortImpl* port, Type type, int16_t value); - bool bind(RunContext& context, Key key); + bool finish_learn(RunContext& ctx, Key key); - Atom control_to_port_value(RunContext& context, - const PortImpl* port, - Type type, - int16_t value) const; + float control_to_port_value(RunContext& ctx, + const PortImpl* port, + Type type, + int16_t value) const; - int16_t port_value_to_control(RunContext& context, + int16_t port_value_to_control(RunContext& ctx, PortImpl* port, Type type, const Atom& value) const; - Engine& _engine; - PortImpl* _learn_port; - SPtr _bindings; - BufferRef _feedback; - LV2_Atom_Forge _forge; + Engine& _engine; + std::atomic _learn_binding; + SPtr _bindings; + BufferRef _feedback; + LV2_Atom_Forge _forge; }; } // namespace Server diff --git a/src/server/events/Delete.cpp b/src/server/events/Delete.cpp index d04f176c..b77f7c3e 100644 --- a/src/server/events/Delete.cpp +++ b/src/server/events/Delete.cpp @@ -56,6 +56,9 @@ Delete::~Delete() { delete _disconnect_event; delete _compiled_graph; + for (ControlBindings::Binding* b : _removed_bindings) { + delete b; + } } bool @@ -65,7 +68,7 @@ Delete::pre_process(PreProcessContext& ctx) return Event::pre_process_done(Status::NOT_DELETABLE, _path); } - _removed_bindings = _engine.control_bindings()->remove(_path); + _engine.control_bindings()->get_all(_path, _removed_bindings); Store::iterator iter = _engine.store()->find(_path); if (iter == _engine.store()->end()) { @@ -131,6 +134,10 @@ Delete::execute(RunContext& context) _disconnect_event->execute(context); } + if (!_removed_bindings.empty()) { + _engine.control_bindings()->remove(context, _removed_bindings); + } + GraphImpl* parent = _block ? _block->parent_graph() : NULL; if (_port) { parent = _port->parent_graph(); @@ -150,8 +157,6 @@ Delete::execute(RunContext& context) void Delete::post_process() { - _removed_bindings.reset(); - Broadcaster::Transfer t(*_engine.broadcaster()); if (respond() == Status::SUCCESS && (_block || _port)) { if (_block) { diff --git a/src/server/events/Delete.hpp b/src/server/events/Delete.hpp index fd797804..1b34faf6 100644 --- a/src/server/events/Delete.hpp +++ b/src/server/events/Delete.hpp @@ -68,9 +68,9 @@ private: Raul::Array* _ports_array; ///< New (external) ports for Graph CompiledGraph* _compiled_graph; ///< Graph's new process order DisconnectAll* _disconnect_event; + Store::Objects _removed_objects; - SPtr _removed_bindings; - Store::Objects _removed_objects; + std::vector _removed_bindings; }; } // namespace Events diff --git a/src/server/events/Delta.cpp b/src/server/events/Delta.cpp index 66897c66..c623d51e 100644 --- a/src/server/events/Delta.cpp +++ b/src/server/events/Delta.cpp @@ -61,6 +61,7 @@ Delta::Delta(Engine& engine, , _object(NULL) , _graph(NULL) , _compiled_graph(NULL) + , _binding(NULL) , _state(NULL) , _context(context) , _type(type) @@ -223,8 +224,9 @@ Delta::pre_process(PreProcessContext& ctx) const Atom& value = r.second; if (key == uris.midi_binding && value == uris.patch_wildcard) { PortImpl* port = dynamic_cast(_object); - if (port) - _old_bindings = _engine.control_bindings()->remove(port); + if (port) { + _engine.control_bindings()->get_all(port->path(), _removed_bindings); + } } if (_object) { _removed.emplace(key, value); @@ -299,9 +301,10 @@ Delta::pre_process(PreProcessContext& ctx) } else if (key == uris.midi_binding) { if (port->is_a(PortType::CONTROL) || port->is_a(PortType::CV)) { if (value == uris.patch_wildcard) { - _engine.control_bindings()->learn(port); + _engine.control_bindings()->start_learn(port); } else if (value.type() == uris.atom_Object) { - op = SpecialType::CONTROL_BINDING; + op = SpecialType::CONTROL_BINDING; + _binding = new ControlBindings::Binding(); } else { _status = Status::BAD_VALUE_TYPE; } @@ -372,24 +375,23 @@ Delta::pre_process(PreProcessContext& ctx) if (!_create_event && key == uris.ingen_polyphonic) { GraphImpl* parent = dynamic_cast(obj->parent()); - if (parent) { - if (value.type() == uris.forge.Bool) { - poly_changed = true; - op = SpecialType::POLYPHONIC; - obj->set_property(key, value, value.context()); - BlockImpl* block = dynamic_cast(obj); - if (block) - block->set_polyphonic(value.get()); - if (value.get()) { - obj->prepare_poly(*_engine.buffer_factory(), parent->internal_poly()); - } else { - obj->prepare_poly(*_engine.buffer_factory(), 1); - } + if (!parent) { + _status = Status::BAD_OBJECT_TYPE; + } else if (value.type() != uris.forge.Bool) { + _status = Status::BAD_VALUE_TYPE; + } else { + poly_changed = true; + op = SpecialType::POLYPHONIC; + obj->set_property(key, value, value.context()); + BlockImpl* block = dynamic_cast(obj); + if (block) { + block->set_polyphonic(value.get()); + } + if (value.get()) { + obj->prepare_poly(*_engine.buffer_factory(), parent->internal_poly()); } else { - _status = Status::BAD_VALUE_TYPE; + obj->prepare_poly(*_engine.buffer_factory(), 1); } - } else { - _status = Status::BAD_OBJECT_TYPE; } } } else if (is_client && key == uris.ingen_broadcast) { @@ -454,6 +456,10 @@ Delta::execute(RunContext& context) s->execute(context); } + if (!_removed_bindings.empty()) { + _engine.control_bindings()->remove(context, _removed_bindings); + } + NodeImpl* const object = dynamic_cast(_object); BlockImpl* const block = dynamic_cast(_object); PortImpl* const port = dynamic_cast(_object); @@ -501,7 +507,9 @@ Delta::execute(RunContext& context) break; case SpecialType::CONTROL_BINDING: if (port) { - _engine.control_bindings()->port_binding_changed(context, port, value); + if (!_engine.control_bindings()->set_port_binding(context, port, _binding, value)) { + _status = Status::BAD_VALUE; + } } else if (block) { if (uris.ingen_Internal == block->plugin_impl()->type()) { block->learn(); @@ -513,9 +521,9 @@ Delta::execute(RunContext& context) break; case SpecialType::NONE: if (port) { - if (key == uris.lv2_minimum) { + if (!strcmp(uris.lv2_minimum.c_str(), key.c_str())) { port->set_minimum(value); - } else if (key == uris.lv2_maximum) { + } else if (!strcmp(uris.lv2_maximum.c_str(), key.c_str())) { port->set_maximum(value); } } diff --git a/src/server/events/Delta.hpp b/src/server/events/Delta.hpp index 98570210..ca9d0276 100644 --- a/src/server/events/Delta.hpp +++ b/src/server/events/Delta.hpp @@ -92,26 +92,26 @@ private: typedef std::vector SetEvents; - Event* _create_event; - SetEvents _set_events; - std::vector _types; - std::vector _remove_types; - Raul::URI _subject; - Resource::Properties _properties; - Resource::Properties _remove; - ClientUpdate _update; - Ingen::Resource* _object; - GraphImpl* _graph; - CompiledGraph* _compiled_graph; - LilvState* _state; - Resource::Graph _context; - ControlBindings::Key _binding; - Type _type; + Event* _create_event; + SetEvents _set_events; + std::vector _types; + std::vector _remove_types; + Raul::URI _subject; + Resource::Properties _properties; + Resource::Properties _remove; + ClientUpdate _update; + Ingen::Resource* _object; + GraphImpl* _graph; + CompiledGraph* _compiled_graph; + ControlBindings::Binding* _binding; + LilvState* _state; + Resource::Graph _context; + Type _type; Resource::Properties _added; Resource::Properties _removed; - SPtr _old_bindings; + std::vector _removed_bindings; boost::optional _preset; -- cgit v1.2.1