From ab51bc443b70165c0e68fad6736e7fd5d54e59bf Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Sat, 15 Jul 2017 22:40:38 -0400 Subject: [PATCH 1/3] Eliminated foolish double indirection on phase history. --- Storage/Disk/DigitalPhaseLockedLoop.cpp | 12 +++++------- Storage/Disk/DigitalPhaseLockedLoop.hpp | 2 +- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/Storage/Disk/DigitalPhaseLockedLoop.cpp b/Storage/Disk/DigitalPhaseLockedLoop.cpp index c22d17053..231b71e4a 100644 --- a/Storage/Disk/DigitalPhaseLockedLoop.cpp +++ b/Storage/Disk/DigitalPhaseLockedLoop.cpp @@ -17,9 +17,8 @@ DigitalPhaseLockedLoop::DigitalPhaseLockedLoop(int clocks_per_bit, int tolerance tolerance_(tolerance), phase_(0), window_length_(clocks_per_bit), - phase_error_pointer_(0) { - phase_error_history_.reset(new std::vector(length_of_history, 0)); -} + phase_error_pointer_(0), + phase_error_history_(length_of_history, 0) {} void DigitalPhaseLockedLoop::run_for_cycles(int number_of_cycles) { phase_ += number_of_cycles; @@ -51,15 +50,14 @@ void DigitalPhaseLockedLoop::post_phase_error(int error) { phase_ -= (error + 1) >> 1; // use the average of the last few errors to affect frequency - std::vector *phase_error_history = phase_error_history_.get(); - size_t phase_error_history_size = phase_error_history->size(); + size_t phase_error_history_size = phase_error_history_.size(); - (*phase_error_history)[phase_error_pointer_] = error; + phase_error_history_[phase_error_pointer_] = error; phase_error_pointer_ = (phase_error_pointer_ + 1)%phase_error_history_size; int total_error = 0; for(size_t c = 0; c < phase_error_history_size; c++) { - total_error += (*phase_error_history)[c]; + total_error += phase_error_history_[c]; } int denominator = (int)(phase_error_history_size * 4); window_length_ += (total_error + (denominator >> 1)) / denominator; diff --git a/Storage/Disk/DigitalPhaseLockedLoop.hpp b/Storage/Disk/DigitalPhaseLockedLoop.hpp index a1e5dd7a5..d4b38153e 100644 --- a/Storage/Disk/DigitalPhaseLockedLoop.hpp +++ b/Storage/Disk/DigitalPhaseLockedLoop.hpp @@ -52,7 +52,7 @@ class DigitalPhaseLockedLoop { Delegate *delegate_; void post_phase_error(int error); - std::unique_ptr> phase_error_history_; + std::vector phase_error_history_; size_t phase_error_pointer_; int phase_; From f931cd582d9936fa1cd29eb35edb2ece98d2c4a1 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Sun, 16 Jul 2017 13:54:07 -0400 Subject: [PATCH 2/3] Switched to use of `std::vector` in those few remaining places where I was still using a `unique_ptr` to a native type and `new`ing for myself. So, some of my earliest bits of code. --- Machines/Commodore/1540/C1540.cpp | 4 ++-- Machines/Commodore/1540/C1540.hpp | 2 +- Machines/Commodore/Vic-20/Vic20.cpp | 10 +++++----- Machines/Commodore/Vic-20/Vic20.hpp | 2 +- Outputs/Speaker.hpp | 21 +++++++++++---------- 5 files changed, 20 insertions(+), 19 deletions(-) diff --git a/Machines/Commodore/1540/C1540.cpp b/Machines/Commodore/1540/C1540.cpp index 5efcd6c24..ca92c51c2 100644 --- a/Machines/Commodore/1540/C1540.cpp +++ b/Machines/Commodore/1540/C1540.cpp @@ -69,8 +69,8 @@ unsigned int Machine::perform_bus_operation(CPU::MOS6502::BusOperation operation return 1; } -void Machine::set_rom(const uint8_t *rom) { - memcpy(rom_, rom, sizeof(rom_)); +void Machine::set_rom(const std::vector &rom) { + memcpy(rom_, rom.data(), std::min(sizeof(rom_), rom.size())); } void Machine::set_disk(std::shared_ptr disk) { diff --git a/Machines/Commodore/1540/C1540.hpp b/Machines/Commodore/1540/C1540.hpp index 8fdfdc69c..5fdcf4294 100644 --- a/Machines/Commodore/1540/C1540.hpp +++ b/Machines/Commodore/1540/C1540.hpp @@ -131,7 +131,7 @@ class Machine: /*! Sets the ROM image to use for this drive; it is assumed that the buffer provided will be at least 16 kb in size. */ - void set_rom(const uint8_t *rom); + void set_rom(const std::vector &rom); /*! Sets the serial bus to which this drive should attach itself. diff --git a/Machines/Commodore/Vic-20/Vic20.cpp b/Machines/Commodore/Vic-20/Vic20.cpp index 871610a42..67513e38a 100644 --- a/Machines/Commodore/Vic-20/Vic20.cpp +++ b/Machines/Commodore/Vic-20/Vic20.cpp @@ -246,8 +246,8 @@ void Machine::set_rom(ROMSlot slot, size_t length, const uint8_t *data) { case Characters: target = character_rom_; max_length = 0x1000; break; case BASIC: target = basic_rom_; break; case Drive: - drive_rom_.reset(new uint8_t[length]); - memcpy(drive_rom_.get(), data, length); + drive_rom_.resize(length); + memcpy(drive_rom_.data(), data, length); install_disk_rom(); return; } @@ -313,10 +313,10 @@ void Machine::tape_did_change_input(Storage::Tape::BinaryTapePlayer *tape) { #pragma mark - Disc void Machine::install_disk_rom() { - if(drive_rom_ && c1540_) { - c1540_->set_rom(drive_rom_.get()); + if(!drive_rom_.empty() && c1540_) { + c1540_->set_rom(drive_rom_); c1540_->run_for_cycles(2000000); - drive_rom_.reset(); + drive_rom_.clear(); } } diff --git a/Machines/Commodore/Vic-20/Vic20.hpp b/Machines/Commodore/Vic-20/Vic20.hpp index 4b3f7cf19..86b4c88ca 100644 --- a/Machines/Commodore/Vic-20/Vic20.hpp +++ b/Machines/Commodore/Vic-20/Vic20.hpp @@ -199,7 +199,7 @@ class Machine: uint8_t user_basic_memory_[0x0400]; uint8_t screen_memory_[0x1000]; uint8_t colour_memory_[0x0400]; - std::unique_ptr drive_rom_; + std::vector drive_rom_; uint8_t *processor_read_memory_map_[64]; uint8_t *processor_write_memory_map_[64]; diff --git a/Outputs/Speaker.hpp b/Outputs/Speaker.hpp index e7350da6e..29434ba0f 100644 --- a/Outputs/Speaker.hpp +++ b/Outputs/Speaker.hpp @@ -15,6 +15,7 @@ #include #include +#include #include "../SignalProcessing/Stepper.hpp" #include "../SignalProcessing/FIRFilter.hpp" @@ -54,7 +55,7 @@ class Speaker { void set_output_rate(float cycles_per_second, int buffer_size) { output_cycles_per_second_ = cycles_per_second; if(buffer_size_ != buffer_size) { - buffer_in_progress_.reset(new int16_t[buffer_size]); + buffer_in_progress_.resize((size_t)buffer_size); buffer_size_ = buffer_size; } set_needs_updated_filter_coefficients(); @@ -104,7 +105,7 @@ class Speaker { } std::shared_ptr>> queued_functions_; - std::unique_ptr buffer_in_progress_; + std::vector buffer_in_progress_; float high_frequency_cut_off_; int buffer_size_; int buffer_in_progress_pointer_; @@ -154,14 +155,14 @@ template class Filter: public Speaker { unsigned int cycles_to_read = (unsigned int)(buffer_size_ - buffer_in_progress_pointer_); if(cycles_to_read > cycles_remaining) cycles_to_read = cycles_remaining; - static_cast(this)->get_samples(cycles_to_read, &buffer_in_progress_.get()[buffer_in_progress_pointer_]); + static_cast(this)->get_samples(cycles_to_read, &buffer_in_progress_[(size_t)buffer_in_progress_pointer_]); buffer_in_progress_pointer_ += cycles_to_read; // announce to delegate if full if(buffer_in_progress_pointer_ == buffer_size_) { buffer_in_progress_pointer_ = 0; if(delegate_) { - delegate_->speaker_did_complete_samples(this, buffer_in_progress_.get(), buffer_size_); + delegate_->speaker_did_complete_samples(this, buffer_in_progress_.data(), buffer_size_); } } @@ -175,19 +176,19 @@ template class Filter: public Speaker { if(input_cycles_per_second_ > output_cycles_per_second_ || (input_cycles_per_second_ == output_cycles_per_second_ && high_frequency_cut_off_ >= 0.0)) { while(cycles_remaining) { unsigned int cycles_to_read = (unsigned int)std::min((int)cycles_remaining, number_of_taps_ - input_buffer_depth_); - static_cast(this)->get_samples(cycles_to_read, &input_buffer_.get()[input_buffer_depth_]); + static_cast(this)->get_samples(cycles_to_read, &input_buffer_[(size_t)input_buffer_depth_]); cycles_remaining -= cycles_to_read; input_buffer_depth_ += cycles_to_read; if(input_buffer_depth_ == number_of_taps_) { - buffer_in_progress_.get()[buffer_in_progress_pointer_] = filter_->apply(input_buffer_.get()); + buffer_in_progress_[(size_t)buffer_in_progress_pointer_] = filter_->apply(input_buffer_.data()); buffer_in_progress_pointer_++; // announce to delegate if full if(buffer_in_progress_pointer_ == buffer_size_) { buffer_in_progress_pointer_ = 0; if(delegate_) { - delegate_->speaker_did_complete_samples(this, buffer_in_progress_.get(), buffer_size_); + delegate_->speaker_did_complete_samples(this, buffer_in_progress_.data(), buffer_size_); } } @@ -196,7 +197,7 @@ template class Filter: public Speaker { // anything. Otherwise skip as required to get to the next sample batch and don't expect to reuse. uint64_t steps = stepper_->step(); if(steps < number_of_taps_) { - int16_t *input_buffer = input_buffer_.get(); + int16_t *input_buffer = input_buffer_.data(); memmove(input_buffer, &input_buffer[steps], sizeof(int16_t) * ((size_t)number_of_taps_ - (size_t)steps)); input_buffer_depth_ -= steps; } else { @@ -218,7 +219,7 @@ template class Filter: public Speaker { std::unique_ptr stepper_; std::unique_ptr filter_; - std::unique_ptr input_buffer_; + std::vector input_buffer_; int input_buffer_depth_; void update_filter_coefficients() { @@ -244,7 +245,7 @@ template class Filter: public Speaker { } filter_.reset(new SignalProcessing::FIRFilter((unsigned int)number_of_taps_, (float)input_cycles_per_second_, 0.0, high_pass_frequency, SignalProcessing::FIRFilter::DefaultAttenuation)); - input_buffer_.reset(new int16_t[number_of_taps_]); + input_buffer_.resize((size_t)number_of_taps_); input_buffer_depth_ = 0; } }; From 279f4760d7eb87d27366cc6f5e216ff41c605e50 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Sun, 16 Jul 2017 15:01:39 -0400 Subject: [PATCH 3/3] Eliminated `buffer_size_` as something explicitly stored, and reduced size of delegate call out. --- .../Mac/Clock Signal/Machine/CSMachine.mm | 4 ++-- Outputs/Speaker.hpp | 18 +++++++----------- 2 files changed, 9 insertions(+), 13 deletions(-) diff --git a/OSBindings/Mac/Clock Signal/Machine/CSMachine.mm b/OSBindings/Mac/Clock Signal/Machine/CSMachine.mm index 950427945..16861bdaa 100644 --- a/OSBindings/Mac/Clock Signal/Machine/CSMachine.mm +++ b/OSBindings/Mac/Clock Signal/Machine/CSMachine.mm @@ -21,8 +21,8 @@ struct SpeakerDelegate: public Outputs::Speaker::Delegate { __weak CSMachine *machine; - void speaker_did_complete_samples(Outputs::Speaker *speaker, const int16_t *buffer, int buffer_size) { - [machine speaker:speaker didCompleteSamples:buffer length:buffer_size]; + void speaker_did_complete_samples(Outputs::Speaker *speaker, const std::vector &buffer) { + [machine speaker:speaker didCompleteSamples:buffer.data() length:(int)buffer.size()]; } }; diff --git a/Outputs/Speaker.hpp b/Outputs/Speaker.hpp index 29434ba0f..4fd2b0fd8 100644 --- a/Outputs/Speaker.hpp +++ b/Outputs/Speaker.hpp @@ -35,7 +35,7 @@ class Speaker { public: class Delegate { public: - virtual void speaker_did_complete_samples(Speaker *speaker, const int16_t *buffer, int buffer_size) = 0; + virtual void speaker_did_complete_samples(Speaker *speaker, const std::vector &buffer) = 0; }; float get_ideal_clock_rate_in_range(float minimum, float maximum) { @@ -54,10 +54,7 @@ class Speaker { void set_output_rate(float cycles_per_second, int buffer_size) { output_cycles_per_second_ = cycles_per_second; - if(buffer_size_ != buffer_size) { - buffer_in_progress_.resize((size_t)buffer_size); - buffer_size_ = buffer_size; - } + buffer_in_progress_.resize((size_t)buffer_size); set_needs_updated_filter_coefficients(); } @@ -107,7 +104,6 @@ class Speaker { std::vector buffer_in_progress_; float high_frequency_cut_off_; - int buffer_size_; int buffer_in_progress_pointer_; int number_of_taps_, requested_number_of_taps_; bool coefficients_are_dirty_; @@ -152,17 +148,17 @@ template class Filter: public Speaker { // if input and output rates exactly match, just accumulate results and pass on if(input_cycles_per_second_ == output_cycles_per_second_ && high_frequency_cut_off_ < 0.0) { while(cycles_remaining) { - unsigned int cycles_to_read = (unsigned int)(buffer_size_ - buffer_in_progress_pointer_); + unsigned int cycles_to_read = (unsigned int)(buffer_in_progress_.size() - (size_t)buffer_in_progress_pointer_); if(cycles_to_read > cycles_remaining) cycles_to_read = cycles_remaining; static_cast(this)->get_samples(cycles_to_read, &buffer_in_progress_[(size_t)buffer_in_progress_pointer_]); buffer_in_progress_pointer_ += cycles_to_read; // announce to delegate if full - if(buffer_in_progress_pointer_ == buffer_size_) { + if(buffer_in_progress_pointer_ == buffer_in_progress_.size()) { buffer_in_progress_pointer_ = 0; if(delegate_) { - delegate_->speaker_did_complete_samples(this, buffer_in_progress_.data(), buffer_size_); + delegate_->speaker_did_complete_samples(this, buffer_in_progress_); } } @@ -185,10 +181,10 @@ template class Filter: public Speaker { buffer_in_progress_pointer_++; // announce to delegate if full - if(buffer_in_progress_pointer_ == buffer_size_) { + if(buffer_in_progress_pointer_ == buffer_in_progress_.size()) { buffer_in_progress_pointer_ = 0; if(delegate_) { - delegate_->speaker_did_complete_samples(this, buffer_in_progress_.data(), buffer_size_); + delegate_->speaker_did_complete_samples(this, buffer_in_progress_); } }