Skip to content

Commit db7eb99

Browse files
committed
Let the outputs be changed without restarting anything
Phase 5's exit criterion is "launch to synced output without documentation, a text editor, or a restart", so the window will have to switch Link on, point OSC somewhere and pick a MIDI port while the app is open — and the transports were built once from a config and never touched again. This makes them settable, and gives the output thread the same way in that the tracker already has. **Link is now always built, whether or not it is switched on**, and that is the load-bearing decision here. `BeatEngine::setHostTimeSource` is handed the session and the audio thread reads it every hop (§4.3); building it on demand would mean destroying one under a running audio thread the first time an operator switched Link off, and no ordering makes that safe. Built once, switched on and off, never replaced — so `OutputRunner::hostTimeClock()` is stable for the runner's life and that hazard cannot occur. It costs nothing idle: Link's own header says construction "does not touch the network; nothing is visible to peers until the session is enabled", and a publisher with no targets sends to nobody. MIDI is the exception and stays on demand, because it is a named device rather than a switch. Switching Link on while stopped says what to do rather than doing it: the session joins at startOutputs, so ticking a box before Start does not put takt4 in front of peers before it is tracking anything. Switching it off and on again resends the tempo on the next beat, because a rejoined session knows nothing and the "unchanged, do not resend" rule would otherwise leave peers waiting for a change. `OutputCommand` and `OutputRunner::post` are `engine::Command` and `BeatEngine::post` again, for the same reason: the transports belong to the output thread, and a caller must not reach into something another thread is using. Posting while stopped applies immediately instead — an operator ticking Link with nothing running should not have to press Start to find out whether it took. The runner owns the `Transports` now rather than borrowing them, which turns "one thread touches these" from a comment into something the type system enforces: `transports()` hands back a const reference, and the mutating members are not on it. A MIDI port that will not open is the failure that actually happens. `Transports` builds the new port before tearing the old one down, so a bad name leaves a working clock exactly where it was; `OutputRunner` catches it and keeps the message in `lastError()`, because the transports carrying on is precisely what makes the failure invisible otherwise. A port opened mid-set starts its clock from now rather than from when the outputs started — `advance` would otherwise try to emit every tick of the intervening set at once. `OscPublisher::clearTargets` forgets the published state along with the targets. Only changed values are sent between beats, so a target typed in mid-set would otherwise wait for the tempo to move before learning what it is. Eight more tests: that the Link session is the same object across a switch, that switching on before start does not join, that replacing OSC targets resends the state, that a bad MIDI port leaves the transports usable and is reported rather than swallowed, and that a change posted while running actually travels. 174 tests with the UI off, 190 with it on. `track` over the excerpt still ends "499 frames, 21 beats (5 downbeats), ending at 127.7 BPM in 4/4, locked", and a live run with --link and --osc still sends and shuts down clean.
1 parent f6be09e commit db7eb99

9 files changed

Lines changed: 504 additions & 104 deletions

File tree

‎src/cli/main.cpp‎

