From 9b3b365a6645c16b0f93dcd798da83494e481d26 Mon Sep 17 00:00:00 2001 From: Bent Bisballe Nyeng Date: Tue, 26 Mar 2024 09:21:22 +0100 Subject: Linter fixes for audiocache.cc --- src/Makefile.am | 1 + src/audiocache.cc | 142 +++++++++++++++++++++++----------------------- src/audiocache.h | 19 ++++--- src/audiocacheidmanager.h | 12 ++-- src/owner.h | 24 ++++++++ 5 files changed, 114 insertions(+), 84 deletions(-) create mode 100644 src/owner.h diff --git a/src/Makefile.am b/src/Makefile.am index eac6653..a8bdb59 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -122,6 +122,7 @@ EXTRA_DIST = \ midimapper.h \ nolocale.h \ notifier.h \ + owner.h \ path.h \ platform.h \ powerlist.h \ diff --git a/src/audiocache.cc b/src/audiocache.cc index a2d26d9..fa144fd 100644 --- a/src/audiocache.cc +++ b/src/audiocache.cc @@ -28,8 +28,8 @@ #include -#include -#include +#include +#include #include @@ -64,9 +64,9 @@ void AudioCache::deinit() } // Invariant: initial_samples_needed < preloaded audio data -sample_t* AudioCache::open(const AudioFile& file, - std::size_t initial_samples_needed, - int channel, cacheid_t& id) +gsl::owner AudioCache::open(const AudioFile& file, + std::size_t initial_samples_needed, + int channel, cacheid_t& new_cacheid) { assert(chunk_size); // Assert updateChunkSize was called before processing. @@ -74,16 +74,16 @@ sample_t* AudioCache::open(const AudioFile& file, { settings.number_of_underruns.fetch_add(1); // File preload not yet ready - skip this sample. - id = CACHE_DUMMYID; + new_cacheid = CACHE_DUMMYID; assert(nodata); return nodata; } // Register a new id for this cache session. - id = id_manager.registerID({}); + new_cacheid = id_manager.registerID({}); // If we are out of available ids we get CACHE_DUMMYID - if(id == CACHE_DUMMYID) + if(new_cacheid == CACHE_DUMMYID) { settings.number_of_underruns.fetch_add(1); // Use nodata buffer instead. @@ -92,19 +92,19 @@ sample_t* AudioCache::open(const AudioFile& file, } // Get the cache_t connected with the registered id. - cache_t& c = id_manager.getCache(id); + cache_t& cache = id_manager.getCache(new_cacheid); - c.afile = nullptr; // File is opened when needed. - c.channel = channel; + cache.afile = nullptr; // File is opened when needed. + cache.channel = channel; // Next call to 'next()' will read from this point. - c.localpos = initial_samples_needed; + cache.localpos = initial_samples_needed; - c.ready = false; - c.front = nullptr; // This is allocated when needed. - c.back = nullptr; // This is allocated when needed. + cache.ready = false; + cache.front = nullptr; // This is allocated when needed. + cache.back = nullptr; // This is allocated when needed. - std::size_t cropped_size; + std::size_t cropped_size{}; if(file.preloadedsize == file.size) { @@ -122,162 +122,162 @@ sample_t* AudioCache::open(const AudioFile& file, // \----------------------v-------------------/ // cropped_size - cropped_size = file.preloadedsize - c.localpos; + cropped_size = file.preloadedsize - cache.localpos; cropped_size -= cropped_size % framesize; cropped_size += initial_samples_needed; } - c.preloaded_samples = file.data; - c.preloaded_samples_size = cropped_size; + cache.preloaded_samples = file.data; + cache.preloaded_samples_size = cropped_size; // Next potential read from disk will read from this point. - c.pos = cropped_size; + cache.pos = cropped_size; // Only load next buffer if there is more data in the file to be loaded... - if(c.pos < file.size) + if(cache.pos < file.size) { - c.afile = &event_handler.openFile(file.filename); + cache.afile = &event_handler.openFile(file.filename); - if(c.back == nullptr) + if(cache.back == nullptr) { - c.back = new sample_t[chunk_size]; + cache.back = new sample_t[chunk_size]; } - event_handler.pushLoadNextEvent(c.afile, c.channel, c.pos, - c.back, &c.ready); + event_handler.pushLoadNextEvent(cache.afile, cache.channel, cache.pos, + cache.back, &cache.ready); } - return c.preloaded_samples; // return preloaded data + return cache.preloaded_samples; // return preloaded data } -sample_t* AudioCache::next(cacheid_t id, std::size_t& size) +gsl::owner AudioCache::next(cacheid_t cacheid, std::size_t& size) { - if(id == CACHE_DUMMYID) + if(cacheid == CACHE_DUMMYID) { settings.number_of_underruns.fetch_add(1); assert(nodata); return nodata; } - cache_t& c = id_manager.getCache(id); + cache_t& cache = id_manager.getCache(cacheid); - if(c.preloaded_samples) + if(cache.preloaded_samples != nullptr) { // We are playing from memory: - if(c.localpos < c.preloaded_samples_size) + if(cache.localpos < cache.preloaded_samples_size) { - sample_t* s = c.preloaded_samples + c.localpos; + sample_t* samples = cache.preloaded_samples + cache.localpos; // NOLINT: Fix with span? // If only a partial frame is returned. Reflect this in the size - size = std::min(size, c.preloaded_samples_size - c.localpos); + size = std::min(size, cache.preloaded_samples_size - cache.localpos); - c.localpos += size; - return s; + cache.localpos += size; + return samples; } - c.preloaded_samples = nullptr; // Start using samples from disk. + cache.preloaded_samples = nullptr; // Start using samples from disk. } else { // We are playing from cache: - if(c.localpos < chunk_size) + if(cache.localpos < chunk_size) { - if(c.front == nullptr) + if(cache.front == nullptr) { // Just return silence. settings.number_of_underruns.fetch_add(1); - c.localpos += size; // Skip these samples so we don't loose sync. + cache.localpos += size; // Skip these samples so we don't loose sync. assert(nodata); return nodata; } - sample_t* s = c.front + c.localpos; + sample_t* samples = cache.front + cache.localpos;// NOLINT: Fix with span? // If only a partial frame is returned. Reflect this in the size - size = std::min(size, chunk_size - c.localpos); - c.localpos += size; - return s; + size = std::min(size, chunk_size - cache.localpos); + cache.localpos += size; + return samples; } } // Check for buffer underrun - if(!c.ready) + if(!cache.ready) { // Just return silence. settings.number_of_underruns.fetch_add(1); - c.localpos += size; // Skip these samples so we don't loose sync. + cache.localpos += size; // Skip these samples so we don't loose sync. assert(nodata); return nodata; } // Swap buffers - std::swap(c.front, c.back); + std::swap(cache.front, cache.back); // Next time we go here we have already read the first frame. - c.localpos = size; + cache.localpos = size; - c.pos += chunk_size; + cache.pos += chunk_size; // Does the file have remaining unread samples? - assert(c.afile); // Assert that we have an audio file. - if(c.pos < c.afile->getSize()) + assert(cache.afile); // Assert that we have an audio file. + if(cache.pos < cache.afile->getSize()) { // Do we have a back buffer to read into? - if(c.back == nullptr) + if(cache.back == nullptr) { - c.back = new sample_t[chunk_size]; + cache.back = new sample_t[chunk_size]; } - event_handler.pushLoadNextEvent(c.afile, c.channel, c.pos, - c.back, &c.ready); + event_handler.pushLoadNextEvent(cache.afile, cache.channel, cache.pos, + cache.back, &cache.ready); } // We should always have a front buffer at this point. - assert(c.front); - return c.front; + assert(cache.front); + return cache.front; } -bool AudioCache::isReady(cacheid_t id) +bool AudioCache::isReady(cacheid_t cacheid) { - if(id == CACHE_DUMMYID) + if(cacheid == CACHE_DUMMYID) { return true; } - cache_t& cache = id_manager.getCache(id); + const cache_t& cache = id_manager.getCache(cacheid); return cache.ready; } -void AudioCache::close(cacheid_t id) +void AudioCache::close(cacheid_t cacheid) { - if(id == CACHE_DUMMYID) + if(cacheid == CACHE_DUMMYID) { return; } - event_handler.pushCloseEvent(id); + event_handler.pushCloseEvent(cacheid); } void AudioCache::setFrameSize(std::size_t framesize) { // Make sure the event handler thread is stalled while we set the framesize // state. - std::lock_guard event_handler_lock(event_handler); + const std::lock_guard event_handler_lock(event_handler); // NOTE: Not threaded... //std::lock_guard id_manager_lock(id_manager); if(framesize > nodata_framesize) { - if(nodata) + if(nodata != nullptr) { - nodata_dirty.emplace_back(std::move(nodata)); // Store for later deletion. + nodata_dirty.emplace_back(nodata); // Store for later deletion. } nodata = new sample_t[framesize]; nodata_framesize = framesize; for(std::size_t i = 0; i < framesize; ++i) { - nodata[i] = 0.0f; + nodata[i] = 0.0f;// NOLINT: Fix with span? } } @@ -292,9 +292,11 @@ std::size_t AudioCache::getFrameSize() const void AudioCache::updateChunkSize(std::size_t output_channels) { // Make sure we won't get out-of-range chunk sizes. - std::size_t disk_cache_chunk_size = - std::max(settings.disk_cache_chunk_size.load(), std::size_t(512u * 1024u)); - output_channels = std::max(output_channels, std::size_t(1u)); + constexpr std::size_t max_cache_chunk_size{512ul * 1024ul}; + const auto disk_cache_chunk_size = + std::max(settings.disk_cache_chunk_size.load(), max_cache_chunk_size); + constexpr std::size_t min_output_channels{1}; + output_channels = std::max(output_channels, min_output_channels); // 1MB pr. chunk divided over 16 channels, 4 bytes pr. sample. const auto ideal_chunk_size = diff --git a/src/audiocache.h b/src/audiocache.h index 9c6fa53..632aae9 100644 --- a/src/audiocache.h +++ b/src/audiocache.h @@ -38,6 +38,7 @@ #include "audiocacheidmanager.h" #include "audiocacheeventhandler.h" #include "settings.h" +#include "owner.h" class AudioCache { @@ -68,8 +69,10 @@ public: //! \param [out] new_id The newly created cache id. //! \return A pointer to the first buffer containing the //! 'initial_samples_needed' number of samples. - sample_t* open(const AudioFile& file, std::size_t initial_samples_needed, int channel, - cacheid_t& new_id); + gsl::owner open(const AudioFile& file, + std::size_t initial_samples_needed, + int channel, + cacheid_t& new_cacheid); //! Get next buffer. //! Returns the next buffer for reading based on cache id. @@ -78,9 +81,9 @@ public: //! \param id The cache id to read from. //! \param [out] size The size of the returned buffer. //! \return A pointer to the buffer. - sample_t* next(cacheid_t id, std::size_t &size); + gsl::owner next(cacheid_t id, std::size_t &size); - //! Returns true iff the next chunk of the supplied id has been read from disk. + //! Returns true iff next chunk of the supplied id has been read from disk. bool isReady(cacheid_t id); //! Unregister cache entry. @@ -103,10 +106,10 @@ public: bool isAsyncMode() const; private: - std::size_t framesize{0}; - sample_t* nodata{nullptr}; - std::size_t nodata_framesize{0}; - std::size_t chunk_size{0}; + std::size_t framesize{}; + gsl::owner nodata{}; + std::size_t nodata_framesize{}; + std::size_t chunk_size{}; std::list> nodata_dirty; AudioCacheIDManager id_manager; diff --git a/src/audiocacheidmanager.h b/src/audiocacheidmanager.h index 0aa40e3..2bbe09d 100644 --- a/src/audiocacheidmanager.h +++ b/src/audiocacheidmanager.h @@ -26,14 +26,14 @@ */ #pragma once -#include +#include #include - -#include - #include +#include "audiotypes.h" +#include "owner.h" + class AudioCacheFile; #define CACHE_DUMMYID -2 @@ -49,8 +49,8 @@ struct cache_t size_t channel{0}; size_t pos{0}; //< File position volatile bool ready{false}; - sample_t* front{nullptr}; - sample_t* back{nullptr}; + gsl::owner front{nullptr}; + gsl::owner back{nullptr}; size_t localpos{0}; //< Intra buffer (front) position. sample_t* preloaded_samples{nullptr}; // nullptr means preload buffer not active. diff --git a/src/owner.h b/src/owner.h new file mode 100644 index 0000000..0c67ae5 --- /dev/null +++ b/src/owner.h @@ -0,0 +1,24 @@ +//! -*- c++ -*- +#pragma once + +//#include // for enable_if_t, is_convertible, is_assignable + +// Taken from the GSL Core Guidelines Support Library +namespace gsl +{ + +// +// owner +// +// `gsl::owner` is designed as a safety mechanism for code that must deal directly with raw pointers that own memory. +// Ideally such code should be restricted to the implementation of low-level abstractions. `gsl::owner` can also be used +// as a stepping point in converting legacy code to use more modern RAII constructs, such as smart pointers. +// +// T must be a pointer type +// - disallow construction from any type other than pointer type +// +//template ::value>> +template +using owner = T; + +}// gsl:: -- cgit v1.2.3