From 7886c2df7abae474733620ef73b80a938fe7821e Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Thu, 7 Jul 2022 10:48:42 -0400 Subject: [PATCH 01/32] Start experimenting with a more event-based approach to timing. --- Concurrency/AsyncUpdater.hpp | 77 +++++++++++++++++++ .../Clock Signal.xcodeproj/project.pbxproj | 2 + .../Mac/Clock Signal/Machine/CSMachine.mm | 9 +++ 3 files changed, 88 insertions(+) create mode 100644 Concurrency/AsyncUpdater.hpp diff --git a/Concurrency/AsyncUpdater.hpp b/Concurrency/AsyncUpdater.hpp new file mode 100644 index 000000000..5596a1dcd --- /dev/null +++ b/Concurrency/AsyncUpdater.hpp @@ -0,0 +1,77 @@ +// +// AsyncUpdater.h +// Clock Signal +// +// Created by Thomas Harte on 06/07/2022. +// Copyright © 2022 Thomas Harte. All rights reserved. +// + +#ifndef AsyncUpdater_hpp +#define AsyncUpdater_hpp + +#include +#include +#include + +#include "../ClockReceiver/TimeTypes.hpp" + +namespace Concurrency { + +template class AsyncUpdater { + public: + template AsyncUpdater(Args&&... args) + : performer_(std::forward(args)...), + performer_thread_{ + [this] { + Time::Nanos last_fired = Time::nanos_now(); + + while(!should_quit) { + // Wait for a new action to be signalled, and grab it. + std::unique_lock lock(condition_mutex_); + while(actions_.empty()) { + condition_.wait(lock); + } + auto action = actions_.pop_back(); + lock.unlock(); + + // Update to now. + auto time_now = Time::nanos_now(); + performer_.perform(time_now - last_fired); + last_fired = time_now; + + // Perform the action. + action(); + } + } + } {} + + /// Run the performer up to 'now' and then perform @c post_action. + /// + /// @c post_action will be performed asynchronously, on the same + /// thread as the performer. + void update(const std::function &post_action) { + std::lock_guard guard(condition_mutex_); + actions_.push_back(post_action); + condition_.notify_all(); + } + + ~AsyncUpdater() { + should_quit = true; + update([] {}); + performer_thread_.join(); + } + + private: + Performer performer_; + + std::thread performer_thread_; + std::mutex condition_mutex_; + std::condition_variable condition_; + std::vector> actions_; + std::atomic should_quit = false; +}; + + +} + +#endif /* AsyncUpdater_hpp */ diff --git a/OSBindings/Mac/Clock Signal.xcodeproj/project.pbxproj b/OSBindings/Mac/Clock Signal.xcodeproj/project.pbxproj index 3311a4686..87026c868 100644 --- a/OSBindings/Mac/Clock Signal.xcodeproj/project.pbxproj +++ b/OSBindings/Mac/Clock Signal.xcodeproj/project.pbxproj @@ -2138,6 +2138,7 @@ 4BDCC5F81FB27A5E001220C5 /* ROMMachine.hpp */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.h; path = ROMMachine.hpp; sourceTree = ""; }; 4BDDBA981EF3451200347E61 /* Z80MachineCycleTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = Z80MachineCycleTests.swift; sourceTree = ""; }; 4BE0151C286A8C8E00EA42E9 /* MemorySwitches.hpp */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.h; path = MemorySwitches.hpp; sourceTree = ""; }; + 4BE0151E28766ECF00EA42E9 /* AsyncUpdater.hpp */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.h; path = AsyncUpdater.hpp; sourceTree = ""; }; 4BE0A3EC237BB170002AB46F /* ST.cpp */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.cpp; path = ST.cpp; sourceTree = ""; }; 4BE0A3ED237BB170002AB46F /* ST.hpp */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.h; path = ST.hpp; sourceTree = ""; }; 4BE211DD253E4E4800435408 /* 65C02_no_Rockwell_test.bin */ = {isa = PBXFileReference; lastKnownFileType = archive.macbinary; name = 65C02_no_Rockwell_test.bin; path = "Klaus Dormann/65C02_no_Rockwell_test.bin"; sourceTree = ""; }; @@ -2754,6 +2755,7 @@ children = ( 4B3940E51DA83C8300427841 /* AsyncTaskQueue.cpp */, 4B3940E61DA83C8300427841 /* AsyncTaskQueue.hpp */, + 4BE0151E28766ECF00EA42E9 /* AsyncUpdater.hpp */, ); name = Concurrency; path = ../../Concurrency; diff --git a/OSBindings/Mac/Clock Signal/Machine/CSMachine.mm b/OSBindings/Mac/Clock Signal/Machine/CSMachine.mm index 61206bd56..214aa280c 100644 --- a/OSBindings/Mac/Clock Signal/Machine/CSMachine.mm +++ b/OSBindings/Mac/Clock Signal/Machine/CSMachine.mm @@ -24,6 +24,7 @@ #include "../../../../ClockReceiver/TimeTypes.hpp" #include "../../../../ClockReceiver/ScanSynchroniser.hpp" +#include "../../../../Concurrency/AsyncUpdater.hpp" #import "CSStaticAnalyser+TargetVector.h" #import "NSBundle+DataResource.h" @@ -34,6 +35,14 @@ #include #include +namespace { + +struct Updater {}; + +Concurrency::AsyncUpdater updater(); + +} + @interface CSMachine() - (void)speaker:(Outputs::Speaker::Speaker *)speaker didCompleteSamples:(const int16_t *)samples length:(int)length; - (void)speakerDidChangeInputClock:(Outputs::Speaker::Speaker *)speaker; From 01a309909b60ced2981c6788108de9595223fa19 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Thu, 7 Jul 2022 11:10:54 -0400 Subject: [PATCH 02/32] Elide actions when running behind. --- Concurrency/AsyncUpdater.hpp | 31 ++++++++++++++++++++++--------- 1 file changed, 22 insertions(+), 9 deletions(-) diff --git a/Concurrency/AsyncUpdater.hpp b/Concurrency/AsyncUpdater.hpp index 5596a1dcd..700bd7951 100644 --- a/Concurrency/AsyncUpdater.hpp +++ b/Concurrency/AsyncUpdater.hpp @@ -19,19 +19,21 @@ namespace Concurrency { template class AsyncUpdater { public: - template AsyncUpdater(Args&&... args) - : performer_(std::forward(args)...), + template AsyncUpdater(Args&&... args) : + performer_(std::forward(args)...), + actions_(std::make_unique()), performer_thread_{ [this] { Time::Nanos last_fired = Time::nanos_now(); + auto actions = std::make_unique(); while(!should_quit) { - // Wait for a new action to be signalled, and grab it. + // Wait for new actions to be signalled, and grab them. std::unique_lock lock(condition_mutex_); - while(actions_.empty()) { + while(!actions_) { condition_.wait(lock); } - auto action = actions_.pop_back(); + std::swap(actions, actions_); lock.unlock(); // Update to now. @@ -39,8 +41,11 @@ template class AsyncUpdater { performer_.perform(time_now - last_fired); last_fired = time_now; - // Perform the action. - action(); + // Perform the actions. + for(const auto& action: *actions) { + action(); + } + actions->clear(); } } } {} @@ -49,9 +54,11 @@ template class AsyncUpdater { /// /// @c post_action will be performed asynchronously, on the same /// thread as the performer. + /// + /// Actions may be elided, void update(const std::function &post_action) { std::lock_guard guard(condition_mutex_); - actions_.push_back(post_action); + actions_->push_back(post_action); condition_.notify_all(); } @@ -62,12 +69,18 @@ template class AsyncUpdater { } private: + // The object that will actually receive time advances. Performer performer_; + // The list of actions waiting be performed. These will be elided, + // increasing their latency, if the emulation thread falls behind. + using ActionVector = std::vector>; + std::unique_ptr actions_; + + // Necessary synchronisation parts. std::thread performer_thread_; std::mutex condition_mutex_; std::condition_variable condition_; - std::vector> actions_; std::atomic should_quit = false; }; From 3e2a6ef3f48f8f361afd61f194a068bdc6ef017a Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Thu, 7 Jul 2022 16:35:45 -0400 Subject: [PATCH 03/32] Hacks up an [unsafe] return to something best-effort-updater-esque. For profiling, mainly. --- ClockReceiver/TimeTypes.hpp | 5 ++ Concurrency/AsyncUpdater.hpp | 10 ++-- .../Documents/MachineDocument.swift | 6 --- .../Mac/Clock Signal/Machine/CSMachine.mm | 54 +++++++++++++++---- 4 files changed, 54 insertions(+), 21 deletions(-) diff --git a/ClockReceiver/TimeTypes.hpp b/ClockReceiver/TimeTypes.hpp index c8ef2fe1c..c697b274b 100644 --- a/ClockReceiver/TimeTypes.hpp +++ b/ClockReceiver/TimeTypes.hpp @@ -20,6 +20,11 @@ inline Nanos nanos_now() { return std::chrono::duration_cast(std::chrono::high_resolution_clock::now().time_since_epoch()).count(); } +inline Seconds seconds(Nanos nanos) { + return double(nanos) / 1e9; +} + } #endif /* TimeTypes_h */ + diff --git a/Concurrency/AsyncUpdater.hpp b/Concurrency/AsyncUpdater.hpp index 700bd7951..74e1fd334 100644 --- a/Concurrency/AsyncUpdater.hpp +++ b/Concurrency/AsyncUpdater.hpp @@ -20,7 +20,7 @@ namespace Concurrency { template class AsyncUpdater { public: template AsyncUpdater(Args&&... args) : - performer_(std::forward(args)...), + performer(std::forward(args)...), actions_(std::make_unique()), performer_thread_{ [this] { @@ -30,7 +30,7 @@ template class AsyncUpdater { while(!should_quit) { // Wait for new actions to be signalled, and grab them. std::unique_lock lock(condition_mutex_); - while(!actions_) { + while(actions_->empty()) { condition_.wait(lock); } std::swap(actions, actions_); @@ -38,7 +38,7 @@ template class AsyncUpdater { // Update to now. auto time_now = Time::nanos_now(); - performer_.perform(time_now - last_fired); + performer.perform(time_now - last_fired); last_fired = time_now; // Perform the actions. @@ -68,10 +68,10 @@ template class AsyncUpdater { performer_thread_.join(); } - private: // The object that will actually receive time advances. - Performer performer_; + Performer performer; + private: // The list of actions waiting be performed. These will be elided, // increasing their latency, if the emulation thread falls behind. using ActionVector = std::vector>; diff --git a/OSBindings/Mac/Clock Signal/Documents/MachineDocument.swift b/OSBindings/Mac/Clock Signal/Documents/MachineDocument.swift index fd9b5810b..f4d3deccc 100644 --- a/OSBindings/Mac/Clock Signal/Documents/MachineDocument.swift +++ b/OSBindings/Mac/Clock Signal/Documents/MachineDocument.swift @@ -15,7 +15,6 @@ class MachineDocument: NSWindowDelegate, CSMachineDelegate, CSScanTargetViewResponderDelegate, - CSAudioQueueDelegate, CSROMReciverViewDelegate { // MARK: - Mutual Exclusion. @@ -274,17 +273,12 @@ class MachineDocument: if self.audioQueue == nil || self.audioQueue.samplingRate != selectedSamplingRate || self.audioQueue != self.machine.audioQueue { self.machine.audioQueue = nil self.audioQueue = CSAudioQueue(samplingRate: Float64(selectedSamplingRate), isStereo:isStereo) - self.audioQueue.delegate = self self.machine.audioQueue = self.audioQueue self.machine.setAudioSamplingRate(Float(selectedSamplingRate), bufferSize:audioQueue.preferredBufferSize, stereo:isStereo) } } } - /// Responds to the CSAudioQueueDelegate dry-queue warning message by requesting a machine update. - final func audioQueueIsRunningDry(_ audioQueue: CSAudioQueue) { - } - // MARK: - Pasteboard Forwarding. /// Forwards any text currently on the pasteboard into the active machine. diff --git a/OSBindings/Mac/Clock Signal/Machine/CSMachine.mm b/OSBindings/Mac/Clock Signal/Machine/CSMachine.mm index 214aa280c..f1e11b04b 100644 --- a/OSBindings/Mac/Clock Signal/Machine/CSMachine.mm +++ b/OSBindings/Mac/Clock Signal/Machine/CSMachine.mm @@ -37,9 +37,16 @@ namespace { -struct Updater {}; +struct MachineUpdater { + void perform(Time::Nanos duration) { + // Top out at 0.1 seconds; this is a safeguard against a negative + // feedback loop if emulation starts running slowly. + const auto seconds = std::min(Time::seconds(duration), 0.1); + machine->run_for(seconds); + } -Concurrency::AsyncUpdater updater(); + MachineTypes::TimedMachine *machine = nullptr; +}; } @@ -49,6 +56,10 @@ Concurrency::AsyncUpdater updater(); - (void)addLED:(NSString *)led isPersistent:(BOOL)isPersistent; @end +@interface CSMachine() +- (void)audioQueueIsRunningDry:(nonnull CSAudioQueue *)audioQueue; +@end + struct LockProtectedDelegate { // Contractual promise is: machine, the pointer **and** the object **, may be accessed only // in sections protected by the machineAccessLock; @@ -106,6 +117,7 @@ struct ActivityObserver: public Activity::Observer { CSStaticAnalyser *_analyser; std::unique_ptr _machine; MachineTypes::JoystickMachine *_joystickMachine; + Concurrency::AsyncUpdater updater; CSJoystickManager *_joystickManager; NSMutableArray *_leds; @@ -142,6 +154,7 @@ struct ActivityObserver: public Activity::Observer { [missingROMs appendString:[NSString stringWithUTF8String:wstring_converter.to_bytes(description).c_str()]]; return nil; } + updater.performer.machine = _machine->timed_machine(); // Use the keyboard as a joystick if the machine has no keyboard, or if it has a 'non-exclusive' keyboard. _inputMode = @@ -219,6 +232,11 @@ struct ActivityObserver: public Activity::Observer { } } +- (void)setAudioQueue:(CSAudioQueue *)audioQueue { + _audioQueue = audioQueue; + audioQueue.delegate = self; +} + - (void)setAudioSamplingRate:(float)samplingRate bufferSize:(NSUInteger)bufferSize stereo:(BOOL)stereo { @synchronized(self) { [self setSpeakerDelegate:&_speakerDelegate sampleRate:samplingRate bufferSize:bufferSize stereo:stereo]; @@ -442,9 +460,12 @@ struct ActivityObserver: public Activity::Observer { } - (void)applyInputEvent:(dispatch_block_t)event { - @synchronized(_inputEvents) { - [_inputEvents addObject:event]; - } + updater.update([event] { + event(); + }); +// @synchronized(_inputEvents) { +// [_inputEvents addObject:event]; +// } } - (void)clearAllKeys { @@ -655,9 +676,21 @@ struct ActivityObserver: public Activity::Observer { #pragma mark - Timer +- (void)audioQueueIsRunningDry:(nonnull CSAudioQueue *)audioQueue { + // TODO: Make audio flushes overt, and do one here. + updater.update([] {}); +} + - (void)scanTargetViewDisplayLinkDidFire:(CSScanTargetView *)view now:(const CVTimeStamp *)now outputTime:(const CVTimeStamp *)outputTime { + updater.update([self] { + dispatch_async(dispatch_get_main_queue(), ^{ + [self.view updateBacking]; + [self.view draw]; + }); + }); + // First order of business: grab a timestamp. - const auto timeNow = Time::nanos_now(); +/* const auto timeNow = Time::nanos_now(); BOOL isSyncLocking; @synchronized(self) { @@ -682,13 +715,14 @@ struct ActivityObserver: public Activity::Observer { // Draw the current output. (TODO: do this within the timer if either raster racing or, at least, sync matching). if(!isSyncLocking) { [self.view draw]; - } + }*/ } -#define TICKS 1000 +#define TICKS 120 - (void)start { - __block auto lastTime = Time::nanos_now(); +// updater.performer.machine = _machine->timed_machine(); +/* __block auto lastTime = Time::nanos_now(); _timer = [[CSHighPrecisionTimer alloc] initWithTask:^{ // Grab the time now and, therefore, the amount of time since the timer last fired @@ -762,7 +796,7 @@ struct ActivityObserver: public Activity::Observer { } lastTime = timeNow; - } interval:uint64_t(1000000000) / uint64_t(TICKS)]; + } interval:uint64_t(1000000000) / uint64_t(TICKS)];*/ } #undef TICKS From fc0dc4e5e24bafbd850b1437a74cab5aafd69cd2 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Thu, 7 Jul 2022 16:41:49 -0400 Subject: [PATCH 04/32] Amiga only, temporarily: attempt to reduce audio maintenance costs. --- Machines/Amiga/Amiga.cpp | 9 ++++++--- Machines/TimedMachine.hpp | 3 +++ 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/Machines/Amiga/Amiga.cpp b/Machines/Amiga/Amiga.cpp index deb09d162..5a345359b 100644 --- a/Machines/Amiga/Amiga.cpp +++ b/Machines/Amiga/Amiga.cpp @@ -209,14 +209,17 @@ class ConcreteMachine: chipset_.set_scan_target(scan_target); } - Outputs::Display::ScanStatus get_scaled_scan_status() const { + Outputs::Display::ScanStatus get_scaled_scan_status() const final { return chipset_.get_scaled_scan_status(); } // MARK: - MachineTypes::TimedMachine. - void run_for(const Cycles cycles) { + void run_for(const Cycles cycles) final { mc68000_.run_for(cycles); + } + + void flush_output() final { flush(); } @@ -228,7 +231,7 @@ class ConcreteMachine: // MARK: - MachineTypes::JoystickMachine. - const std::vector> &get_joysticks() { + const std::vector> &get_joysticks() final { return chipset_.get_joysticks(); } diff --git a/Machines/TimedMachine.hpp b/Machines/TimedMachine.hpp index 5e4306b0d..df17baece 100644 --- a/Machines/TimedMachine.hpp +++ b/Machines/TimedMachine.hpp @@ -61,6 +61,9 @@ class TimedMachine { virtual float get_confidence() { return 0.5f; } virtual std::string debug_type() { return ""; } + /// Ensures all locally-buffered output is posted onward. + virtual void flush_output() {} + protected: /// Runs the machine for @c cycles. virtual void run_for(const Cycles cycles) = 0; From 96189bde4bb46392167b99381c28392bdda8c405 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Thu, 7 Jul 2022 16:46:08 -0400 Subject: [PATCH 05/32] Loop the Master System into the experiment. --- Machines/MasterSystem/MasterSystem.cpp | 9 ++++++--- OSBindings/Mac/Clock Signal/Machine/CSMachine.mm | 5 ++++- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/Machines/MasterSystem/MasterSystem.cpp b/Machines/MasterSystem/MasterSystem.cpp index a3ceff903..507b318df 100644 --- a/Machines/MasterSystem/MasterSystem.cpp +++ b/Machines/MasterSystem/MasterSystem.cpp @@ -210,6 +210,12 @@ class ConcreteMachine: z80_.run_for(cycles); } + void flush_output() final { + vdp_.flush(); + update_audio(); + audio_queue_.perform(); + } + forceinline HalfCycles perform_machine_cycle(const CPU::Z80::PartialMachineCycle &cycle) { if(vdp_ += cycle.length) { z80_.set_interrupt_line(vdp_->get_interrupt_line(), vdp_.last_sequence_point_overrun()); @@ -383,9 +389,6 @@ class ConcreteMachine: } void flush() { - vdp_.flush(); - update_audio(); - audio_queue_.perform(); } const std::vector> &get_joysticks() final { diff --git a/OSBindings/Mac/Clock Signal/Machine/CSMachine.mm b/OSBindings/Mac/Clock Signal/Machine/CSMachine.mm index f1e11b04b..ef588b7c2 100644 --- a/OSBindings/Mac/Clock Signal/Machine/CSMachine.mm +++ b/OSBindings/Mac/Clock Signal/Machine/CSMachine.mm @@ -678,11 +678,14 @@ struct ActivityObserver: public Activity::Observer { - (void)audioQueueIsRunningDry:(nonnull CSAudioQueue *)audioQueue { // TODO: Make audio flushes overt, and do one here. - updater.update([] {}); + updater.update([self] { + updater.performer.machine->flush_output(); + }); } - (void)scanTargetViewDisplayLinkDidFire:(CSScanTargetView *)view now:(const CVTimeStamp *)now outputTime:(const CVTimeStamp *)outputTime { updater.update([self] { + updater.performer.machine->flush_output(); dispatch_async(dispatch_get_main_queue(), ^{ [self.view updateBacking]; [self.view draw]; From 07ce0f0133a257f845423d11e37cb522c6fb1dcf Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Thu, 7 Jul 2022 16:56:10 -0400 Subject: [PATCH 06/32] Attempt safe shutdown. --- Concurrency/AsyncUpdater.hpp | 12 +++++++++--- OSBindings/Mac/Clock Signal/Machine/CSMachine.mm | 12 +++++++++--- 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/Concurrency/AsyncUpdater.hpp b/Concurrency/AsyncUpdater.hpp index 74e1fd334..12b937895 100644 --- a/Concurrency/AsyncUpdater.hpp +++ b/Concurrency/AsyncUpdater.hpp @@ -62,10 +62,16 @@ template class AsyncUpdater { condition_.notify_all(); } + void stop() { + if(performer_thread_.joinable()) { + should_quit = true; + update([] {}); + performer_thread_.join(); + } + } + ~AsyncUpdater() { - should_quit = true; - update([] {}); - performer_thread_.join(); + stop(); } // The object that will actually receive time advances. diff --git a/OSBindings/Mac/Clock Signal/Machine/CSMachine.mm b/OSBindings/Mac/Clock Signal/Machine/CSMachine.mm index ef588b7c2..bb66f3461 100644 --- a/OSBindings/Mac/Clock Signal/Machine/CSMachine.mm +++ b/OSBindings/Mac/Clock Signal/Machine/CSMachine.mm @@ -677,7 +677,6 @@ struct ActivityObserver: public Activity::Observer { #pragma mark - Timer - (void)audioQueueIsRunningDry:(nonnull CSAudioQueue *)audioQueue { - // TODO: Make audio flushes overt, and do one here. updater.update([self] { updater.performer.machine->flush_output(); }); @@ -692,6 +691,8 @@ struct ActivityObserver: public Activity::Observer { }); }); + // TODO: restory sync locking; see below. + // First order of business: grab a timestamp. /* const auto timeNow = Time::nanos_now(); @@ -724,6 +725,10 @@ struct ActivityObserver: public Activity::Observer { #define TICKS 120 - (void)start { +// _timer = [[CSHighPrecisionTimer alloc] initWithTask:^{ +// updater.update([] {}); +// } interval:uint64_t(1000000000) / uint64_t(TICKS)]; + // updater.performer.machine = _machine->timed_machine(); /* __block auto lastTime = Time::nanos_now(); @@ -805,8 +810,9 @@ struct ActivityObserver: public Activity::Observer { #undef TICKS - (void)stop { - [_timer invalidate]; - _timer = nil; + updater.stop(); +// [_timer invalidate]; +// _timer = nil; } + (BOOL)attemptInstallROM:(NSURL *)url { From 5c3084c37c9842e80cc02e447e75d24136155213 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Thu, 7 Jul 2022 20:09:37 -0400 Subject: [PATCH 07/32] Fix construction order. --- Concurrency/AsyncUpdater.hpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/Concurrency/AsyncUpdater.hpp b/Concurrency/AsyncUpdater.hpp index 12b937895..62625d452 100644 --- a/Concurrency/AsyncUpdater.hpp +++ b/Concurrency/AsyncUpdater.hpp @@ -84,10 +84,13 @@ template class AsyncUpdater { std::unique_ptr actions_; // Necessary synchronisation parts. - std::thread performer_thread_; + std::atomic should_quit = false; std::mutex condition_mutex_; std::condition_variable condition_; - std::atomic should_quit = false; + + // Ensure the thread isn't constructed until after the mutex + // and condition variable. + std::thread performer_thread_; }; From b03d91d5dd470386f5aba15eefb1d020bdf20ab2 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Fri, 8 Jul 2022 15:38:29 -0400 Subject: [PATCH 08/32] Permit granular specification of what to flush. --- Machines/Amiga/Amiga.cpp | 2 +- Machines/MasterSystem/MasterSystem.cpp | 12 ++++++++---- Machines/TimedMachine.hpp | 9 +++++++-- OSBindings/Mac/Clock Signal/Machine/CSMachine.mm | 4 ++-- 4 files changed, 18 insertions(+), 9 deletions(-) diff --git a/Machines/Amiga/Amiga.cpp b/Machines/Amiga/Amiga.cpp index 5a345359b..30afa9f43 100644 --- a/Machines/Amiga/Amiga.cpp +++ b/Machines/Amiga/Amiga.cpp @@ -219,7 +219,7 @@ class ConcreteMachine: mc68000_.run_for(cycles); } - void flush_output() final { + void flush_output(Output) final { flush(); } diff --git a/Machines/MasterSystem/MasterSystem.cpp b/Machines/MasterSystem/MasterSystem.cpp index 507b318df..6c5b9dc49 100644 --- a/Machines/MasterSystem/MasterSystem.cpp +++ b/Machines/MasterSystem/MasterSystem.cpp @@ -210,10 +210,14 @@ class ConcreteMachine: z80_.run_for(cycles); } - void flush_output() final { - vdp_.flush(); - update_audio(); - audio_queue_.perform(); + void flush_output(Output output) final { + if(int(output) & int(Output::Video)) { + vdp_.flush(); + } + if(int(output) & int(Output::Audio)) { + update_audio(); + audio_queue_.perform(); + } } forceinline HalfCycles perform_machine_cycle(const CPU::Z80::PartialMachineCycle &cycle) { diff --git a/Machines/TimedMachine.hpp b/Machines/TimedMachine.hpp index df17baece..c741d9016 100644 --- a/Machines/TimedMachine.hpp +++ b/Machines/TimedMachine.hpp @@ -61,8 +61,13 @@ class TimedMachine { virtual float get_confidence() { return 0.5f; } virtual std::string debug_type() { return ""; } - /// Ensures all locally-buffered output is posted onward. - virtual void flush_output() {} + enum class Output { + Video = 1 << 0, + Audio = 1 << 1 + }; + /// Ensures all locally-buffered output is posted onward for the types of output indicated + /// by the bitfield argument. + virtual void flush_output(Output) {} protected: /// Runs the machine for @c cycles. diff --git a/OSBindings/Mac/Clock Signal/Machine/CSMachine.mm b/OSBindings/Mac/Clock Signal/Machine/CSMachine.mm index bb66f3461..579a66451 100644 --- a/OSBindings/Mac/Clock Signal/Machine/CSMachine.mm +++ b/OSBindings/Mac/Clock Signal/Machine/CSMachine.mm @@ -678,13 +678,13 @@ struct ActivityObserver: public Activity::Observer { - (void)audioQueueIsRunningDry:(nonnull CSAudioQueue *)audioQueue { updater.update([self] { - updater.performer.machine->flush_output(); + updater.performer.machine->flush_output(MachineTypes::TimedMachine::Output::Audio); }); } - (void)scanTargetViewDisplayLinkDidFire:(CSScanTargetView *)view now:(const CVTimeStamp *)now outputTime:(const CVTimeStamp *)outputTime { updater.update([self] { - updater.performer.machine->flush_output(); + updater.performer.machine->flush_output(MachineTypes::TimedMachine::Output::Video); dispatch_async(dispatch_get_main_queue(), ^{ [self.view updateBacking]; [self.view draw]; From b097b1296b357730c7a7ba6c40c1adeff3ca91e7 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Fri, 8 Jul 2022 16:04:32 -0400 Subject: [PATCH 09/32] Adopt granular flushing widely. --- Machines/Amiga/Amiga.cpp | 6 +----- Machines/AmstradCPC/AmstradCPC.cpp | 12 ++++++++---- Machines/Apple/AppleII/AppleII.cpp | 13 +++++++++---- Machines/Apple/AppleIIgs/AppleIIgs.cpp | 12 ++++++++---- Machines/Apple/Macintosh/Macintosh.cpp | 3 +-- Machines/Atari/2600/Atari2600.cpp | 2 +- Machines/Atari/ST/AtariST.cpp | 14 +++++++++----- Machines/ColecoVision/ColecoVision.cpp | 12 ++++++++---- Machines/Commodore/Vic-20/Vic20.cpp | 10 +++++++--- Machines/Electron/Electron.cpp | 12 ++++++++---- Machines/Enterprise/Enterprise.cpp | 12 ++++++++---- Machines/MSX/MSX.cpp | 12 ++++++++---- Machines/MasterSystem/MasterSystem.cpp | 3 --- Machines/Oric/Oric.cpp | 10 +++++++--- Machines/Sinclair/ZX8081/ZX8081.cpp | 13 +++++++++---- Machines/Sinclair/ZXSpectrum/ZXSpectrum.cpp | 13 +++++++++---- 16 files changed, 101 insertions(+), 58 deletions(-) diff --git a/Machines/Amiga/Amiga.cpp b/Machines/Amiga/Amiga.cpp index 30afa9f43..4e214c21a 100644 --- a/Machines/Amiga/Amiga.cpp +++ b/Machines/Amiga/Amiga.cpp @@ -176,10 +176,6 @@ class ConcreteMachine: return total_length - cycle.length; } - void flush() { - chipset_.flush(); - } - private: CPU::MC68000Mk2::Processor mc68000_; @@ -220,7 +216,7 @@ class ConcreteMachine: } void flush_output(Output) final { - flush(); + chipset_.flush(); } // MARK: - MachineTypes::MouseMachine. diff --git a/Machines/AmstradCPC/AmstradCPC.cpp b/Machines/AmstradCPC/AmstradCPC.cpp index f1405d905..18d672657 100644 --- a/Machines/AmstradCPC/AmstradCPC.cpp +++ b/Machines/AmstradCPC/AmstradCPC.cpp @@ -1048,11 +1048,15 @@ template class ConcreteMachine: return HalfCycles(0); } - /// Another Z80 entry point; indicates that a partcular run request has concluded. - void flush() { + /// Fields requests to pump all output. + void flush_output(Output output) final { // Just flush the AY. - ay_.update(); - ay_.flush(); + if(int(output) & int(Output::Audio)) { + ay_.update(); + ay_.flush(); + } + + // Always flush the FDC. flush_fdc(); } diff --git a/Machines/Apple/AppleII/AppleII.cpp b/Machines/Apple/AppleII/AppleII.cpp index c1f88e26f..450ad4452 100644 --- a/Machines/Apple/AppleII/AppleII.cpp +++ b/Machines/Apple/AppleII/AppleII.cpp @@ -810,11 +810,16 @@ template class ConcreteMachine: return Cycles(1); } - void flush() { - update_video(); - update_audio(); + void flush_output(Output output) final { update_just_in_time_cards(); - audio_queue_.perform(); + + if(int(output) & int(Output::Video)) { + update_video(); + } + if(int(output) & int(Output::Audio)) { + update_audio(); + audio_queue_.perform(); + } } void run_for(const Cycles cycles) final { diff --git a/Machines/Apple/AppleIIgs/AppleIIgs.cpp b/Machines/Apple/AppleIIgs/AppleIIgs.cpp index 35b33377c..bebde58bf 100644 --- a/Machines/Apple/AppleIIgs/AppleIIgs.cpp +++ b/Machines/Apple/AppleIIgs/AppleIIgs.cpp @@ -326,13 +326,17 @@ class ConcreteMachine: m65816_.run_for(cycles); } - void flush() { - video_.flush(); + void flush_output(Output output) final { iwm_.flush(); adb_glu_.flush(); - AudioUpdater updater(this); - audio_queue_.perform(); + if(int(output) & int(Output::Video)) { + video_.flush(); + } + if(int(output) & int(Output::Audio)) { + AudioUpdater updater(this); + audio_queue_.perform(); + } } void set_scan_target(Outputs::Display::ScanTarget *target) override { diff --git a/Machines/Apple/Macintosh/Macintosh.cpp b/Machines/Apple/Macintosh/Macintosh.cpp index add620202..09eed25e3 100644 --- a/Machines/Apple/Macintosh/Macintosh.cpp +++ b/Machines/Apple/Macintosh/Macintosh.cpp @@ -190,7 +190,6 @@ template class ConcreteMachin void run_for(const Cycles cycles) final { mc68000_.run_for(cycles); - flush(); } using Microcycle = CPU::MC68000Mk2::Microcycle; @@ -366,7 +365,7 @@ template class ConcreteMachin return delay; } - void flush() { + void flush_output(Output) { // Flush the video before the audio queue; in a Mac the // video is responsible for providing part of the // audio signal, so the two aren't as distinct as in diff --git a/Machines/Atari/2600/Atari2600.cpp b/Machines/Atari/2600/Atari2600.cpp index 6aeaa4f9c..9fb138353 100644 --- a/Machines/Atari/2600/Atari2600.cpp +++ b/Machines/Atari/2600/Atari2600.cpp @@ -174,7 +174,7 @@ class ConcreteMachine: bus_->apply_confidence(confidence_counter_); } - void flush() { + void flush_output(Output) final { bus_->flush(); } diff --git a/Machines/Atari/ST/AtariST.cpp b/Machines/Atari/ST/AtariST.cpp index c87092646..4464f6da7 100644 --- a/Machines/Atari/ST/AtariST.cpp +++ b/Machines/Atari/ST/AtariST.cpp @@ -154,7 +154,6 @@ class ConcreteMachine: } mc68000_.run_for(cycles); - flush(); } // MARK: MC68000::BusHandler @@ -414,14 +413,19 @@ class ConcreteMachine: return HalfCycles(0); } - void flush() { + void flush_output(Output output) final { dma_.flush(); mfp_.flush(); keyboard_acia_.flush(); midi_acia_.flush(); - video_.flush(); - update_audio(); - audio_queue_.perform(); + + if(int(output) & int(Output::Video)) { + video_.flush(); + } + if(int(output) & int(Output::Audio)) { + update_audio(); + audio_queue_.perform(); + } } private: diff --git a/Machines/ColecoVision/ColecoVision.cpp b/Machines/ColecoVision/ColecoVision.cpp index 659f90a5d..61f710907 100644 --- a/Machines/ColecoVision/ColecoVision.cpp +++ b/Machines/ColecoVision/ColecoVision.cpp @@ -342,10 +342,14 @@ class ConcreteMachine: return penalty; } - void flush() { - vdp_.flush(); - update_audio(); - audio_queue_.perform(); + void flush_output(Output output) final { + if(int(output) & int(Output::Video)) { + vdp_.flush(); + } + if(int(output) & int(Output::Audio)) { + update_audio(); + audio_queue_.perform(); + } } float get_confidence() final { diff --git a/Machines/Commodore/Vic-20/Vic20.cpp b/Machines/Commodore/Vic-20/Vic20.cpp index 49432e44d..b63796eec 100644 --- a/Machines/Commodore/Vic-20/Vic20.cpp +++ b/Machines/Commodore/Vic-20/Vic20.cpp @@ -620,9 +620,13 @@ class ConcreteMachine: return Cycles(1); } - void flush() { - update_video(); - mos6560_.flush(); + void flush_output(Output output) final { + if(int(output) & int(Output::Video)) { + update_video(); + } + if(int(output) & int(Output::Audio)) { + mos6560_.flush(); + } } void run_for(const Cycles cycles) final { diff --git a/Machines/Electron/Electron.cpp b/Machines/Electron/Electron.cpp index 16201ae76..0eef67e3d 100644 --- a/Machines/Electron/Electron.cpp +++ b/Machines/Electron/Electron.cpp @@ -501,10 +501,14 @@ template class ConcreteMachine: return Cycles(int(cycles)); } - forceinline void flush() { - video_.flush(); - update_audio(); - audio_queue_.perform(); + void flush_output(Output output) final { + if(int(output) & int(Output::Video)) { + video_.flush(); + } + if(int(output) & int(Output::Audio)) { + update_audio(); + audio_queue_.perform(); + } } void set_scan_target(Outputs::Display::ScanTarget *scan_target) final { diff --git a/Machines/Enterprise/Enterprise.cpp b/Machines/Enterprise/Enterprise.cpp index 7443370ba..1c54c7b88 100644 --- a/Machines/Enterprise/Enterprise.cpp +++ b/Machines/Enterprise/Enterprise.cpp @@ -539,10 +539,14 @@ template class ConcreteMachine: return penalty; } - void flush() { - nick_.flush(); - update_audio(); - audio_queue_.perform(); + void flush_output(Output output) final { + if(int(output) & int(Output::Video)) { + nick_.flush(); + } + if(int(output) & int(Output::Audio)) { + update_audio(); + audio_queue_.perform(); + } } private: diff --git a/Machines/MSX/MSX.cpp b/Machines/MSX/MSX.cpp index e4353d965..7934b36e8 100644 --- a/Machines/MSX/MSX.cpp +++ b/Machines/MSX/MSX.cpp @@ -615,10 +615,14 @@ class ConcreteMachine: return addition; } - void flush() { - vdp_.flush(); - update_audio(); - audio_queue_.perform(); + void flush_output(Output output) final { + if(int(output) & int(Output::Video)) { + vdp_.flush(); + } + if(int(output) & int(Output::Audio)) { + update_audio(); + audio_queue_.perform(); + } } void set_keyboard_line(int line) { diff --git a/Machines/MasterSystem/MasterSystem.cpp b/Machines/MasterSystem/MasterSystem.cpp index 6c5b9dc49..51db116c4 100644 --- a/Machines/MasterSystem/MasterSystem.cpp +++ b/Machines/MasterSystem/MasterSystem.cpp @@ -392,9 +392,6 @@ class ConcreteMachine: return HalfCycles(0); } - void flush() { - } - const std::vector> &get_joysticks() final { return joysticks_; } diff --git a/Machines/Oric/Oric.cpp b/Machines/Oric/Oric.cpp index b06d05b7c..4cc079904 100644 --- a/Machines/Oric/Oric.cpp +++ b/Machines/Oric/Oric.cpp @@ -578,9 +578,13 @@ template class ConcreteMachine: return HalfCycles(0); } - forceinline void flush() { - video_.flush(); + void flush_output(Output output) final { + if(int(output) & int(Output::Video)) { + video_.flush(); + } + if constexpr (is_zx81) { - update_audio(); - audio_queue_.perform(); + if(int(output) & int(Output::Audio)) { + update_audio(); + audio_queue_.perform(); + } } } diff --git a/Machines/Sinclair/ZXSpectrum/ZXSpectrum.cpp b/Machines/Sinclair/ZXSpectrum/ZXSpectrum.cpp index 649b7c0eb..06a5d8631 100644 --- a/Machines/Sinclair/ZXSpectrum/ZXSpectrum.cpp +++ b/Machines/Sinclair/ZXSpectrum/ZXSpectrum.cpp @@ -263,10 +263,15 @@ template class ConcreteMachine: } } - void flush() { - video_.flush(); - update_audio(); - audio_queue_.perform(); + void flush_output(Output output) override { + if(int(output) & int(Output::Video)) { + video_.flush(); + } + + if(int(output) & int(Output::Audio)) { + update_audio(); + audio_queue_.perform(); + } if constexpr (model == Model::Plus3) { fdc_.flush(); From 51ed3f2ed08016532e7ded7449235d89a1ba78fe Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Sat, 9 Jul 2022 13:03:45 -0400 Subject: [PATCH 10/32] Reduce modal-related thread hopping. --- .../Mac/Clock Signal/ScanTarget/CSScanTarget.mm | 14 ++++++++------ Outputs/ScanTargets/BufferingScanTarget.cpp | 12 +++++++++--- Outputs/ScanTargets/BufferingScanTarget.hpp | 7 ++++++- 3 files changed, 23 insertions(+), 10 deletions(-) diff --git a/OSBindings/Mac/Clock Signal/ScanTarget/CSScanTarget.mm b/OSBindings/Mac/Clock Signal/ScanTarget/CSScanTarget.mm index 42548e778..a58a499b8 100644 --- a/OSBindings/Mac/Clock Signal/ScanTarget/CSScanTarget.mm +++ b/OSBindings/Mac/Clock Signal/ScanTarget/CSScanTarget.mm @@ -939,12 +939,14 @@ using BufferingScanTarget = Outputs::Display::BufferingScanTarget; - (void)updateFrameBuffer { // TODO: rethink BufferingScanTarget::perform. Is it now really just for guarding the modals? - _scanTarget.perform([=] { - const Outputs::Display::ScanTarget::Modals *const newModals = _scanTarget.new_modals(); - if(newModals) { - [self setModals:*newModals]; - } - }); + if(_scanTarget.has_new_modals()) { + _scanTarget.perform([=] { + const Outputs::Display::ScanTarget::Modals *const newModals = _scanTarget.new_modals(); + if(newModals) { + [self setModals:*newModals]; + } + }); + } @synchronized(self) { if(!_frameBufferRenderPass) return; diff --git a/Outputs/ScanTargets/BufferingScanTarget.cpp b/Outputs/ScanTargets/BufferingScanTarget.cpp index c5bc95002..e65b9a136 100644 --- a/Outputs/ScanTargets/BufferingScanTarget.cpp +++ b/Outputs/ScanTargets/BufferingScanTarget.cpp @@ -302,7 +302,7 @@ size_t BufferingScanTarget::write_area_data_size() const { void BufferingScanTarget::set_modals(Modals modals) { perform([=] { modals_ = modals; - modals_are_dirty_ = true; + modals_are_dirty_.store(true, std::memory_order::memory_order_relaxed); }); } @@ -374,10 +374,12 @@ void BufferingScanTarget::set_line_buffer(Line *line_buffer, LineMetadata *metad } const Outputs::Display::ScanTarget::Modals *BufferingScanTarget::new_modals() { - if(!modals_are_dirty_) { + const auto modals_are_dirty = modals_are_dirty_.load(std::memory_order::memory_order_relaxed); + if(!modals_are_dirty) { return nullptr; } - modals_are_dirty_ = false; + + modals_are_dirty_.store(false, std::memory_order::memory_order_relaxed); // MAJOR SHARP EDGE HERE: assume that because the new_modals have been fetched then the caller will // now ensure their texture buffer is appropriate. They might provide a new pointer and might now. @@ -392,3 +394,7 @@ const Outputs::Display::ScanTarget::Modals *BufferingScanTarget::new_modals() { const Outputs::Display::ScanTarget::Modals &BufferingScanTarget::modals() const { return modals_; } + +bool BufferingScanTarget::has_new_modals() const { + return modals_are_dirty_.load(std::memory_order::memory_order_relaxed); +} diff --git a/Outputs/ScanTargets/BufferingScanTarget.hpp b/Outputs/ScanTargets/BufferingScanTarget.hpp index 4b41d0a8b..83d8a3013 100644 --- a/Outputs/ScanTargets/BufferingScanTarget.hpp +++ b/Outputs/ScanTargets/BufferingScanTarget.hpp @@ -161,6 +161,11 @@ class BufferingScanTarget: public Outputs::Display::ScanTarget { /// @returns the current @c Modals. const Modals &modals() const; + /// @returns @c true if new modals are available; @c false otherwise. + /// + /// Safe to call from any thread. + bool has_new_modals() const; + private: // ScanTarget overrides. void set_modals(Modals) final; @@ -253,7 +258,7 @@ class BufferingScanTarget: public Outputs::Display::ScanTarget { // Current modals and whether they've yet been returned // from a call to @c get_new_modals. Modals modals_; - bool modals_are_dirty_ = false; + std::atomic modals_are_dirty_ = false; // Provides a per-data size implementation of end_data; a previous // implementation used blind memcpy and that turned into something From 6dabdaca45c3990f7588d2cc77f458b1f7108d8c Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Sat, 9 Jul 2022 13:33:46 -0400 Subject: [PATCH 11/32] Switch to `int`; attempt to do a better job of initial audio filling. --- Machines/Amiga/Amiga.cpp | 2 +- Machines/AmstradCPC/AmstradCPC.cpp | 4 ++-- Machines/Apple/AppleII/AppleII.cpp | 6 +++--- Machines/Apple/AppleIIgs/AppleIIgs.cpp | 6 +++--- Machines/Apple/Macintosh/Macintosh.cpp | 2 +- Machines/Atari/2600/Atari2600.cpp | 2 +- Machines/Atari/ST/AtariST.cpp | 6 +++--- Machines/ColecoVision/ColecoVision.cpp | 6 +++--- Machines/Commodore/Vic-20/Vic20.cpp | 6 +++--- Machines/Electron/Electron.cpp | 6 +++--- Machines/Enterprise/Enterprise.cpp | 6 +++--- Machines/MSX/MSX.cpp | 6 +++--- Machines/MasterSystem/MasterSystem.cpp | 6 +++--- Machines/Oric/Oric.cpp | 6 +++--- Machines/Sinclair/ZX8081/ZX8081.cpp | 6 +++--- Machines/Sinclair/ZXSpectrum/ZXSpectrum.cpp | 6 +++--- Machines/TimedMachine.hpp | 10 +++++----- .../Mac/Clock Signal/Audio/CSAudioQueue.h | 5 +++++ .../Mac/Clock Signal/Audio/CSAudioQueue.m | 17 +++++++++++------ .../Mac/Clock Signal/Machine/CSMachine.mm | 12 ++++++++++-- 20 files changed, 72 insertions(+), 54 deletions(-) diff --git a/Machines/Amiga/Amiga.cpp b/Machines/Amiga/Amiga.cpp index 4e214c21a..370217d7c 100644 --- a/Machines/Amiga/Amiga.cpp +++ b/Machines/Amiga/Amiga.cpp @@ -215,7 +215,7 @@ class ConcreteMachine: mc68000_.run_for(cycles); } - void flush_output(Output) final { + void flush_output(int) final { chipset_.flush(); } diff --git a/Machines/AmstradCPC/AmstradCPC.cpp b/Machines/AmstradCPC/AmstradCPC.cpp index 18d672657..db4c88391 100644 --- a/Machines/AmstradCPC/AmstradCPC.cpp +++ b/Machines/AmstradCPC/AmstradCPC.cpp @@ -1049,9 +1049,9 @@ template class ConcreteMachine: } /// Fields requests to pump all output. - void flush_output(Output output) final { + void flush_output(int outputs) final { // Just flush the AY. - if(int(output) & int(Output::Audio)) { + if(outputs & Output::Audio) { ay_.update(); ay_.flush(); } diff --git a/Machines/Apple/AppleII/AppleII.cpp b/Machines/Apple/AppleII/AppleII.cpp index 450ad4452..706e4cb6c 100644 --- a/Machines/Apple/AppleII/AppleII.cpp +++ b/Machines/Apple/AppleII/AppleII.cpp @@ -810,13 +810,13 @@ template class ConcreteMachine: return Cycles(1); } - void flush_output(Output output) final { + void flush_output(int outputs) final { update_just_in_time_cards(); - if(int(output) & int(Output::Video)) { + if(outputs & Output::Video) { update_video(); } - if(int(output) & int(Output::Audio)) { + if(outputs & Output::Audio) { update_audio(); audio_queue_.perform(); } diff --git a/Machines/Apple/AppleIIgs/AppleIIgs.cpp b/Machines/Apple/AppleIIgs/AppleIIgs.cpp index bebde58bf..546eeb68a 100644 --- a/Machines/Apple/AppleIIgs/AppleIIgs.cpp +++ b/Machines/Apple/AppleIIgs/AppleIIgs.cpp @@ -326,14 +326,14 @@ class ConcreteMachine: m65816_.run_for(cycles); } - void flush_output(Output output) final { + void flush_output(int outputs) final { iwm_.flush(); adb_glu_.flush(); - if(int(output) & int(Output::Video)) { + if(outputs & Output::Video) { video_.flush(); } - if(int(output) & int(Output::Audio)) { + if(outputs & Output::Audio) { AudioUpdater updater(this); audio_queue_.perform(); } diff --git a/Machines/Apple/Macintosh/Macintosh.cpp b/Machines/Apple/Macintosh/Macintosh.cpp index 09eed25e3..02378af24 100644 --- a/Machines/Apple/Macintosh/Macintosh.cpp +++ b/Machines/Apple/Macintosh/Macintosh.cpp @@ -365,7 +365,7 @@ template class ConcreteMachin return delay; } - void flush_output(Output) { + void flush_output(int) { // Flush the video before the audio queue; in a Mac the // video is responsible for providing part of the // audio signal, so the two aren't as distinct as in diff --git a/Machines/Atari/2600/Atari2600.cpp b/Machines/Atari/2600/Atari2600.cpp index 9fb138353..72ff213d6 100644 --- a/Machines/Atari/2600/Atari2600.cpp +++ b/Machines/Atari/2600/Atari2600.cpp @@ -174,7 +174,7 @@ class ConcreteMachine: bus_->apply_confidence(confidence_counter_); } - void flush_output(Output) final { + void flush_output(int) final { bus_->flush(); } diff --git a/Machines/Atari/ST/AtariST.cpp b/Machines/Atari/ST/AtariST.cpp index 4464f6da7..2721de88c 100644 --- a/Machines/Atari/ST/AtariST.cpp +++ b/Machines/Atari/ST/AtariST.cpp @@ -413,16 +413,16 @@ class ConcreteMachine: return HalfCycles(0); } - void flush_output(Output output) final { + void flush_output(int outputs) final { dma_.flush(); mfp_.flush(); keyboard_acia_.flush(); midi_acia_.flush(); - if(int(output) & int(Output::Video)) { + if(outputs & Output::Video) { video_.flush(); } - if(int(output) & int(Output::Audio)) { + if(outputs & Output::Audio) { update_audio(); audio_queue_.perform(); } diff --git a/Machines/ColecoVision/ColecoVision.cpp b/Machines/ColecoVision/ColecoVision.cpp index 61f710907..5123abdb5 100644 --- a/Machines/ColecoVision/ColecoVision.cpp +++ b/Machines/ColecoVision/ColecoVision.cpp @@ -342,11 +342,11 @@ class ConcreteMachine: return penalty; } - void flush_output(Output output) final { - if(int(output) & int(Output::Video)) { + void flush_output(int outputs) final { + if(outputs & Output::Video) { vdp_.flush(); } - if(int(output) & int(Output::Audio)) { + if(outputs & Output::Audio) { update_audio(); audio_queue_.perform(); } diff --git a/Machines/Commodore/Vic-20/Vic20.cpp b/Machines/Commodore/Vic-20/Vic20.cpp index b63796eec..03d8292e4 100644 --- a/Machines/Commodore/Vic-20/Vic20.cpp +++ b/Machines/Commodore/Vic-20/Vic20.cpp @@ -620,11 +620,11 @@ class ConcreteMachine: return Cycles(1); } - void flush_output(Output output) final { - if(int(output) & int(Output::Video)) { + void flush_output(int outputs) final { + if(outputs & Output::Video) { update_video(); } - if(int(output) & int(Output::Audio)) { + if(outputs & Output::Audio) { mos6560_.flush(); } } diff --git a/Machines/Electron/Electron.cpp b/Machines/Electron/Electron.cpp index 0eef67e3d..b0f3591e3 100644 --- a/Machines/Electron/Electron.cpp +++ b/Machines/Electron/Electron.cpp @@ -501,11 +501,11 @@ template class ConcreteMachine: return Cycles(int(cycles)); } - void flush_output(Output output) final { - if(int(output) & int(Output::Video)) { + void flush_output(int outputs) final { + if(outputs & Output::Video) { video_.flush(); } - if(int(output) & int(Output::Audio)) { + if(outputs & Output::Audio) { update_audio(); audio_queue_.perform(); } diff --git a/Machines/Enterprise/Enterprise.cpp b/Machines/Enterprise/Enterprise.cpp index 1c54c7b88..1481d8e13 100644 --- a/Machines/Enterprise/Enterprise.cpp +++ b/Machines/Enterprise/Enterprise.cpp @@ -539,11 +539,11 @@ template class ConcreteMachine: return penalty; } - void flush_output(Output output) final { - if(int(output) & int(Output::Video)) { + void flush_output(int outputs) final { + if(outputs & Output::Video) { nick_.flush(); } - if(int(output) & int(Output::Audio)) { + if(outputs & Output::Audio) { update_audio(); audio_queue_.perform(); } diff --git a/Machines/MSX/MSX.cpp b/Machines/MSX/MSX.cpp index 7934b36e8..71c598c9f 100644 --- a/Machines/MSX/MSX.cpp +++ b/Machines/MSX/MSX.cpp @@ -615,11 +615,11 @@ class ConcreteMachine: return addition; } - void flush_output(Output output) final { - if(int(output) & int(Output::Video)) { + void flush_output(int outputs) final { + if(outputs & Output::Video) { vdp_.flush(); } - if(int(output) & int(Output::Audio)) { + if(outputs & Output::Audio) { update_audio(); audio_queue_.perform(); } diff --git a/Machines/MasterSystem/MasterSystem.cpp b/Machines/MasterSystem/MasterSystem.cpp index 51db116c4..3e5fab9c9 100644 --- a/Machines/MasterSystem/MasterSystem.cpp +++ b/Machines/MasterSystem/MasterSystem.cpp @@ -210,11 +210,11 @@ class ConcreteMachine: z80_.run_for(cycles); } - void flush_output(Output output) final { - if(int(output) & int(Output::Video)) { + void flush_output(int outputs) final { + if(outputs & Output::Video) { vdp_.flush(); } - if(int(output) & int(Output::Audio)) { + if(outputs & Output::Audio) { update_audio(); audio_queue_.perform(); } diff --git a/Machines/Oric/Oric.cpp b/Machines/Oric/Oric.cpp index 4cc079904..808c80725 100644 --- a/Machines/Oric/Oric.cpp +++ b/Machines/Oric/Oric.cpp @@ -578,11 +578,11 @@ template class ConcreteMachine: return HalfCycles(0); } - void flush_output(Output output) final { - if(int(output) & int(Output::Video)) { + void flush_output(int outputs) final { + if(outputs & Output::Video) { video_.flush(); } if constexpr (is_zx81) { - if(int(output) & int(Output::Audio)) { + if(outputs & Output::Audio) { update_audio(); audio_queue_.perform(); } diff --git a/Machines/Sinclair/ZXSpectrum/ZXSpectrum.cpp b/Machines/Sinclair/ZXSpectrum/ZXSpectrum.cpp index 06a5d8631..c01671947 100644 --- a/Machines/Sinclair/ZXSpectrum/ZXSpectrum.cpp +++ b/Machines/Sinclair/ZXSpectrum/ZXSpectrum.cpp @@ -263,12 +263,12 @@ template class ConcreteMachine: } } - void flush_output(Output output) override { - if(int(output) & int(Output::Video)) { + void flush_output(int outputs) override { + if(outputs & Output::Video) { video_.flush(); } - if(int(output) & int(Output::Audio)) { + if(outputs & Output::Audio) { update_audio(); audio_queue_.perform(); } diff --git a/Machines/TimedMachine.hpp b/Machines/TimedMachine.hpp index c741d9016..f97a5ac66 100644 --- a/Machines/TimedMachine.hpp +++ b/Machines/TimedMachine.hpp @@ -61,13 +61,13 @@ class TimedMachine { virtual float get_confidence() { return 0.5f; } virtual std::string debug_type() { return ""; } - enum class Output { - Video = 1 << 0, - Audio = 1 << 1 + struct Output { + static constexpr int Video = 1 << 0; + static constexpr int Audio = 1 << 1; }; /// Ensures all locally-buffered output is posted onward for the types of output indicated - /// by the bitfield argument. - virtual void flush_output(Output) {} + /// by the bitfield argument, which is comprised of flags from the namespace @c Output. + virtual void flush_output(int) {} protected: /// Runs the machine for @c cycles. diff --git a/OSBindings/Mac/Clock Signal/Audio/CSAudioQueue.h b/OSBindings/Mac/Clock Signal/Audio/CSAudioQueue.h index 370bea589..8a8721dc9 100644 --- a/OSBindings/Mac/Clock Signal/Audio/CSAudioQueue.h +++ b/OSBindings/Mac/Clock Signal/Audio/CSAudioQueue.h @@ -58,4 +58,9 @@ */ @property (nonatomic, readonly) NSUInteger preferredBufferSize; +/*! + @returns @C YES if this queue is running low or is completely exhausted of new audio buffers. +*/ +@property (atomic, readonly) BOOL isRunningDry; + @end diff --git a/OSBindings/Mac/Clock Signal/Audio/CSAudioQueue.m b/OSBindings/Mac/Clock Signal/Audio/CSAudioQueue.m index f65432cc6..1c1a5f7e5 100644 --- a/OSBindings/Mac/Clock Signal/Audio/CSAudioQueue.m +++ b/OSBindings/Mac/Clock Signal/Audio/CSAudioQueue.m @@ -8,6 +8,7 @@ #import "CSAudioQueue.h" @import AudioToolbox; +#include #define AudioQueueBufferMaxLength 8192 #define NumberOfStoredAudioQueueBuffer 16 @@ -29,7 +30,7 @@ static NSLock *CSAudioQueueDeallocLock; AudioQueueRef _audioQueue; NSLock *_storedBuffersLock; CSWeakAudioQueuePointer *_weakPointer; - int _enqueuedBuffers; + atomic_int _enqueuedBuffers; } #pragma mark - AudioQueue callbacks @@ -39,14 +40,14 @@ static NSLock *CSAudioQueueDeallocLock; */ - (BOOL)audioQueue:(AudioQueueRef)theAudioQueue didCallbackWithBuffer:(AudioQueueBufferRef)buffer { [_storedBuffersLock lock]; - --_enqueuedBuffers; + const int buffers = atomic_fetch_add(&_enqueuedBuffers, -1); // If that leaves nothing in the queue, re-enqueue whatever just came back in order to keep the // queue going. AudioQueues seem to stop playing and never restart no matter how much encouragement // if exhausted. - if(!_enqueuedBuffers) { + if(!buffers) { AudioQueueEnqueueBuffer(theAudioQueue, buffer, 0, NULL); - ++_enqueuedBuffers; + atomic_fetch_add(&_enqueuedBuffers, 1); } else { AudioQueueFreeBuffer(_audioQueue, buffer); } @@ -71,6 +72,10 @@ static void audioOutputCallback( } } +- (BOOL)isRunningDry { + return atomic_load_explicit(&_enqueuedBuffers, memory_order_relaxed) < 2; +} + #pragma mark - Standard object lifecycle - (instancetype)initWithSamplingRate:(Float64)samplingRate isStereo:(BOOL)isStereo { @@ -158,11 +163,11 @@ static void audioOutputCallback( [_storedBuffersLock lock]; // Don't enqueue more than 4 buffers ahead of now, to ensure not too much latency accrues. - if(_enqueuedBuffers > 4) { + if(atomic_load_explicit(&_enqueuedBuffers, memory_order_relaxed) > 4) { [_storedBuffersLock unlock]; return; } - ++_enqueuedBuffers; + atomic_fetch_add(&_enqueuedBuffers, 1); AudioQueueBufferRef newBuffer; AudioQueueAllocateBuffer(_audioQueue, (UInt32)bufferBytes * 2, &newBuffer); diff --git a/OSBindings/Mac/Clock Signal/Machine/CSMachine.mm b/OSBindings/Mac/Clock Signal/Machine/CSMachine.mm index 579a66451..45c93b0b4 100644 --- a/OSBindings/Mac/Clock Signal/Machine/CSMachine.mm +++ b/OSBindings/Mac/Clock Signal/Machine/CSMachine.mm @@ -678,13 +678,21 @@ struct ActivityObserver: public Activity::Observer { - (void)audioQueueIsRunningDry:(nonnull CSAudioQueue *)audioQueue { updater.update([self] { - updater.performer.machine->flush_output(MachineTypes::TimedMachine::Output::Audio); + updater.performer.machine->flush_output(MachineTypes::TimedMachine::Output::Audio);// | MachineTypes::TimedMachine::Output::Video); +// dispatch_async(dispatch_get_global_queue(QOS_CLASS_USER_INTERACTIVE, 0), ^{ +// [self.view updateBacking]; +// }); }); } - (void)scanTargetViewDisplayLinkDidFire:(CSScanTargetView *)view now:(const CVTimeStamp *)now outputTime:(const CVTimeStamp *)outputTime { updater.update([self] { - updater.performer.machine->flush_output(MachineTypes::TimedMachine::Output::Video); + auto outputs = MachineTypes::TimedMachine::Output::Video; + if(_audioQueue.isRunningDry) { + outputs |= MachineTypes::TimedMachine::Output::Audio; + } + updater.performer.machine->flush_output(outputs); + dispatch_async(dispatch_get_main_queue(), ^{ [self.view updateBacking]; [self.view draw]; From f2fb9cf5964618cba06f7052adb9a55a5b4fe0b1 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Sun, 10 Jul 2022 21:35:05 -0400 Subject: [PATCH 12/32] Avoid unnecessary queue jump. --- OSBindings/Mac/Clock Signal/Machine/CSMachine.mm | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/OSBindings/Mac/Clock Signal/Machine/CSMachine.mm b/OSBindings/Mac/Clock Signal/Machine/CSMachine.mm index 45c93b0b4..b75edb10a 100644 --- a/OSBindings/Mac/Clock Signal/Machine/CSMachine.mm +++ b/OSBindings/Mac/Clock Signal/Machine/CSMachine.mm @@ -678,10 +678,8 @@ struct ActivityObserver: public Activity::Observer { - (void)audioQueueIsRunningDry:(nonnull CSAudioQueue *)audioQueue { updater.update([self] { - updater.performer.machine->flush_output(MachineTypes::TimedMachine::Output::Audio);// | MachineTypes::TimedMachine::Output::Video); -// dispatch_async(dispatch_get_global_queue(QOS_CLASS_USER_INTERACTIVE, 0), ^{ -// [self.view updateBacking]; -// }); + updater.performer.machine->flush_output(MachineTypes::TimedMachine::Output::Audio); /* | MachineTypes::TimedMachine::Output::Video); + [self.view updateBacking];*/ }); } @@ -692,9 +690,9 @@ struct ActivityObserver: public Activity::Observer { outputs |= MachineTypes::TimedMachine::Output::Audio; } updater.performer.machine->flush_output(outputs); + [self.view updateBacking]; dispatch_async(dispatch_get_main_queue(), ^{ - [self.view updateBacking]; [self.view draw]; }); }); From a1544f303364c590ac48665b408ee43230e1b97b Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Mon, 11 Jul 2022 20:50:02 -0400 Subject: [PATCH 13/32] Do a better job of keeping the queue populated. --- OSBindings/Mac/Clock Signal/Audio/CSAudioQueue.m | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/OSBindings/Mac/Clock Signal/Audio/CSAudioQueue.m b/OSBindings/Mac/Clock Signal/Audio/CSAudioQueue.m index 1c1a5f7e5..fce2ebc44 100644 --- a/OSBindings/Mac/Clock Signal/Audio/CSAudioQueue.m +++ b/OSBindings/Mac/Clock Signal/Audio/CSAudioQueue.m @@ -42,10 +42,10 @@ static NSLock *CSAudioQueueDeallocLock; [_storedBuffersLock lock]; const int buffers = atomic_fetch_add(&_enqueuedBuffers, -1); - // If that leaves nothing in the queue, re-enqueue whatever just came back in order to keep the - // queue going. AudioQueues seem to stop playing and never restart no matter how much encouragement - // if exhausted. - if(!buffers) { + // If that suggests the queue may be exhausted soon, re-enqueue whatever just came back in order to + // keep the queue going. AudioQueues seem to stop playing and never restart no matter how much + // encouragement if exhausted. + if(buffers == 1) { AudioQueueEnqueueBuffer(theAudioQueue, buffer, 0, NULL); atomic_fetch_add(&_enqueuedBuffers, 1); } else { @@ -73,7 +73,7 @@ static void audioOutputCallback( } - (BOOL)isRunningDry { - return atomic_load_explicit(&_enqueuedBuffers, memory_order_relaxed) < 2; + return atomic_load_explicit(&_enqueuedBuffers, memory_order_relaxed) < 3; } #pragma mark - Standard object lifecycle @@ -167,7 +167,7 @@ static void audioOutputCallback( [_storedBuffersLock unlock]; return; } - atomic_fetch_add(&_enqueuedBuffers, 1); + const int enqueuedBuffers = atomic_fetch_add(&_enqueuedBuffers, 1); AudioQueueBufferRef newBuffer; AudioQueueAllocateBuffer(_audioQueue, (UInt32)bufferBytes * 2, &newBuffer); @@ -179,7 +179,9 @@ static void audioOutputCallback( // 'Start' the queue. This is documented to be a no-op if the queue is already started, // and it's better to defer starting it until at least some data is available. - AudioQueueStart(_audioQueue, NULL); + if(enqueuedBuffers > 3) { + AudioQueueStart(_audioQueue, NULL); + } } #pragma mark - Sampling Rate getters From d16dc3a5d7fb7594695aede4febce90be2acb233 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Tue, 12 Jul 2022 07:45:07 -0400 Subject: [PATCH 14/32] Move limit up to 20fps. --- OSBindings/Mac/Clock Signal/Machine/CSMachine.h | 2 +- OSBindings/Mac/Clock Signal/Machine/CSMachine.mm | 7 ++----- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/OSBindings/Mac/Clock Signal/Machine/CSMachine.h b/OSBindings/Mac/Clock Signal/Machine/CSMachine.h index 745bc9eb3..ae801b5f1 100644 --- a/OSBindings/Mac/Clock Signal/Machine/CSMachine.h +++ b/OSBindings/Mac/Clock Signal/Machine/CSMachine.h @@ -73,7 +73,7 @@ typedef NS_ENUM(NSInteger, CSMachineKeyboardInputMode) { - (void)setMouseButton:(int)button isPressed:(BOOL)isPressed; - (void)addMouseMotionX:(CGFloat)deltaX y:(CGFloat)deltaY; -@property (atomic, strong, nullable) CSAudioQueue *audioQueue; +@property (nonatomic, strong, nullable) CSAudioQueue *audioQueue; @property (nonatomic, readonly, nonnull) CSScanTargetView *view; @property (nonatomic, weak, nullable) id delegate; diff --git a/OSBindings/Mac/Clock Signal/Machine/CSMachine.mm b/OSBindings/Mac/Clock Signal/Machine/CSMachine.mm index b75edb10a..b9b312b79 100644 --- a/OSBindings/Mac/Clock Signal/Machine/CSMachine.mm +++ b/OSBindings/Mac/Clock Signal/Machine/CSMachine.mm @@ -39,9 +39,9 @@ namespace { struct MachineUpdater { void perform(Time::Nanos duration) { - // Top out at 0.1 seconds; this is a safeguard against a negative + // Top out at 1/20th of a second; this is a safeguard against a negative // feedback loop if emulation starts running slowly. - const auto seconds = std::min(Time::seconds(duration), 0.1); + const auto seconds = std::min(Time::seconds(duration), 0.05); machine->run_for(seconds); } @@ -463,9 +463,6 @@ struct ActivityObserver: public Activity::Observer { updater.update([event] { event(); }); -// @synchronized(_inputEvents) { -// [_inputEvents addObject:event]; -// } } - (void)clearAllKeys { From 4e9ae654592efe34138e86f4c03098c4ca87c119 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Tue, 12 Jul 2022 09:56:13 -0400 Subject: [PATCH 15/32] Reintroduce sync matching. --- Machines/TimedMachine.hpp | 4 + .../Mac/Clock Signal/Machine/CSMachine.mm | 137 ++---------------- .../Mac/Clock Signal/Views/CSScanTargetView.h | 5 + .../Mac/Clock Signal/Views/CSScanTargetView.m | 6 +- 4 files changed, 26 insertions(+), 126 deletions(-) diff --git a/Machines/TimedMachine.hpp b/Machines/TimedMachine.hpp index f97a5ac66..fef5015b1 100644 --- a/Machines/TimedMachine.hpp +++ b/Machines/TimedMachine.hpp @@ -39,6 +39,10 @@ class TimedMachine { fiction: it will apply across the system, including to the CRT. */ virtual void set_speed_multiplier(double multiplier) { + if(speed_multiplier_ == multiplier) { + return; + } + speed_multiplier_ = multiplier; auto audio_producer = dynamic_cast(this); diff --git a/OSBindings/Mac/Clock Signal/Machine/CSMachine.mm b/OSBindings/Mac/Clock Signal/Machine/CSMachine.mm index b9b312b79..a178049db 100644 --- a/OSBindings/Mac/Clock Signal/Machine/CSMachine.mm +++ b/OSBindings/Mac/Clock Signal/Machine/CSMachine.mm @@ -117,18 +117,11 @@ struct ActivityObserver: public Activity::Observer { CSStaticAnalyser *_analyser; std::unique_ptr _machine; MachineTypes::JoystickMachine *_joystickMachine; - Concurrency::AsyncUpdater updater; CSJoystickManager *_joystickManager; NSMutableArray *_leds; - CSHighPrecisionTimer *_timer; - std::atomic_flag _isUpdating; - Time::Nanos _syncTime; - Time::Nanos _timeDiff; - double _refreshPeriod; - BOOL _isSyncLocking; - + Concurrency::AsyncUpdater updater; Time::ScanSynchroniser _scanSynchroniser; NSTimer *_joystickTimer; @@ -177,7 +170,6 @@ struct ActivityObserver: public Activity::Observer { _joystickMachine = _machine->joystick_machine(); [self updateJoystickTimer]; - _isUpdating.clear(); } return self; } @@ -687,135 +679,30 @@ struct ActivityObserver: public Activity::Observer { outputs |= MachineTypes::TimedMachine::Output::Audio; } updater.performer.machine->flush_output(outputs); + + const auto scanStatus = self->_machine->scan_producer()->get_scan_status(); + const bool canSynchronise = self->_scanSynchroniser.can_synchronise(scanStatus, self.view.refreshPeriod); + if(canSynchronise) { + const double multiplier = self->_scanSynchroniser.next_speed_multiplier(self->_machine->scan_producer()->get_scan_status()); + self->_machine->timed_machine()->set_speed_multiplier(multiplier); + } else { + self->_machine->timed_machine()->set_speed_multiplier(1.0); + } + [self.view updateBacking]; dispatch_async(dispatch_get_main_queue(), ^{ [self.view draw]; }); }); - - // TODO: restory sync locking; see below. - - // First order of business: grab a timestamp. -/* const auto timeNow = Time::nanos_now(); - - BOOL isSyncLocking; - @synchronized(self) { - // Store a means to map from CVTimeStamp.hostTime to Time::Nanos; - // there is an extremely dodgy assumption here that the former is in ns. - // If you can find a well-defined way to get the CVTimeStamp.hostTime units, - // whether at runtime or via preprocessor define, I'd love to know about it. - if(!_timeDiff) { - _timeDiff = int64_t(timeNow) - int64_t(now->hostTime); - } - - // Store the next end-of-frame time. TODO: and start of next and implied visible duration, if raster racing? - _syncTime = int64_t(now->hostTime) + _timeDiff; - - // Set the current refresh period. - _refreshPeriod = double(now->videoRefreshPeriod) / double(now->videoTimeScale); - - // Determine where responsibility lies for drawing. - isSyncLocking = _isSyncLocking; - } - - // Draw the current output. (TODO: do this within the timer if either raster racing or, at least, sync matching). - if(!isSyncLocking) { - [self.view draw]; - }*/ } -#define TICKS 120 - - (void)start { -// _timer = [[CSHighPrecisionTimer alloc] initWithTask:^{ -// updater.update([] {}); -// } interval:uint64_t(1000000000) / uint64_t(TICKS)]; - -// updater.performer.machine = _machine->timed_machine(); -/* __block auto lastTime = Time::nanos_now(); - - _timer = [[CSHighPrecisionTimer alloc] initWithTask:^{ - // Grab the time now and, therefore, the amount of time since the timer last fired - // (subject to a cap to avoid potential perpetual regression). - const auto timeNow = Time::nanos_now(); - lastTime = std::max(timeNow - Time::Nanos(10'000'000'000 / TICKS), lastTime); - const auto duration = timeNow - lastTime; - - BOOL splitAndSync = NO; - @synchronized(self) { - // Post on input events. - @synchronized(self->_inputEvents) { - for(dispatch_block_t action: self->_inputEvents) { - action(); - } - [self->_inputEvents removeAllObjects]; - } - - // If this tick includes vsync then inspect the machine. - // - // _syncTime = 0 is used here as a sentinel to mark that a sync time is known; - // this with the >= test ensures that no syncs are missed even if some sort of - // performance problem is afoot (e.g. I'm debugging). - if(self->_syncTime && timeNow >= self->_syncTime) { - splitAndSync = self->_isSyncLocking = self->_scanSynchroniser.can_synchronise(self->_machine->scan_producer()->get_scan_status(), self->_refreshPeriod); - - // If the time window is being split, run up to the split, then check out machine speed, possibly - // adjusting multiplier, then run after the split. Include a sanity check against an out-of-bounds - // _syncTime; that can happen when debugging (possibly inter alia?). - if(splitAndSync) { - if(self->_syncTime >= lastTime) { - self->_machine->timed_machine()->run_for((double)(self->_syncTime - lastTime) / 1e9); - self->_machine->timed_machine()->set_speed_multiplier( - self->_scanSynchroniser.next_speed_multiplier(self->_machine->scan_producer()->get_scan_status()) - ); - self->_machine->timed_machine()->run_for((double)(timeNow - self->_syncTime) / 1e9); - } else { - self->_machine->timed_machine()->run_for((double)(timeNow - lastTime) / 1e9); - } - } - - self->_syncTime = 0; - } - - // If the time window is being split, run up to the split, then check out machine speed, possibly - // adjusting multiplier, then run after the split. - if(!splitAndSync) { - self->_machine->timed_machine()->run_for((double)duration / 1e9); - } - } - - // If this was not a split-and-sync then dispatch the update request asynchronously, unless - // there is an earlier one not yet finished, in which case don't worry about it for now. - // - // If it was a split-and-sync then spin until it is safe to dispatch, and dispatch with - // a concluding draw. Implicit assumption here: whatever is left to be done in the final window - // can be done within the retrace period. - auto wasUpdating = self->_isUpdating.test_and_set(); - if(wasUpdating && splitAndSync) { - while(self->_isUpdating.test_and_set()); - wasUpdating = false; - } - if(!wasUpdating) { - dispatch_async(dispatch_get_global_queue(QOS_CLASS_USER_INTERACTIVE, 0), ^{ - [self.view updateBacking]; - if(splitAndSync) { - [self.view draw]; - } - self->_isUpdating.clear(); - }); - } - - lastTime = timeNow; - } interval:uint64_t(1000000000) / uint64_t(TICKS)];*/ + // A no-op; retained in case of future changes to the manner of scheduling. } -#undef TICKS - - (void)stop { updater.stop(); -// [_timer invalidate]; -// _timer = nil; } + (BOOL)attemptInstallROM:(NSURL *)url { diff --git a/OSBindings/Mac/Clock Signal/Views/CSScanTargetView.h b/OSBindings/Mac/Clock Signal/Views/CSScanTargetView.h index 5373e2be2..e53d2318e 100644 --- a/OSBindings/Mac/Clock Signal/Views/CSScanTargetView.h +++ b/OSBindings/Mac/Clock Signal/Views/CSScanTargetView.h @@ -170,4 +170,9 @@ */ - (void)willChangeScanTargetOwner; +/*! + @returns The period of a frame. +*/ +@property(nonatomic, readonly) double refreshPeriod; + @end diff --git a/OSBindings/Mac/Clock Signal/Views/CSScanTargetView.m b/OSBindings/Mac/Clock Signal/Views/CSScanTargetView.m index 3b7af0af5..4d54dceb8 100644 --- a/OSBindings/Mac/Clock Signal/Views/CSScanTargetView.m +++ b/OSBindings/Mac/Clock Signal/Views/CSScanTargetView.m @@ -103,8 +103,12 @@ static CVReturn DisplayLinkCallback(__unused CVDisplayLinkRef displayLink, const [self stopDisplayLink]; } +- (double)refreshPeriod { + return CVDisplayLinkGetActualOutputVideoRefreshPeriod(_displayLink); +} + - (void)stopDisplayLink { - const double duration = CVDisplayLinkGetActualOutputVideoRefreshPeriod(_displayLink); + const double duration = [self refreshPeriod]; CVDisplayLinkStop(_displayLink); // This is a workaround; CVDisplayLinkStop does not wait for any existing call to the From 4ddbf095f3e654b40cdae63168ce756cc87ce095 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Tue, 12 Jul 2022 10:49:53 -0400 Subject: [PATCH 16/32] Fully banish `flush` from the processors. --- OSBindings/Mac/Clock SignalTests/68000OldVsNew.mm | 2 -- Processors/6502/Implementation/6502Implementation.hpp | 1 - Processors/6502Esque/6502Esque.hpp | 6 ------ Processors/65816/Implementation/65816Implementation.hpp | 1 - Processors/68000/68000.hpp | 2 -- Processors/68000/Implementation/68000Implementation.hpp | 1 - Processors/68000Mk2/68000Mk2.hpp | 2 -- Processors/Z80/Implementation/Z80Implementation.hpp | 2 -- Processors/Z80/Z80.hpp | 6 ------ 9 files changed, 23 deletions(-) diff --git a/OSBindings/Mac/Clock SignalTests/68000OldVsNew.mm b/OSBindings/Mac/Clock SignalTests/68000OldVsNew.mm index 3b1efcdef..8ba3bd9ef 100644 --- a/OSBindings/Mac/Clock SignalTests/68000OldVsNew.mm +++ b/OSBindings/Mac/Clock SignalTests/68000OldVsNew.mm @@ -176,8 +176,6 @@ struct BusHandler { return HalfCycles(0); } - void flush() {} - int transaction_delay; int instructions; diff --git a/Processors/6502/Implementation/6502Implementation.hpp b/Processors/6502/Implementation/6502Implementation.hpp index 6caafebbc..617eefedb 100644 --- a/Processors/6502/Implementation/6502Implementation.hpp +++ b/Processors/6502/Implementation/6502Implementation.hpp @@ -650,7 +650,6 @@ template void Proces } cycles_left_to_run_ = number_of_cycles; - bus_handler_.flush(); } template void Processor::set_ready_line(bool active) { diff --git a/Processors/6502Esque/6502Esque.hpp b/Processors/6502Esque/6502Esque.hpp index 84edc52d2..89227804b 100644 --- a/Processors/6502Esque/6502Esque.hpp +++ b/Processors/6502Esque/6502Esque.hpp @@ -141,12 +141,6 @@ template class BusHandler { Cycles perform_bus_operation([[maybe_unused]] BusOperation operation, [[maybe_unused]] addr_t address, [[maybe_unused]] uint8_t *value) { return Cycles(1); } - - /*! - Announces completion of all the cycles supplied to a .run_for request on the 6502. Intended to allow - bus handlers to perform any deferred output work. - */ - void flush() {} }; } diff --git a/Processors/65816/Implementation/65816Implementation.hpp b/Processors/65816/Implementation/65816Implementation.hpp index f28fb2552..b0ced2550 100644 --- a/Processors/65816/Implementation/65816Implementation.hpp +++ b/Processors/65816/Implementation/65816Implementation.hpp @@ -1022,7 +1022,6 @@ template void Processor void Proces #undef destination #undef destination_address - bus_handler_.flush(); e_clock_phase_ = (e_clock_phase_ + cycles_run_for) % 20; half_cycles_left_to_run_ = remaining_duration - cycles_run_for; } diff --git a/Processors/68000Mk2/68000Mk2.hpp b/Processors/68000Mk2/68000Mk2.hpp index 9382e8a48..00269f06d 100644 --- a/Processors/68000Mk2/68000Mk2.hpp +++ b/Processors/68000Mk2/68000Mk2.hpp @@ -347,8 +347,6 @@ class BusHandler { return HalfCycles(0); } - void flush() {} - /*! Provides information about the path of execution if enabled via the template. */ diff --git a/Processors/Z80/Implementation/Z80Implementation.hpp b/Processors/Z80/Implementation/Z80Implementation.hpp index 56cf63fd9..0d8df3a89 100644 --- a/Processors/Z80/Implementation/Z80Implementation.hpp +++ b/Processors/Z80/Implementation/Z80Implementation.hpp @@ -48,7 +48,6 @@ template < class T, static PartialMachineCycle bus_acknowledge_cycle = {PartialMachineCycle::BusAcknowledge, HalfCycles(2), nullptr, nullptr, false}; number_of_cycles_ -= bus_handler_.perform_machine_cycle(bus_acknowledge_cycle) + HalfCycles(1); if(!number_of_cycles_) { - bus_handler_.flush(); return; } } @@ -70,7 +69,6 @@ template < class T, case MicroOp::BusOperation: if(number_of_cycles_ < operation->machine_cycle.length) { scheduled_program_counter_--; - bus_handler_.flush(); return; } if(uses_wait_line && operation->machine_cycle.was_requested) { diff --git a/Processors/Z80/Z80.hpp b/Processors/Z80/Z80.hpp index d5e2e91da..921c9d5dc 100644 --- a/Processors/Z80/Z80.hpp +++ b/Processors/Z80/Z80.hpp @@ -399,12 +399,6 @@ class BusHandler { HalfCycles perform_machine_cycle([[maybe_unused]] const PartialMachineCycle &cycle) { return HalfCycles(0); } - - /*! - Announces completion of all the cycles supplied to a .run_for request on the Z80. Intended to allow - bus handlers to perform any deferred output work. - */ - void flush() {} }; #include "Implementation/Z80Storage.hpp" From 59da143e6a71d0a96648868cccea2254c04ef0ff Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Tue, 12 Jul 2022 10:57:22 -0400 Subject: [PATCH 17/32] Add overt flushes to the SDL target. --- Machines/TimedMachine.hpp | 2 ++ .../xcshareddata/xcschemes/Clock Signal Kiosk.xcscheme | 8 ++++---- OSBindings/SDL/main.cpp | 3 +++ 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/Machines/TimedMachine.hpp b/Machines/TimedMachine.hpp index fef5015b1..6c1580342 100644 --- a/Machines/TimedMachine.hpp +++ b/Machines/TimedMachine.hpp @@ -68,6 +68,8 @@ class TimedMachine { struct Output { static constexpr int Video = 1 << 0; static constexpr int Audio = 1 << 1; + + static constexpr int All = Video | Audio; }; /// Ensures all locally-buffered output is posted onward for the types of output indicated /// by the bitfield argument, which is comprised of flags from the namespace @c Output. diff --git a/OSBindings/Mac/Clock Signal.xcodeproj/xcshareddata/xcschemes/Clock Signal Kiosk.xcscheme b/OSBindings/Mac/Clock Signal.xcodeproj/xcshareddata/xcschemes/Clock Signal Kiosk.xcscheme index 04eb4197f..575580d92 100644 --- a/OSBindings/Mac/Clock Signal.xcodeproj/xcshareddata/xcschemes/Clock Signal Kiosk.xcscheme +++ b/OSBindings/Mac/Clock Signal.xcodeproj/xcshareddata/xcschemes/Clock Signal Kiosk.xcscheme @@ -82,19 +82,19 @@ + isEnabled = "NO"> + isEnabled = "NO"> + isEnabled = "NO"> + isEnabled = "YES"> run_for(double(vsync_time - last_time_) / 1e9); + timed_machine->flush_output(MachineTypes::TimedMachine::Output::All); timed_machine->set_speed_multiplier( scan_synchroniser_.next_speed_multiplier(scan_producer->get_scan_status()) ); @@ -163,9 +164,11 @@ struct MachineRunner { lock_guard.lock(); timed_machine->run_for(double(time_now - vsync_time) / 1e9); + timed_machine->flush_output(MachineTypes::TimedMachine::Output::All); } else { timed_machine->set_speed_multiplier(scan_synchroniser_.get_base_speed_multiplier()); timed_machine->run_for(double(time_now - last_time_) / 1e9); + timed_machine->flush_output(MachineTypes::TimedMachine::Output::All); } last_time_ = time_now; } From a0e01d4c34bd6094b33d35e8d44a8f1d7dd28def Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Tue, 12 Jul 2022 11:03:58 -0400 Subject: [PATCH 18/32] Add overt flushes to the SDL target. --- OSBindings/Qt/timer.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/OSBindings/Qt/timer.cpp b/OSBindings/Qt/timer.cpp index f8728ca86..bd4af0a2e 100644 --- a/OSBindings/Qt/timer.cpp +++ b/OSBindings/Qt/timer.cpp @@ -31,6 +31,7 @@ void Timer::tick() { std::lock_guard lock_guard(*machineMutex); machine->run_for(double(duration) / 1e9); + machine->flush_output(MachineTypes::TimedMachine::Output::All); } Timer::~Timer() { From df15d60b9e3cdb90f1f6cc3fac2316abc210cc5a Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Tue, 12 Jul 2022 15:03:35 -0400 Subject: [PATCH 19/32] Switch to `AudioQueueNewOutputWithDispatchQueue`, reducing runloop contention. --- .../Mac/Clock Signal/Audio/CSAudioQueue.h | 2 +- .../Mac/Clock Signal/Audio/CSAudioQueue.m | 92 +++++++------------ 2 files changed, 32 insertions(+), 62 deletions(-) diff --git a/OSBindings/Mac/Clock Signal/Audio/CSAudioQueue.h b/OSBindings/Mac/Clock Signal/Audio/CSAudioQueue.h index 8a8721dc9..f91011618 100644 --- a/OSBindings/Mac/Clock Signal/Audio/CSAudioQueue.h +++ b/OSBindings/Mac/Clock Signal/Audio/CSAudioQueue.h @@ -29,7 +29,7 @@ @returns An instance of CSAudioQueue if successful; @c nil otherwise. */ -- (nonnull instancetype)initWithSamplingRate:(Float64)samplingRate isStereo:(BOOL)isStereo NS_DESIGNATED_INITIALIZER; +- (nullable instancetype)initWithSamplingRate:(Float64)samplingRate isStereo:(BOOL)isStereo NS_DESIGNATED_INITIALIZER; - (nonnull instancetype)init __attribute((unavailable)); /*! diff --git a/OSBindings/Mac/Clock Signal/Audio/CSAudioQueue.m b/OSBindings/Mac/Clock Signal/Audio/CSAudioQueue.m index fce2ebc44..05cfae7db 100644 --- a/OSBindings/Mac/Clock Signal/Audio/CSAudioQueue.m +++ b/OSBindings/Mac/Clock Signal/Audio/CSAudioQueue.m @@ -13,23 +13,9 @@ #define AudioQueueBufferMaxLength 8192 #define NumberOfStoredAudioQueueBuffer 16 -static NSLock *CSAudioQueueDeallocLock; - -/*! - Holds a weak reference to a CSAudioQueue. Used to work around an apparent AudioQueue bug. - See -[CSAudioQueue dealloc]. -*/ -@interface CSWeakAudioQueuePointer: NSObject -@property(nonatomic, weak) CSAudioQueue *queue; -@end - -@implementation CSWeakAudioQueuePointer -@end - @implementation CSAudioQueue { AudioQueueRef _audioQueue; - NSLock *_storedBuffersLock; - CSWeakAudioQueuePointer *_weakPointer; + NSLock *_storedBuffersLock, *_deallocLock; atomic_int _enqueuedBuffers; } @@ -56,22 +42,6 @@ static NSLock *CSAudioQueueDeallocLock; return YES; } -static void audioOutputCallback( - void *inUserData, - AudioQueueRef inAQ, - AudioQueueBufferRef inBuffer) { - // Pull the delegate call for audio queue running dry outside of the locked region, to allow non-deadlocking - // lifecycle -dealloc events to result from it. - if([CSAudioQueueDeallocLock tryLock]) { - CSAudioQueue *queue = ((__bridge CSWeakAudioQueuePointer *)inUserData).queue; - BOOL isRunningDry = NO; - isRunningDry = [queue audioQueue:inAQ didCallbackWithBuffer:inBuffer]; - id delegate = queue.delegate; - [CSAudioQueueDeallocLock unlock]; - if(isRunningDry) [delegate audioQueueIsRunningDry:queue]; - } -} - - (BOOL)isRunningDry { return atomic_load_explicit(&_enqueuedBuffers, memory_order_relaxed) < 3; } @@ -82,10 +52,8 @@ static void audioOutputCallback( self = [super init]; if(self) { - if(!CSAudioQueueDeallocLock) { - CSAudioQueueDeallocLock = [[NSLock alloc] init]; - } _storedBuffersLock = [[NSLock alloc] init]; + _deallocLock = [[NSLock alloc] init]; _samplingRate = samplingRate; @@ -113,16 +81,31 @@ static void audioOutputCallback( outputDescription.mReserved = 0; // create an audio output queue along those lines; see -dealloc re: the CSWeakAudioQueuePointer - _weakPointer = [[CSWeakAudioQueuePointer alloc] init]; - _weakPointer.queue = self; - if(!AudioQueueNewOutput( + __weak CSAudioQueue *weakSelf = self; + if(AudioQueueNewOutputWithDispatchQueue( + &_audioQueue, &outputDescription, - audioOutputCallback, - (__bridge void *)(_weakPointer), - NULL, - kCFRunLoopCommonModes, 0, - &_audioQueue)) { + dispatch_get_global_queue(QOS_CLASS_USER_INTERACTIVE, 0), + ^(AudioQueueRef inAQ, AudioQueueBufferRef inBuffer) { + CSAudioQueue *queue = weakSelf; + if(!queue) { + return; + } + + if([queue->_deallocLock tryLock]) { + BOOL isRunningDry = NO; + isRunningDry = [queue audioQueue:inAQ didCallbackWithBuffer:inBuffer]; + + id delegate = queue.delegate; + [queue->_deallocLock unlock]; + + if(isRunningDry) [delegate audioQueueIsRunningDry:queue]; + } + } + ) + ) { + return nil; } } @@ -130,30 +113,17 @@ static void audioOutputCallback( } - (void)dealloc { - [CSAudioQueueDeallocLock lock]; + [_deallocLock lock]; if(_audioQueue) { AudioQueueDispose(_audioQueue, true); _audioQueue = NULL; } - [CSAudioQueueDeallocLock unlock]; - // Yuck. Horrid hack happening here. At least under macOS v10.12, I am frequently seeing calls to - // my registered audio callback (audioOutputCallback in this case) that occur **after** the call - // to AudioQueueDispose above, even though the second parameter there asks for a synchronous shutdown. - // So this appears to be a bug on Apple's side. - // - // Since the audio callback receives a void * pointer that identifies the class it should branch into, - // it's therefore unsafe to pass 'self'. Instead I pass a CSWeakAudioQueuePointer which points to the actual - // queue. The lifetime of that class is the lifetime of this instance plus 1 second, as effected by the - // artificial dispatch_after below; it serves only to keep pointerSaviour alive for an extra second. - // - // Why a second? That's definitely quite a lot longer than any amount of audio that may be queued. So - // probably safe. As and where Apple's audio queue works properly, CSAudioQueueDeallocLock should provide - // absolute safety; elsewhere the CSWeakAudioQueuePointer provides probabilistic. - CSWeakAudioQueuePointer *pointerSaviour = _weakPointer; - dispatch_after(dispatch_time(DISPATCH_TIME_NOW, (int64_t)(1 * NSEC_PER_SEC)), dispatch_get_main_queue(), ^{ - [pointerSaviour hash]; - }); + // nil out the dealloc lock before entering the critical section such + // that it becomes impossible for anyone else to acquire. + NSLock *deallocLock = _deallocLock; + _deallocLock = nil; + [deallocLock unlock]; } #pragma mark - Audio enqueuer From 5b69324ee9bbb6fc8f4f647a119743e93057f586 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Tue, 12 Jul 2022 15:58:16 -0400 Subject: [PATCH 20/32] Tidy up comments. --- OSBindings/Mac/Clock Signal/Audio/CSAudioQueue.m | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/OSBindings/Mac/Clock Signal/Audio/CSAudioQueue.m b/OSBindings/Mac/Clock Signal/Audio/CSAudioQueue.m index 05cfae7db..3b595968b 100644 --- a/OSBindings/Mac/Clock Signal/Audio/CSAudioQueue.m +++ b/OSBindings/Mac/Clock Signal/Audio/CSAudioQueue.m @@ -11,7 +11,6 @@ #include #define AudioQueueBufferMaxLength 8192 -#define NumberOfStoredAudioQueueBuffer 16 @implementation CSAudioQueue { AudioQueueRef _audioQueue; @@ -57,22 +56,19 @@ _samplingRate = samplingRate; - // determine preferred buffer sizes + // Determine preferred buffer size as being the first power of two less than _preferredBufferSize = AudioQueueBufferMaxLength; while((Float64)_preferredBufferSize*100.0 > samplingRate) _preferredBufferSize >>= 1; - /* - Describe a mono 16bit stream of the requested sampling rate - */ + // Describe a 16bit stream of the requested sampling rate. AudioStreamBasicDescription outputDescription; outputDescription.mSampleRate = samplingRate; + outputDescription.mChannelsPerFrame = isStereo ? 2 : 1; outputDescription.mFormatID = kAudioFormatLinearPCM; outputDescription.mFormatFlags = kLinearPCMFormatFlagIsSignedInteger; - - outputDescription.mChannelsPerFrame = isStereo ? 2 : 1; outputDescription.mFramesPerPacket = 1; outputDescription.mBytesPerFrame = 2 * outputDescription.mChannelsPerFrame; outputDescription.mBytesPerPacket = outputDescription.mBytesPerFrame * outputDescription.mFramesPerPacket; @@ -80,7 +76,7 @@ outputDescription.mReserved = 0; - // create an audio output queue along those lines; see -dealloc re: the CSWeakAudioQueuePointer + // Create an audio output queue along those lines. __weak CSAudioQueue *weakSelf = self; if(AudioQueueNewOutputWithDispatchQueue( &_audioQueue, From 4b9d92929ad735294d15e9aec81e8a4c8ab44eaa Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Tue, 12 Jul 2022 16:02:30 -0400 Subject: [PATCH 21/32] Tweak logic. --- OSBindings/Mac/Clock Signal/Audio/CSAudioQueue.m | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/OSBindings/Mac/Clock Signal/Audio/CSAudioQueue.m b/OSBindings/Mac/Clock Signal/Audio/CSAudioQueue.m index 3b595968b..e3acc374a 100644 --- a/OSBindings/Mac/Clock Signal/Audio/CSAudioQueue.m +++ b/OSBindings/Mac/Clock Signal/Audio/CSAudioQueue.m @@ -30,7 +30,7 @@ // If that suggests the queue may be exhausted soon, re-enqueue whatever just came back in order to // keep the queue going. AudioQueues seem to stop playing and never restart no matter how much // encouragement if exhausted. - if(buffers == 1) { + if(!buffers) { AudioQueueEnqueueBuffer(theAudioQueue, buffer, 0, NULL); atomic_fetch_add(&_enqueuedBuffers, 1); } else { @@ -145,7 +145,7 @@ // 'Start' the queue. This is documented to be a no-op if the queue is already started, // and it's better to defer starting it until at least some data is available. - if(enqueuedBuffers > 3) { + if(enqueuedBuffers > 2) { AudioQueueStart(_audioQueue, NULL); } } From 0270997acd9f164d0f828c3bc787a446d8e4f3dd Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Tue, 12 Jul 2022 16:03:09 -0400 Subject: [PATCH 22/32] Add insurance against calls before setup. --- Outputs/Speaker/Implementation/LowpassSpeaker.hpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Outputs/Speaker/Implementation/LowpassSpeaker.hpp b/Outputs/Speaker/Implementation/LowpassSpeaker.hpp index 202c7f194..b225d2585 100644 --- a/Outputs/Speaker/Implementation/LowpassSpeaker.hpp +++ b/Outputs/Speaker/Implementation/LowpassSpeaker.hpp @@ -170,6 +170,10 @@ template class LowpassBase: public Speaker } inline void resample_input_buffer(int scale) { + if(output_buffer_.empty()) { + return; + } + if constexpr (is_stereo) { output_buffer_[output_buffer_pointer_ + 0] = filter_->apply(input_buffer_.data(), 2); output_buffer_[output_buffer_pointer_ + 1] = filter_->apply(input_buffer_.data() + 1, 2); From 1c537a877ed3207901cd15f768c901e6a57a769a Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Tue, 12 Jul 2022 16:22:19 -0400 Subject: [PATCH 23/32] Remove unnecessary lock. --- OSBindings/Mac/Clock Signal/Audio/CSAudioQueue.m | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/OSBindings/Mac/Clock Signal/Audio/CSAudioQueue.m b/OSBindings/Mac/Clock Signal/Audio/CSAudioQueue.m index e3acc374a..9d80a4543 100644 --- a/OSBindings/Mac/Clock Signal/Audio/CSAudioQueue.m +++ b/OSBindings/Mac/Clock Signal/Audio/CSAudioQueue.m @@ -14,7 +14,7 @@ @implementation CSAudioQueue { AudioQueueRef _audioQueue; - NSLock *_storedBuffersLock, *_deallocLock; + NSLock *_deallocLock; atomic_int _enqueuedBuffers; } @@ -24,7 +24,6 @@ @returns @c YES if the queue is running dry; @c NO otherwise. */ - (BOOL)audioQueue:(AudioQueueRef)theAudioQueue didCallbackWithBuffer:(AudioQueueBufferRef)buffer { - [_storedBuffersLock lock]; const int buffers = atomic_fetch_add(&_enqueuedBuffers, -1); // If that suggests the queue may be exhausted soon, re-enqueue whatever just came back in order to @@ -37,7 +36,6 @@ AudioQueueFreeBuffer(_audioQueue, buffer); } - [_storedBuffersLock unlock]; return YES; } @@ -51,7 +49,6 @@ self = [super init]; if(self) { - _storedBuffersLock = [[NSLock alloc] init]; _deallocLock = [[NSLock alloc] init]; _samplingRate = samplingRate; @@ -127,10 +124,8 @@ - (void)enqueueAudioBuffer:(const int16_t *)buffer numberOfSamples:(size_t)lengthInSamples { size_t bufferBytes = lengthInSamples * sizeof(int16_t); - [_storedBuffersLock lock]; // Don't enqueue more than 4 buffers ahead of now, to ensure not too much latency accrues. if(atomic_load_explicit(&_enqueuedBuffers, memory_order_relaxed) > 4) { - [_storedBuffersLock unlock]; return; } const int enqueuedBuffers = atomic_fetch_add(&_enqueuedBuffers, 1); @@ -141,7 +136,6 @@ newBuffer->mAudioDataByteSize = (UInt32)bufferBytes; AudioQueueEnqueueBuffer(_audioQueue, newBuffer, 0, NULL); - [_storedBuffersLock unlock]; // 'Start' the queue. This is documented to be a no-op if the queue is already started, // and it's better to defer starting it until at least some data is available. From b7ad94c67610d2cb763ba7d1042753de0d49c1e4 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Tue, 12 Jul 2022 21:43:33 -0400 Subject: [PATCH 24/32] Attempt to get a bit more rigorous in diagnosing queue stoppages. --- .../Mac/Clock Signal/Audio/CSAudioQueue.m | 66 ++++++++++--------- 1 file changed, 35 insertions(+), 31 deletions(-) diff --git a/OSBindings/Mac/Clock Signal/Audio/CSAudioQueue.m b/OSBindings/Mac/Clock Signal/Audio/CSAudioQueue.m index 9d80a4543..6c6d9bb83 100644 --- a/OSBindings/Mac/Clock Signal/Audio/CSAudioQueue.m +++ b/OSBindings/Mac/Clock Signal/Audio/CSAudioQueue.m @@ -12,44 +12,36 @@ #define AudioQueueBufferMaxLength 8192 +#define OSSGuard(x) { \ + const OSStatus status = x; \ + assert(!status); \ + (void)status; \ +} + +#define IsDry(x) (x) < 3 + @implementation CSAudioQueue { AudioQueueRef _audioQueue; NSLock *_deallocLock; + NSLock *_queueLock; atomic_int _enqueuedBuffers; } -#pragma mark - AudioQueue callbacks - -/*! - @returns @c YES if the queue is running dry; @c NO otherwise. -*/ -- (BOOL)audioQueue:(AudioQueueRef)theAudioQueue didCallbackWithBuffer:(AudioQueueBufferRef)buffer { - const int buffers = atomic_fetch_add(&_enqueuedBuffers, -1); - - // If that suggests the queue may be exhausted soon, re-enqueue whatever just came back in order to - // keep the queue going. AudioQueues seem to stop playing and never restart no matter how much - // encouragement if exhausted. - if(!buffers) { - AudioQueueEnqueueBuffer(theAudioQueue, buffer, 0, NULL); - atomic_fetch_add(&_enqueuedBuffers, 1); - } else { - AudioQueueFreeBuffer(_audioQueue, buffer); - } - - return YES; -} +#pragma mark - Status - (BOOL)isRunningDry { - return atomic_load_explicit(&_enqueuedBuffers, memory_order_relaxed) < 3; + return IsDry(atomic_load_explicit(&_enqueuedBuffers, memory_order_relaxed)); } -#pragma mark - Standard object lifecycle +#pragma mark - Object lifecycle - (instancetype)initWithSamplingRate:(Float64)samplingRate isStereo:(BOOL)isStereo { self = [super init]; if(self) { _deallocLock = [[NSLock alloc] init]; + _queueLock = [[NSLock alloc] init]; + atomic_store_explicit(&_enqueuedBuffers, 0, memory_order_relaxed); _samplingRate = samplingRate; @@ -87,13 +79,21 @@ } if([queue->_deallocLock tryLock]) { - BOOL isRunningDry = NO; - isRunningDry = [queue audioQueue:inAQ didCallbackWithBuffer:inBuffer]; + [queue->_queueLock lock]; + + OSSGuard(AudioQueueFreeBuffer(inAQ, inBuffer)); + + const int buffers = atomic_fetch_add(&queue->_enqueuedBuffers, -1) - 1; +// if(!buffers) { +// OSSGuard(AudioQueueStop(inAQ, true)); +// } + + [queue->_queueLock unlock]; id delegate = queue.delegate; [queue->_deallocLock unlock]; - if(isRunningDry) [delegate audioQueueIsRunningDry:queue]; + if(IsDry(buffers)) [delegate audioQueueIsRunningDry:queue]; } } ) @@ -108,7 +108,7 @@ - (void)dealloc { [_deallocLock lock]; if(_audioQueue) { - AudioQueueDispose(_audioQueue, true); + OSSGuard(AudioQueueDispose(_audioQueue, true)); _audioQueue = NULL; } @@ -125,23 +125,27 @@ size_t bufferBytes = lengthInSamples * sizeof(int16_t); // Don't enqueue more than 4 buffers ahead of now, to ensure not too much latency accrues. - if(atomic_load_explicit(&_enqueuedBuffers, memory_order_relaxed) > 4) { + if(atomic_load_explicit(&_enqueuedBuffers, memory_order_relaxed) == 4) { return; } - const int enqueuedBuffers = atomic_fetch_add(&_enqueuedBuffers, 1); + const int enqueuedBuffers = atomic_fetch_add(&_enqueuedBuffers, 1) + 1; + + [_queueLock lock]; AudioQueueBufferRef newBuffer; - AudioQueueAllocateBuffer(_audioQueue, (UInt32)bufferBytes * 2, &newBuffer); + OSSGuard(AudioQueueAllocateBuffer(_audioQueue, (UInt32)bufferBytes * 2, &newBuffer)); memcpy(newBuffer->mAudioData, buffer, bufferBytes); newBuffer->mAudioDataByteSize = (UInt32)bufferBytes; - AudioQueueEnqueueBuffer(_audioQueue, newBuffer, 0, NULL); + OSSGuard(AudioQueueEnqueueBuffer(_audioQueue, newBuffer, 0, NULL)); // 'Start' the queue. This is documented to be a no-op if the queue is already started, // and it's better to defer starting it until at least some data is available. if(enqueuedBuffers > 2) { - AudioQueueStart(_audioQueue, NULL); + OSSGuard(AudioQueueStart(_audioQueue, NULL)); } + + [_queueLock unlock]; } #pragma mark - Sampling Rate getters From 10108303e702596fb0879172f4a573db591efc57 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Wed, 13 Jul 2022 15:04:58 -0400 Subject: [PATCH 25/32] Eliminate `AudioQueueStop`, which is very slow, use `AudioQueueStart` only as required. --- OSBindings/Mac/Clock Signal/Audio/CSAudioQueue.m | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/OSBindings/Mac/Clock Signal/Audio/CSAudioQueue.m b/OSBindings/Mac/Clock Signal/Audio/CSAudioQueue.m index 6c6d9bb83..67c4f1d51 100644 --- a/OSBindings/Mac/Clock Signal/Audio/CSAudioQueue.m +++ b/OSBindings/Mac/Clock Signal/Audio/CSAudioQueue.m @@ -80,15 +80,10 @@ if([queue->_deallocLock tryLock]) { [queue->_queueLock lock]; - OSSGuard(AudioQueueFreeBuffer(inAQ, inBuffer)); + [queue->_queueLock unlock]; const int buffers = atomic_fetch_add(&queue->_enqueuedBuffers, -1) - 1; -// if(!buffers) { -// OSSGuard(AudioQueueStop(inAQ, true)); -// } - - [queue->_queueLock unlock]; id delegate = queue.delegate; [queue->_deallocLock unlock]; @@ -139,9 +134,10 @@ OSSGuard(AudioQueueEnqueueBuffer(_audioQueue, newBuffer, 0, NULL)); - // 'Start' the queue. This is documented to be a no-op if the queue is already started, - // and it's better to defer starting it until at least some data is available. - if(enqueuedBuffers > 2) { + // Start the queue if it isn't started yet, and there are now some packets waiting. + UInt32 isRunning = 0, bytesReceived = sizeof(UInt32); + OSSGuard(AudioQueueGetProperty(_audioQueue, kAudioQueueProperty_IsRunning, &isRunning, &bytesReceived)); + if(!isRunning && enqueuedBuffers > 2) { OSSGuard(AudioQueueStart(_audioQueue, NULL)); } From 75f3f1a77fffc6cca13f08c38e48383e83fd0f9e Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Wed, 13 Jul 2022 15:05:34 -0400 Subject: [PATCH 26/32] Avoid the whole thread hop for a zero-length run_for. --- Outputs/Speaker/Implementation/LowpassSpeaker.hpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/Outputs/Speaker/Implementation/LowpassSpeaker.hpp b/Outputs/Speaker/Implementation/LowpassSpeaker.hpp index b225d2585..4a7638a93 100644 --- a/Outputs/Speaker/Implementation/LowpassSpeaker.hpp +++ b/Outputs/Speaker/Implementation/LowpassSpeaker.hpp @@ -368,6 +368,10 @@ template class PullLowpass: public LowpassBase class PullLowpass: public LowpassBase Date: Wed, 13 Jul 2022 15:24:43 -0400 Subject: [PATCH 27/32] Eliminate `AudioQueueBufferMaxLength`. --- OSBindings/Mac/Clock Signal/Audio/CSAudioQueue.m | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/OSBindings/Mac/Clock Signal/Audio/CSAudioQueue.m b/OSBindings/Mac/Clock Signal/Audio/CSAudioQueue.m index 67c4f1d51..d10a3f791 100644 --- a/OSBindings/Mac/Clock Signal/Audio/CSAudioQueue.m +++ b/OSBindings/Mac/Clock Signal/Audio/CSAudioQueue.m @@ -10,8 +10,6 @@ @import AudioToolbox; #include -#define AudioQueueBufferMaxLength 8192 - #define OSSGuard(x) { \ const OSStatus status = x; \ assert(!status); \ @@ -45,9 +43,11 @@ _samplingRate = samplingRate; - // Determine preferred buffer size as being the first power of two less than - _preferredBufferSize = AudioQueueBufferMaxLength; - while((Float64)_preferredBufferSize*100.0 > samplingRate) _preferredBufferSize >>= 1; + // Determine preferred buffer size as being the first power of two + // not less than 1/100th of a second. + _preferredBufferSize = 1; + const NSUInteger oneHundredthOfRate = (NSUInteger)(samplingRate / 100.0); + while(_preferredBufferSize < oneHundredthOfRate) _preferredBufferSize <<= 1; // Describe a 16bit stream of the requested sampling rate. AudioStreamBasicDescription outputDescription; From 6a509c12805fcf00d79b907881877611d87c44fd Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Wed, 13 Jul 2022 18:36:40 -0400 Subject: [PATCH 28/32] Improve comments, marginally reduce `dynamic_cast`ing. --- .../Mac/Clock Signal/Machine/CSMachine.mm | 20 +++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/OSBindings/Mac/Clock Signal/Machine/CSMachine.mm b/OSBindings/Mac/Clock Signal/Machine/CSMachine.mm index a178049db..d38bb7b39 100644 --- a/OSBindings/Mac/Clock Signal/Machine/CSMachine.mm +++ b/OSBindings/Mac/Clock Signal/Machine/CSMachine.mm @@ -667,30 +667,38 @@ struct ActivityObserver: public Activity::Observer { - (void)audioQueueIsRunningDry:(nonnull CSAudioQueue *)audioQueue { updater.update([self] { - updater.performer.machine->flush_output(MachineTypes::TimedMachine::Output::Audio); /* | MachineTypes::TimedMachine::Output::Video); - [self.view updateBacking];*/ + updater.performer.machine->flush_output(MachineTypes::TimedMachine::Output::Audio); }); } - (void)scanTargetViewDisplayLinkDidFire:(CSScanTargetView *)view now:(const CVTimeStamp *)now outputTime:(const CVTimeStamp *)outputTime { updater.update([self] { + // Grab a pointer to the timed machine from somewhere where it has already + // been dynamically cast, to avoid that cost here. + MachineTypes::TimedMachine *const timed_machine = updater.performer.machine; + + // Definitely update video; update audio too if that pipeline is looking a little dry. auto outputs = MachineTypes::TimedMachine::Output::Video; if(_audioQueue.isRunningDry) { outputs |= MachineTypes::TimedMachine::Output::Audio; } - updater.performer.machine->flush_output(outputs); + timed_machine->flush_output(outputs); + // Attempt sync-matching if this machine is a fit. + // + // TODO: either cache scan_producer(), or possibly start caching these things + // inside the DynamicMachine? const auto scanStatus = self->_machine->scan_producer()->get_scan_status(); const bool canSynchronise = self->_scanSynchroniser.can_synchronise(scanStatus, self.view.refreshPeriod); if(canSynchronise) { const double multiplier = self->_scanSynchroniser.next_speed_multiplier(self->_machine->scan_producer()->get_scan_status()); - self->_machine->timed_machine()->set_speed_multiplier(multiplier); + timed_machine->set_speed_multiplier(multiplier); } else { - self->_machine->timed_machine()->set_speed_multiplier(1.0); + timed_machine->set_speed_multiplier(1.0); } + // Ask Metal to rasterise all that just happened and present it. [self.view updateBacking]; - dispatch_async(dispatch_get_main_queue(), ^{ [self.view draw]; }); From 92efad4970af34da59c982f1d5eb8ed806ea3818 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Wed, 13 Jul 2022 21:36:01 -0400 Subject: [PATCH 29/32] Switch to std::vector. --- Concurrency/AsyncTaskQueue.hpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Concurrency/AsyncTaskQueue.hpp b/Concurrency/AsyncTaskQueue.hpp index 225f49580..3a000ef09 100644 --- a/Concurrency/AsyncTaskQueue.hpp +++ b/Concurrency/AsyncTaskQueue.hpp @@ -12,9 +12,9 @@ #include #include #include -#include #include #include +#include #if defined(__APPLE__) && !defined(IGNORE_APPLE) #include @@ -23,7 +23,7 @@ namespace Concurrency { -using TaskList = std::list>; +using TaskList = std::vector>; /*! An async task queue allows a caller to enqueue void(void) functions. Those functions are guaranteed From 79f8cab5e28985b5b653e10b0d92384a94c59d84 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Wed, 13 Jul 2022 21:41:04 -0400 Subject: [PATCH 30/32] Attempt to reduce memory allocations. --- Concurrency/AsyncTaskQueue.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/Concurrency/AsyncTaskQueue.cpp b/Concurrency/AsyncTaskQueue.cpp index 12615430e..ccc44a990 100644 --- a/Concurrency/AsyncTaskQueue.cpp +++ b/Concurrency/AsyncTaskQueue.cpp @@ -89,6 +89,7 @@ DeferringAsyncTaskQueue::~DeferringAsyncTaskQueue() { void DeferringAsyncTaskQueue::defer(std::function function) { if(!deferred_tasks_) { deferred_tasks_ = std::make_unique(); + deferred_tasks_->reserve(16); } deferred_tasks_->push_back(function); } From 4c031bd335b94fbeb5f75293f7f49b81000407f7 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Wed, 13 Jul 2022 22:22:19 -0400 Subject: [PATCH 31/32] Don't use `kAudioQueueProperty_IsRunning` as it seems not to be trustworthy. --- OSBindings/Mac/Clock Signal/Audio/CSAudioQueue.m | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/OSBindings/Mac/Clock Signal/Audio/CSAudioQueue.m b/OSBindings/Mac/Clock Signal/Audio/CSAudioQueue.m index d10a3f791..b66a5e371 100644 --- a/OSBindings/Mac/Clock Signal/Audio/CSAudioQueue.m +++ b/OSBindings/Mac/Clock Signal/Audio/CSAudioQueue.m @@ -16,7 +16,7 @@ (void)status; \ } -#define IsDry(x) (x) < 3 +#define IsDry(x) (x) < 2 @implementation CSAudioQueue { AudioQueueRef _audioQueue; @@ -84,6 +84,9 @@ [queue->_queueLock unlock]; const int buffers = atomic_fetch_add(&queue->_enqueuedBuffers, -1) - 1; + if(!buffers) { + OSSGuard(AudioQueuePause(queue->_audioQueue)); + } id delegate = queue.delegate; [queue->_deallocLock unlock]; @@ -135,9 +138,7 @@ OSSGuard(AudioQueueEnqueueBuffer(_audioQueue, newBuffer, 0, NULL)); // Start the queue if it isn't started yet, and there are now some packets waiting. - UInt32 isRunning = 0, bytesReceived = sizeof(UInt32); - OSSGuard(AudioQueueGetProperty(_audioQueue, kAudioQueueProperty_IsRunning, &isRunning, &bytesReceived)); - if(!isRunning && enqueuedBuffers > 2) { + if(enqueuedBuffers > 1) { OSSGuard(AudioQueueStart(_audioQueue, NULL)); } From 18f01bcd480970beb3471cdc5c33a9d41c0d776b Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Wed, 13 Jul 2022 22:26:59 -0400 Subject: [PATCH 32/32] Switch back to std::list as a kneejerk fix for `AsyncTaskQueue`. --- Concurrency/AsyncTaskQueue.hpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Concurrency/AsyncTaskQueue.hpp b/Concurrency/AsyncTaskQueue.hpp index 3a000ef09..f301a8717 100644 --- a/Concurrency/AsyncTaskQueue.hpp +++ b/Concurrency/AsyncTaskQueue.hpp @@ -12,6 +12,7 @@ #include #include #include +#include #include #include #include @@ -56,7 +57,7 @@ class AsyncTaskQueue { std::atomic_bool should_destruct_; std::condition_variable processing_condition_; std::mutex queue_mutex_; - TaskList pending_tasks_; + std::list> pending_tasks_; std::thread thread_; #endif