From 43da08a1e3620cc296d27ef5cdf387693b063c68 Mon Sep 17 00:00:00 2001 From: Bent Bisballe Nyeng Date: Sun, 16 Feb 2020 21:36:24 +0100 Subject: Fix style, minor code fixes, and add review comments. --- src/drumgizmo.cc | 1 + src/events.h | 5 +++-- src/events_ds.cc | 15 ++++++++------- src/events_ds.h | 35 +++++++++++++++++++++++------------ src/inputprocessor.cc | 3 +-- src/inputprocessor.h | 2 +- src/memory_heap.h | 10 ++++++---- test/Makefile.am | 12 +++++++++++- test/eventsdstest.cc | 49 +++++++++++++++++++++++++++++++++++++++++++++++++ 9 files changed, 103 insertions(+), 29 deletions(-) create mode 100644 test/eventsdstest.cc diff --git a/src/drumgizmo.cc b/src/drumgizmo.cc index 6b01be2..fcc54ad 100644 --- a/src/drumgizmo.cc +++ b/src/drumgizmo.cc @@ -357,6 +357,7 @@ void DrumGizmo::getSamples(int ch, int pos, sample_t* s, size_t sz) } { + // TODO: We should make all audiofiles reference counted and get rid of this lock. std::lock_guard guard(af.mutex); renderSampleEvent(sample_event, pos, s, sz); diff --git a/src/events.h b/src/events.h index 1b3c376..e673329 100644 --- a/src/events.h +++ b/src/events.h @@ -53,14 +53,15 @@ using EventIDs = std::vector; class Event { public: - enum class Type { + enum class Type + { SampleEvent, }; Event(Type type, channel_t channel, timepos_t offset = 0) : type(type), channel(channel), offset(offset) {} - virtual ~Event() {} + virtual ~Event() = default; EventID id; EventGroupID group_id; diff --git a/src/events_ds.cc b/src/events_ds.cc index 988cb21..9edbe16 100644 --- a/src/events_ds.cc +++ b/src/events_ds.cc @@ -32,7 +32,7 @@ void EventsDS::remove(EventID event_id) { - auto const& event_info = id_to_info.get(event_id); + const auto& event_info = id_to_info.get(event_id); if (event_info.type == Event::Type::SampleEvent) { @@ -64,12 +64,12 @@ std::size_t EventsDS::numberOfEvents(int ch) const return channel_data.sample_events.size(); } -EventGroupIDs const& EventsDS::getSampleEventGroupIDsOf(InstrumentID instrument_id) const +const EventGroupIDs& EventsDS::getSampleEventGroupIDsOf(InstrumentID instrument_id) const { return instruments_sample_event_group_ids[instrument_id]; } -EventIDs const& EventsDS::getEventIDsOf(EventGroupID event_group_id) const +const EventIDs& EventsDS::getEventIDsOf(EventGroupID event_group_id) const { return id_to_group_data.get(event_group_id).event_ids; } @@ -90,7 +90,8 @@ void EventsDS::startAddingNewGroup(InstrumentID instrument_id) group_ids.push_back(current_group_id); id_to_group_data.get(current_group_id).instrument_index = group_ids.size() - 1; } - else { + else + { current_groups_instrument_id.invalidate(); } } @@ -106,11 +107,11 @@ void EventsDS::removeGroup(EventGroupID group_id, InstrumentID instrument_id) if (instrument_id.valid()) { - auto index = id_to_group_data.get(group_id).instrument_index; + auto instrument_index = id_to_group_data.get(group_id).instrument_index; auto& ids = instruments_sample_event_group_ids[instrument_id]; - id_to_group_data.get(ids.back()).instrument_index = index; - ids[index] = ids.back(); + id_to_group_data.get(ids.back()).instrument_index = instrument_index; + ids[instrument_index] = ids.back(); ids.pop_back(); } diff --git a/src/events_ds.h b/src/events_ds.h index 4a31f47..1b3a4b6 100644 --- a/src/events_ds.h +++ b/src/events_ds.h @@ -38,38 +38,47 @@ #include "range.h" #include "instrument.h" +//! TODO: document class as a whole +//! TODO: What s does DS stand for? // TODO: make it possible to choose sizes - -struct EventsDS +class EventsDS { - // - // member functions - // - EventsDS() = default; +public: + EventsDS() = default; // TODO: Is this needed? + //! TODO: document all the (public) things! template T& emplace(int ch, Args&&... args); + //! TODO: document all the (public) things! void remove(EventID event_id); + //! TODO: document all the (public) things! std::size_t numberOfEvents(int ch) const; + //! TODO: document all the (public) things! template ContainerRange> iterateOver(int ch); - EventGroupIDs const& getSampleEventGroupIDsOf(InstrumentID instrument_id) const; - EventIDs const& getEventIDsOf(EventGroupID event_group_id) const; + //! TODO: document all the (public) things! + const EventGroupIDs& getSampleEventGroupIDsOf(InstrumentID instrument_id) const; + + //! TODO: document all the (public) things! + const EventIDs& getEventIDsOf(EventGroupID event_group_id) const; + //! TODO: document all the (public) things! void startAddingNewGroup(InstrumentID instrument_id = InstrumentID()); private: - struct ChannelData { + struct ChannelData + { std::vector sample_events; }; using ChannelEventIndex = std::size_t; - struct EventInfo { + struct EventInfo + { Event::Type type; int ch; ChannelEventIndex channel_event_index; @@ -77,7 +86,9 @@ private: EventInfo(Event::Type type, int ch, ChannelEventIndex channel_event_index) : type(type), ch(ch), channel_event_index(channel_event_index) {} }; - struct GroupData { + + struct GroupData + { EventIDs event_ids; Event::Type type; @@ -121,7 +132,7 @@ T& EventsDS::emplace(int ch, Args&&... args) return sample_event; } - assert(false); + assert(false); // TODO: This should probably be a static_assert instead? } template diff --git a/src/inputprocessor.cc b/src/inputprocessor.cc index a29ec77..6249c5b 100644 --- a/src/inputprocessor.cc +++ b/src/inputprocessor.cc @@ -44,7 +44,6 @@ InputProcessor::InputProcessor(Settings& settings, Random& random) : kit(kit) , events_ds(events_ds) - , is_stopping(false) , settings(settings) { // Build filter list @@ -191,7 +190,7 @@ bool InputProcessor::processOnset(event_t& event, std::size_t pos, } } - // Mute other instruments in the same group + // Mute other instruments in the same instrument/choke group applyChokeGroup(settings, kit, *instr, event, events_ds); // Apply directed chokes to mute other instruments if needed diff --git a/src/inputprocessor.h b/src/inputprocessor.h index 98623d5..546f348 100644 --- a/src/inputprocessor.h +++ b/src/inputprocessor.h @@ -58,7 +58,7 @@ public: private: DrumKit& kit; EventsDS& events_ds; - bool is_stopping; ///< Is set to true when a EventType::Stop event has been seen. + bool is_stopping{false}; ///< Is set to true when a EventType::Stop event has been seen. bool processOnset(event_t& event, std::size_t pos, double resample_ratio); bool processChoke(event_t& event, std::size_t pos, double resample_ratio); diff --git a/src/memory_heap.h b/src/memory_heap.h index e2901b8..d90c50c 100644 --- a/src/memory_heap.h +++ b/src/memory_heap.h @@ -51,9 +51,9 @@ public: template Index emplace(Args&&... args); - Index add(T const& element); + Index add(const T& element); T& get(Index index); - T const& get(Index index) const; + const T& get(Index index) const; void remove(Index index); private: @@ -62,7 +62,7 @@ private: }; template -auto MemoryHeap::add(T const& element) -> Index +auto MemoryHeap::add(const T& element) -> Index { if (free_indices.empty()) { @@ -92,6 +92,8 @@ auto MemoryHeap::emplace(Args&&... args) -> Index return free_index; } +// Note: MemoryHeap never really deletes anything -- it just overwrites, so +// old indices will always return a valid item wrt. memory. template T& MemoryHeap::get(Index index) { @@ -100,7 +102,7 @@ T& MemoryHeap::get(Index index) } template -T const& MemoryHeap::get(Index index) const +const T& MemoryHeap::get(Index index) const { assert(index < memory.size()); return memory[index]; diff --git a/test/Makefile.am b/test/Makefile.am index 56d7fa3..d0f718d 100644 --- a/test/Makefile.am +++ b/test/Makefile.am @@ -7,7 +7,8 @@ TESTS = resource enginetest paintertest configfile audiocache \ audiocachefile audiocacheidmanager audiocacheeventhandler \ randomtest atomictest syncedsettingstest imagecachetest \ semaphoretest drumkitcreatortest bytesizeparsertest notifiertest \ - dgxmlparsertest domloadertest configparsertest midimapparsertest + dgxmlparsertest domloadertest configparsertest midimapparsertest \ + eventsdstest EXTRA_DIST = \ dgunit.h \ @@ -259,4 +260,13 @@ midimapparsertest_SOURCES = \ midimapparsertest.cc \ dgtest.cc +eventsdstest_CXXFLAGS = -DOUTPUT=\"eventsdstest\" \ + $(DEBUG_FLAGS) \ + -I$(top_srcdir)/src +eventsdstest_LDFLAGS = +eventsdstest_SOURCES = \ + $(top_srcdir)/src/events_ds.cc \ + eventsdstest.cc \ + dgtest.cc + endif diff --git a/test/eventsdstest.cc b/test/eventsdstest.cc new file mode 100644 index 0000000..2616053 --- /dev/null +++ b/test/eventsdstest.cc @@ -0,0 +1,49 @@ +/* -*- Mode: c++ -*- */ +/*************************************************************************** + * eventsdstest.cc + * + * Sun Feb 16 21:38:59 CET 2020 + * Copyright 2020 Bent Bisballe Nyeng + * deva@aasimon.org + ****************************************************************************/ + +/* + * This file is part of DrumGizmo. + * + * DrumGizmo is free software; you can redistribute it and/or modify + * it under the terms of the GNU Lesser General Public License as published by + * the Free Software Foundation; either version 3 of the License, or + * (at your option) any later version. + * + * DrumGizmo is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with DrumGizmo; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA. + */ +#include "dgunit.h" + +#include "../src/events_ds.h" + +class EventsDSTest + : public DGUnit +{ +public: + EventsDSTest() + { + DGUNIT_TEST(EventsDSTest::todo); + } + +public: + void todo() + { + // TODO: Add some tests here + DGUNIT_ASSERT(false); + } +}; + +// Registers the fixture into the 'registry' +static EventsDSTest test; -- cgit v1.2.3