Lines changed: 21 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1015,7 +1015,7 @@ int runTrackFile(const std::filesystem::path& in, const takt4::model::ModelWeigh
10151015
offline.midiClockPort.reset();
10161016

10171017
auto engine = std::make_unique<takt4::engine::BeatEngine>(weights, model, engineOptions(args));
1018-
takt4::output::Transports transports(transportConfig(offline));
1018+
takt4::output::Transports transports{transportConfig(offline)};
10191019
BeatPrinter printer;
10201020
if (args.beatsOut) {
10211021
printer.writeTo(*args.beatsOut);
@@ -1068,7 +1068,10 @@ int runTrackDevice(const TrackArgs& args, const takt4::model::ModelWeights& weig
10681068
options.sampleRate = args.beats.stream.rate;
10691069
options.forceSoftwareSlice = args.beats.stream.software;
10701070
takt4::audio::InputStream stream(session, device, selection, *engine, options);
1071-
takt4::output::Transports transports(transportConfig(args));
1071+
// The runner owns the transports (§4.2): one thread touches them, and that is
1072+
// structural rather than a comment now.
1073+
takt4::output::OutputRunner runner(*engine, transportConfig(args));
1074+
const takt4::output::Transports& transports = runner.transports();
10721075
BeatPrinter printer;
10731076
if (args.beatsOut) {
10741077
printer.writeTo(*args.beatsOut);
@@ -1077,9 +1080,7 @@ int runTrackDevice(const TrackArgs& args, const takt4::model::ModelWeights& weig
10771080
// beat carries the host time of the audio it was found in rather than of the moment
10781081
// this loop happened to notice it. Before the stream is started, because the stamp is
10791082
// taken on the audio thread and there has to be a clock in place before there is one.
1080-
if (transports.link() != nullptr) {
1081-
engine->setHostTimeSource(transports.link());
1082-
}
1083+
engine->setHostTimeSource(&runner.hostTimeClock());
10831084

10841085
std::cout << "device: " << device.hostApiName << " / " << device.name << '\n'
10851086
<< "channel: " << selection.channels[0] + 1;
@@ -1101,13 +1102,11 @@ int runTrackDevice(const TrackArgs& args, const takt4::model::ModelWeights& weig
11011102
if (!args.anyOutput()) {
11021103
std::cout << "none (--link, --osc HOST:PORT, --midi-clock PORT)";
11031104
}
1104-
if (transports.link() != nullptr) {
1105+
if (transports.linkEnabled()) {
11051106
std::cout << "Link ";
11061107
}
1107-
if (transports.osc() != nullptr) {
1108-
for (std::size_t i = 0; i < transports.osc()->targetCount(); ++i) {
1109-
std::cout << "OSC " << transports.osc()->target(i).resolved() << " ";
1110-
}
1108+
for (std::size_t i = 0; i < transports.osc().targetCount(); ++i) {
1109+
std::cout << "OSC " << transports.osc().target(i).resolved() << " ";
11111110
}
11121111
if (transports.midiPort() != nullptr) {
11131112
std::cout << "MIDI clock to \"" << transports.midiPort()->portName() << "\"";
@@ -1165,7 +1164,7 @@ int runTrackDevice(const TrackArgs& args, const takt4::model::ModelWeights& weig
11651164
takt4::tracking::TempoTracker::Options live = engine->tempoOptions();
11661165
live.latencyOffsetSeconds += key == '[' ? -0.005 : 0.005;
11671166
(void)engine->post(Command::setTempoOptions(live));
1168-
transports.setLatencySeconds(live.latencyOffsetSeconds);
1167+
runner.setLatencySeconds(live.latencyOffsetSeconds);
11691168
std::cout << " latency offset " << fixed1(live.latencyOffsetSeconds * 1000.0)
11701169
<< " ms\n";
11711170
break;
@@ -1201,11 +1200,10 @@ int runTrackDevice(const TrackArgs& args, const takt4::model::ModelWeights& weig
12011200
std::mutex publishedMutex;
12021201
std::vector<takt4::engine::EngineBeat> published;
12031202

1204-
// §4.2's output thread. It is the single consumer of the engine's beat ring from here
1205-
// on, so this loop must not drain it: it polls keys, prints, and drains the *frame*
1206-
// ring, which nothing else wants. The tracking waits on neither — the inference thread
1207-
// runs at the audio's pace — and now neither does a MIDI tick.
1208-
takt4::output::OutputRunner runner(*engine, transports);
1203+
// §4.2's output thread. It is the single consumer of the engine's beat ring, so this
1204+
// loop must not drain it: it polls keys, prints, and drains the *frame* ring, which
1205+
// nothing else wants. The tracking waits on neither — the inference thread runs at the
1206+
// audio's pace — and now neither does a MIDI tick.
12091207
runner.setBeatObserver([&](const takt4::engine::EngineBeat& beat) {
12101208
const std::lock_guard<std::mutex> lock(publishedMutex);
12111209
published.push_back(beat);
@@ -1272,18 +1270,18 @@ int runTrackDevice(const TrackArgs& args, const takt4::model::ModelWeights& weig
12721270
? ", " + std::to_string(engine->framesDropped()) + " frames not drained"
12731271
: "")
12741272
<< '\n';
1275-
if (transports.osc() != nullptr) {
1276-
std::cout << "OSC: " << transports.osc()->messagesSent() << " messages sent, "
1277-
<< transports.osc()->messagesFailed() << " failed\n";
1273+
if (transports.osc().targetCount() != 0) {
1274+
std::cout << "OSC: " << transports.osc().messagesSent() << " messages sent, "
1275+
<< transports.osc().messagesFailed() << " failed\n";
12781276
}
12791277
if (transports.midiClock() != nullptr) {
12801278
std::cout << "MIDI clock: " << transports.midiClock()->ticksSent() << " ticks, "
12811279
<< transports.midiClock()->ticksSkipped() << " skipped\n";
12821280
}
1283-
if (transports.link() != nullptr) {
1284-
std::cout << "Link: " << transports.link()->tempoUpdates() << " tempo updates, "
1285-
<< transports.link()->beatRequests() << " beat requests, "
1286-
<< transports.link()->numPeers() << " peers at the end\n";
1281+
if (transports.linkEnabled()) {
1282+
std::cout << "Link: " << transports.link().tempoUpdates() << " tempo updates, "
1283+
<< transports.link().beatRequests() << " beat requests, "
1284+
<< transports.link().numPeers() << " peers at the end\n";
12871285
}
12881286
return 0;
12891287
}

‎src/core/output/osc_publisher.cpp‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,16 @@ void OscPublisher::addTarget(std::string_view host, std::uint16_t port) {
4646
targets_.push_back(std::make_unique<OscSender>(host, port));
4747
}
4848

49+
void OscPublisher::clearTargets() noexcept {
50+
targets_.clear();
51+
// Back to exactly what a freshly built publisher holds, so a target added after this
52+
// is told the same things a target present from the start would have been.
53+
lastBpm_ = -1.0;
54+
lastConfidence_ = -1.0;
55+
lastLocked_ = -1;
56+
lastMeter_ = 0;
57+
}
58+
4959
void OscPublisher::sendInt(std::string_view address, std::int32_t value) {
5060
OscMessage message(address);
5161
message.addInt(value);

‎src/core/output/osc_publisher.hpp‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,14 @@ class OscPublisher {
4545
/// Adds a target. Throws std::runtime_error if the host cannot be resolved.
4646
void addTarget(std::string_view host, std::uint16_t port);
4747

48+
/// Removes every target, and forgets what was last published with them.
49+
///
50+
/// The forgetting is the point: only changed values are sent between beats, so a
51+
/// target added after this has never been told the tempo and would otherwise wait for
52+
/// it to move before learning it. An operator who types an address mid-set expects
53+
/// the next message, not the next tempo change.
54+
void clearTargets() noexcept;
55+
4856
std::size_t targetCount() const noexcept { return targets_.size(); }
4957
const OscSender& target(std::size_t index) const noexcept { return *targets_[index]; }
5058

‎src/core/output/output_runner.cpp‎

Lines changed: 64 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
#include "core/output/output_runner.hpp"
22

3+
#include <exception>
4+
#include <utility>
5+
36
#if defined(_WIN32)
47
#include <windows.h>
58
// timeapi.h must follow windows.h.
@@ -34,8 +37,8 @@ void restoreTimerResolution(bool raised) noexcept {
3437

3538
} // namespace
3639

37-
OutputRunner::OutputRunner(engine::BeatEngine& engine, Transports& transports)
38-
: engine_(engine), transports_(transports) {}
40+
OutputRunner::OutputRunner(engine::BeatEngine& engine, const Transports::Config& config)
41+
: engine_(engine), transports_(config) {}
3942

4043
OutputRunner::~OutputRunner() {
4144
stop();
@@ -69,6 +72,10 @@ void OutputRunner::stop() noexcept {
6972
running_.store(false, std::memory_order_release);
7073
worker_.join();
7174

75+
// Anything posted in the moments before the stop still meant something, and the
76+
// transports are this thread's now.
77+
applyCommands();
78+
7279
// The last beats of a set are still beats: the engine may have called one between the
7380
// final round and the join. Safe on this thread now — the only other consumer of that
7481
// ring has been joined.
@@ -84,8 +91,63 @@ void OutputRunner::stop() noexcept {
8491
raisedTimer_ = false;
8592
}
8693

94+
void OutputRunner::post(OutputCommand command) {
95+
if (!running()) {
96+
// Nothing else is touching the transports, so there is no reason to make an
97+
// operator press Start before a setting takes: this is how an app is configured
98+
// before it is running at all.
99+
apply(command);
100+
return;
101+
}
102+
const std::lock_guard<std::mutex> lock(commandMutex_);
103+
pending_.push_back(std::move(command));
104+
}
105+
106+
std::string OutputRunner::lastError() const {
107+
const std::lock_guard<std::mutex> lock(errorMutex_);
108+
return lastError_;
109+
}
110+
111+
void OutputRunner::apply(const OutputCommand& command) {
112+
try {
113+
switch (command.kind) {
114+
case OutputCommand::Kind::LinkEnabled:
115+
transports_.setLinkEnabled(command.enabled);
116+
break;
117+
case OutputCommand::Kind::OscTargets:
118+
transports_.setOscTargets(command.targets);
119+
break;
120+
case OutputCommand::Kind::MidiClockPort:
121+
transports_.setMidiClockPort(command.port);
122+
break;
123+
}
124+
const std::lock_guard<std::mutex> lock(errorMutex_);
125+
lastError_.clear();
126+
} catch (const std::exception& e) {
127+
// A MIDI port that is not there. Transports leaves what was working alone, so the
128+
// failure is only that the change did not happen — which somebody has to be told.
129+
const std::lock_guard<std::mutex> lock(errorMutex_);
130+
lastError_ = e.what();
131+
}
132+
}
133+
134+
void OutputRunner::applyCommands() noexcept {
135+
{
136+
const std::lock_guard<std::mutex> lock(commandMutex_);
137+
if (pending_.empty()) {
138+
return;
139+
}
140+
applying_.swap(pending_);
141+
}
142+
for (const OutputCommand& command : applying_) {
143+
apply(command);
144+
}
145+
applying_.clear();
146+
}
147+
87148
void OutputRunner::run() noexcept {
88149
while (running_.load(std::memory_order_acquire)) {
150+
applyCommands();
89151
try {
90152
drainOnce(elapsed());
91153
} catch (...) {

‎src/core/output/output_runner.hpp‎

Lines changed: 88 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,55 @@
11
#pragma once
22

3+
#include "core/audio/host_time.hpp"
34
#include "core/engine/beat_engine.hpp"
45
#include "core/output/transports.hpp"
56

67
#include <atomic>
78
#include <chrono>
89
#include <cstdint>
910
#include <functional>
11+
#include <mutex>
12+
#include <optional>
13+
#include <string>
1014
#include <thread>
15+
#include <vector>
1116

1217
namespace takt4::output {
1318

19+
/// A change to what the transports are sending.
20+
///
21+
/// Posted by whoever owns the controls — a window, §5.7's inbound OSC later — and applied
22+
/// by the output thread between rounds, because the transports belong to it. Exactly the
23+
/// shape `engine::Command` has for the tracker, and for the same reason: many producers,
24+
/// one consumer, and no caller reaching into something another thread is using.
25+
struct OutputCommand {
26+
enum class Kind : std::uint8_t { LinkEnabled, OscTargets, MidiClockPort };
27+
28+
static OutputCommand linkEnabled(bool on) {
29+
OutputCommand command;
30+
command.kind = Kind::LinkEnabled;
31+
command.enabled = on;
32+
return command;
33+
}
34+
static OutputCommand oscTargets(std::vector<Transports::OscTarget> targets) {
35+
OutputCommand command;
36+
command.kind = Kind::OscTargets;
37+
command.targets = std::move(targets);
38+
return command;
39+
}
40+
static OutputCommand midiClockPort(std::optional<std::string> port) {
41+
OutputCommand command;
42+
command.kind = Kind::MidiClockPort;
43+
command.port = std::move(port);
44+
return command;
45+
}
46+
47+
Kind kind = Kind::LinkEnabled;
48+
bool enabled = false;
49+
std::vector<Transports::OscTarget> targets;
50+
std::optional<std::string> port;
51+
};
52+
1453
/// HANDOFF §4.2's output thread.
1554
///
1655
/// §4.2 wants the transports off the audio thread *and* off the caller's loop, so that
@@ -30,9 +69,9 @@ namespace takt4::output {
3069
///
3170
/// The caller still owns two things that have to happen around it:
3271
///
33-
/// * `engine.setHostTimeSource(transports.link())` **before the engine is started**,
34-
/// because §4.3's stamp is taken on the audio thread and there has to be a clock in
35-
/// place before there is one.
72+
/// * `engine.setHostTimeSource(&runner.hostTimeClock())` **before the audio stream is
73+
/// opened**, because §4.3's stamp is taken on the audio thread and there has to be a
74+
/// clock in place before there is one.
3675
/// * Draining `popFrame`, if anything wants the frames. Nobody has to, but a ring that
3776
/// nobody drains fills and the engine starts counting frames lost.
3877
class OutputRunner {
@@ -46,8 +85,13 @@ class OutputRunner {
4685
/// loop has to come round well inside that or the ticks inherit its period as jitter.
4786
static constexpr std::chrono::milliseconds kPeriod{1};
4887

49-
/// Both must outlive this. Nothing is sent until `start()`.
50-
OutputRunner(engine::BeatEngine& engine, Transports& transports);
88+
/// The engine must outlive this. The transports are built here and owned here, which
89+
/// is what makes "one thread touches them" structural rather than a comment: nothing
90+
/// else can reach a mutating member of them. Changes go through `post`.
91+
///
92+
/// Throws whatever `Transports` throws — a MIDI port that is not on the machine.
93+
/// Nothing is sent until `start()`.
94+
OutputRunner(engine::BeatEngine& engine, const Transports::Config& config);
5195
~OutputRunner();
5296

5397
OutputRunner(const OutputRunner&) = delete;
@@ -78,15 +122,53 @@ class OutputRunner {
78122
/// Seconds since `start()`, on the steady clock the transports are driven from.
79123
double elapsed() const noexcept;
80124

125+
/// The transports, for reading. Their counters are atomic and Link's own state is
126+
/// safe to query, so a UI may call this while the thread is sending; the members that
127+
/// change what is sent are not reachable through it, and that is deliberate.
128+
const Transports& transports() const noexcept { return transports_; }
129+
130+
/// Link's clock, for `engine::BeatEngine::setHostTimeSource`.
131+
///
132+
/// §4.3's stamp is taken on the audio thread, so this has to be installed before the
133+
/// stream is opened. It is stable for this runner's life — the session is built once
134+
/// and switched on and off, never replaced — which is exactly why switching Link off
135+
/// mid-set cannot leave the audio thread holding a destroyed clock.
136+
audio::HostTimeSource& hostTimeClock() noexcept { return transports_.link(); }
137+
138+
/// §5.5's latency offset. Any thread: it writes one atomic.
139+
void setLatencySeconds(double seconds) noexcept { transports_.setLatencySeconds(seconds); }
140+
141+
/// Asks for a change to what is being sent. Any thread; applied by the output thread
142+
/// before its next round, so it has happened within a millisecond. Applied
143+
/// immediately on the calling thread when the runner is not running, which is what
144+
/// lets an app be configured before it is started.
145+
void post(OutputCommand command);
146+
147+
/// What went wrong applying the last posted change, or empty. A MIDI port that is not
148+
/// on the machine is the one that happens; an operator has to be told rather than
149+
/// left wondering why nothing ticks.
150+
std::string lastError() const;
151+
81152
private:
82153
void run() noexcept;
83154
/// One round: every beat waiting, then the clock. On the output thread, or on the
84155
/// caller's in `stop()` once the thread has been joined — never on both at once.
85156
void drainOnce(double now);
157+
/// Everything posted since the last round, in order. On whichever thread owns the
158+
/// transports at the time.
159+
void applyCommands() noexcept;
160+
void apply(const OutputCommand& command);
86161

87162
engine::BeatEngine& engine_;
88-
Transports& transports_;
163+
Transports transports_;
89164
BeatObserver observer_;
165+
166+
mutable std::mutex commandMutex_;
167+
std::vector<OutputCommand> pending_;
168+
/// Drained into, and reused, so applying commands allocates nothing after the first.
169+
std::vector<OutputCommand> applying_;
170+
mutable std::mutex errorMutex_;
171+
std::string lastError_;
90172
std::thread worker_;
91173
std::atomic<bool> running_{false};
92174
std::atomic<std::uint64_t> rounds_{0};

0 commit comments

Comments
 (0)