From b82eff51dd855828a5920a47af768c15e7d7966d Mon Sep 17 00:00:00 2001 From: Jared Boone Date: Tue, 10 May 2016 14:12:37 -0700 Subject: [PATCH] Refactor of capture buffer management between cores. Instead of copying data into and out of FIFO, passing buffer pointers between cores that are sized to match preferred/ideal SD card write size. --- firmware/application/analog_audio_app.hpp | 2 +- firmware/application/capture_app.hpp | 2 +- firmware/application/capture_thread.cpp | 48 +++++++------- firmware/application/capture_thread.hpp | 4 +- firmware/application/ui_record_view.cpp | 10 +-- firmware/application/ui_record_view.hpp | 8 +-- firmware/baseband/Makefile | 1 + firmware/baseband/stream_input.cpp | 76 +++++++++++++++++++++++ firmware/baseband/stream_input.hpp | 43 ++++--------- firmware/common/message.hpp | 59 +++++++++++++++--- 10 files changed, 180 insertions(+), 73 deletions(-) create mode 100644 firmware/baseband/stream_input.cpp diff --git a/firmware/application/analog_audio_app.hpp b/firmware/application/analog_audio_app.hpp index ebef06e3e..0da7a7ad7 100644 --- a/firmware/application/analog_audio_app.hpp +++ b/firmware/application/analog_audio_app.hpp @@ -143,7 +143,7 @@ private: RecordView record_view { { 0 * 8, 2 * 16, 30 * 8, 1 * 16 }, - "AUD_????", RecordView::FileType::WAV, 12, 2, + "AUD_????", RecordView::FileType::WAV, 4096, 4 }; spectrum::WaterfallWidget waterfall; diff --git a/firmware/application/capture_app.hpp b/firmware/application/capture_app.hpp index bc896f9e5..6ece4b89a 100644 --- a/firmware/application/capture_app.hpp +++ b/firmware/application/capture_app.hpp @@ -78,7 +78,7 @@ private: RecordView record_view { { 0 * 8, 2 * 16, 30 * 8, 1 * 16 }, - "BBD_????", RecordView::FileType::RawS16, 14, 1, + "BBD_????", RecordView::FileType::RawS16, 16384, 3 }; spectrum::WaterfallWidget waterfall; diff --git a/firmware/application/capture_thread.cpp b/firmware/application/capture_thread.cpp index 6bccea57b..7d1eb5572 100644 --- a/firmware/application/capture_thread.cpp +++ b/firmware/application/capture_thread.cpp @@ -31,20 +31,29 @@ public: ~StreamOutput(); size_t available() { - return fifo->len(); + return fifo_buffers_full->len(); } - size_t read(void* const data, const size_t length) { - return fifo->out(reinterpret_cast(data), length); + StreamBuffer* get_buffer() { + StreamBuffer* p { nullptr }; + fifo_buffers_full->out(p); + return p; } - static FIFO* fifo; + bool release_buffer(StreamBuffer* const p) { + p->empty(); + return fifo_buffers_empty->in(p); + } + + static FIFO* fifo_buffers_empty; + static FIFO* fifo_buffers_full; private: CaptureConfig* const config; }; -FIFO* StreamOutput::fifo = nullptr; +FIFO* StreamOutput::fifo_buffers_empty = nullptr; +FIFO* StreamOutput::fifo_buffers_full = nullptr; StreamOutput::StreamOutput( CaptureConfig* const config @@ -53,11 +62,13 @@ StreamOutput::StreamOutput( shared_memory.baseband_queue.push_and_wait( CaptureConfigMessage { config } ); - fifo = config->fifo; + fifo_buffers_empty = config->fifo_buffers_empty; + fifo_buffers_full = config->fifo_buffers_full; } StreamOutput::~StreamOutput() { - fifo = nullptr; + fifo_buffers_full = nullptr; + fifo_buffers_empty = nullptr; shared_memory.baseband_queue.push_and_wait( CaptureConfigMessage { nullptr } ); @@ -69,9 +80,9 @@ Thread* CaptureThread::thread = nullptr; CaptureThread::CaptureThread( std::unique_ptr writer, - size_t write_size_log2, - size_t buffer_count_log2 -) : config { write_size_log2, buffer_count_log2 }, + size_t write_size, + size_t buffer_count +) : config { write_size, buffer_count }, writer { std::move(writer) } { // Need significant stack for FATFS @@ -90,29 +101,22 @@ CaptureThread::~CaptureThread() { void CaptureThread::check_fifo_isr() { // TODO: Prevent over-signalling by transmitting a set of // flags from the baseband core. - const auto fifo = StreamOutput::fifo; + const auto fifo = StreamOutput::fifo_buffers_full; if( fifo ) { chEvtSignalI(thread, EVT_MASK_CAPTURE_THREAD); } } msg_t CaptureThread::run() { - const size_t write_size = 1U << config.write_size_log2; - const auto write_buffer = std::make_unique(write_size); - if( !write_buffer ) { - return false; - } - StreamOutput stream { &config }; while( !chThdShouldTerminate() ) { - if( stream.available() >= write_size ) { - if( stream.read(write_buffer.get(), write_size) != write_size ) { - return false; - } - if( !writer->write(write_buffer.get(), write_size) ) { + if( stream.available() ) { + auto buffer = stream.get_buffer(); + if( !writer->write(buffer->data(), buffer->size()) ) { return false; } + stream.release_buffer(buffer); } else { chEvtWaitAny(EVT_MASK_CAPTURE_THREAD); } diff --git a/firmware/application/capture_thread.hpp b/firmware/application/capture_thread.hpp index c4bdbb1c5..5ebd49447 100644 --- a/firmware/application/capture_thread.hpp +++ b/firmware/application/capture_thread.hpp @@ -40,8 +40,8 @@ class CaptureThread { public: CaptureThread( std::unique_ptr writer, - size_t write_size_log2, - size_t buffer_count_log2 + size_t write_size, + size_t buffer_count ); ~CaptureThread(); diff --git a/firmware/application/ui_record_view.cpp b/firmware/application/ui_record_view.cpp index 3f905f063..8db26f2b0 100644 --- a/firmware/application/ui_record_view.cpp +++ b/firmware/application/ui_record_view.cpp @@ -139,13 +139,13 @@ RecordView::RecordView( const Rect parent_rect, std::string filename_stem_pattern, const FileType file_type, - const size_t buffer_size_k, - const size_t buffer_count_k + const size_t write_size, + const size_t buffer_count ) : View { parent_rect }, filename_stem_pattern { filename_stem_pattern }, file_type { file_type }, - buffer_size_k { buffer_size_k }, - buffer_count_k { buffer_count_k } + write_size { write_size }, + buffer_count { buffer_count } { add_children({ { &button_record, @@ -222,7 +222,7 @@ void RecordView::start() { button_record.set_bitmap(&bitmap_stop); capture_thread = std::make_unique( std::move(writer), - buffer_size_k, buffer_count_k + write_size, buffer_count ); } } diff --git a/firmware/application/ui_record_view.hpp b/firmware/application/ui_record_view.hpp index 0c8c24bb6..88d77fd29 100644 --- a/firmware/application/ui_record_view.hpp +++ b/firmware/application/ui_record_view.hpp @@ -46,8 +46,8 @@ public: const Rect parent_rect, std::string filename_stem_pattern, FileType file_type, - const size_t buffer_size_k, - const size_t buffer_count_k + const size_t write_size, + const size_t buffer_count ); ~RecordView(); @@ -73,8 +73,8 @@ private: const std::string filename_stem_pattern; const FileType file_type; - const size_t buffer_size_k; - const size_t buffer_count_k; + const size_t write_size; + const size_t buffer_count; size_t sampling_rate { 0 }; SignalToken signal_token_tick_second; diff --git a/firmware/baseband/Makefile b/firmware/baseband/Makefile index 5881ff660..dda8f28ca 100755 --- a/firmware/baseband/Makefile +++ b/firmware/baseband/Makefile @@ -146,6 +146,7 @@ CPPSRC = main.cpp \ proc_tpms.cpp \ proc_ert.cpp \ proc_capture.cpp \ + stream_input.cpp \ dsp_squelch.cpp \ clock_recovery.cpp \ packet_builder.cpp \ diff --git a/firmware/baseband/stream_input.cpp b/firmware/baseband/stream_input.cpp new file mode 100644 index 000000000..f383be9c6 --- /dev/null +++ b/firmware/baseband/stream_input.cpp @@ -0,0 +1,76 @@ +/* + * Copyright (C) 2016 Jared Boone, ShareBrained Technology, Inc. + * + * This file is part of PortaPack. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2, or (at your option) + * any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; see the file COPYING. If not, write to + * the Free Software Foundation, Inc., 51 Franklin Street, + * Boston, MA 02110-1301, USA. + */ + +#include "stream_input.hpp" + +#include "lpc43xx_cpp.hpp" +using namespace lpc43xx; + +StreamInput::StreamInput(CaptureConfig* const config) : + fifo_buffers_empty { buffers_empty.data(), buffer_count_max_log2 }, + fifo_buffers_full { buffers_full.data(), buffer_count_max_log2 }, + config { config }, + data { std::make_unique(config->write_size * config->buffer_count) } +{ + config->fifo_buffers_empty = &fifo_buffers_empty; + config->fifo_buffers_full = &fifo_buffers_full; + + for(size_t i=0; ibuffer_count; i++) { + buffers[i] = { &(data.get()[i * config->write_size]), config->write_size }; + fifo_buffers_empty.in(&buffers[i]); + } +} + +size_t StreamInput::write(const void* const data, const size_t length) { + const uint8_t* p = static_cast(data); + size_t written = 0; + + while( written < length ) { + if( !active_buffer ) { + // We need an empty buffer... + if( !fifo_buffers_empty.out(active_buffer) ) { + // ...but none are available. Samples were dropped. + break; + } + } + + const auto remaining = length - written; + written += active_buffer->write(&p[written], remaining); + + if( active_buffer->is_full() ) { + if( !fifo_buffers_full.in(active_buffer) ) { + // FIFO is fuil of buffers, there's no place for this one. + // Bail out of the loop, and try submitting the buffer in the + // next pass. + // This should never happen if the number of buffers is less + // than the capacity of the FIFO. + break; + } + active_buffer = nullptr; + creg::m4txevent::assert(); + } + } + + config->baseband_bytes_received += length; + config->baseband_bytes_dropped += (length - written); + + return written; +} diff --git a/firmware/baseband/stream_input.hpp b/firmware/baseband/stream_input.hpp index cb4bbe4e1..ce4a29a42 100644 --- a/firmware/baseband/stream_input.hpp +++ b/firmware/baseband/stream_input.hpp @@ -25,46 +25,29 @@ #include "message.hpp" #include "fifo.hpp" -#include "lpc43xx_cpp.hpp" -using namespace lpc43xx; - #include #include +#include #include class StreamInput { public: - StreamInput(CaptureConfig* const config) : - config { config }, - K { config->write_size_log2 + config->buffer_count_log2 }, - event_bytes_mask { (1UL << config->write_size_log2) - 1 }, - data { std::make_unique(1UL << K) }, - fifo { data.get(), K } - { - config->fifo = &fifo; - } + StreamInput(CaptureConfig* const config); - size_t write(const void* const data, const size_t length) { - const auto written = fifo.in(reinterpret_cast(data), length); - - const auto last_bytes_written = bytes_written; - bytes_written += written; - if( (bytes_written & event_bytes_mask) < (last_bytes_written & event_bytes_mask) ) { - creg::m4txevent::assert(); - } - config->baseband_bytes_received += length; - config->baseband_bytes_dropped = config->baseband_bytes_received - bytes_written; - - return written; - } + size_t write(const void* const data, const size_t length); private: - CaptureConfig* const config; - const size_t K; - const uint64_t event_bytes_mask; - uint64_t bytes_written = 0; + static constexpr size_t buffer_count_max_log2 = 3; + static constexpr size_t buffer_count_max = 1U << buffer_count_max_log2; + + FIFO fifo_buffers_empty; + FIFO fifo_buffers_full; + std::array buffers; + std::array buffers_empty; + std::array buffers_full; + StreamBuffer* active_buffer { nullptr }; + CaptureConfig* const config { nullptr }; std::unique_ptr data; - FIFO fifo; }; #endif/*__STREAM_INPUT_H__*/ diff --git a/firmware/common/message.hpp b/firmware/common/message.hpp index f1b20a4e6..bc94fe12c 100644 --- a/firmware/common/message.hpp +++ b/firmware/common/message.hpp @@ -24,6 +24,7 @@ #include #include +#include #include #include #include @@ -411,21 +412,63 @@ public: const iir_biquad_config_t audio_hpf_config; }; +// TODO: Put this somewhere else, or at least the implementation part. +class StreamBuffer { + uint8_t* data_; + size_t used_; + size_t capacity_; + +public: + constexpr StreamBuffer( + void* const data = nullptr, + const size_t capacity = 0 + ) : data_ { static_cast(data) }, + used_ { 0 }, + capacity_ { capacity } + { + } + + size_t write(const void* p, const size_t count) { + const auto copy_size = std::min(capacity_ - used_, count); + memcpy(&data_[used_], p, copy_size); + used_ += copy_size; + return copy_size; + } + + bool is_full() const { + return used_ >= capacity_; + } + + const void* data() const { + return data_; + } + + size_t size() const { + return used_; + } + + void empty() { + used_ = 0; + } +}; + struct CaptureConfig { - const size_t write_size_log2; - const size_t buffer_count_log2; + const size_t write_size; + const size_t buffer_count; uint64_t baseband_bytes_received; uint64_t baseband_bytes_dropped; - FIFO* fifo; + FIFO* fifo_buffers_empty; + FIFO* fifo_buffers_full; constexpr CaptureConfig( - const size_t write_size_log2, - const size_t buffer_count_log2 - ) : write_size_log2 { write_size_log2 }, - buffer_count_log2 { buffer_count_log2 }, + const size_t write_size, + const size_t buffer_count + ) : write_size { write_size }, + buffer_count { buffer_count }, baseband_bytes_received { 0 }, baseband_bytes_dropped { 0 }, - fifo { nullptr } + fifo_buffers_empty { nullptr }, + fifo_buffers_full { nullptr } { }