From 4ad5108ff1acc309ebc96d91d61d8cb24637f07e Mon Sep 17 00:00:00 2001 From: Ryan Trauntvein Date: Fri, 21 Aug 2026 14:28:50 -0700 Subject: [PATCH 01/15] Fix mixer hot-swap reliability Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1b2cfc57-c6c7-44ce-95c9-72368d409ca8 --- src/AudioLib/Mixer.cpp | 29 ++- src/AudioLib/Mixer.h | 1 + src/AudioLib/SourceAAC.cpp | 7 +- src/AudioLib/SourceAAC.h | 1 + src/AudioLib/Systems/MixSystem.cpp | 283 ++++++++++++++++++++++------- src/AudioLib/Systems/MixSystem.h | 24 ++- 6 files changed, 257 insertions(+), 88 deletions(-) diff --git a/src/AudioLib/Mixer.cpp b/src/AudioLib/Mixer.cpp index c929c02..723b2f8 100644 --- a/src/AudioLib/Mixer.cpp +++ b/src/AudioLib/Mixer.cpp @@ -2,10 +2,10 @@ #include "../AudioSetup.hpp" //clipping wave to avoid overflows -int16_t clip(int32_t input){ +int16_t clip(int32_t input){ - if (input > 0x7FFF) return 0x7FFFF; - if (input < - 0x7FFF) return -0x7FFF; + if (input > INT16_MAX) return INT16_MAX; + if (input < INT16_MIN) return INT16_MIN; return input; } @@ -23,7 +23,7 @@ Mixer::~Mixer() size_t Mixer::generate(int16_t *outBuffer){ memset(outBuffer, 0, BUFFER_SIZE); - std::vector receivedSamples(sourceList.size(), 0); + std::fill(receivedSamples.begin(), receivedSamples.end(), 0); for(uint8_t i = 0; i < sourceList.size(); i++){ if(pauseList[i]) continue; @@ -31,9 +31,6 @@ size_t Mixer::generate(int16_t *outBuffer){ int16_t* buffer = bufferList[i]; if(generator != nullptr && buffer != nullptr){ receivedSamples[i] = generator->generate(buffer); - if(receivedSamples[i] == 0){ - pauseList[i] = true; - } } } @@ -41,7 +38,7 @@ size_t Mixer::generate(int16_t *outBuffer){ int32_t wave = 0; for(uint8_t j = 0; j < sourceList.size(); j++){ if(pauseList[j]) continue; - if(bufferList[j] == nullptr || receivedSamples[j] < i/NUM_CHANNELS) break; + if(bufferList[j] == nullptr || receivedSamples[j] <= i/NUM_CHANNELS) continue; if(sourceList.size() == 2){ wave += bufferList[j][i] * (float)((j == 1 ? (float)(mixRatio) : (float)(255.0 - mixRatio))/255.0); //use the mixer if only 2 tracks found @@ -51,17 +48,14 @@ size_t Mixer::generate(int16_t *outBuffer){ } outBuffer[i] = clip(wave); } - size_t longestBuffer = *std::max_element(receivedSamples.begin(), receivedSamples.end()); + size_t longestBuffer = receivedSamples.empty() + ? 0 + : *std::max_element(receivedSamples.begin(), receivedSamples.end()); if(longestBuffer == 0){ - bool allPaused = true; - for(bool p : pauseList){ - allPaused &= p; - } - - if(allPaused){ - return BUFFER_SAMPLES; - } + // SD-backed decoders can briefly underflow; silence this block without + // turning a recoverable read delay into a persistent deck pause. + return BUFFER_SAMPLES; } return longestBuffer; @@ -86,6 +80,7 @@ void Mixer::addSource(Generator* generator){ } bufferList.push_back(buffer); + receivedSamples.push_back(0); pauseList.push_back(false); } diff --git a/src/AudioLib/Mixer.h b/src/AudioLib/Mixer.h index 1fbb9af..0a2c496 100644 --- a/src/AudioLib/Mixer.h +++ b/src/AudioLib/Mixer.h @@ -28,6 +28,7 @@ class Mixer : public Generator private: std::vector sourceList; std::vector bufferList; + std::vector receivedSamples; uint8_t mixRatio = 122; //half-half by default, 0 = only first track, 255 = only second track std::vector pauseList; }; diff --git a/src/AudioLib/SourceAAC.cpp b/src/AudioLib/SourceAAC.cpp index 54b3bbf..a30ecca 100644 --- a/src/AudioLib/SourceAAC.cpp +++ b/src/AudioLib/SourceAAC.cpp @@ -47,10 +47,14 @@ void SourceAAC::setSongDoneCallback(void (*callback)()) { songDoneCallback = callback; } +bool SourceAAC::isReadReady() const { + return !readJobPending || readResult != nullptr; +} + void SourceAAC::close(){ if(readJobPending){ while(readResult == nullptr){ - delayMicroseconds(1); + Sched.loop(0); } free(readResult->buffer); @@ -306,4 +310,3 @@ void SourceAAC::resetDecoding() { void SourceAAC::setRepeat(bool repeat) { SourceAAC::repeat = repeat; } - diff --git a/src/AudioLib/SourceAAC.h b/src/AudioLib/SourceAAC.h index 172df00..e8fae7a 100644 --- a/src/AudioLib/SourceAAC.h +++ b/src/AudioLib/SourceAAC.h @@ -33,6 +33,7 @@ class SourceAAC : public Source void setRepeat(bool repeat); void setSongDoneCallback(void (*callback)()); + bool isReadReady() const; private: fs::File file; diff --git a/src/AudioLib/Systems/MixSystem.cpp b/src/AudioLib/Systems/MixSystem.cpp index ed51cc7..7beecd4 100644 --- a/src/AudioLib/Systems/MixSystem.cpp +++ b/src/AudioLib/Systems/MixSystem.cpp @@ -13,7 +13,7 @@ MixSystem::MixSystem(const fs::File& f1, const fs::File& f2) : MixSystem(){ open(1, f2); } -MixSystem::MixSystem() : audioTask("MixAudio", audioThread, 16 * 1024, this), queue(6, sizeof(MixRequest*)){ +MixSystem::MixSystem() : audioTask("MixAudio", audioThread, 16 * 1024, this), queue(requestCapacity, sizeof(uint8_t)){ mixer = new Mixer(); for(int i = 0; i < 2; i++){ @@ -64,73 +64,209 @@ MixSystem::~MixSystem(){ delete speed[i]; delete source[i]; + delete retiredSource[i]; } } bool MixSystem::open(uint8_t c, const fs::File& file){ - this->file[c] = file; - if(!file){ + if(c >= 2 || !file){ Serial.println("MixSystem: file not open"); return false; } - delete source[c]; - auto source = this->source[c] = new SourceAAC(file); + auto newSource = new SourceAAC(file); + if(newSource == nullptr){ + Serial.println("MixSystem: source allocation failed"); + return false; + } + newSource->setRepeat(true); + while(!newSource->isReadReady()) Sched.loop(0); + return replaceSource(c, newSource); +} - source->setRepeat(true); +bool MixSystem::replaceSource(uint8_t c, SourceAAC* newSource){ + if(c >= 2 || newSource == nullptr){ + delete newSource; + return false; + } + sourceMutex.lock(); + if(retiredSource[c] != nullptr){ + sourceMutex.unlock(); + delete newSource; + return false; + } + newSource->setVolume(volume[c]); + auto oldSource = source[c]; + source[c] = newSource; if(speed[c]){ - speed[c]->setSource(source); + speed[c]->setSource(newSource); }else{ - effector[c]->setSource(source); + effector[c]->setSource(newSource); } + retiredSource[c] = oldSource; + sourceMutex.unlock(); return true; } +bool MixSystem::openChannel(uint8_t channel, const fs::File& file){ + if(channel >= 2 || !file) return false; + + if(!out->isRunning()){ + return open(channel, file); + } + + cleanupRetiredSources(); + sourceMutex.lock(); + const bool canReplace = retiredSource[channel] == nullptr; + sourceMutex.unlock(); + if(!canReplace) return false; + + const int8_t requestIndex = reserveRequest({ MixRequest::OPEN, channel }); + if(requestIndex < 0) return false; + + auto newSource = new SourceAAC(file); + if(newSource == nullptr){ + releaseRequest(requestIndex); + return false; + } + newSource->setRepeat(true); + while(!newSource->isReadReady()) Sched.loop(0); + requests[requestIndex].value = reinterpret_cast(newSource); + + if(sendRequest(requestIndex)) return true; + + delete newSource; + releaseRequest(requestIndex); + return false; +} + +void MixSystem::_openChannel(uint8_t channel, SourceAAC* newSource){ + if(channel >= 2 || newSource == nullptr){ + delete newSource; + return; + } + + const bool wasPaused = mixer->isChannelPaused(channel); + mixer->pauseChannel(channel); + if(replaceSource(channel, newSource) || !wasPaused) mixer->resumeChannel(channel); +} + +int8_t MixSystem::reserveRequest(const MixRequest& request){ + queueMutex.lock(); + for(uint8_t i = 0; i < requestCapacity; i++){ + if(requestUsed[i] && request.type == MixRequest::OPEN && + requests[i].type == MixRequest::OPEN && requests[i].channel == request.channel){ + queueMutex.unlock(); + return -1; + } + } + + for(uint8_t i = 0; i < requestCapacity; i++){ + if(requestUsed[i]) continue; + requests[i] = request; + requestUsed[i] = true; + queueMutex.unlock(); + return i; + } + queueMutex.unlock(); + return -1; +} + +bool MixSystem::sendRequest(uint8_t index){ + if(index >= requestCapacity || !requestUsed[index]) return false; + return queue.send(&index); +} + +bool MixSystem::enqueueRequest(const MixRequest& request){ + const int8_t index = reserveRequest(request); + if(index < 0) return false; + if(sendRequest(index)) return true; + releaseRequest(index); + return false; +} + +void MixSystem::releaseRequest(uint8_t index){ + if(index >= requestCapacity) return; + queueMutex.lock(); + requests[index] = {}; + requestUsed[index] = false; + queueMutex.unlock(); +} + +void MixSystem::clearRequests(){ + uint8_t index; + while(queue.count()){ + if(!queue.receive(&index)) break; + if(index < requestCapacity && requestUsed[index] && requests[index].type == MixRequest::OPEN){ + delete reinterpret_cast(requests[index].value); + } + releaseRequest(index); + } +} + +void MixSystem::cleanupRetiredSources(){ + SourceAAC* ready[2] = {}; + sourceMutex.lock(); + for(uint8_t channel = 0; channel < 2; channel++){ + if(retiredSource[channel] && retiredSource[channel]->isReadReady()){ + ready[channel] = retiredSource[channel]; + retiredSource[channel] = nullptr; + } + } + sourceMutex.unlock(); + delete ready[0]; + delete ready[1]; +} + void MixSystem::audioThread(Task* task){ MixSystem* system = static_cast(task->arg); Serial.println("-- MixSystem started --"); while(task->running){ - MixRequest* request; + uint8_t requestIndex; while(system->queue.count()){ - system->queue.receive(&request); + if(!system->queue.receive(&requestIndex)) break; + if(requestIndex >= requestCapacity || !system->requestUsed[requestIndex]) continue; + const MixRequest request = system->requests[requestIndex]; - switch(request->type){ + switch(request.type){ case MixRequest::ADD_SPEED: - system->_addSpeed(request->channel); + system->_addSpeed(request.channel); break; case MixRequest::REMOVE_SPEED: - system->_removeSpeed(request->channel); + system->_removeSpeed(request.channel); break; case MixRequest::SET_SPEED: - system->_setSpeed(request->channel, request->value); + system->_setSpeed(request.channel, request.value); break; case MixRequest::SET_EFFECT: - system->_setEffect(request->channel, request->slot, static_cast(request->value)); + system->_setEffect(request.channel, request.slot, static_cast(request.value)); break; case MixRequest::SET_EFFECT_INTENSITY: - system->_setEffectIntensity(request->channel, request->slot, request->value); + system->_setEffectIntensity(request.channel, request.slot, request.value); break; case MixRequest::SET_INFO: - system->_setInfoGenerator(request->channel, (InfoGenerator*) request->value); + system->_setInfoGenerator(request.channel, reinterpret_cast(request.value)); break; case MixRequest::SET_SEEK: - system->_seekChannel(request->channel, (uint16_t) request->value); + system->_seekChannel(request.channel, static_cast(request.value)); break; case MixRequest::RECORD: - if(request->value == system->isRecording()) break; - if(request->value){ + if(request.value == system->isRecording()) break; + if(request.value){ system->_startRecording(); }else{ system->_stopRecording(); } break; + case MixRequest::OPEN: + system->_openChannel(request.channel, reinterpret_cast(request.value)); + break; } - - delete request; + system->releaseRequest(requestIndex); } if(system->out->isRunning()){ @@ -154,18 +290,19 @@ void MixSystem::start(){ } void MixSystem::stop(){ - if(!running) return; - - audioTask.stop(); - - while(!audioTask.isStopped()){ - Sched.loop(0); + if(!audioTask.isStopped()){ + audioTask.stop(); + while(!audioTask.isStopped()){ + Sched.loop(0); + } } + running = false; _stopRecording(); fileOut.close(); out->stop(); + clearRequests(); } bool MixSystem::isRunning(){ @@ -173,21 +310,49 @@ bool MixSystem::isRunning(){ } uint16_t MixSystem::getDuration(uint8_t c){ - if(c >= 2 || !source[c]) return 0; - return source[c]->getDuration(); + if(c >= 2) return 0; + cleanupRetiredSources(); + sourceMutex.lock(); + uint16_t duration = source[c] ? source[c]->getDuration() : 0; + sourceMutex.unlock(); + return duration; } uint16_t MixSystem::getElapsed(uint8_t c){ - if(c >= 2 || !source[c]) return 0; + if(c >= 2) return 0; + cleanupRetiredSources(); if(seekPending[c] > 0){ return seek[c]; } - return source[c]->getElapsed(); + sourceMutex.lock(); + uint16_t elapsed = source[c] ? source[c]->getElapsed() : 0; + sourceMutex.unlock(); + return elapsed; +} + +bool MixSystem::hasChannel(uint8_t c){ + if(c >= 2) return false; + cleanupRetiredSources(); + sourceMutex.lock(); + bool loaded = source[c] != nullptr; + sourceMutex.unlock(); + return loaded; +} + +uint8_t MixSystem::getVolume(uint8_t c){ + return c < 2 ? volume[c] : 0; +} + +uint8_t MixSystem::getMix(){ + return mixer ? mixer->getMixRatio() : 128; } void MixSystem::setVolume(uint8_t c, uint8_t volume){ - if(c >= 2 || !source[c]) return; - source[c]->setVolume(volume); + if(c >= 2) return; + this->volume[c] = volume; + sourceMutex.lock(); + if(source[c]) source[c]->setVolume(volume); + sourceMutex.unlock(); } void MixSystem::setMix(uint8_t ratio){ @@ -201,9 +366,7 @@ void MixSystem::addSpeed(uint8_t channel){ return; } - if(queue.count() == queue.getQueueSize()) return; - MixRequest* request = new MixRequest({ MixRequest::ADD_SPEED, channel }); - queue.send(&request); + enqueueRequest({ MixRequest::ADD_SPEED, channel }); } void MixSystem::removeSpeed(uint8_t channel){ @@ -212,9 +375,7 @@ void MixSystem::removeSpeed(uint8_t channel){ return; } - if(queue.count() == queue.getQueueSize()) return; - MixRequest* request = new MixRequest({ MixRequest::REMOVE_SPEED, channel }); - queue.send(&request); + enqueueRequest({ MixRequest::REMOVE_SPEED, channel }); } void MixSystem::setSpeed(uint8_t channel, uint8_t speed){ @@ -223,9 +384,7 @@ void MixSystem::setSpeed(uint8_t channel, uint8_t speed){ return; } - if(queue.count() == queue.getQueueSize()) return; - MixRequest* request = new MixRequest({ MixRequest::SET_SPEED, channel, 0, speed }); - queue.send(&request); + enqueueRequest({ MixRequest::SET_SPEED, channel, 0, speed }); } void MixSystem::setEffect(uint8_t channel, uint8_t slot, EffectType type){ @@ -234,9 +393,7 @@ void MixSystem::setEffect(uint8_t channel, uint8_t slot, EffectType type){ return; } - if(queue.count() == queue.getQueueSize()) return; - MixRequest* request = new MixRequest({ MixRequest::SET_EFFECT, channel, slot, static_cast(type) }); - queue.send(&request); + enqueueRequest({ MixRequest::SET_EFFECT, channel, slot, static_cast(type) }); } void MixSystem::setEffectIntensity(uint8_t channel, uint8_t slot, uint8_t intensity){ @@ -245,13 +402,11 @@ void MixSystem::setEffectIntensity(uint8_t channel, uint8_t slot, uint8_t intens return; } - if(queue.count() == queue.getQueueSize()) return; - MixRequest* request = new MixRequest({ MixRequest::SET_EFFECT_INTENSITY, channel, slot, intensity }); - queue.send(&request); + enqueueRequest({ MixRequest::SET_EFFECT_INTENSITY, channel, slot, intensity }); } void MixSystem::_addSpeed(uint8_t c){ - if(c >= 2 || !effector[c] || speed[c]) return; + if(c >= 2 || !effector[c] || !source[c] || speed[c]) return; auto speed = this->speed[c] = new SpeedModifier(source[c]); effector[c]->setSource(speed); } @@ -309,9 +464,7 @@ void MixSystem::setChannelInfo(uint8_t channel, InfoGenerator* channelInfoGen){ return; } - if(queue.count() == queue.getQueueSize()) return; - MixRequest* request = new MixRequest({ MixRequest::SET_INFO, channel, 0, (size_t) channelInfoGen }); - queue.send(&request); + enqueueRequest({ MixRequest::SET_INFO, channel, 0, reinterpret_cast(channelInfoGen) }); } void MixSystem::pauseChannel(uint8_t channel){ @@ -326,6 +479,8 @@ void MixSystem::resumeChannel(uint8_t channel){ } void MixSystem::seekChannel(uint8_t channel, uint16_t time){ + if(channel >= 2) return; + if(!out->isRunning()){ _seekChannel(channel, time); return; @@ -333,19 +488,14 @@ void MixSystem::seekChannel(uint8_t channel, uint16_t time){ seek[channel] = time; seekPending[channel]++; - - if(queue.count() == queue.getQueueSize()) return; - MixRequest* request = new MixRequest({ MixRequest::SET_SEEK, channel, 0, time }); - queue.send(&request); + if(!enqueueRequest({ MixRequest::SET_SEEK, channel, 0, time })) seekPending[channel]--; } void MixSystem::_seekChannel(uint8_t channel, uint16_t time){ if(channel > 1) return; + if(seekPending[channel] > 0) seekPending[channel]--; + if(!source[channel]) return; - if(i2s->isRunning()){ - seekPending[channel]--; - //i2s_zero_dma_buffer((i2s_port_t) 0); - } source[channel]->seek(time, SeekSet); } @@ -359,8 +509,7 @@ void MixSystem::startRecording(){ return; } - MixRequest* request = new MixRequest({ MixRequest::RECORD, 0, 0, 1 }); - queue.send(&request); + enqueueRequest({ MixRequest::RECORD, 0, 0, 1 }); } void MixSystem::stopRecording(){ @@ -369,8 +518,7 @@ void MixSystem::stopRecording(){ return; } - MixRequest* request = new MixRequest({ MixRequest::RECORD, 0, 0, 0 }); - queue.send(&request); + enqueueRequest({ MixRequest::RECORD, 0, 0, 0 }); } void MixSystem::_startRecording(){ @@ -410,5 +558,8 @@ bool MixSystem::isChannelPaused(uint8_t channel){ } void MixSystem::setChannelDoneCallback(uint8_t channel, void(*callback)()) { - source[channel]->setSongDoneCallback(callback); + if(channel >= 2) return; + sourceMutex.lock(); + if(source[channel]) source[channel]->setSongDoneCallback(callback); + sourceMutex.unlock(); } diff --git a/src/AudioLib/Systems/MixSystem.h b/src/AudioLib/Systems/MixSystem.h index 6bae1e0..73cfc86 100644 --- a/src/AudioLib/Systems/MixSystem.h +++ b/src/AudioLib/Systems/MixSystem.h @@ -13,11 +13,12 @@ #include "../EffectType.hpp" #include "../SourceAAC.h" #include +#include #include "../InfoGenerator.h" #include "../OutputWAV.h" struct MixRequest { - enum { ADD_SPEED, REMOVE_SPEED, SET_SPEED, SET_EFFECT, SET_EFFECT_INTENSITY, SET_INFO, SET_SEEK, RECORD } type; + enum { ADD_SPEED, REMOVE_SPEED, SET_SPEED, SET_EFFECT, SET_EFFECT_INTENSITY, SET_INFO, SET_SEEK, RECORD, OPEN } type; uint8_t channel; uint8_t slot; size_t value; @@ -32,6 +33,7 @@ class MixSystem { constexpr static const char* const recordPath = "/.Jay-D_Recording.wav"; bool open(uint8_t channel, const fs::File& file); + bool openChannel(uint8_t channel, const fs::File& file); Task audioTask; static void audioThread(Task* task); @@ -42,6 +44,9 @@ class MixSystem { uint16_t getDuration(uint8_t channel); uint16_t getElapsed(uint8_t channel); + bool hasChannel(uint8_t channel); + uint8_t getVolume(uint8_t channel); + uint8_t getMix(); void setVolume(uint8_t channel, uint8_t volume); void setMix(uint8_t ratio); @@ -68,14 +73,20 @@ class MixSystem { void setChannelDoneCallback(uint8_t channel, void(*callback)()); private: + static constexpr uint8_t requestCapacity = 6; bool running = false; Queue queue; + Mutex queueMutex; + Mutex sourceMutex; + MixRequest requests[requestCapacity] = {}; + bool requestUsed[requestCapacity] = {}; - fs::File file[2]; fs::File fileOut; SourceAAC* source[2] = { nullptr }; + SourceAAC* retiredSource[2] = { nullptr }; + uint8_t volume[2] = { 255, 255 }; EffectProcessor* effector[2]; Mixer* mixer; @@ -94,12 +105,19 @@ class MixSystem { void _seekChannel(uint8_t channel, uint16_t time); void _startRecording(); void _stopRecording(); + void _openChannel(uint8_t channel, SourceAAC* source); + bool replaceSource(uint8_t channel, SourceAAC* source); + int8_t reserveRequest(const MixRequest& request); + bool sendRequest(uint8_t index); + bool enqueueRequest(const MixRequest& request); + void releaseRequest(uint8_t index); + void clearRequests(); + void cleanupRetiredSources(); static Effect* (* getEffect[EffectType::COUNT])(); uint16_t seek[2]; int seekPending[2] = { 0 }; - }; #endif //JAYD_LIBRARY_MIXSYSTEM_H From ec469121f12184a1f206372e8da275e079974d8c Mon Sep 17 00:00:00 2001 From: Ryan Trauntvein Date: Fri, 21 Aug 2026 21:03:55 -0700 Subject: [PATCH 02/15] Add frame-accurate AAC timing and seek Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- README.md | 24 ++ src/AudioLib/ADTSTiming.h | 83 ++++++ src/AudioLib/SourceAAC.cpp | 442 ++++++++++++++++++++++------- src/AudioLib/SourceAAC.h | 72 ++--- src/AudioLib/SpeedModifier.cpp | 5 + src/AudioLib/SpeedModifier.h | 1 + src/AudioLib/Systems/MixSystem.cpp | 103 +++++-- src/AudioLib/Systems/MixSystem.h | 11 +- tests/adts_timing_self_check.cpp | 52 ++++ 9 files changed, 641 insertions(+), 152 deletions(-) create mode 100644 src/AudioLib/ADTSTiming.h create mode 100644 tests/adts_timing_self_check.cpp diff --git a/README.md b/README.md index 18856a6..a7184d2 100644 --- a/README.md +++ b/README.md @@ -43,6 +43,30 @@ cmake --build . --target CMBuild ``` This will compile the binaries, and place the .bin and .elf files in the build/ directory located in the root of the repository. +## AAC timing and seeking + +`SourceAAC` measures time in source sample frames: one frame is one sample +instant across all encoded channels, at the ADTS sample rate. Channel count +does not multiply duration. The seconds-based API remains available and rounds +down to whole seconds. + +ADTS indexing runs while a source is opened, before it is published to the +audio thread. Each seek entry is 8 bytes. The bounded index uses at most 128 KiB +per deck in PSRAM (16,384 frames), or 16 KiB without PSRAM (2,048 frames); +the temporary scan cache is 4 KiB. The larger strict-frame decode buffers add +23 KiB per source over the previous buffers. `getFrameIndexQuality()` reports +whether the whole track is seekable; duration remains frame-counted even when +the seek index reaches its cap. A `cm:esp32:jayd` build measured 1,026,334 +bytes of flash and 44,464 bytes of static RAM: +312 bytes of flash and no +static-RAM increase versus `4ad5108`. + +Seek targets use a 64-bit source-frame API, but the compact index stores 32-bit +frame positions, limiting frame-accurate seeking to the first 2^32 source +frames (about 24.9 hours at 48 kHz). Seeking starts at the preceding ADTS frame +and discards decoded PCM up to the target. Accuracy is therefore bounded by +the decoder's sample-rate conversion and AAC priming; one 256-sample output +block of accuracy has not been established on hardware. + To compile the binary, and upload it according to the port set in CMakeLists.txt, run ```cmake --build . --target CMBuild``` diff --git a/src/AudioLib/ADTSTiming.h b/src/AudioLib/ADTSTiming.h new file mode 100644 index 0000000..597bffd --- /dev/null +++ b/src/AudioLib/ADTSTiming.h @@ -0,0 +1,83 @@ +#ifndef JAYD_ADTSTIMING_H +#define JAYD_ADTSTIMING_H + +#include +#include + +namespace ADTSTiming { + +enum ParseResult : uint8_t { + INVALID, + NEED_MORE, + VALID +}; + +struct Header { + uint16_t frameLength; + uint16_t sourceFrames; + uint32_t sampleRate; + uint8_t channels; + uint8_t headerLength; +}; + +struct FrameIndexEntry { + uint32_t offset; + uint32_t sourceFrame; +}; + +inline ParseResult parseHeader(const uint8_t* data, size_t size, Header& header){ + static const uint32_t sampleRates[] = { + 96000, 88200, 64000, 48000, 44100, 32000, 24000, + 22050, 16000, 12000, 11025, 8000, 7350 + }; + + if(size < 7) return NEED_MORE; + if(data[0] != 0xff || (data[1] & 0xf6) != 0xf0) return INVALID; + + const uint8_t profile = (data[2] >> 6) & 0x03; + const uint8_t sampleRateIndex = (data[2] >> 2) & 0x0f; + if(profile == 3 || sampleRateIndex >= sizeof(sampleRates) / sizeof(sampleRates[0])) return INVALID; + + header.headerLength = (data[1] & 0x01) ? 7 : 9; + if(size < header.headerLength) return NEED_MORE; + + header.frameLength = uint16_t((uint16_t(data[3] & 0x03) << 11) | + (uint16_t(data[4]) << 3) | + (data[5] >> 5)); + if(header.frameLength < header.headerLength) return INVALID; + + header.sampleRate = sampleRates[sampleRateIndex]; + header.channels = uint8_t(((data[2] & 0x01) << 2) | (data[3] >> 6)); + header.sourceFrames = uint16_t(1024u * ((data[6] & 0x03) + 1u)); + return VALID; +} + +inline ParseResult parseFrame(const uint8_t* data, size_t size, Header& header){ + const ParseResult result = parseHeader(data, size, header); + if(result != VALID) return result; + return header.frameLength <= size ? VALID : NEED_MORE; +} + +inline bool secondsToFrames(uint64_t seconds, uint32_t sampleRate, uint64_t& frames){ + if(sampleRate == 0 || seconds > UINT64_MAX / sampleRate) return false; + frames = seconds * sampleRate; + return true; +} + +inline size_t findPreceding(const FrameIndexEntry* entries, size_t count, uint64_t target){ + size_t low = 0; + size_t high = count; + while(low < high){ + const size_t middle = low + (high - low) / 2; + if(entries[middle].sourceFrame <= target){ + low = middle + 1; + }else{ + high = middle; + } + } + return low == 0 ? 0 : low - 1; +} + +} + +#endif diff --git a/src/AudioLib/SourceAAC.cpp b/src/AudioLib/SourceAAC.cpp index a30ecca..3987797 100644 --- a/src/AudioLib/SourceAAC.cpp +++ b/src/AudioLib/SourceAAC.cpp @@ -1,14 +1,20 @@ #include "SourceAAC.h" #include "../PerfMon.h" +#include #define AAC_READ_BUFFER 1024 * 64 #define AAC_READ_CHUNK 1024 * 4 // should be bigger than min input -#define AAC_DECODE_MIN_INPUT 1024 // should be smaller than read chunk -#define AAC_OUT_BUFFER 1024 * 4 +#define AAC_DECODE_BUFFER 8192 +#define AAC_OUT_BUFFER 20480 +#define AAC_MAX_DECODED_BYTES 8192 +#define AAC_MAX_MONO_BLOCK_BYTES 4096 +#define AAC_INDEX_PSRAM_ENTRIES 16384 +#define AAC_INDEX_INTERNAL_ENTRIES 2048 +#define AAC_INDEX_READ_CHUNK 4096 SourceAAC::SourceAAC() : readBuffer(AAC_READ_BUFFER), - fillBuffer(AAC_DECODE_MIN_INPUT), + fillBuffer(AAC_DECODE_BUFFER), dataBuffer(AAC_OUT_BUFFER){ } @@ -21,7 +27,7 @@ void SourceAAC::open(fs::File file){ close(); this->file = file; - channels = sampleRate = bytesPerSample = bitrate = movedBytes = 0; + channels = sampleRate = bytesPerSample = 0; readBuffer.clear(); dataBuffer.clear(); fillBuffer.clear(); @@ -30,14 +36,18 @@ void SourceAAC::open(fs::File file){ return; } - dataSize = file.size(); - bitrate = 64000; bytesPerSample = 2; + buildFrameIndex(); + if(sourceSampleRate == 0){ + Serial.println("SourceAAC: no valid ADTS frames"); + return; + } + file.seek(firstFrameOffset); hAACDecoder = AACInitDecoder(); if(hAACDecoder == nullptr){ Serial.println("Decoder construct fail"); - + return; } addReadJob(true); @@ -59,9 +69,11 @@ void SourceAAC::close(){ free(readResult->buffer); delete readResult; + readResult = nullptr; + readJobPending = false; } - channels = sampleRate = bytesPerSample = bitrate = movedBytes = 0; + channels = sampleRate = bytesPerSample = 0; readBuffer.clear(); dataBuffer.clear(); fillBuffer.clear(); @@ -70,6 +82,17 @@ void SourceAAC::close(){ AACFreeDecoder(hAACDecoder); hAACDecoder = nullptr; } + freeFrameIndex(); + durationSourceFrames = 0; + indexedSourceFrameEnd = 0; + portENTER_CRITICAL(&timingMux); + elapsedSourceFrames = 0; + portEXIT_CRITICAL(&timingMux); + decodedSourceFrame = seekTargetSourceFrame = elapsedFrameRemainder = 0; + sourceSampleRate = firstFrameOffset = 0; + sourceChannels = adtsChannelConfiguration = 0; + readEof = false; + discardPendingRead = false; } SourceAAC::~SourceAAC(){ @@ -77,7 +100,7 @@ SourceAAC::~SourceAAC(){ } void SourceAAC::addReadJob(bool full){ - if(readJobPending) return; + if(readJobPending || readEof || !file) return; delete readResult; readResult = nullptr; @@ -90,12 +113,17 @@ void SourceAAC::addReadJob(bool full){ return; } - uint8_t* buf; + uint8_t* buf = nullptr; if(size <= AAC_READ_CHUNK || !psramFound()){ buf = static_cast(malloc(size)); }else{ buf = static_cast(ps_malloc(size)); } + if(buf == nullptr){ + Serial.println("SourceAAC: read buffer allocation failed"); + readEof = true; + return; + } Sched.addJob(new SDJob{ .type = SDJob::SD_READ, @@ -108,9 +136,10 @@ void SourceAAC::addReadJob(bool full){ readJobPending = true; } -void SourceAAC::processReadJob(){ +void SourceAAC::processReadJob(bool wait){ + if(!readJobPending) return; if(readResult == nullptr){ - if(readBuffer.readAvailable() + fillBuffer.readAvailable() < AAC_DECODE_MIN_INPUT){ + if(wait || readBuffer.readAvailable() + fillBuffer.readAvailable() < 7){ while(readResult == nullptr){ delayMicroseconds(1); } @@ -120,12 +149,67 @@ void SourceAAC::processReadJob(){ } readBuffer.write(readResult->buffer, readResult->size); + if(readResult->size == 0) readEof = true; free(readResult->buffer); delete readResult; readResult = nullptr; readJobPending = false; + if(discardPendingRead){ + discardPendingRead = false; + readBuffer.clear(); + readEof = false; + addReadJob(); + if(wait) processReadJob(true); + } +} + +bool SourceAAC::prepareNextFrame(ADTSTiming::Header& header){ + for(;;){ + refill(); + const size_t available = fillBuffer.readAvailable(); + const uint8_t* data = fillBuffer.readData(); + bool waitingForData = false; + + for(size_t offset = 0; offset + 7 <= available; offset++){ + const ADTSTiming::ParseResult result = ADTSTiming::parseHeader(data + offset, available - offset, header); + if(result == ADTSTiming::INVALID) continue; + if(result == ADTSTiming::VALID && + header.sampleRate == sourceSampleRate && + (adtsChannelConfiguration == 0 || header.channels == adtsChannelConfiguration)){ + if(header.frameLength <= available - offset){ + fillBuffer.readMove(offset); + return true; + } + if(!readEof){ + fillBuffer.readMove(offset); + addReadJob(); + processReadJob(true); + waitingForData = true; + break; + } + } + if(result == ADTSTiming::NEED_MORE && !readEof){ + fillBuffer.readMove(offset); + addReadJob(); + processReadJob(true); + waitingForData = true; + break; + } + } + if(waitingForData) continue; + + if(readEof){ + fillBuffer.readMove(fillBuffer.readAvailable()); + return false; + } + + const size_t keep = min(size_t(8), fillBuffer.readAvailable()); + fillBuffer.readMove(fillBuffer.readAvailable() - keep); + addReadJob(); + processReadJob(true); + } } size_t SourceAAC::generate(int16_t* outBuffer){ @@ -143,81 +227,65 @@ size_t SourceAAC::generate(int16_t* outBuffer){ processReadJob(); Profiler.end(); - refill(); - if(fillBuffer.readAvailable() < AAC_DECODE_MIN_INPUT){ - seek(0, SeekSet); - Serial.println("if fillbuffer < aac_decode_min"); -// if(songDoneCallback != nullptr) { -// songDoneCallback(); -// } - if(repeat){ - processReadJob(); - refill(); - }else{ - return 0; - } - } - while(dataBuffer.readAvailable() < BUFFER_SIZE){ - // Serial.printf("Grabbing, available %ld, taking %ld\n", readBuffer.readAvailable(), fillBuffer.writeAvailable()); - - refill(); - if(fillBuffer.readAvailable() < AAC_DECODE_MIN_INPUT){ - addReadJob(); - processReadJob(); - refill(); - } + ADTSTiming::Header adts; + if(!prepareNextFrame(adts)) break; + const size_t rawBlocks = adts.sourceFrames / 1024; + const size_t requiredBytes = + (rawBlocks - 1) * AAC_MAX_MONO_BLOCK_BYTES + AAC_MAX_DECODED_BYTES; + if(dataBuffer.writeAvailable() < requiredBytes) break; - /*ADTSHeader* adts = (ADTSHeader*) fillBuffer.readData(); - if(adts->syncword_0_to_8 != 0xff || adts->syncword_9_to_12 != 0xf){ - Serial.println("Incorrect frame, searching..."); - - size_t bytesMoved = 0; - while((adts->syncword_0_to_8 != 0xff || adts->syncword_9_to_12 != 0xf) && (++bytesMoved + sizeof(ADTSHeader)) < fillBuffer.readAvailable()){ - adts = (ADTSHeader*) (fillBuffer.readData() + bytesMoved); + uint8_t* data = const_cast(fillBuffer.readData()); + int bytesLeft = adts.frameLength; + size_t decodedBlocks = 0; + for(; decodedBlocks < rawBlocks; decodedBlocks++){ + int16_t* pcm = reinterpret_cast(dataBuffer.writeData()); + const int ret = AACDecode(hAACDecoder, &data, &bytesLeft, pcm); + if(ret){ + Serial.printf("decode error %d, frame size %u B\n", ret, adts.frameLength); + AACFlushCodec(hAACDecoder); + break; } - if(adts->syncword_0_to_8 != 0xff || adts->syncword_9_to_12 != 0xf && (bytesMoved + sizeof(ADTSHeader)) == fillBuffer.readAvailable()){ - Serial.printf("Can't find frame. searched %lu bytes\n", bytesMoved); - fillBuffer.readMove(bytesMoved); + AACFrameInfo fi; + AACGetLastFrameInfo(hAACDecoder, &fi); + sampleRate = fi.sampRateOut; + channels = fi.nChans; + if(sampleRate == 0 || channels == 0 || + fi.outputSamps <= 0 || fi.outputSamps % channels != 0){ + Serial.println("SourceAAC: invalid decoder frame info"); + AACFlushCodec(hAACDecoder); break; - }else{ - Serial.printf("Frame found after %lu bytes\n", bytesMoved); + } + if(sourceChannels == 0) sourceChannels = channels; + + const size_t outputFrames = fi.outputSamps / channels; + if(channels > 1){ + for(size_t frame = 0; frame < outputFrames; frame++){ + int32_t mixed = 0; + for(uint8_t channel = 0; channel < channels; channel++){ + mixed += pcm[frame * channels + channel]; + } + pcm[frame] = int16_t(mixed / channels); + } } - readData += bytesMoved / (NUM_CHANNELS * BYTES_PER_SAMPLE); - fillBuffer.readMove(bytesMoved); - refill(); - } - - if(fillBuffer.readAvailable() < AAC_DECODE_MIN_INPUT){ - break; - } - - size_t frameSize = adts->frame_length_0_to_1 << 11 | adts->frame_length_2_to_9 << 3 | adts->frame_length_10_to_12;*/ + size_t discardedFrames = 0; + if(seekTargetSourceFrame > decodedSourceFrame){ + const uint64_t sourceFramesToDiscard = min( + uint64_t(1024), + seekTargetSourceFrame - decodedSourceFrame); + discardedFrames = size_t((sourceFramesToDiscard * outputFrames + 1023) / 1024); + discardedFrames = min(discardedFrames, outputFrames); + memmove(pcm, pcm + discardedFrames, (outputFrames - discardedFrames) * bytesPerSample); + } + decodedSourceFrame += 1024; + if(decodedSourceFrame >= seekTargetSourceFrame) seekTargetSourceFrame = 0; - uint8_t* data = const_cast(fillBuffer.readData()); - int bytesLeft = fillBuffer.readAvailable(); - // Serial.printf("Decoding, available %ld\n", fillBuffer.readAvailable()); - int ret = AACDecode(hAACDecoder, &data, &bytesLeft, reinterpret_cast(dataBuffer.writeData())); - if(ret){ - size_t frameSize = fillBuffer.readAvailable() - bytesLeft; - Serial.printf("decode error %d, frame size %d B\n", ret, frameSize); - size_t size = min(frameSize, fillBuffer.readAvailable()); - movedBytes++; - fillBuffer.readMove(1); - continue; + dataBuffer.writeMove((outputFrames - discardedFrames) * bytesPerSample); } - movedBytes += fillBuffer.readAvailable() - bytesLeft; - fillBuffer.readMove(fillBuffer.readAvailable() - bytesLeft); - - AACFrameInfo fi; - AACGetLastFrameInfo(hAACDecoder, &fi); - - sampleRate = fi.sampRateOut; - channels = fi.nChans; - - dataBuffer.writeMove(fi.outputSamps * channels * bytesPerSample); + decodedSourceFrame += (rawBlocks - decodedBlocks) * 1024; + fillBuffer.readMove(adts.frameLength); } Profiler.end(); @@ -231,15 +299,27 @@ size_t SourceAAC::generate(int16_t* outBuffer){ } if(samples == 0){ - seek(0, SeekSet); - if(songDoneCallback != nullptr) { songDoneCallback(); } - if(repeat){ - return generate(outBuffer); + if(repeat && !rewindAttempted){ + rewindAttempted = true; + if(seekSourceFrame(0)){ + const size_t repeatedSamples = generate(outBuffer); + rewindAttempted = false; + return repeatedSamples; + } + rewindAttempted = false; } }else{ + const uint32_t outputRate = sampleRate == 0 ? sourceSampleRate : sampleRate; + const uint64_t numerator = elapsedFrameRemainder + uint64_t(samples) * sourceSampleRate; + const uint64_t advanced = numerator / outputRate; + elapsedFrameRemainder = numerator % outputRate; + portENTER_CRITICAL(&timingMux); + elapsedSourceFrames = min(durationSourceFrames, elapsedSourceFrames + advanced); + portEXIT_CRITICAL(&timingMux); + rewindAttempted = false; addReadJob(); } @@ -253,35 +333,58 @@ void SourceAAC::refill(){ } int SourceAAC::available(){ - if(sampleRate == 0 || channels == 0 || bytesPerSample == 0) return 0; - return (file.available() / (channels * bytesPerSample)); + const uint64_t elapsed = getElapsedSourceFrames(); + const uint64_t remaining = elapsed < durationSourceFrames ? durationSourceFrames - elapsed : 0; + return remaining > INT_MAX ? INT_MAX : int(remaining); } uint16_t SourceAAC::getDuration(){ - if(bitrate == 0) return 0; - return 8 * dataSize / bitrate; + if(sourceSampleRate == 0) return 0; + const uint64_t seconds = durationSourceFrames / sourceSampleRate; + return seconds > UINT16_MAX ? UINT16_MAX : uint16_t(seconds); } uint16_t SourceAAC::getElapsed(){ - if(bitrate == 0) return 0; - return 8 * movedBytes / bitrate; + if(sourceSampleRate == 0) return 0; + const uint64_t seconds = getElapsedSourceFrames() / sourceSampleRate; + return seconds > UINT16_MAX ? UINT16_MAX : uint16_t(seconds); } void SourceAAC::seek(uint16_t time, fs::SeekMode mode){ - size_t offset = time * bitrate / 8; - if(offset >= file.size()){ - return; + uint64_t frames; + if(!ADTSTiming::secondsToFrames(time, sourceSampleRate, frames)) return; + if(mode == SeekCur){ + const uint64_t elapsed = getElapsedSourceFrames(); + frames = frames > UINT64_MAX - elapsed ? UINT64_MAX : frames + elapsed; + }else if(mode == SeekEnd){ + frames = frames >= durationSourceFrames ? 0 : durationSourceFrames - frames; } + seekSourceFrame(frames); +} - if(readJobPending){ - while (readResult == nullptr){ - delayMicroseconds(1); +bool SourceAAC::seekSourceFrame(uint64_t frame){ + if(sourceSampleRate == 0 || frame > UINT32_MAX) return false; + frame = min(frame, durationSourceFrames); + uint32_t offset = firstFrameOffset; + uint64_t indexedFrame = 0; + if(frame != 0){ + if(frameIndexCount == 0) return false; + const size_t index = ADTSTiming::findPreceding(frameIndex, frameIndexCount, frame); + if(frameIndex[index].sourceFrame > frame || + (frameIndexQuality != INDEX_COMPLETE && frame > indexedSourceFrameEnd)){ + return false; } - + offset = frameIndex[index].offset; + indexedFrame = frameIndex[index].sourceFrame; + } + if(readJobPending && readResult != nullptr){ free(readResult->buffer); delete readResult; readResult = nullptr; readJobPending = false; + discardPendingRead = false; + }else if(readJobPending){ + discardPendingRead = true; } Sched.addJob(new SDJob{ .type = SDJob::SD_SEEK, @@ -290,8 +393,14 @@ void SourceAAC::seek(uint16_t time, fs::SeekMode mode){ .buffer = nullptr, .result = nullptr }); - movedBytes = offset; + portENTER_CRITICAL(&timingMux); + elapsedSourceFrames = frame; + portEXIT_CRITICAL(&timingMux); + decodedSourceFrame = indexedFrame; + seekTargetSourceFrame = frame; + elapsedFrameRemainder = 0; resetDecoding(); + return true; } void SourceAAC::setVolume(uint8_t volume){ @@ -303,6 +412,7 @@ void SourceAAC::resetDecoding() { dataBuffer.clear(); fillBuffer.clear(); AACFlushCodec(hAACDecoder); + readEof = false; addReadJob(); } @@ -310,3 +420,143 @@ void SourceAAC::resetDecoding() { void SourceAAC::setRepeat(bool repeat) { SourceAAC::repeat = repeat; } + +uint64_t SourceAAC::getDurationSourceFrames() const { + return durationSourceFrames; +} + +uint64_t SourceAAC::getElapsedSourceFrames() const { + portENTER_CRITICAL(&timingMux); + const uint64_t frames = elapsedSourceFrames; + portEXIT_CRITICAL(&timingMux); + return frames; +} + +uint32_t SourceAAC::getSourceSampleRate() const { + return sourceSampleRate; +} + +uint8_t SourceAAC::getSourceChannels() const { + return sourceChannels; +} + +SourceAAC::FrameIndexQuality SourceAAC::getFrameIndexQuality() const { + return frameIndexQuality; +} + +void SourceAAC::freeFrameIndex(){ + free(frameIndex); + frameIndex = nullptr; + frameIndexCount = frameIndexCapacity = 0; + frameIndexQuality = INDEX_UNAVAILABLE; +} + +void SourceAAC::buildFrameIndex(){ + freeFrameIndex(); + durationSourceFrames = 0; + indexedSourceFrameEnd = 0; + sourceSampleRate = 0; + sourceChannels = adtsChannelConfiguration = 0; + firstFrameOffset = 0; + + const size_t fileSize = file.size(); + if(fileSize < 7) return; + + frameIndexCapacity = min( + fileSize / size_t(7), + size_t(psramFound() ? AAC_INDEX_PSRAM_ENTRIES : AAC_INDEX_INTERNAL_ENTRIES)); + if(frameIndexCapacity > 0){ + const size_t bytes = frameIndexCapacity * sizeof(ADTSTiming::FrameIndexEntry); + frameIndex = static_cast( + psramFound() ? ps_malloc(bytes) : malloc(bytes)); + if(frameIndex == nullptr) frameIndexCapacity = 0; + } + + uint8_t* cache = static_cast( + psramFound() ? ps_malloc(AAC_INDEX_READ_CHUNK) : malloc(AAC_INDEX_READ_CHUNK)); + if(cache == nullptr){ + Serial.println("SourceAAC: ADTS scan buffer allocation failed"); + return; + } + + size_t cacheStart = fileSize; + size_t cacheSize = 0; + auto readAt = [&](size_t position, uint8_t* out, size_t count) -> bool { + size_t copied = 0; + while(copied < count){ + if(position < cacheStart || position >= cacheStart + cacheSize){ + if(!file.seek(position)) return false; + cacheStart = position; + cacheSize = file.read(cache, min(size_t(AAC_INDEX_READ_CHUNK), fileSize - position)); + Sched.loop(0); + if(cacheSize == 0) return false; + } + const size_t cacheOffset = position - cacheStart; + const size_t part = min(count - copied, cacheSize - cacheOffset); + memcpy(out + copied, cache + cacheOffset, part); + position += part; + copied += part; + } + return true; + }; + + uint8_t bytes[9]; + size_t offset = 0; + bool foundFrame = false; + bool indexFull = false; + bool scanFailed = false; + while(offset + 7 <= fileSize){ + if(!readAt(offset, bytes, 7)){ + scanFailed = true; + break; + } + ADTSTiming::Header header; + ADTSTiming::ParseResult result = ADTSTiming::parseHeader(bytes, 7, header); + if(result == ADTSTiming::NEED_MORE && offset + 9 <= fileSize){ + if(!readAt(offset, bytes, 9)){ + scanFailed = true; + break; + } + result = ADTSTiming::parseHeader(bytes, 9, header); + } + if(result != ADTSTiming::VALID || + header.frameLength > fileSize - offset || + (foundFrame && (header.sampleRate != sourceSampleRate || + (sourceChannels != 0 && header.channels != sourceChannels)))){ + offset++; + continue; + } + + if(!foundFrame){ + foundFrame = true; + firstFrameOffset = uint32_t(offset); + sourceSampleRate = header.sampleRate; + sourceChannels = adtsChannelConfiguration = header.channels; + } + const bool stored = frameIndexCount < frameIndexCapacity && durationSourceFrames <= UINT32_MAX; + if(stored){ + frameIndex[frameIndexCount++] = { + uint32_t(offset), + uint32_t(durationSourceFrames) + }; + }else{ + indexFull = true; + } + if(durationSourceFrames > UINT64_MAX - header.sourceFrames){ + durationSourceFrames = UINT64_MAX; + indexFull = true; + break; + } + durationSourceFrames += header.sourceFrames; + if(stored) indexedSourceFrameEnd = durationSourceFrames; + offset += header.frameLength; + } + free(cache); + + if(frameIndexCount == 0){ + frameIndexQuality = INDEX_UNAVAILABLE; + }else{ + frameIndexQuality = indexFull || scanFailed ? INDEX_PARTIAL : INDEX_COMPLETE; + } + if(scanFailed) Serial.println("SourceAAC: ADTS index scan read failed"); +} diff --git a/src/AudioLib/SourceAAC.h b/src/AudioLib/SourceAAC.h index e8fae7a..543234d 100644 --- a/src/AudioLib/SourceAAC.h +++ b/src/AudioLib/SourceAAC.h @@ -11,6 +11,7 @@ #include #include #include +#include "ADTSTiming.h" class SourceAAC : public Source { @@ -25,6 +26,19 @@ class SourceAAC : public Source uint16_t getElapsed() override; void seek(uint16_t time, fs::SeekMode mode) override; + uint64_t getDurationSourceFrames() const; + uint64_t getElapsedSourceFrames() const; + uint32_t getSourceSampleRate() const; + uint8_t getSourceChannels() const; + bool seekSourceFrame(uint64_t frame); + + enum FrameIndexQuality : uint8_t { + INDEX_UNAVAILABLE, + INDEX_PARTIAL, + INDEX_COMPLETE + }; + FrameIndexQuality getFrameIndexQuality() const; + void open(fs::File file); void close() override; @@ -37,10 +51,6 @@ class SourceAAC : public Source private: fs::File file; - uint32_t bitrate = 0; - - size_t dataSize = 0; - size_t movedBytes = 0; float volume = 1.0f; @@ -50,43 +60,35 @@ class SourceAAC : public Source DataBuffer fillBuffer; DataBuffer dataBuffer; void refill(); + bool prepareNextFrame(ADTSTiming::Header& header); HAACDecoder hAACDecoder = nullptr; - struct ADTSHeader { - unsigned char syncword_0_to_8: 8; - - unsigned char protection_absent: 1; - unsigned char layer: 2; - unsigned char ID: 1; - unsigned char syncword_9_to_12: 4; - - unsigned char channel_configuration_0_bit: 1; - unsigned char private_bit: 1; - unsigned char sampling_frequency_index: 4; - unsigned char profile: 2; - - unsigned char frame_length_0_to_1: 2; - unsigned char copyrignt_identification_start: 1; - unsigned char copyright_identification_bit: 1; - unsigned char home: 1; - unsigned char original_or_copy: 1; - unsigned char channel_configuration_1_to_2: 2; - - unsigned char frame_length_2_to_9: 8; - - unsigned char adts_buffer_fullness_0_to_4: 5; - unsigned char frame_length_10_to_12: 3; - - unsigned char number_of_raw_data_blocks_in_frame: 2; - unsigned char adts_buffer_fullness_5_to_10: 6; - }; - - SDResult* readResult = nullptr; void addReadJob(bool full = false); - void processReadJob(); + void processReadJob(bool wait = false); void resetDecoding(); + void buildFrameIndex(); + void freeFrameIndex(); + + ADTSTiming::FrameIndexEntry* frameIndex = nullptr; + size_t frameIndexCount = 0; + size_t frameIndexCapacity = 0; + FrameIndexQuality frameIndexQuality = INDEX_UNAVAILABLE; + uint64_t durationSourceFrames = 0; + uint64_t indexedSourceFrameEnd = 0; + mutable portMUX_TYPE timingMux = portMUX_INITIALIZER_UNLOCKED; + uint64_t elapsedSourceFrames = 0; + uint64_t decodedSourceFrame = 0; + uint64_t seekTargetSourceFrame = 0; + uint64_t elapsedFrameRemainder = 0; + uint32_t sourceSampleRate = 0; + uint8_t sourceChannels = 0; + uint8_t adtsChannelConfiguration = 0; + uint32_t firstFrameOffset = 0; + bool readEof = false; + bool discardPendingRead = false; + bool rewindAttempted = false; bool repeat = false; void (*songDoneCallback)() = nullptr; diff --git a/src/AudioLib/SpeedModifier.cpp b/src/AudioLib/SpeedModifier.cpp index 34b03ef..446e39b 100644 --- a/src/AudioLib/SpeedModifier.cpp +++ b/src/AudioLib/SpeedModifier.cpp @@ -51,3 +51,8 @@ void SpeedModifier::fillBuffer(){ void SpeedModifier::setSource(Source* source){ SpeedModifier::source = source; } + +void SpeedModifier::reset(){ + dataBuffer->clear(); + remainder = 0; +} diff --git a/src/AudioLib/SpeedModifier.h b/src/AudioLib/SpeedModifier.h index 08c5cc7..b056bcc 100644 --- a/src/AudioLib/SpeedModifier.h +++ b/src/AudioLib/SpeedModifier.h @@ -29,6 +29,7 @@ class SpeedModifier : public Generator { void setSpeed(float speed); void setSource(Source* source); + void reset(); private: Source *source = nullptr; diff --git a/src/AudioLib/Systems/MixSystem.cpp b/src/AudioLib/Systems/MixSystem.cpp index 7beecd4..fc9313d 100644 --- a/src/AudioLib/Systems/MixSystem.cpp +++ b/src/AudioLib/Systems/MixSystem.cpp @@ -133,7 +133,7 @@ bool MixSystem::openChannel(uint8_t channel, const fs::File& file){ } newSource->setRepeat(true); while(!newSource->isReadReady()) Sched.loop(0); - requests[requestIndex].value = reinterpret_cast(newSource); + requests[requestIndex].value = reinterpret_cast(newSource); if(sendRequest(requestIndex)) return true; @@ -200,7 +200,7 @@ void MixSystem::clearRequests(){ while(queue.count()){ if(!queue.receive(&index)) break; if(index < requestCapacity && requestUsed[index] && requests[index].type == MixRequest::OPEN){ - delete reinterpret_cast(requests[index].value); + delete reinterpret_cast(uintptr_t(requests[index].value)); } releaseRequest(index); } @@ -249,10 +249,10 @@ void MixSystem::audioThread(Task* task){ system->_setEffectIntensity(request.channel, request.slot, request.value); break; case MixRequest::SET_INFO: - system->_setInfoGenerator(request.channel, reinterpret_cast(request.value)); + system->_setInfoGenerator(request.channel, reinterpret_cast(uintptr_t(request.value))); break; case MixRequest::SET_SEEK: - system->_seekChannel(request.channel, static_cast(request.value)); + system->_seekChannel(request.channel, request.value); break; case MixRequest::RECORD: if(request.value == system->isRecording()) break; @@ -263,7 +263,7 @@ void MixSystem::audioThread(Task* task){ } break; case MixRequest::OPEN: - system->_openChannel(request.channel, reinterpret_cast(request.value)); + system->_openChannel(request.channel, reinterpret_cast(uintptr_t(request.value))); break; } system->releaseRequest(requestIndex); @@ -321,15 +321,57 @@ uint16_t MixSystem::getDuration(uint8_t c){ uint16_t MixSystem::getElapsed(uint8_t c){ if(c >= 2) return 0; cleanupRetiredSources(); - if(seekPending[c] > 0){ - return seek[c]; - } sourceMutex.lock(); - uint16_t elapsed = source[c] ? source[c]->getElapsed() : 0; + uint16_t elapsed = 0; + if(source[c]){ + const uint64_t frames = seekPending[c] > 0 ? seekFrame[c] : source[c]->getElapsedSourceFrames(); + const uint32_t rate = source[c]->getSourceSampleRate(); + const uint64_t seconds = rate == 0 ? 0 : frames / rate; + elapsed = seconds > UINT16_MAX ? UINT16_MAX : uint16_t(seconds); + } sourceMutex.unlock(); return elapsed; } +uint64_t MixSystem::getDurationSourceFrames(uint8_t c){ + if(c >= 2) return 0; + cleanupRetiredSources(); + sourceMutex.lock(); + const uint64_t frames = source[c] ? source[c]->getDurationSourceFrames() : 0; + sourceMutex.unlock(); + return frames; +} + +uint64_t MixSystem::getElapsedSourceFrames(uint8_t c){ + if(c >= 2) return 0; + cleanupRetiredSources(); + sourceMutex.lock(); + const uint64_t frames = source[c] + ? (seekPending[c] > 0 ? seekFrame[c] : source[c]->getElapsedSourceFrames()) + : 0; + sourceMutex.unlock(); + return frames; +} + +uint32_t MixSystem::getSourceSampleRate(uint8_t c){ + if(c >= 2) return 0; + cleanupRetiredSources(); + sourceMutex.lock(); + const uint32_t rate = source[c] ? source[c]->getSourceSampleRate() : 0; + sourceMutex.unlock(); + return rate; +} + +SourceAAC::FrameIndexQuality MixSystem::getFrameIndexQuality(uint8_t c){ + if(c >= 2) return SourceAAC::INDEX_UNAVAILABLE; + cleanupRetiredSources(); + sourceMutex.lock(); + const SourceAAC::FrameIndexQuality quality = + source[c] ? source[c]->getFrameIndexQuality() : SourceAAC::INDEX_UNAVAILABLE; + sourceMutex.unlock(); + return quality; +} + bool MixSystem::hasChannel(uint8_t c){ if(c >= 2) return false; cleanupRetiredSources(); @@ -464,7 +506,7 @@ void MixSystem::setChannelInfo(uint8_t channel, InfoGenerator* channelInfoGen){ return; } - enqueueRequest({ MixRequest::SET_INFO, channel, 0, reinterpret_cast(channelInfoGen) }); + enqueueRequest({ MixRequest::SET_INFO, channel, 0, reinterpret_cast(channelInfoGen) }); } void MixSystem::pauseChannel(uint8_t channel){ @@ -480,23 +522,48 @@ void MixSystem::resumeChannel(uint8_t channel){ void MixSystem::seekChannel(uint8_t channel, uint16_t time){ if(channel >= 2) return; + const uint32_t rate = getSourceSampleRate(channel); + uint64_t frame; + if(!ADTSTiming::secondsToFrames(time, rate, frame)) return; + seekChannelSourceFrame(channel, frame); +} +bool MixSystem::seekChannelSourceFrame(uint8_t channel, uint64_t frame){ + if(channel >= 2) return false; if(!out->isRunning()){ - _seekChannel(channel, time); - return; + sourceMutex.lock(); + const bool success = source[channel] && source[channel]->seekSourceFrame(frame); + if(success && speed[channel]) speed[channel]->reset(); + sourceMutex.unlock(); + return success; } - seek[channel] = time; + sourceMutex.lock(); + if(!source[channel]){ + sourceMutex.unlock(); + return false; + } + seekFrame[channel] = frame; seekPending[channel]++; - if(!enqueueRequest({ MixRequest::SET_SEEK, channel, 0, time })) seekPending[channel]--; + sourceMutex.unlock(); + if(!enqueueRequest({ MixRequest::SET_SEEK, channel, 0, frame })){ + sourceMutex.lock(); + seekPending[channel]--; + sourceMutex.unlock(); + return false; + } + return true; } -void MixSystem::_seekChannel(uint8_t channel, uint16_t time){ +void MixSystem::_seekChannel(uint8_t channel, uint64_t frame){ if(channel > 1) return; + sourceMutex.lock(); if(seekPending[channel] > 0) seekPending[channel]--; - if(!source[channel]) return; - - source[channel]->seek(time, SeekSet); + SourceAAC* channelSource = source[channel]; + sourceMutex.unlock(); + if(channelSource && channelSource->seekSourceFrame(frame) && speed[channel]){ + speed[channel]->reset(); + } } bool MixSystem::isRecording(){ diff --git a/src/AudioLib/Systems/MixSystem.h b/src/AudioLib/Systems/MixSystem.h index 73cfc86..de04189 100644 --- a/src/AudioLib/Systems/MixSystem.h +++ b/src/AudioLib/Systems/MixSystem.h @@ -21,7 +21,7 @@ struct MixRequest { enum { ADD_SPEED, REMOVE_SPEED, SET_SPEED, SET_EFFECT, SET_EFFECT_INTENSITY, SET_INFO, SET_SEEK, RECORD, OPEN } type; uint8_t channel; uint8_t slot; - size_t value; + uint64_t value; }; class MixSystem { @@ -44,6 +44,10 @@ class MixSystem { uint16_t getDuration(uint8_t channel); uint16_t getElapsed(uint8_t channel); + uint64_t getDurationSourceFrames(uint8_t channel); + uint64_t getElapsedSourceFrames(uint8_t channel); + uint32_t getSourceSampleRate(uint8_t channel); + SourceAAC::FrameIndexQuality getFrameIndexQuality(uint8_t channel); bool hasChannel(uint8_t channel); uint8_t getVolume(uint8_t channel); uint8_t getMix(); @@ -65,6 +69,7 @@ class MixSystem { bool isChannelPaused(uint8_t channel); void seekChannel(uint8_t channel, uint16_t time); + bool seekChannelSourceFrame(uint8_t channel, uint64_t frame); void startRecording(); void stopRecording(); @@ -102,7 +107,7 @@ class MixSystem { void _setEffect(uint8_t channel, uint8_t slot, EffectType type); void _setEffectIntensity(uint8_t channel, uint8_t slot, uint8_t intensity); void _setInfoGenerator(uint8_t channel, InfoGenerator* generator); - void _seekChannel(uint8_t channel, uint16_t time); + void _seekChannel(uint8_t channel, uint64_t frame); void _startRecording(); void _stopRecording(); void _openChannel(uint8_t channel, SourceAAC* source); @@ -116,7 +121,7 @@ class MixSystem { static Effect* (* getEffect[EffectType::COUNT])(); - uint16_t seek[2]; + uint64_t seekFrame[2] = {}; int seekPending[2] = { 0 }; }; diff --git a/tests/adts_timing_self_check.cpp b/tests/adts_timing_self_check.cpp new file mode 100644 index 0000000..2297892 --- /dev/null +++ b/tests/adts_timing_self_check.cpp @@ -0,0 +1,52 @@ +#include "../src/AudioLib/ADTSTiming.h" +#include +#include +#include + +static void makeFrame(uint8_t* frame, size_t size, uint8_t sampleRateIndex = 6){ + assert(size >= 7 && size <= 8191); + memset(frame, 0, size); + frame[0] = 0xff; + frame[1] = 0xf1; + frame[2] = uint8_t((1u << 6) | (sampleRateIndex << 2)); + frame[3] = uint8_t((1u << 6) | ((size >> 11) & 0x03)); + frame[4] = uint8_t(size >> 3); + frame[5] = uint8_t((size & 0x07) << 5); +} + +int main(){ + static_assert(sizeof(ADTSTiming::FrameIndexEntry) == 8, "seek entry size changed"); + + uint8_t cbr[20]; + uint8_t vbr[33]; + makeFrame(cbr, sizeof(cbr)); + makeFrame(vbr, sizeof(vbr)); + + ADTSTiming::Header header; + assert(ADTSTiming::parseFrame(cbr, sizeof(cbr), header) == ADTSTiming::VALID); + assert(header.frameLength == sizeof(cbr)); + assert(header.sampleRate == 24000); + assert(header.channels == 1); + assert(header.sourceFrames == 1024); + assert(ADTSTiming::parseFrame(vbr, sizeof(vbr), header) == ADTSTiming::VALID); + assert(header.frameLength == sizeof(vbr)); + + cbr[0] = 0xfe; + assert(ADTSTiming::parseFrame(cbr, sizeof(cbr), header) == ADTSTiming::INVALID); + makeFrame(cbr, sizeof(cbr)); + assert(ADTSTiming::parseFrame(cbr, sizeof(cbr) - 1, header) == ADTSTiming::NEED_MORE); + + uint64_t frames; + assert(ADTSTiming::secondsToFrames(60, 24000, frames) && frames == 1440000); + assert(!ADTSTiming::secondsToFrames(UINT64_MAX, 24000, frames)); + + const ADTSTiming::FrameIndexEntry index[] = { + { 10, 0 }, + { 30, 1024 }, + { 63, 2048 } + }; + assert(ADTSTiming::findPreceding(index, 3, 0) == 0); + assert(ADTSTiming::findPreceding(index, 3, 1023) == 0); + assert(ADTSTiming::findPreceding(index, 3, 1024) == 1); + assert(ADTSTiming::findPreceding(index, 3, UINT64_MAX) == 2); +} From 6750262bad110fe3534ebcab54e20d9cc3e14250 Mon Sep 17 00:00:00 2001 From: Ryan Trauntvein Date: Fri, 21 Aug 2026 21:48:44 -0700 Subject: [PATCH 03/15] Fix AAC decode capacity and EOF callbacks Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- README.md | 2 +- src/AudioLib/ADTSTiming.h | 26 ++++++++++++++++++++++++++ src/AudioLib/SourceAAC.cpp | 24 ++++++++++++++++++------ src/AudioLib/SourceAAC.h | 1 + tests/adts_timing_self_check.cpp | 16 ++++++++++++++++ 5 files changed, 62 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index a7184d2..6bd91dc 100644 --- a/README.md +++ b/README.md @@ -54,7 +54,7 @@ ADTS indexing runs while a source is opened, before it is published to the audio thread. Each seek entry is 8 bytes. The bounded index uses at most 128 KiB per deck in PSRAM (16,384 frames), or 16 KiB without PSRAM (2,048 frames); the temporary scan cache is 4 KiB. The larger strict-frame decode buffers add -23 KiB per source over the previous buffers. `getFrameIndexQuality()` reports +35 KiB per source over the previous buffers. `getFrameIndexQuality()` reports whether the whole track is seekable; duration remains frame-counted even when the seek index reaches its cap. A `cm:esp32:jayd` build measured 1,026,334 bytes of flash and 44,464 bytes of static RAM: +312 bytes of flash and no diff --git a/src/AudioLib/ADTSTiming.h b/src/AudioLib/ADTSTiming.h index 597bffd..c132db8 100644 --- a/src/AudioLib/ADTSTiming.h +++ b/src/AudioLib/ADTSTiming.h @@ -25,6 +25,32 @@ struct FrameIndexEntry { uint32_t sourceFrame; }; +class EofNotification { +public: + bool take(){ + if(notified) return false; + notified = true; + return true; + } + + void reset(){ + notified = false; + } + +private: + bool notified = false; +}; + +inline bool requiredDecodeBytes(size_t rawBlocks, size_t bytesPerBlock, + size_t capacity, size_t& required){ + if(rawBlocks == 0 || bytesPerBlock == 0 || + rawBlocks > SIZE_MAX / bytesPerBlock){ + return false; + } + required = rawBlocks * bytesPerBlock; + return required <= capacity; +} + inline ParseResult parseHeader(const uint8_t* data, size_t size, Header& header){ static const uint32_t sampleRates[] = { 96000, 88200, 64000, 48000, 44100, 32000, 24000, diff --git a/src/AudioLib/SourceAAC.cpp b/src/AudioLib/SourceAAC.cpp index 3987797..6b8bdee 100644 --- a/src/AudioLib/SourceAAC.cpp +++ b/src/AudioLib/SourceAAC.cpp @@ -5,9 +5,8 @@ #define AAC_READ_BUFFER 1024 * 64 #define AAC_READ_CHUNK 1024 * 4 // should be bigger than min input #define AAC_DECODE_BUFFER 8192 -#define AAC_OUT_BUFFER 20480 +#define AAC_OUT_BUFFER 32768 #define AAC_MAX_DECODED_BYTES 8192 -#define AAC_MAX_MONO_BLOCK_BYTES 4096 #define AAC_INDEX_PSRAM_ENTRIES 16384 #define AAC_INDEX_INTERNAL_ENTRIES 2048 #define AAC_INDEX_READ_CHUNK 4096 @@ -93,6 +92,7 @@ void SourceAAC::close(){ sourceChannels = adtsChannelConfiguration = 0; readEof = false; discardPendingRead = false; + eofNotification.reset(); } SourceAAC::~SourceAAC(){ @@ -227,18 +227,29 @@ size_t SourceAAC::generate(int16_t* outBuffer){ processReadJob(); Profiler.end(); + Profiler.start("AAC decode"); while(dataBuffer.readAvailable() < BUFFER_SIZE){ ADTSTiming::Header adts; if(!prepareNextFrame(adts)) break; const size_t rawBlocks = adts.sourceFrames / 1024; - const size_t requiredBytes = - (rawBlocks - 1) * AAC_MAX_MONO_BLOCK_BYTES + AAC_MAX_DECODED_BYTES; - if(dataBuffer.writeAvailable() < requiredBytes) break; + size_t requiredBytes = 0; + if(!ADTSTiming::requiredDecodeBytes( + rawBlocks, AAC_MAX_DECODED_BYTES, + AAC_OUT_BUFFER, requiredBytes) || + dataBuffer.writeAvailable() < requiredBytes){ + Serial.println("SourceAAC: decoded frame exceeds output buffer"); + break; + } uint8_t* data = const_cast(fillBuffer.readData()); int bytesLeft = adts.frameLength; size_t decodedBlocks = 0; for(; decodedBlocks < rawBlocks; decodedBlocks++){ + if(dataBuffer.writeAvailable() < AAC_MAX_DECODED_BYTES){ + Serial.println("SourceAAC: insufficient decoder output space"); + AACFlushCodec(hAACDecoder); + break; + } int16_t* pcm = reinterpret_cast(dataBuffer.writeData()); const int ret = AACDecode(hAACDecoder, &data, &bytesLeft, pcm); if(ret){ @@ -299,7 +310,7 @@ size_t SourceAAC::generate(int16_t* outBuffer){ } if(samples == 0){ - if(songDoneCallback != nullptr) { + if(readEof && eofNotification.take() && songDoneCallback != nullptr) { songDoneCallback(); } if(repeat && !rewindAttempted){ @@ -399,6 +410,7 @@ bool SourceAAC::seekSourceFrame(uint64_t frame){ decodedSourceFrame = indexedFrame; seekTargetSourceFrame = frame; elapsedFrameRemainder = 0; + eofNotification.reset(); resetDecoding(); return true; } diff --git a/src/AudioLib/SourceAAC.h b/src/AudioLib/SourceAAC.h index 543234d..c0cdf83 100644 --- a/src/AudioLib/SourceAAC.h +++ b/src/AudioLib/SourceAAC.h @@ -89,6 +89,7 @@ class SourceAAC : public Source bool readEof = false; bool discardPendingRead = false; bool rewindAttempted = false; + ADTSTiming::EofNotification eofNotification; bool repeat = false; void (*songDoneCallback)() = nullptr; diff --git a/tests/adts_timing_self_check.cpp b/tests/adts_timing_self_check.cpp index 2297892..b9c9c65 100644 --- a/tests/adts_timing_self_check.cpp +++ b/tests/adts_timing_self_check.cpp @@ -49,4 +49,20 @@ int main(){ assert(ADTSTiming::findPreceding(index, 3, 1023) == 0); assert(ADTSTiming::findPreceding(index, 3, 1024) == 1); assert(ADTSTiming::findPreceding(index, 3, UINT64_MAX) == 2); + + size_t required = 0; + assert(ADTSTiming::requiredDecodeBytes(4, 8192, 32768, required)); + assert(required == 32768); + assert(!ADTSTiming::requiredDecodeBytes(4, 8192, 32767, required)); + assert(!ADTSTiming::requiredDecodeBytes(SIZE_MAX, 8192, SIZE_MAX, required)); + + ADTSTiming::EofNotification eof; + assert(eof.take()); // first non-repeat EOF generate + assert(!eof.take()); // repeated generate after the same EOF + eof.reset(); // repeat rewind + assert(eof.take()); + eof.reset(); // seek after EOF + assert(eof.take()); + eof.reset(); // reopen + assert(eof.take()); } From a664b0491da80ed6a7cab676316e4957c20c1ce3 Mon Sep 17 00:00:00 2001 From: Ryan Trauntvein Date: Fri, 21 Aug 2026 20:42:00 -0700 Subject: [PATCH 04/15] Make deck rate control deterministic Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> (cherry picked from commit 2afe71c3526e0966f08802f83cc87e258f195094) --- README.md | 9 +++ src/AudioLib/SpeedModifier.cpp | 79 +++++++++++++++++++++++--- src/AudioLib/SpeedModifier.h | 36 +++++++++--- src/AudioLib/Systems/MixSystem.cpp | 59 +++++++++++++++++++ src/AudioLib/Systems/MixSystem.h | 7 ++- tests/SpeedModifierSelfCheck.cpp | 79 ++++++++++++++++++++++++++ tests/run-speed-modifier-self-check.sh | 14 +++++ tests/stubs/Arduino.h | 9 +++ tests/stubs/Buffer/DataBuffer.h | 57 +++++++++++++++++++ tests/stubs/FS.h | 12 ++++ 10 files changed, 344 insertions(+), 17 deletions(-) create mode 100644 tests/SpeedModifierSelfCheck.cpp create mode 100755 tests/run-speed-modifier-self-check.sh create mode 100644 tests/stubs/Arduino.h create mode 100644 tests/stubs/Buffer/DataBuffer.h create mode 100644 tests/stubs/FS.h diff --git a/README.md b/README.md index 6bd91dc..ba1c184 100644 --- a/README.md +++ b/README.md @@ -72,6 +72,15 @@ To compile the binary, and upload it according to the port set in CMakeLists.txt ```cmake --build . --target CMBuild``` in the cmake directory. + +## Deck rate control + +`SpeedModifier` exposes a bounded Q16.16 rate (`0.5x` to `1.5x`, with `65536` +as neutral). The legacy 0-255 API maps `127` to neutral. Rate changes ramp over +at most 256 output samples and use linear interpolation without allocating in +`generate()`. This is resampling, so pitch changes with rate; key lock and time +stretching are not provided. + # Used libraries and copyright notices [See NOTICE](https://github.com/CircuitMess/JayD-Library/blob/master/NOTICE.md) diff --git a/src/AudioLib/SpeedModifier.cpp b/src/AudioLib/SpeedModifier.cpp index 446e39b..ab2b7e2 100644 --- a/src/AudioLib/SpeedModifier.cpp +++ b/src/AudioLib/SpeedModifier.cpp @@ -10,20 +10,29 @@ SpeedModifier::~SpeedModifier() noexcept{ } size_t SpeedModifier::generate(int16_t *outBuffer){ + if(source == nullptr) return 0; + if(dataBuffer->readAvailable() < (float) BUFFER_SIZE * 2){ fillBuffer(); } - float sourcePtr = remainder; size_t destinationPtr = 0; + const size_t availableSamples = dataBuffer->readAvailable() / (BYTES_PER_SAMPLE * NUM_CHANNELS); + const int16_t* samples = reinterpret_cast(dataBuffer->readData()); + + while(destinationPtr < BUFFER_SAMPLES && (sourcePosition >> 16) + 1 < availableSamples){ + const size_t sourceIndex = sourcePosition >> 16; + const int32_t fraction = (sourcePosition & 0xffff) >> 1; + const int32_t difference = int32_t(samples[sourceIndex + 1]) - samples[sourceIndex]; + outBuffer[destinationPtr++] = samples[sourceIndex] + difference * fraction / 32768; - while(destinationPtr < BUFFER_SAMPLES && floor(sourcePtr) < dataBuffer->readAvailable()){ - outBuffer[destinationPtr++] = reinterpret_cast(dataBuffer->readData())[(int) floor(sourcePtr)]; - sourcePtr += speed; + advanceRate(); + sourcePosition += currentRate; } - remainder = sourcePtr - floor(sourcePtr); - dataBuffer->readMove(floor(sourcePtr) * BYTES_PER_SAMPLE * NUM_CHANNELS); + const size_t consumedSamples = sourcePosition >> 16; + sourcePosition &= 0xffff; + dataBuffer->readMove(consumedSamples * BYTES_PER_SAMPLE * NUM_CHANNELS); return destinationPtr; } @@ -34,22 +43,74 @@ int SpeedModifier::available(){ } void SpeedModifier::setModifier(uint8_t modifier){ - speed = (float) modifier * (1.0f / 255.0f) + 0.5f; + setRate(modifierToRate(modifier)); } void SpeedModifier::setSpeed(float speed){ - this->speed = speed; + if(!(speed > 0.5f)){ + setRate(MinRate); + }else if(speed >= 1.5f){ + setRate(MaxRate); + }else{ + setRate(static_cast(speed * RateScale + 0.5f)); + } +} + +void SpeedModifier::setRate(Rate rate){ + if(rate < MinRate) rate = MinRate; + if(rate > MaxRate) rate = MaxRate; + requestedRate = rate; +} + +SpeedModifier::Rate SpeedModifier::getRate() const{ + return requestedRate; +} + +SpeedModifier::Rate SpeedModifier::getCurrentRate() const{ + return currentRate; +} + +void SpeedModifier::nudgeRate(int32_t amount){ + const int64_t nudged = int64_t(requestedRate) + amount; + if(nudged <= MinRate){ + setRate(MinRate); + }else if(nudged >= MaxRate){ + setRate(MaxRate); + }else{ + setRate(static_cast(nudged)); + } +} + +SpeedModifier::Rate SpeedModifier::modifierToRate(uint8_t modifier){ + if(modifier <= 127){ + return MinRate + (uint32_t(modifier) * (NeutralRate - MinRate) + 63) / 127; + } + return NeutralRate + (uint32_t(modifier - 127) * (MaxRate - NeutralRate) + 64) / 128; +} + +void SpeedModifier::advanceRate(){ + static constexpr Rate step = RateScale / BUFFER_SAMPLES; + if(currentRate < requestedRate){ + const Rate remaining = requestedRate - currentRate; + currentRate += remaining < step ? remaining : step; + }else if(currentRate > requestedRate){ + const Rate remaining = currentRate - requestedRate; + currentRate -= remaining < step ? remaining : step; + } } void SpeedModifier::fillBuffer(){ - while(dataBuffer->readAvailable() < BUFFER_SIZE * 2){ + while(source && dataBuffer->readAvailable() < BUFFER_SIZE * 2 && dataBuffer->writeAvailable() >= BUFFER_SIZE){ size_t generated = source->generate(reinterpret_cast(dataBuffer->writeData())); + if(generated == 0) break; dataBuffer->writeMove(generated * BYTES_PER_SAMPLE * NUM_CHANNELS); } } void SpeedModifier::setSource(Source* source){ SpeedModifier::source = source; + dataBuffer->clear(); + sourcePosition = 0; } void SpeedModifier::reset(){ diff --git a/src/AudioLib/SpeedModifier.h b/src/AudioLib/SpeedModifier.h index b056bcc..e9db24d 100644 --- a/src/AudioLib/SpeedModifier.h +++ b/src/AudioLib/SpeedModifier.h @@ -9,25 +9,44 @@ class SpeedModifier : public Generator { public: + typedef uint32_t Rate; + + // Unsigned Q16.16 input-samples per output-sample. Setters clamp to + // 0.5x-1.5x; positive halfway cases round up to the nearest Rate unit. + static constexpr Rate RateScale = 1UL << 16; + static constexpr Rate MinRate = RateScale / 2; + static constexpr Rate NeutralRate = RateScale; + static constexpr Rate MaxRate = RateScale + RateScale / 2; SpeedModifier(Source* source); - ~SpeedModifier(); + ~SpeedModifier() noexcept; size_t generate(int16_t* outBuffer) override; int available() override; /** - * Set speed multiplier as modifier. Will get mapped from 0-255 to 0.5 - 2.0 - * @param modifier + * Set the requested rate using the legacy 0-255 control. The two halves + * map linearly to 0.5x-1.0x and 1.0x-1.5x, with 127 exactly neutral. */ void setModifier(uint8_t modifier); /** - * Set speed multiplier. - * @param speed + * Compatibility float setter. Prefer setRate() for exact control. */ void setSpeed(float speed); + /** + * Set/get the requested Q16.16 rate. getCurrentRate() reports the + * click-reduction ramp position, which converges within 256 samples. + * + * This is resampling: playback pitch changes with rate. It does not + * provide key lock or time stretching. + */ + void setRate(Rate rate); + Rate getRate() const; + Rate getCurrentRate() const; + void nudgeRate(int32_t amount); + void setSource(Source* source); void reset(); @@ -35,9 +54,12 @@ class SpeedModifier : public Generator { Source *source = nullptr; DataBuffer* dataBuffer = nullptr; - float speed = 1; - float remainder = 0; + Rate requestedRate = NeutralRate; + Rate currentRate = NeutralRate; + uint32_t sourcePosition = 0; + static Rate modifierToRate(uint8_t modifier); + void advanceRate(); void fillBuffer(); }; diff --git a/src/AudioLib/Systems/MixSystem.cpp b/src/AudioLib/Systems/MixSystem.cpp index fc9313d..d8a6b5d 100644 --- a/src/AudioLib/Systems/MixSystem.cpp +++ b/src/AudioLib/Systems/MixSystem.cpp @@ -242,6 +242,13 @@ void MixSystem::audioThread(Task* task){ case MixRequest::SET_SPEED: system->_setSpeed(request.channel, request.value); break; + case MixRequest::SET_RATE: + system->_setRate(request.channel, request.value); + break; + case MixRequest::NUDGE_RATE: + system->_nudgeRate(request.channel, request.slot ? + -static_cast(request.value) : static_cast(request.value)); + break; case MixRequest::SET_EFFECT: system->_setEffect(request.channel, request.slot, static_cast(request.value)); break; @@ -429,6 +436,38 @@ void MixSystem::setSpeed(uint8_t channel, uint8_t speed){ enqueueRequest({ MixRequest::SET_SPEED, channel, 0, speed }); } +void MixSystem::setRate(uint8_t channel, SpeedModifier::Rate rate){ + if(!out->isRunning()){ + _setRate(channel, rate); + return; + } + + enqueueRequest({ MixRequest::SET_RATE, channel, 0, rate }); +} + +SpeedModifier::Rate MixSystem::getRate(uint8_t channel){ + if(channel >= 2) return SpeedModifier::NeutralRate; + sourceMutex.lock(); + const SpeedModifier::Rate rate = speed[channel] ? speed[channel]->getRate() : SpeedModifier::NeutralRate; + sourceMutex.unlock(); + return rate; +} + +void MixSystem::nudgeRate(uint8_t channel, int32_t amount){ + const int32_t maxNudge = SpeedModifier::MaxRate - SpeedModifier::MinRate; + if(amount < -maxNudge) amount = -maxNudge; + if(amount > maxNudge) amount = maxNudge; + + if(!out->isRunning()){ + _nudgeRate(channel, amount); + return; + } + + const bool negative = amount < 0; + const uint32_t magnitude = negative ? static_cast(-amount) : amount; + enqueueRequest({ MixRequest::NUDGE_RATE, channel, negative, magnitude }); +} + void MixSystem::setEffect(uint8_t channel, uint8_t slot, EffectType type){ if(!out->isRunning()){ _setEffect(channel, slot, type); @@ -449,20 +488,40 @@ void MixSystem::setEffectIntensity(uint8_t channel, uint8_t slot, uint8_t intens void MixSystem::_addSpeed(uint8_t c){ if(c >= 2 || !effector[c] || !source[c] || speed[c]) return; + sourceMutex.lock(); auto speed = this->speed[c] = new SpeedModifier(source[c]); effector[c]->setSource(speed); + sourceMutex.unlock(); } void MixSystem::_removeSpeed(uint8_t c){ if(c >= 2 || !effector[c] || !speed[c]) return; + sourceMutex.lock(); effector[c]->setSource(source[c]); delete speed[c]; speed[c] = nullptr; + sourceMutex.unlock(); } void MixSystem::_setSpeed(uint8_t c, uint8_t modifier){ if(c >= 2 || !this->speed[c]) return; + sourceMutex.lock(); this->speed[c]->setModifier(modifier); + sourceMutex.unlock(); +} + +void MixSystem::_setRate(uint8_t c, SpeedModifier::Rate rate){ + if(c >= 2 || !speed[c]) return; + sourceMutex.lock(); + speed[c]->setRate(rate); + sourceMutex.unlock(); +} + +void MixSystem::_nudgeRate(uint8_t c, int32_t amount){ + if(c >= 2 || !speed[c]) return; + sourceMutex.lock(); + speed[c]->nudgeRate(amount); + sourceMutex.unlock(); } void MixSystem::_setEffect(uint8_t c, uint8_t s, EffectType type){ diff --git a/src/AudioLib/Systems/MixSystem.h b/src/AudioLib/Systems/MixSystem.h index de04189..b05c8cf 100644 --- a/src/AudioLib/Systems/MixSystem.h +++ b/src/AudioLib/Systems/MixSystem.h @@ -18,7 +18,7 @@ #include "../OutputWAV.h" struct MixRequest { - enum { ADD_SPEED, REMOVE_SPEED, SET_SPEED, SET_EFFECT, SET_EFFECT_INTENSITY, SET_INFO, SET_SEEK, RECORD, OPEN } type; + enum { ADD_SPEED, REMOVE_SPEED, SET_SPEED, SET_RATE, NUDGE_RATE, SET_EFFECT, SET_EFFECT_INTENSITY, SET_INFO, SET_SEEK, RECORD, OPEN } type; uint8_t channel; uint8_t slot; uint64_t value; @@ -58,6 +58,9 @@ class MixSystem { void addSpeed(uint8_t channel); void removeSpeed(uint8_t channel); void setSpeed(uint8_t channel, uint8_t speed); + void setRate(uint8_t channel, SpeedModifier::Rate rate); + SpeedModifier::Rate getRate(uint8_t channel); + void nudgeRate(uint8_t channel, int32_t amount); void setEffect(uint8_t channel, uint8_t slot, EffectType type); void setEffectIntensity(uint8_t channel, uint8_t slot, uint8_t intensity); @@ -104,6 +107,8 @@ class MixSystem { void _addSpeed(uint8_t channel); void _removeSpeed(uint8_t channel); void _setSpeed(uint8_t channel, uint8_t speed); + void _setRate(uint8_t channel, SpeedModifier::Rate rate); + void _nudgeRate(uint8_t channel, int32_t amount); void _setEffect(uint8_t channel, uint8_t slot, EffectType type); void _setEffectIntensity(uint8_t channel, uint8_t slot, uint8_t intensity); void _setInfoGenerator(uint8_t channel, InfoGenerator* generator); diff --git a/tests/SpeedModifierSelfCheck.cpp b/tests/SpeedModifierSelfCheck.cpp new file mode 100644 index 0000000..c515c16 --- /dev/null +++ b/tests/SpeedModifierSelfCheck.cpp @@ -0,0 +1,79 @@ +#include + +#include +#include +#include + +class FakeSource : public Source { +public: + FakeSource(int16_t first, int16_t increment) : next(first), increment(increment){} + + size_t generate(int16_t* outBuffer) override{ + for(size_t i = 0; i < 256; i++){ + outBuffer[i] = next; + next += increment; + } + return 256; + } + + int available() override{ + return 4096; + } + + uint16_t getDuration() override{ + return 1; + } + + uint16_t getElapsed() override{ + return 0; + } + + void seek(uint16_t, fs::SeekMode) override{} + void close() override{} + +private: + int16_t next; + int16_t increment; +}; + +int main(){ + int16_t output[256] = {}; + FakeSource ramp(0, 1); + SpeedModifier speed(&ramp); + + assert(speed.getRate() == SpeedModifier::NeutralRate); + assert(speed.getCurrentRate() == SpeedModifier::NeutralRate); + assert(speed.generate(output) == 256); + for(size_t i = 0; i < 256; i++) assert(output[i] == static_cast(i)); + + speed.setRate(0); + assert(speed.getRate() == SpeedModifier::MinRate); + assert(speed.generate(output) == 256); + assert(speed.getCurrentRate() == SpeedModifier::MinRate); + + speed.setRate(UINT32_MAX); + assert(speed.getRate() == SpeedModifier::MaxRate); + assert(speed.generate(output) == 256); + assert(speed.getCurrentRate() == SpeedModifier::MaxRate); + + speed.setModifier(127); + assert(speed.getRate() == SpeedModifier::NeutralRate); + speed.setModifier(0); + assert(speed.getRate() == SpeedModifier::MinRate); + speed.setModifier(255); + assert(speed.getRate() == SpeedModifier::MaxRate); + speed.nudgeRate(INT32_MIN); + assert(speed.getRate() == SpeedModifier::MinRate); + speed.nudgeRate(INT32_MAX); + assert(speed.getRate() == SpeedModifier::MaxRate); + + FakeSource first(100, 0); + FakeSource replacement(200, 0); + SpeedModifier swapped(&first); + swapped.setRate(SpeedModifier::MaxRate); + assert(swapped.generate(output) == 256); + swapped.setSource(&replacement); + assert(swapped.getRate() == SpeedModifier::MaxRate); + assert(swapped.generate(output) == 256); + for(size_t i = 0; i < 256; i++) assert(output[i] == 200); +} diff --git a/tests/run-speed-modifier-self-check.sh b/tests/run-speed-modifier-self-check.sh new file mode 100755 index 0000000..dc0dba5 --- /dev/null +++ b/tests/run-speed-modifier-self-check.sh @@ -0,0 +1,14 @@ +#!/bin/sh +set -eu + +repo=$(CDPATH= cd -- "$(dirname -- "$0")/.." && pwd) +binary="${TMPDIR:-/tmp}/jayd-speed-modifier-self-check" +trap 'rm -f "$binary"' EXIT + +c++ -std=c++11 -Wall -Wextra -Werror \ + -I"$repo/tests/stubs" -I"$repo/src" \ + "$repo/tests/SpeedModifierSelfCheck.cpp" \ + "$repo/src/AudioLib/SpeedModifier.cpp" \ + "$repo/src/AudioLib/Source.cpp" \ + -o "$binary" +"$binary" diff --git a/tests/stubs/Arduino.h b/tests/stubs/Arduino.h new file mode 100644 index 0000000..596a5d6 --- /dev/null +++ b/tests/stubs/Arduino.h @@ -0,0 +1,9 @@ +#ifndef ARDUINO_H +#define ARDUINO_H + +#include +#include + +using std::size_t; + +#endif diff --git a/tests/stubs/Buffer/DataBuffer.h b/tests/stubs/Buffer/DataBuffer.h new file mode 100644 index 0000000..cebedcc --- /dev/null +++ b/tests/stubs/Buffer/DataBuffer.h @@ -0,0 +1,57 @@ +#ifndef DATABUFFER_H +#define DATABUFFER_H + +#include +#include +#include +#include +#include + +class DataBuffer { +public: + DataBuffer(size_t size, bool = false) : buffer(size){} + + size_t readAvailable(){ + return writeCursor - readCursor; + } + + bool readMove(size_t amount){ + if(readCursor + amount > writeCursor) return false; + readCursor += amount; + return true; + } + + const uint8_t* readData(){ + return buffer.data() + readCursor; + } + + size_t writeAvailable(){ + return readCursor + buffer.size() - writeCursor; + } + + bool writeMove(size_t amount){ + if(writeCursor + amount > buffer.size()) return false; + writeCursor += amount; + return true; + } + + uint8_t* writeData(){ + const size_t left = readAvailable(); + if(readCursor != 0 && left != 0) std::memmove(buffer.data(), buffer.data() + readCursor, left); + readCursor = 0; + writeCursor = left; + return buffer.data() + writeCursor; + } + + void clear(){ + readCursor = 0; + writeCursor = 0; + } + +private: + std::vector buffer; + size_t readCursor = 0; + size_t writeCursor = 0; +}; + +#endif diff --git a/tests/stubs/FS.h b/tests/stubs/FS.h new file mode 100644 index 0000000..b85a422 --- /dev/null +++ b/tests/stubs/FS.h @@ -0,0 +1,12 @@ +#ifndef FS_H +#define FS_H + +namespace fs { +enum SeekMode { + SeekSet, + SeekCur, + SeekEnd +}; +} + +#endif From d6457a9b5a20438e1726775a66ff3958f8b48d93 Mon Sep 17 00:00:00 2001 From: Ryan Trauntvein Date: Fri, 21 Aug 2026 20:44:48 -0700 Subject: [PATCH 05/15] Harden post-mix WAV recording Add explicit recording status and errors, detect short/failed SD writes without blocking playback, and finalize WAV headers through the existing scheduler before closing files.\n\nCo-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> (cherry picked from commit 6caffb569ced2021a97ac9a26ba609375bf6acd3) --- src/AudioLib/OutputWAV.cpp | 275 +++++++++++++++++++---------- src/AudioLib/OutputWAV.h | 42 ++++- src/AudioLib/Systems/MixSystem.cpp | 136 +++++++++++--- src/AudioLib/Systems/MixSystem.h | 28 ++- src/AudioLib/WavHeader.h | 48 +++++ src/Services/SDScheduler.cpp | 12 +- src/Services/SDScheduler.h | 4 +- tests/wav_header_selfcheck.cpp | 29 +++ 8 files changed, 441 insertions(+), 133 deletions(-) create mode 100644 src/AudioLib/WavHeader.h create mode 100644 tests/wav_header_selfcheck.cpp diff --git a/src/AudioLib/OutputWAV.cpp b/src/AudioLib/OutputWAV.cpp index 520e594..04d5cc4 100644 --- a/src/AudioLib/OutputWAV.cpp +++ b/src/AudioLib/OutputWAV.cpp @@ -1,25 +1,7 @@ #include "OutputWAV.h" -#include #include "../AudioSetup.hpp" #include "../PerfMon.h" - -struct WavHeader{ - char RIFF[4]; - uint32_t chunkSize; - char WAVE[4]; - char fmt[3]; - uint32_t fmtSize; - uint16_t audioFormat; - uint16_t numChannels; - uint32_t sampleRate; - uint32_t byteRate; // == SampleRate * NumChannels * BitsPerSample/8 - uint16_t blockAlign; // == NumChannels * BitsPerSample/8 - uint16_t bitsPerSample; - char data[4]; - uint32_t dataSize; // == NumSamples * NumChannels * BitsPerSample/8 -}; - OutputWAV::OutputWAV(){ freeBuffers.reserve(OUTWAV_BUFCOUNT); for(int i = 0; i < OUTWAV_BUFCOUNT; i++){ @@ -33,6 +15,10 @@ OutputWAV::OutputWAV(const fs::File& file) : OutputWAV(){ } OutputWAV::~OutputWAV(){ + delete finalizeResult; + for(auto result : writeResult){ + delete result; + } for(auto& outBuffer : outBuffers){ delete outBuffer; } @@ -48,71 +34,175 @@ void OutputWAV::setFile(const fs::File& file){ void OutputWAV::output(size_t numSamples){ Profiler.start("WAV write process"); - processWriteJob(); + service(); Profiler.end(); - while(freeBuffers.empty()){ - processWriteJob(); + const size_t size = numSamples * NUM_CHANNELS * BYTES_PER_SAMPLE; + if(error != RecordingError::NONE || finalizeStage != FinalizeStage::ACTIVE){ + droppedBytes += size; + return; } - DataBuffer* buffer = outBuffers[freeBuffers.front()]; - - size_t size = numSamples * NUM_CHANNELS * BYTES_PER_SAMPLE; - dataLength += size; + if(freeBuffers.empty()){ + droppedBytes += size; + fail(RecordingError::BUFFER_OVERRUN); + return; + } + DataBuffer* buffer = outBuffers[freeBuffers.front()]; memcpy(buffer->writeData(), this->inBuffer, size); buffer->writeMove(size); if(buffer->readAvailable() >= OUTWAV_WRITESIZE){ Profiler.start("WAV write add"); - addWriteJob(); + if(!addWriteJob()){ + droppedBytes += buffer->readAvailable(); + buffer->clear(); + fail(RecordingError::QUEUE_FULL); + } Profiler.end(); } } void OutputWAV::init(){ - dataLength = 0; + if(!prepared && file) begin(file); + if(!prepared || !file || finalizeStage != FinalizeStage::ACTIVE){ + Serial.println("Output file not open"); + fail(RecordingError::OPEN_FAILED); + } +} + +void OutputWAV::deinit(){ + finish(); +} + +bool OutputWAV::begin(const fs::File& outputFile){ + if(finalizeStage != FinalizeStage::DONE || hasPendingWrites()) return false; + + file = outputFile; + bytesWritten = 0; + droppedBytes = 0; + error = RecordingError::NONE; + prepared = false; + freeBuffers.clear(); + for(uint8_t i = 0; i < OUTWAV_BUFCOUNT; i++){ + outBuffers[i]->clear(); + writePending[i] = false; + writeSize[i] = 0; + freeBuffers.push_back(i); + } if(!file){ - Serial.println("Output file not open"); - return; + fail(RecordingError::OPEN_FAILED); + return false; } - writeHeaderWAV(0); + header = makeWavHeader(0, NUM_CHANNELS, SAMPLE_RATE, BYTES_PER_SAMPLE); + if(!writeInitialHeader()) return false; + prepared = true; + finalizeStage = FinalizeStage::ACTIVE; + return true; } -void OutputWAV::deinit(){ - Serial.println("Stopping WAV"); - if(!freeBuffers.empty() && outBuffers[freeBuffers.front()]->readAvailable() > 0){ - Serial.println("Writing last buffer"); - addWriteJob(); +void OutputWAV::finish(){ + if(finalizeStage != FinalizeStage::ACTIVE || !prepared) return; + + if(!freeBuffers.empty()){ + DataBuffer* buffer = outBuffers[freeBuffers.front()]; + if(buffer->readAvailable() > 0){ + if(error != RecordingError::NONE){ + droppedBytes += buffer->readAvailable(); + buffer->clear(); + }else if(!addWriteJob()){ + droppedBytes += buffer->readAvailable(); + buffer->clear(); + fail(RecordingError::QUEUE_FULL); + } + } + } + finalizeStage = FinalizeStage::DRAIN; +} + +void OutputWAV::service(){ + processWriteJob(); + if(finalizeStage == FinalizeStage::DONE || + finalizeStage == FinalizeStage::ACTIVE || + (finalizeStage == FinalizeStage::DRAIN && hasPendingWrites())) return; + + if(finalizeStage == FinalizeStage::DRAIN){ + header = makeWavHeader(bytesWritten, NUM_CHANNELS, SAMPLE_RATE, BYTES_PER_SAMPLE); + if(!queueFinalizeJob(SDJob::SD_SEEK)){ + fail(RecordingError::FINALIZE_FAILED); + finalizeStage = FinalizeStage::DONE; + }else{ + finalizeStage = FinalizeStage::SEEK; + } + return; } - Serial.println("Waiting jobs"); + if(finalizeResult == nullptr) return; + const bool success = finalizeResult->error == 0 && + (finalizeStage == FinalizeStage::SEEK || finalizeResult->size == sizeof(WavHeader)); + delete finalizeResult; + finalizeResult = nullptr; + if(!success){ + fail(RecordingError::FINALIZE_FAILED); + finalizeStage = FinalizeStage::DONE; + return; + } - while(freeBuffers.size() != OUTWAV_BUFCOUNT){ - processWriteJob(); + if(finalizeStage == FinalizeStage::SEEK){ + if(!queueFinalizeJob(SDJob::SD_WRITE)){ + fail(RecordingError::FINALIZE_FAILED); + finalizeStage = FinalizeStage::DONE; + }else{ + finalizeStage = FinalizeStage::HEADER; + } + }else{ + finalizeStage = FinalizeStage::DONE; + prepared = false; } +} + +bool OutputWAV::isFinalized() const{ + return finalizeStage == FinalizeStage::DONE; +} - Serial.println("Writing header"); +RecordingError OutputWAV::getError() const{ + return error; +} + +uint32_t OutputWAV::getBytesWritten() const{ + return bytesWritten; +} - writeHeaderWAV(dataLength); +uint32_t OutputWAV::getDroppedBytes() const{ + return droppedBytes; } -void OutputWAV::addWriteJob(){ - if(freeBuffers.empty()) return; - uint8_t i = freeBuffers.front(); +uint32_t OutputWAV::getDurationMs() const{ + const uint32_t byteRate = SAMPLE_RATE * NUM_CHANNELS * BYTES_PER_SAMPLE; + return byteRate == 0 ? 0 : static_cast(bytesWritten) * 1000 / byteRate; +} - Sched.addJob(new SDJob{ +bool OutputWAV::addWriteJob(){ + if(freeBuffers.empty()) return false; + const uint8_t i = freeBuffers.front(); + const size_t size = outBuffers[i]->readAvailable(); + if(size == 0) return true; + + if(!Sched.addJob(new SDJob{ .type = SDJob::SD_WRITE, .file = file, - .size = outBuffers[i]->readAvailable(), + .size = size, .buffer = const_cast(outBuffers[i]->readData()), .result = &writeResult[i] - }); + })) return false; freeBuffers.erase(freeBuffers.begin()); writePending[i] = true; + writeSize[i] = size; + return true; } void OutputWAV::processWriteJob(){ @@ -120,60 +210,67 @@ void OutputWAV::processWriteJob(){ if(!writePending[i]) continue; if(writeResult[i] == nullptr) continue; - outBuffers[i]->clear(); + const size_t actual = writeResult[i]->size > writeSize[i] + ? writeSize[i] + : writeResult[i]->size; + bytesWritten += actual; + if(writeResult[i]->error != 0 || actual != writeSize[i]){ + droppedBytes += writeSize[i] - actual; + fail(RecordingError::WRITE_FAILED); + } + outBuffers[i]->clear(); delete writeResult[i]; writeResult[i] = nullptr; - writePending[i] = false; + writeSize[i] = 0; freeBuffers.push_back(i); } } -void OutputWAV::writeHeaderWAV(size_t size){ - WavHeader header; - memcpy(header.RIFF, "RIFF", 4); - header.chunkSize = size + 36; - memcpy(header.WAVE, "WAVE", 4); - memcpy(header.fmt, "fmt ", 4); - header.fmtSize = 16; - header.audioFormat = 1; //PCM - header.numChannels = NUM_CHANNELS; //2 channels - header.sampleRate = SAMPLE_RATE; - header.byteRate = SAMPLE_RATE * NUM_CHANNELS * BYTES_PER_SAMPLE; - header.blockAlign = NUM_CHANNELS * BYTES_PER_SAMPLE; - header.bitsPerSample = BYTES_PER_SAMPLE * 8; - memcpy(header.data, "data", 4); - header.dataSize = size; - - Sched.addJob(new SDJob { - .type = SDJob::SD_SEEK, - .file = file, - .size = 0, - .buffer = nullptr, - .result = nullptr - }); +bool OutputWAV::hasPendingWrites() const{ + for(bool pending : writePending){ + if(pending) return true; + } + return false; +} - Sched.addJob(new SDJob { - .type = SDJob::SD_SEEK, - .file = file, - .size = 0, - .buffer = nullptr, - .result = nullptr - }); +bool OutputWAV::writeInitialHeader(){ + if(!queueFinalizeJob(SDJob::SD_SEEK)){ + fail(RecordingError::QUEUE_FULL); + return false; + } + while(finalizeResult == nullptr) Sched.loop(0); + const bool seekSuccess = finalizeResult->error == 0; + delete finalizeResult; + finalizeResult = nullptr; + if(!seekSuccess){ + fail(RecordingError::WRITE_FAILED); + return false; + } + + if(!queueFinalizeJob(SDJob::SD_WRITE)){ + fail(RecordingError::QUEUE_FULL); + return false; + } + while(finalizeResult == nullptr) Sched.loop(0); + const bool writeSuccess = finalizeResult->error == 0 && finalizeResult->size == sizeof(WavHeader); + delete finalizeResult; + finalizeResult = nullptr; + if(!writeSuccess) fail(RecordingError::WRITE_FAILED); + return writeSuccess; +} - SDResult* result = nullptr; - Sched.addJob(new SDJob { - .type = SDJob::SD_WRITE, +bool OutputWAV::queueFinalizeJob(SDJob::Type type){ + return Sched.addJob(new SDJob { + .type = type, .file = file, - .size = sizeof(WavHeader), - .buffer = reinterpret_cast(&header), - .result = &result + .size = type == SDJob::SD_SEEK ? 0 : sizeof(WavHeader), + .buffer = type == SDJob::SD_SEEK ? nullptr : reinterpret_cast(&header), + .result = &finalizeResult }); +} - while(result == nullptr){ - delayMicroseconds(1); - } - - delete result; +void OutputWAV::fail(RecordingError recordingError){ + if(error == RecordingError::NONE) error = recordingError; } diff --git a/src/AudioLib/OutputWAV.h b/src/AudioLib/OutputWAV.h index 0bbe51a..843b6bf 100644 --- a/src/AudioLib/OutputWAV.h +++ b/src/AudioLib/OutputWAV.h @@ -7,11 +7,22 @@ #include #include #include "../Services/SDScheduler.h" +#include "WavHeader.h" #define OUTWAV_BUFSIZE 2 * 1024 * NUM_CHANNELS #define OUTWAV_WRITESIZE 1 * 1024 * NUM_CHANNELS // should be smaller than BUFSIZE #define OUTWAV_BUFCOUNT 16 +enum class RecordingError : uint8_t { + NONE, + SD_UNAVAILABLE, + OPEN_FAILED, + WRITE_FAILED, + FINALIZE_FAILED, + BUFFER_OVERRUN, + QUEUE_FULL +}; + class OutputWAV : public Output { public: @@ -22,24 +33,43 @@ class OutputWAV : public Output void deinit() override; const fs::File& getFile() const; void setFile(const fs::File& file); + bool begin(const fs::File& file); + void finish(); + void service(); + bool isFinalized() const; + RecordingError getError() const; + uint32_t getBytesWritten() const; + uint32_t getDroppedBytes() const; + uint32_t getDurationMs() const; protected: - void output(size_t numBytes) override; + void output(size_t numSamples) override; private: - const char* path; fs::File file; - size_t dataLength; - - void writeHeaderWAV(size_t size); + uint32_t bytesWritten = 0; + uint32_t droppedBytes = 0; + RecordingError error = RecordingError::NONE; + bool prepared = false; bool writePending[OUTWAV_BUFCOUNT] = { false }; SDResult* writeResult[OUTWAV_BUFCOUNT] = { nullptr }; - void addWriteJob(); + size_t writeSize[OUTWAV_BUFCOUNT] = { 0 }; + bool addWriteJob(); void processWriteJob(); + bool hasPendingWrites() const; DataBuffer* outBuffers[OUTWAV_BUFCOUNT] = { nullptr }; std::vector freeBuffers; + + enum class FinalizeStage : uint8_t { DONE, ACTIVE, DRAIN, SEEK, HEADER }; + FinalizeStage finalizeStage = FinalizeStage::DONE; + SDResult* finalizeResult = nullptr; + WavHeader header = {}; + + bool writeInitialHeader(); + bool queueFinalizeJob(SDJob::Type type); + void fail(RecordingError recordingError); }; diff --git a/src/AudioLib/Systems/MixSystem.cpp b/src/AudioLib/Systems/MixSystem.cpp index d8a6b5d..dcb9c70 100644 --- a/src/AudioLib/Systems/MixSystem.cpp +++ b/src/AudioLib/Systems/MixSystem.cpp @@ -226,6 +226,7 @@ void MixSystem::audioThread(Task* task){ Serial.println("-- MixSystem started --"); while(task->running){ + system->serviceRecording(); uint8_t requestIndex; while(system->queue.count()){ if(!system->queue.receive(&requestIndex)) break; @@ -262,7 +263,6 @@ void MixSystem::audioThread(Task* task){ system->_seekChannel(request.channel, request.value); break; case MixRequest::RECORD: - if(request.value == system->isRecording()) break; if(request.value){ system->_startRecording(); }else{ @@ -279,13 +279,14 @@ void MixSystem::audioThread(Task* task){ if(system->out->isRunning()){ Profiler.init(); system->out->loop(0); + system->serviceRecording(); Profiler.report(); }else{ system->running = false; } } - system->fsOut->stop(); + system->_stopRecording(); } void MixSystem::start(){ @@ -306,7 +307,7 @@ void MixSystem::stop(){ running = false; _stopRecording(); - fileOut.close(); + finishRecordingSync(); out->stop(); clearRequests(); @@ -626,56 +627,133 @@ void MixSystem::_seekChannel(uint8_t channel, uint64_t frame){ } bool MixSystem::isRecording(){ - return out->getOutput(1) != nullptr; -} + return recordingState == RecordingState::RECORDING; +} + +RecordingStatus MixSystem::getRecordingStatus() const{ + const RecordingError outputError = fsOut->getError(); + return { + recordingState, + recordingError == RecordingError::NONE ? outputError : recordingError, + fsOut->getBytesWritten(), + fsOut->getDurationMs(), + fsOut->getDroppedBytes() + }; +} + +bool MixSystem::startRecording(){ + if(recordingState == RecordingState::STARTING || + recordingState == RecordingState::RECORDING || + recordingState == RecordingState::STOPPING) return false; + + recordingState = RecordingState::STARTING; + recordingError = RecordingError::NONE; + if(SD.cardType() == CARD_NONE){ + recordingError = RecordingError::SD_UNAVAILABLE; + recordingState = RecordingState::FAILED; + return false; + } + + if(SD.exists(recordPath) && !SD.remove(recordPath)){ + recordingError = RecordingError::SD_UNAVAILABLE; + recordingState = RecordingState::FAILED; + return false; + } + + fileOut = SD.open(recordPath, "w"); + if(!fileOut){ + Serial.printf("Failed opening %s for writing\n", recordPath); + recordingError = RecordingError::OPEN_FAILED; + recordingState = RecordingState::FAILED; + return false; + } + + if(!fsOut->begin(fileOut)){ + recordingError = fsOut->getError(); + recordingState = RecordingState::FAILED; + fileOut.close(); + return false; + } -void MixSystem::startRecording(){ if(!out->isRunning()){ _startRecording(); - return; + return true; } - enqueueRequest({ MixRequest::RECORD, 0, 0, 1 }); + if(enqueueRequest({ MixRequest::RECORD, 0, 0, 1 })) return true; + + recordingError = RecordingError::QUEUE_FULL; + recordingState = RecordingState::STOPPING; + fsOut->finish(); + finishRecordingSync(); + return false; } -void MixSystem::stopRecording(){ +bool MixSystem::stopRecording(){ + if(recordingState == RecordingState::IDLE || + recordingState == RecordingState::COMPLETE || + recordingState == RecordingState::FAILED || + recordingState == RecordingState::STOPPING) return true; + + const RecordingState previousState = recordingState; + recordingState = RecordingState::STOPPING; if(!out->isRunning()){ _stopRecording(); - return; + finishRecordingSync(); + return recordingState == RecordingState::COMPLETE; } - enqueueRequest({ MixRequest::RECORD, 0, 0, 0 }); + if(enqueueRequest({ MixRequest::RECORD, 0, 0, 0 })) return true; + recordingState = previousState; + return false; } void MixSystem::_startRecording(){ - if(isRecording()) return; - - if(SD.exists(recordPath)){ - SD.remove(recordPath); - } - - fileOut = SD.open(recordPath, "w"); - if(!fileOut){ - Serial.printf("Failed opening %s for writing\n", recordPath); - return; - } - - fsOut->setFile(fileOut); - + if(out->getOutput(1) != nullptr) return; + const bool stopping = recordingState == RecordingState::STOPPING; out->addOutput(fsOut); - if(out->isRunning()){ fsOut->start(); } + if(!stopping) recordingState = RecordingState::RECORDING; } void MixSystem::_stopRecording(){ - if(!isRecording()) return; + if(out->getOutput(1) != nullptr){ + out->removeOutput(1); + } + if(fsOut->isRunning()){ + fsOut->stop(); + }else{ + fsOut->finish(); + } + if(recordingState == RecordingState::STARTING || + recordingState == RecordingState::RECORDING) recordingState = RecordingState::STOPPING; +} - out->removeOutput(1); +void MixSystem::serviceRecording(){ + fsOut->service(); + if(recordingState == RecordingState::RECORDING && + fsOut->getError() != RecordingError::NONE){ + recordingState = RecordingState::STOPPING; + _stopRecording(); + } - fsOut->stop(); + if(recordingState != RecordingState::STOPPING || !fsOut->isFinalized()) return; fileOut.close(); + if(fsOut->getError() == RecordingError::NONE && recordingError == RecordingError::NONE){ + recordingState = RecordingState::COMPLETE; + }else{ + if(recordingError == RecordingError::NONE) recordingError = fsOut->getError(); + recordingState = RecordingState::FAILED; + } +} + +void MixSystem::finishRecordingSync(){ + while(recordingState == RecordingState::STOPPING){ + Sched.loop(0); + serviceRecording(); + } } bool MixSystem::isChannelPaused(uint8_t channel){ diff --git a/src/AudioLib/Systems/MixSystem.h b/src/AudioLib/Systems/MixSystem.h index b05c8cf..bea92b3 100644 --- a/src/AudioLib/Systems/MixSystem.h +++ b/src/AudioLib/Systems/MixSystem.h @@ -24,6 +24,23 @@ struct MixRequest { uint64_t value; }; +enum class RecordingState : uint8_t { + IDLE, + STARTING, + RECORDING, + STOPPING, + COMPLETE, + FAILED +}; + +struct RecordingStatus { + RecordingState state; + RecordingError error; + uint32_t bytes; + uint32_t durationMs; + uint32_t droppedBytes; +}; + class MixSystem { public: MixSystem(); @@ -74,9 +91,12 @@ class MixSystem { void seekChannel(uint8_t channel, uint16_t time); bool seekChannelSourceFrame(uint8_t channel, uint64_t frame); - void startRecording(); - void stopRecording(); + // Return values report whether the request was accepted. Poll status for + // asynchronous write/finalization failures. + bool startRecording(); + bool stopRecording(); bool isRecording(); + RecordingStatus getRecordingStatus() const; void setChannelDoneCallback(uint8_t channel, void(*callback)()); @@ -101,6 +121,8 @@ class MixSystem { OutputI2S* i2s; OutputWAV* fsOut; OutputSplitter* out; + volatile RecordingState recordingState = RecordingState::IDLE; + volatile RecordingError recordingError = RecordingError::NONE; SpeedModifier* speed[2] = { nullptr }; @@ -115,6 +137,8 @@ class MixSystem { void _seekChannel(uint8_t channel, uint64_t frame); void _startRecording(); void _stopRecording(); + void serviceRecording(); + void finishRecordingSync(); void _openChannel(uint8_t channel, SourceAAC* source); bool replaceSource(uint8_t channel, SourceAAC* source); int8_t reserveRequest(const MixRequest& request); diff --git a/src/AudioLib/WavHeader.h b/src/AudioLib/WavHeader.h new file mode 100644 index 0000000..81dbc9d --- /dev/null +++ b/src/AudioLib/WavHeader.h @@ -0,0 +1,48 @@ +#ifndef JAYD_LIBRARY_WAVHEADER_H +#define JAYD_LIBRARY_WAVHEADER_H + +#include +#include + +struct WavHeader { + char RIFF[4]; + uint32_t chunkSize; + char WAVE[4]; + char fmt[4]; + uint32_t fmtSize; + uint16_t audioFormat; + uint16_t numChannels; + uint32_t sampleRate; + uint32_t byteRate; + uint16_t blockAlign; + uint16_t bitsPerSample; + char data[4]; + uint32_t dataSize; +}; + +static_assert(sizeof(WavHeader) == 44, "PCM WAV header must be 44 bytes"); + +inline WavHeader makeWavHeader( + uint32_t dataSize, + uint16_t channels, + uint32_t sampleRate, + uint16_t bytesPerSample +){ + WavHeader header = {}; + memcpy(header.RIFF, "RIFF", 4); + header.chunkSize = dataSize + 36; + memcpy(header.WAVE, "WAVE", 4); + memcpy(header.fmt, "fmt ", 4); + header.fmtSize = 16; + header.audioFormat = 1; + header.numChannels = channels; + header.sampleRate = sampleRate; + header.byteRate = sampleRate * channels * bytesPerSample; + header.blockAlign = channels * bytesPerSample; + header.bitsPerSample = bytesPerSample * 8; + memcpy(header.data, "data", 4); + header.dataSize = dataSize; + return header; +} + +#endif //JAYD_LIBRARY_WAVHEADER_H diff --git a/src/Services/SDScheduler.cpp b/src/Services/SDScheduler.cpp index 8971dc4..345cfec 100644 --- a/src/Services/SDScheduler.cpp +++ b/src/Services/SDScheduler.cpp @@ -7,8 +7,11 @@ SDScheduler::SDScheduler() :jobs(8, sizeof(SDJob*)){ } -void SDScheduler::addJob(SDJob *job){ - jobs.send(&job); +bool SDScheduler::addJob(SDJob *job){ + if(job == nullptr) return false; + if(jobs.send(&job)) return true; + delete job; + return false; } void SDScheduler::loop(uint micros) { @@ -42,7 +45,7 @@ void SDScheduler::doJob(SDJob* job){ SDResult* result = new SDResult(); result->size = job->size * success; result->buffer = job->buffer; - result->error = 0; + result->error = success ? 0 : 1; *job->result = result; } @@ -56,11 +59,10 @@ void SDScheduler::doJob(SDJob* job){ if(job->result != nullptr){ SDResult* result = new SDResult(); - result->error = 0; + result->error = job->type == SDJob::SD_WRITE && size != job->size; result->buffer = job->buffer; result->size = size; *job->result = result; } } - diff --git a/src/Services/SDScheduler.h b/src/Services/SDScheduler.h index 431b1d3..19e922e 100644 --- a/src/Services/SDScheduler.h +++ b/src/Services/SDScheduler.h @@ -13,7 +13,7 @@ struct SDResult { }; struct SDJob { - enum { SD_WRITE, SD_READ, SD_SEEK } type; + enum Type { SD_WRITE, SD_READ, SD_SEEK } type; fs::File file; size_t size; uint8_t* buffer; @@ -24,7 +24,7 @@ class SDScheduler : public LoopListener { public: SDScheduler(); - void addJob(SDJob *job); + bool addJob(SDJob *job); void loop(uint micros) override; private: Queue jobs; diff --git a/tests/wav_header_selfcheck.cpp b/tests/wav_header_selfcheck.cpp new file mode 100644 index 0000000..936c154 --- /dev/null +++ b/tests/wav_header_selfcheck.cpp @@ -0,0 +1,29 @@ +#include +#include +#include "../src/AudioLib/WavHeader.h" + +int main(){ + const WavHeader empty = makeWavHeader(0, 1, 24000, 2); + assert(sizeof(empty) == 44); + assert(memcmp(empty.RIFF, "RIFF", 4) == 0); + assert(memcmp(empty.WAVE, "WAVE", 4) == 0); + assert(memcmp(empty.fmt, "fmt ", 4) == 0); + assert(memcmp(empty.data, "data", 4) == 0); + assert(empty.chunkSize == 36); + assert(empty.dataSize == 0); + assert(empty.byteRate == 48000); + assert(empty.blockAlign == 2); + assert(empty.bitsPerSample == 16); + + const WavHeader finalized = makeWavHeader(4096, 2, 44100, 2); + unsigned char fileHeader[sizeof(WavHeader)] = {}; + memcpy(fileHeader, &empty, sizeof(empty)); + memcpy(fileHeader, &finalized, sizeof(finalized)); + WavHeader rewritten = {}; + memcpy(&rewritten, fileHeader, sizeof(rewritten)); + assert(rewritten.chunkSize == 4132); + assert(rewritten.dataSize == 4096); + assert(rewritten.byteRate == 176400); + assert(rewritten.blockAlign == 4); + return 0; +} From e7e61f4b077c5c3a1a0706cbda1423b7172b5d57 Mon Sep 17 00:00:00 2001 From: Ryan Trauntvein Date: Fri, 21 Aug 2026 21:45:22 -0700 Subject: [PATCH 06/15] Fix recording start and finalize races Keep live recorder setup on the audio task and retry finalization queue pressure before reporting an invalid file.\n\nCo-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> (cherry picked from commit 5230446ee3b481e2f407d6f8008fb85e0ef45734) --- src/AudioLib/OutputWAV.cpp | 65 ++++++++++++++---- src/AudioLib/OutputWAV.h | 12 +++- src/AudioLib/RecordingFinalize.h | 24 +++++++ src/AudioLib/Systems/MixSystem.cpp | 105 +++++++++++++++++++---------- src/AudioLib/Systems/MixSystem.h | 3 + tests/wav_header_selfcheck.cpp | 37 ++++++++++ 6 files changed, 197 insertions(+), 49 deletions(-) create mode 100644 src/AudioLib/RecordingFinalize.h diff --git a/src/AudioLib/OutputWAV.cpp b/src/AudioLib/OutputWAV.cpp index 04d5cc4..5f3b9ba 100644 --- a/src/AudioLib/OutputWAV.cpp +++ b/src/AudioLib/OutputWAV.cpp @@ -84,6 +84,8 @@ bool OutputWAV::begin(const fs::File& outputFile){ droppedBytes = 0; error = RecordingError::NONE; prepared = false; + fileValid = false; + finalizeQueueRetries = 0; freeBuffers.clear(); for(uint8_t i = 0; i < OUTWAV_BUFCOUNT; i++){ outBuffers[i]->clear(); @@ -131,11 +133,17 @@ void OutputWAV::service(){ if(finalizeStage == FinalizeStage::DRAIN){ header = makeWavHeader(bytesWritten, NUM_CHANNELS, SAMPLE_RATE, BYTES_PER_SAMPLE); - if(!queueFinalizeJob(SDJob::SD_SEEK)){ - fail(RecordingError::FINALIZE_FAILED); - finalizeStage = FinalizeStage::DONE; - }else{ - finalizeStage = FinalizeStage::SEEK; + if(tryQueueFinalizeJob(SDJob::SD_SEEK, FinalizeStage::SEEK) == + FinalizeEnqueueResult::EXHAUSTED){ + failFinalize(); + } + return; + } + + if(finalizeStage == FinalizeStage::HEADER_QUEUE){ + if(tryQueueFinalizeJob(SDJob::SD_WRITE, FinalizeStage::HEADER) == + FinalizeEnqueueResult::EXHAUSTED){ + failFinalize(); } return; } @@ -146,21 +154,16 @@ void OutputWAV::service(){ delete finalizeResult; finalizeResult = nullptr; if(!success){ - fail(RecordingError::FINALIZE_FAILED); - finalizeStage = FinalizeStage::DONE; + failFinalize(); return; } if(finalizeStage == FinalizeStage::SEEK){ - if(!queueFinalizeJob(SDJob::SD_WRITE)){ - fail(RecordingError::FINALIZE_FAILED); - finalizeStage = FinalizeStage::DONE; - }else{ - finalizeStage = FinalizeStage::HEADER; - } + finalizeStage = FinalizeStage::HEADER_QUEUE; }else{ finalizeStage = FinalizeStage::DONE; prepared = false; + fileValid = true; } } @@ -168,6 +171,18 @@ bool OutputWAV::isFinalized() const{ return finalizeStage == FinalizeStage::DONE; } +bool OutputWAV::isPrepared() const{ + return prepared; +} + +bool OutputWAV::isFileValid() const{ + return fileValid; +} + +void OutputWAV::invalidateFile(){ + fileValid = false; +} + RecordingError OutputWAV::getError() const{ return error; } @@ -185,6 +200,10 @@ uint32_t OutputWAV::getDurationMs() const{ return byteRate == 0 ? 0 : static_cast(bytesWritten) * 1000 / byteRate; } +uint8_t OutputWAV::getFinalizeQueueRetries() const{ + return finalizeQueueRetries; +} + bool OutputWAV::addWriteJob(){ if(freeBuffers.empty()) return false; const uint8_t i = freeBuffers.front(); @@ -271,6 +290,26 @@ bool OutputWAV::queueFinalizeJob(SDJob::Type type){ }); } +FinalizeEnqueueResult OutputWAV::tryQueueFinalizeJob( + SDJob::Type type, + FinalizeStage queuedStage +){ + const FinalizeEnqueueResult result = recordFinalizeEnqueue( + queueFinalizeJob(type), + finalizeQueueRetries, + OUTWAV_FINALIZE_QUEUE_FAILURES + ); + if(result == FinalizeEnqueueResult::QUEUED) finalizeStage = queuedStage; + return result; +} + +void OutputWAV::failFinalize(){ + error = RecordingError::FINALIZE_FAILED; + fileValid = false; + prepared = false; + finalizeStage = FinalizeStage::DONE; +} + void OutputWAV::fail(RecordingError recordingError){ if(error == RecordingError::NONE) error = recordingError; } diff --git a/src/AudioLib/OutputWAV.h b/src/AudioLib/OutputWAV.h index 843b6bf..8006855 100644 --- a/src/AudioLib/OutputWAV.h +++ b/src/AudioLib/OutputWAV.h @@ -7,11 +7,13 @@ #include #include #include "../Services/SDScheduler.h" +#include "RecordingFinalize.h" #include "WavHeader.h" #define OUTWAV_BUFSIZE 2 * 1024 * NUM_CHANNELS #define OUTWAV_WRITESIZE 1 * 1024 * NUM_CHANNELS // should be smaller than BUFSIZE #define OUTWAV_BUFCOUNT 16 +#define OUTWAV_FINALIZE_QUEUE_FAILURES 32 enum class RecordingError : uint8_t { NONE, @@ -37,10 +39,14 @@ class OutputWAV : public Output void finish(); void service(); bool isFinalized() const; + bool isPrepared() const; + bool isFileValid() const; + void invalidateFile(); RecordingError getError() const; uint32_t getBytesWritten() const; uint32_t getDroppedBytes() const; uint32_t getDurationMs() const; + uint8_t getFinalizeQueueRetries() const; protected: void output(size_t numSamples) override; @@ -62,13 +68,17 @@ class OutputWAV : public Output DataBuffer* outBuffers[OUTWAV_BUFCOUNT] = { nullptr }; std::vector freeBuffers; - enum class FinalizeStage : uint8_t { DONE, ACTIVE, DRAIN, SEEK, HEADER }; + enum class FinalizeStage : uint8_t { DONE, ACTIVE, DRAIN, SEEK, HEADER_QUEUE, HEADER }; FinalizeStage finalizeStage = FinalizeStage::DONE; SDResult* finalizeResult = nullptr; WavHeader header = {}; + bool fileValid = false; + uint8_t finalizeQueueRetries = 0; bool writeInitialHeader(); bool queueFinalizeJob(SDJob::Type type); + FinalizeEnqueueResult tryQueueFinalizeJob(SDJob::Type type, FinalizeStage queuedStage); + void failFinalize(); void fail(RecordingError recordingError); }; diff --git a/src/AudioLib/RecordingFinalize.h b/src/AudioLib/RecordingFinalize.h new file mode 100644 index 0000000..15551fd --- /dev/null +++ b/src/AudioLib/RecordingFinalize.h @@ -0,0 +1,24 @@ +#ifndef JAYD_LIBRARY_RECORDINGFINALIZE_H +#define JAYD_LIBRARY_RECORDINGFINALIZE_H + +#include + +enum class FinalizeEnqueueResult : uint8_t { + QUEUED, + RETRY, + EXHAUSTED +}; + +inline FinalizeEnqueueResult recordFinalizeEnqueue( + bool queued, + uint8_t& failures, + uint8_t maxFailures +){ + if(queued) return FinalizeEnqueueResult::QUEUED; + if(failures < maxFailures) failures++; + return failures >= maxFailures + ? FinalizeEnqueueResult::EXHAUSTED + : FinalizeEnqueueResult::RETRY; +} + +#endif //JAYD_LIBRARY_RECORDINGFINALIZE_H diff --git a/src/AudioLib/Systems/MixSystem.cpp b/src/AudioLib/Systems/MixSystem.cpp index dcb9c70..be29bf2 100644 --- a/src/AudioLib/Systems/MixSystem.cpp +++ b/src/AudioLib/Systems/MixSystem.cpp @@ -631,13 +631,26 @@ bool MixSystem::isRecording(){ } RecordingStatus MixSystem::getRecordingStatus() const{ + if(!recordingApplied){ + return { + recordingState, + recordingError, + 0, + 0, + 0, + 0, + false + }; + } const RecordingError outputError = fsOut->getError(); return { recordingState, recordingError == RecordingError::NONE ? outputError : recordingError, fsOut->getBytesWritten(), fsOut->getDurationMs(), - fsOut->getDroppedBytes() + fsOut->getDroppedBytes(), + fsOut->getFinalizeQueueRetries(), + fsOut->isFileValid() }; } @@ -648,44 +661,17 @@ bool MixSystem::startRecording(){ recordingState = RecordingState::STARTING; recordingError = RecordingError::NONE; - if(SD.cardType() == CARD_NONE){ - recordingError = RecordingError::SD_UNAVAILABLE; - recordingState = RecordingState::FAILED; - return false; - } - - if(SD.exists(recordPath) && !SD.remove(recordPath)){ - recordingError = RecordingError::SD_UNAVAILABLE; - recordingState = RecordingState::FAILED; - return false; - } - - fileOut = SD.open(recordPath, "w"); - if(!fileOut){ - Serial.printf("Failed opening %s for writing\n", recordPath); - recordingError = RecordingError::OPEN_FAILED; - recordingState = RecordingState::FAILED; - return false; - } - - if(!fsOut->begin(fileOut)){ - recordingError = fsOut->getError(); - recordingState = RecordingState::FAILED; - fileOut.close(); - return false; - } + recordingApplied = false; if(!out->isRunning()){ _startRecording(); - return true; + return recordingState != RecordingState::FAILED; } if(enqueueRequest({ MixRequest::RECORD, 0, 0, 1 })) return true; recordingError = RecordingError::QUEUE_FULL; - recordingState = RecordingState::STOPPING; - fsOut->finish(); - finishRecordingSync(); + recordingState = RecordingState::FAILED; return false; } @@ -711,11 +697,46 @@ bool MixSystem::stopRecording(){ void MixSystem::_startRecording(){ if(out->getOutput(1) != nullptr) return; const bool stopping = recordingState == RecordingState::STOPPING; + if(!stopping && recordingState != RecordingState::STARTING) return; + fsOut->invalidateFile(); + + if(SD.cardType() == CARD_NONE){ + recordingError = RecordingError::SD_UNAVAILABLE; + recordingState = RecordingState::FAILED; + return; + } + + if(SD.exists(recordPath) && !SD.remove(recordPath)){ + recordingError = RecordingError::SD_UNAVAILABLE; + recordingState = RecordingState::FAILED; + return; + } + + fileOut = SD.open(recordPath, "w"); + if(!fileOut){ + Serial.printf("Failed opening %s for writing\n", recordPath); + recordingError = RecordingError::OPEN_FAILED; + recordingState = RecordingState::FAILED; + return; + } + + if(!fsOut->begin(fileOut)){ + recordingError = fsOut->getError(); + recordingState = RecordingState::FAILED; + fileOut.close(); + return; + } + recordingApplied = true; + out->addOutput(fsOut); if(out->isRunning()){ fsOut->start(); } - if(!stopping) recordingState = RecordingState::RECORDING; + if(stopping){ + _stopRecording(); + }else{ + recordingState = RecordingState::RECORDING; + } } void MixSystem::_stopRecording(){ @@ -727,6 +748,12 @@ void MixSystem::_stopRecording(){ }else{ fsOut->finish(); } + if(!recordingApplied && + (recordingState == RecordingState::STARTING || + recordingState == RecordingState::STOPPING)){ + recordingState = RecordingState::IDLE; + return; + } if(recordingState == RecordingState::STARTING || recordingState == RecordingState::RECORDING) recordingState = RecordingState::STOPPING; } @@ -739,12 +766,20 @@ void MixSystem::serviceRecording(){ _stopRecording(); } - if(recordingState != RecordingState::STOPPING || !fsOut->isFinalized()) return; + if(recordingState != RecordingState::STOPPING || + !recordingApplied || + !fsOut->isFinalized()) return; fileOut.close(); - if(fsOut->getError() == RecordingError::NONE && recordingError == RecordingError::NONE){ + if(fsOut->getError() == RecordingError::NONE && + recordingError == RecordingError::NONE && + fsOut->isFileValid()){ recordingState = RecordingState::COMPLETE; }else{ - if(recordingError == RecordingError::NONE) recordingError = fsOut->getError(); + if(recordingError == RecordingError::NONE){ + recordingError = fsOut->getError() == RecordingError::NONE + ? RecordingError::FINALIZE_FAILED + : fsOut->getError(); + } recordingState = RecordingState::FAILED; } } diff --git a/src/AudioLib/Systems/MixSystem.h b/src/AudioLib/Systems/MixSystem.h index bea92b3..62ff80b 100644 --- a/src/AudioLib/Systems/MixSystem.h +++ b/src/AudioLib/Systems/MixSystem.h @@ -39,6 +39,8 @@ struct RecordingStatus { uint32_t bytes; uint32_t durationMs; uint32_t droppedBytes; + uint8_t finalizeQueueRetries; + bool fileValid; }; class MixSystem { @@ -123,6 +125,7 @@ class MixSystem { OutputSplitter* out; volatile RecordingState recordingState = RecordingState::IDLE; volatile RecordingError recordingError = RecordingError::NONE; + volatile bool recordingApplied = false; SpeedModifier* speed[2] = { nullptr }; diff --git a/tests/wav_header_selfcheck.cpp b/tests/wav_header_selfcheck.cpp index 936c154..08d8ab9 100644 --- a/tests/wav_header_selfcheck.cpp +++ b/tests/wav_header_selfcheck.cpp @@ -1,5 +1,9 @@ #include +#include +#include #include +#include +#include "../src/AudioLib/RecordingFinalize.h" #include "../src/AudioLib/WavHeader.h" int main(){ @@ -25,5 +29,38 @@ int main(){ assert(rewritten.dataSize == 4096); assert(rewritten.byteRate == 176400); assert(rewritten.blockAlign == 4); + + uint8_t queueFailures = 0; + assert(recordFinalizeEnqueue(false, queueFailures, 3) == FinalizeEnqueueResult::RETRY); + assert(recordFinalizeEnqueue(false, queueFailures, 3) == FinalizeEnqueueResult::RETRY); + assert(recordFinalizeEnqueue(true, queueFailures, 3) == FinalizeEnqueueResult::QUEUED); + assert(queueFailures == 2); + assert(recordFinalizeEnqueue(false, queueFailures, 3) == FinalizeEnqueueResult::EXHAUSTED); + + std::ifstream sourceFile("src/AudioLib/Systems/MixSystem.cpp"); + assert(sourceFile.good()); + const std::string source( + (std::istreambuf_iterator(sourceFile)), + std::istreambuf_iterator() + ); + const size_t startBegin = source.find("bool MixSystem::startRecording()"); + const size_t startEnd = source.find("bool MixSystem::stopRecording()", startBegin); + assert(startBegin != std::string::npos); + assert(startEnd != std::string::npos); + const std::string start = source.substr(startBegin, startEnd - startBegin); + assert(start.find("enqueueRequest") != std::string::npos); + assert(start.find("fsOut->") == std::string::npos); + assert(start.find("SD.") == std::string::npos); + assert(start.find("fileOut") == std::string::npos); + + const size_t appliedBegin = source.find("void MixSystem::_startRecording()"); + const size_t appliedEnd = source.find("void MixSystem::_stopRecording()", appliedBegin); + assert(appliedBegin != std::string::npos); + assert(appliedEnd != std::string::npos); + const std::string applied = source.substr(appliedBegin, appliedEnd - appliedBegin); + assert(applied.find("SD.open") != std::string::npos); + assert(applied.find("fsOut->begin") != std::string::npos); + assert(applied.find("fsOut->invalidateFile") != std::string::npos); + assert(applied.find("recordingApplied = true") != std::string::npos); return 0; } From 5accb28ef69ea1a099cc9abbc319c9f49cb2b517 Mon Sep 17 00:00:00 2001 From: Ryan Trauntvein Date: Fri, 21 Aug 2026 22:43:03 -0700 Subject: [PATCH 07/15] Make recording header setup asynchronous Drive initial WAV seek and header writes from the service state machine without blocking mixer output, and make SD scheduler enqueue genuinely nonblocking. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> (cherry picked from commit a4e0fdb0345b8480b5b573f49c58cbf3f881f7a8) --- src/AudioLib/OutputWAV.cpp | 85 +++++++++++++++++++----------- src/AudioLib/OutputWAV.h | 16 +++++- src/AudioLib/Systems/MixSystem.cpp | 79 +++++++++++++++++++-------- src/AudioLib/Systems/MixSystem.h | 1 + src/Services/SDScheduler.cpp | 13 +++-- src/Services/SDScheduler.h | 7 +-- tests/wav_header_selfcheck.cpp | 55 +++++++++++++++++++ 7 files changed, 193 insertions(+), 63 deletions(-) diff --git a/src/AudioLib/OutputWAV.cpp b/src/AudioLib/OutputWAV.cpp index 5f3b9ba..24e84c1 100644 --- a/src/AudioLib/OutputWAV.cpp +++ b/src/AudioLib/OutputWAV.cpp @@ -66,7 +66,7 @@ void OutputWAV::output(size_t numSamples){ void OutputWAV::init(){ if(!prepared && file) begin(file); - if(!prepared || !file || finalizeStage != FinalizeStage::ACTIVE){ + if(!prepared || !file){ Serial.println("Output file not open"); fail(RecordingError::OPEN_FAILED); } @@ -100,9 +100,8 @@ bool OutputWAV::begin(const fs::File& outputFile){ } header = makeWavHeader(0, NUM_CHANNELS, SAMPLE_RATE, BYTES_PER_SAMPLE); - if(!writeInitialHeader()) return false; prepared = true; - finalizeStage = FinalizeStage::ACTIVE; + finalizeStage = FinalizeStage::INIT_SEEK_QUEUE; return true; } @@ -127,9 +126,46 @@ void OutputWAV::finish(){ void OutputWAV::service(){ processWriteJob(); - if(finalizeStage == FinalizeStage::DONE || - finalizeStage == FinalizeStage::ACTIVE || - (finalizeStage == FinalizeStage::DRAIN && hasPendingWrites())) return; + if(finalizeStage == FinalizeStage::DONE || finalizeStage == FinalizeStage::ACTIVE) return; + + if(finalizeStage == FinalizeStage::INIT_SEEK_QUEUE){ + const FinalizeEnqueueResult result = + tryQueueFinalizeJob(SDJob::SD_SEEK, FinalizeStage::INIT_SEEK); + if(result == FinalizeEnqueueResult::EXHAUSTED){ + failInitialize(RecordingError::QUEUE_FULL); + } + return; + } + + if(finalizeStage == FinalizeStage::INIT_HEADER_QUEUE){ + const FinalizeEnqueueResult result = + tryQueueFinalizeJob(SDJob::SD_WRITE, FinalizeStage::INIT_HEADER); + if(result == FinalizeEnqueueResult::EXHAUSTED){ + failInitialize(RecordingError::QUEUE_FULL); + } + return; + } + + if(finalizeStage == FinalizeStage::INIT_SEEK || + finalizeStage == FinalizeStage::INIT_HEADER){ + if(finalizeResult == nullptr) return; + const bool success = finalizeResult->error == 0 && + (finalizeStage == FinalizeStage::INIT_SEEK || + finalizeResult->size == sizeof(WavHeader)); + delete finalizeResult; + finalizeResult = nullptr; + if(!success){ + failInitialize(RecordingError::WRITE_FAILED); + }else if(finalizeStage == FinalizeStage::INIT_SEEK){ + finalizeStage = FinalizeStage::INIT_HEADER_QUEUE; + }else{ + finalizeQueueRetries = 0; + finalizeStage = FinalizeStage::ACTIVE; + } + return; + } + + if(finalizeStage == FinalizeStage::DRAIN && hasPendingWrites()) return; if(finalizeStage == FinalizeStage::DRAIN){ header = makeWavHeader(bytesWritten, NUM_CHANNELS, SAMPLE_RATE, BYTES_PER_SAMPLE); @@ -175,6 +211,10 @@ bool OutputWAV::isPrepared() const{ return prepared; } +bool OutputWAV::isReady() const{ + return finalizeStage == FinalizeStage::ACTIVE; +} + bool OutputWAV::isFileValid() const{ return fileValid; } @@ -254,32 +294,6 @@ bool OutputWAV::hasPendingWrites() const{ return false; } -bool OutputWAV::writeInitialHeader(){ - if(!queueFinalizeJob(SDJob::SD_SEEK)){ - fail(RecordingError::QUEUE_FULL); - return false; - } - while(finalizeResult == nullptr) Sched.loop(0); - const bool seekSuccess = finalizeResult->error == 0; - delete finalizeResult; - finalizeResult = nullptr; - if(!seekSuccess){ - fail(RecordingError::WRITE_FAILED); - return false; - } - - if(!queueFinalizeJob(SDJob::SD_WRITE)){ - fail(RecordingError::QUEUE_FULL); - return false; - } - while(finalizeResult == nullptr) Sched.loop(0); - const bool writeSuccess = finalizeResult->error == 0 && finalizeResult->size == sizeof(WavHeader); - delete finalizeResult; - finalizeResult = nullptr; - if(!writeSuccess) fail(RecordingError::WRITE_FAILED); - return writeSuccess; -} - bool OutputWAV::queueFinalizeJob(SDJob::Type type){ return Sched.addJob(new SDJob { .type = type, @@ -303,6 +317,13 @@ FinalizeEnqueueResult OutputWAV::tryQueueFinalizeJob( return result; } +void OutputWAV::failInitialize(RecordingError recordingError){ + error = recordingError; + fileValid = false; + prepared = false; + finalizeStage = FinalizeStage::DONE; +} + void OutputWAV::failFinalize(){ error = RecordingError::FINALIZE_FAILED; fileValid = false; diff --git a/src/AudioLib/OutputWAV.h b/src/AudioLib/OutputWAV.h index 8006855..3716cda 100644 --- a/src/AudioLib/OutputWAV.h +++ b/src/AudioLib/OutputWAV.h @@ -40,6 +40,7 @@ class OutputWAV : public Output void service(); bool isFinalized() const; bool isPrepared() const; + bool isReady() const; bool isFileValid() const; void invalidateFile(); RecordingError getError() const; @@ -68,16 +69,27 @@ class OutputWAV : public Output DataBuffer* outBuffers[OUTWAV_BUFCOUNT] = { nullptr }; std::vector freeBuffers; - enum class FinalizeStage : uint8_t { DONE, ACTIVE, DRAIN, SEEK, HEADER_QUEUE, HEADER }; + enum class FinalizeStage : uint8_t { + DONE, + INIT_SEEK_QUEUE, + INIT_SEEK, + INIT_HEADER_QUEUE, + INIT_HEADER, + ACTIVE, + DRAIN, + SEEK, + HEADER_QUEUE, + HEADER + }; FinalizeStage finalizeStage = FinalizeStage::DONE; SDResult* finalizeResult = nullptr; WavHeader header = {}; bool fileValid = false; uint8_t finalizeQueueRetries = 0; - bool writeInitialHeader(); bool queueFinalizeJob(SDJob::Type type); FinalizeEnqueueResult tryQueueFinalizeJob(SDJob::Type type, FinalizeStage queuedStage); + void failInitialize(RecordingError recordingError); void failFinalize(); void fail(RecordingError recordingError); }; diff --git a/src/AudioLib/Systems/MixSystem.cpp b/src/AudioLib/Systems/MixSystem.cpp index be29bf2..30a1dad 100644 --- a/src/AudioLib/Systems/MixSystem.cpp +++ b/src/AudioLib/Systems/MixSystem.cpp @@ -655,9 +655,13 @@ RecordingStatus MixSystem::getRecordingStatus() const{ } bool MixSystem::startRecording(){ + recordingMutex.lock(); if(recordingState == RecordingState::STARTING || recordingState == RecordingState::RECORDING || - recordingState == RecordingState::STOPPING) return false; + recordingState == RecordingState::STOPPING){ + recordingMutex.unlock(); + return false; + } recordingState = RecordingState::STARTING; recordingError = RecordingError::NONE; @@ -665,39 +669,57 @@ bool MixSystem::startRecording(){ if(!out->isRunning()){ _startRecording(); - return recordingState != RecordingState::FAILED; + const bool accepted = recordingState != RecordingState::FAILED; + recordingMutex.unlock(); + return accepted; } - if(enqueueRequest({ MixRequest::RECORD, 0, 0, 1 })) return true; + if(enqueueRequest({ MixRequest::RECORD, 0, 0, 1 })){ + recordingMutex.unlock(); + return true; + } recordingError = RecordingError::QUEUE_FULL; recordingState = RecordingState::FAILED; + recordingMutex.unlock(); return false; } bool MixSystem::stopRecording(){ + recordingMutex.lock(); if(recordingState == RecordingState::IDLE || recordingState == RecordingState::COMPLETE || recordingState == RecordingState::FAILED || - recordingState == RecordingState::STOPPING) return true; + recordingState == RecordingState::STOPPING){ + recordingMutex.unlock(); + return true; + } - const RecordingState previousState = recordingState; - recordingState = RecordingState::STOPPING; if(!out->isRunning()){ + recordingState = RecordingState::STOPPING; _stopRecording(); finishRecordingSync(); - return recordingState == RecordingState::COMPLETE; + const bool complete = recordingState == RecordingState::COMPLETE; + recordingMutex.unlock(); + return complete; } - if(enqueueRequest({ MixRequest::RECORD, 0, 0, 0 })) return true; - recordingState = previousState; + if(enqueueRequest({ MixRequest::RECORD, 0, 0, 0 })){ + if(recordingState == RecordingState::STARTING || + recordingState == RecordingState::RECORDING){ + recordingState = RecordingState::STOPPING; + } + recordingMutex.unlock(); + return true; + } + recordingMutex.unlock(); return false; } void MixSystem::_startRecording(){ if(out->getOutput(1) != nullptr) return; - const bool stopping = recordingState == RecordingState::STOPPING; - if(!stopping && recordingState != RecordingState::STARTING) return; + if(recordingState != RecordingState::STARTING && + recordingState != RecordingState::STOPPING) return; fsOut->invalidateFile(); if(SD.cardType() == CARD_NONE){ @@ -727,16 +749,6 @@ void MixSystem::_startRecording(){ return; } recordingApplied = true; - - out->addOutput(fsOut); - if(out->isRunning()){ - fsOut->start(); - } - if(stopping){ - _stopRecording(); - }else{ - recordingState = RecordingState::RECORDING; - } } void MixSystem::_stopRecording(){ @@ -760,6 +772,31 @@ void MixSystem::_stopRecording(){ void MixSystem::serviceRecording(){ fsOut->service(); + if(recordingState == RecordingState::STARTING && recordingApplied){ + if(fsOut->getError() != RecordingError::NONE){ + recordingError = fsOut->getError(); + recordingState = RecordingState::FAILED; + fileOut.close(); + return; + } + if(fsOut->isReady()){ + recordingMutex.lock(); + if(recordingState == RecordingState::STARTING){ + out->addOutput(fsOut); + if(out->isRunning()) fsOut->start(); + recordingState = RecordingState::RECORDING; + } + recordingMutex.unlock(); + } + } + + if(recordingState == RecordingState::STOPPING && + recordingApplied && + fsOut->isReady() && + !fsOut->isRunning()){ + fsOut->finish(); + } + if(recordingState == RecordingState::RECORDING && fsOut->getError() != RecordingError::NONE){ recordingState = RecordingState::STOPPING; diff --git a/src/AudioLib/Systems/MixSystem.h b/src/AudioLib/Systems/MixSystem.h index 62ff80b..8914d5c 100644 --- a/src/AudioLib/Systems/MixSystem.h +++ b/src/AudioLib/Systems/MixSystem.h @@ -109,6 +109,7 @@ class MixSystem { Queue queue; Mutex queueMutex; Mutex sourceMutex; + Mutex recordingMutex; MixRequest requests[requestCapacity] = {}; bool requestUsed[requestCapacity] = {}; diff --git a/src/Services/SDScheduler.cpp b/src/Services/SDScheduler.cpp index 345cfec..a743485 100644 --- a/src/Services/SDScheduler.cpp +++ b/src/Services/SDScheduler.cpp @@ -3,26 +3,29 @@ SDScheduler Sched; -SDScheduler::SDScheduler() :jobs(8, sizeof(SDJob*)){ +SDScheduler::SDScheduler() : jobs(xQueueCreate(jobCapacity, sizeof(SDJob*))){ +} +SDScheduler::~SDScheduler(){ + vQueueDelete(jobs); } bool SDScheduler::addJob(SDJob *job){ if(job == nullptr) return false; - if(jobs.send(&job)) return true; + if(xQueueSend(jobs, &job, 0) == pdTRUE) return true; delete job; return false; } void SDScheduler::loop(uint micros) { - if (jobs.count() == 0) { + if (uxQueueMessagesWaiting(jobs) == 0) { return; } SDJob* request = nullptr; - while(jobs.count() > 0){ - if(!jobs.receive(&request)){ + while(uxQueueMessagesWaiting(jobs) > 0){ + if(xQueueReceive(jobs, &request, 0) != pdTRUE){ Serial.println("Receive error"); return; } diff --git a/src/Services/SDScheduler.h b/src/Services/SDScheduler.h index 19e922e..0fd7b87 100644 --- a/src/Services/SDScheduler.h +++ b/src/Services/SDScheduler.h @@ -2,9 +2,8 @@ #define JAYD_LIBRARY_SDSCHEDULER_H #include -#include #include -#include +#include struct SDResult { uint8_t error; @@ -23,11 +22,13 @@ struct SDJob { class SDScheduler : public LoopListener { public: SDScheduler(); + ~SDScheduler(); bool addJob(SDJob *job); void loop(uint micros) override; private: - Queue jobs; + static constexpr uint8_t jobCapacity = 8; + QueueHandle_t jobs; void doJob(SDJob* job); diff --git a/tests/wav_header_selfcheck.cpp b/tests/wav_header_selfcheck.cpp index 08d8ab9..dd02dad 100644 --- a/tests/wav_header_selfcheck.cpp +++ b/tests/wav_header_selfcheck.cpp @@ -62,5 +62,60 @@ int main(){ assert(applied.find("fsOut->begin") != std::string::npos); assert(applied.find("fsOut->invalidateFile") != std::string::npos); assert(applied.find("recordingApplied = true") != std::string::npos); + assert(applied.find("out->addOutput") == std::string::npos); + assert(applied.find("while(") == std::string::npos); + assert(applied.find("Sched.loop") == std::string::npos); + assert(applied.find("delayMicroseconds") == std::string::npos); + + std::ifstream wavFile("src/AudioLib/OutputWAV.cpp"); + assert(wavFile.good()); + const std::string wavSource( + (std::istreambuf_iterator(wavFile)), + std::istreambuf_iterator() + ); + const size_t beginBegin = wavSource.find("bool OutputWAV::begin("); + const size_t beginEnd = wavSource.find("void OutputWAV::finish()", beginBegin); + assert(beginBegin != std::string::npos); + assert(beginEnd != std::string::npos); + const std::string begin = wavSource.substr(beginBegin, beginEnd - beginBegin); + assert(begin.find("while(") == std::string::npos); + assert(begin.find("Sched.loop") == std::string::npos); + const size_t wavServiceBegin = wavSource.find("void OutputWAV::service()"); + const size_t wavServiceEnd = wavSource.find("bool OutputWAV::isFinalized()", wavServiceBegin); + assert(wavServiceBegin != std::string::npos); + assert(wavServiceEnd != std::string::npos); + const std::string wavService = + wavSource.substr(wavServiceBegin, wavServiceEnd - wavServiceBegin); + assert(wavService.find("while(") == std::string::npos); + assert(wavService.find("Sched.loop") == std::string::npos); + assert(wavSource.find("FinalizeStage::INIT_SEEK_QUEUE") != std::string::npos); + assert(wavSource.find("FinalizeStage::INIT_HEADER_QUEUE") != std::string::npos); + assert(wavSource.find("if(finalizeResult == nullptr) return;") != std::string::npos); + + const size_t serviceBegin = source.find("void MixSystem::serviceRecording()"); + const size_t serviceEnd = source.find("void MixSystem::finishRecordingSync()", serviceBegin); + assert(serviceBegin != std::string::npos); + assert(serviceEnd != std::string::npos); + const std::string service = source.substr(serviceBegin, serviceEnd - serviceBegin); + assert(service.find("fsOut->isReady()") != std::string::npos); + assert(service.find("recordingMutex.lock()") != std::string::npos); + assert(service.find("out->addOutput") != std::string::npos); + + const size_t stopBegin = source.find("bool MixSystem::stopRecording()"); + const size_t stopEnd = source.find("void MixSystem::_startRecording()", stopBegin); + assert(stopBegin != std::string::npos); + assert(stopEnd != std::string::npos); + const std::string stop = source.substr(stopBegin, stopEnd - stopBegin); + assert(stop.find("recordingMutex.lock()") != std::string::npos); + assert(stop.find("recordingState = previousState") == std::string::npos); + + std::ifstream schedulerFile("src/Services/SDScheduler.cpp"); + assert(schedulerFile.good()); + const std::string scheduler( + (std::istreambuf_iterator(schedulerFile)), + std::istreambuf_iterator() + ); + assert(scheduler.find("xQueueSend(jobs, &job, 0)") != std::string::npos); + assert(scheduler.find("portMAX_DELAY") == std::string::npos); return 0; } From 5aa22cc453bcd6aa0949ffc7a358134bd52ca4d6 Mon Sep 17 00:00:00 2001 From: Ryan Trauntvein Date: Sat, 22 Aug 2026 11:07:14 -0700 Subject: [PATCH 08/15] Fix SpeedModifier::reset() to clear sourcePosition The deck-rate branch (2afe71c) replaced SpeedModifier's float remainder accumulator with a Q16.16 fixed-point sourcePosition, but the AAC-timing branch (ec46912) had already added reset(), which zeroed the old remainder field. Cherry-picking both onto the same base left reset() referencing a field that no longer exists, since the two edits touched non-overlapping hunks of the same file. Update reset() to zero sourcePosition instead, so MixSystem::_seekChannel's post-seek speed reset compiles and correctly rewinds the resampler position. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/AudioLib/SpeedModifier.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/AudioLib/SpeedModifier.cpp b/src/AudioLib/SpeedModifier.cpp index ab2b7e2..dbf4f5d 100644 --- a/src/AudioLib/SpeedModifier.cpp +++ b/src/AudioLib/SpeedModifier.cpp @@ -115,5 +115,5 @@ void SpeedModifier::setSource(Source* source){ void SpeedModifier::reset(){ dataBuffer->clear(); - remainder = 0; + sourcePosition = 0; } From bb4c94552da698ab97824d5932421ef9d9a7039d Mon Sep 17 00:00:00 2001 From: Ryan Trauntvein Date: Sat, 22 Aug 2026 11:09:27 -0700 Subject: [PATCH 09/15] Add integration self-check for SpeedModifier::reset() after seek No existing test exercised reset(), which is exactly the glue the deck-rate/AAC-timing cherry-pick conflict broke. Cover the contract MixSystem relies on: a post-seek reset() must drop the stale resampling position but must not silently change the DJ's requested or ramped rate. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- tests/SpeedModifierSelfCheck.cpp | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tests/SpeedModifierSelfCheck.cpp b/tests/SpeedModifierSelfCheck.cpp index c515c16..84fabcb 100644 --- a/tests/SpeedModifierSelfCheck.cpp +++ b/tests/SpeedModifierSelfCheck.cpp @@ -76,4 +76,19 @@ int main(){ assert(swapped.getRate() == SpeedModifier::MaxRate); assert(swapped.generate(output) == 256); for(size_t i = 0; i < 256; i++) assert(output[i] == 200); + + // MixSystem calls reset() after a channel seek to discard the stale + // resampling position, but must not lose the DJ's chosen rate. This is + // also the exact spot where cherry-picking the deck-rate rewrite (Q16.16 + // sourcePosition) together with the AAC-timing seek/reset glue silently + // left reset() referencing the removed float `remainder` field. + FakeSource seekSource(500, 1); + SpeedModifier seeking(&seekSource); + seeking.setRate(SpeedModifier::MaxRate); + assert(seeking.generate(output) == 256); + assert(seeking.getCurrentRate() == SpeedModifier::MaxRate); + seeking.reset(); + assert(seeking.getRate() == SpeedModifier::MaxRate); + assert(seeking.getCurrentRate() == SpeedModifier::MaxRate); + assert(seeking.generate(output) == 256); } From 78a9f3d7e52fd151dc50872812ae2ed1da308af1 Mon Sep 17 00:00:00 2001 From: Ryan Trauntvein Date: Sat, 22 Aug 2026 11:26:48 -0700 Subject: [PATCH 10/15] Fix SDScheduler::addJob() bool contract violations across all callers Independent review of bb4c945 found that SourceAAC ignored addJob()'s return value at its read-job (addReadJob) and seek (seekSourceFrame) call sites. When the queue is full, addJob() deletes the job and returns false, but SourceAAC still set readJobPending = true / mutated seek state as if the job had been queued. readResult then never arrives, so processReadJob(true)/open() spin forever in their busy-wait, and seek silently "succeeds" while the file position never moves. Queue capacity is only 8 vs. two AAC decks plus OutputWAV, so this is reachable under load. Audited every SDScheduler::addJob() caller and fixed the same unchecked-return hang/leak pattern everywhere it appeared: - SourceAAC::addReadJob: only set readJobPending on a successful enqueue; free the allocated buffer on failure instead of leaking it. - SourceAAC::seekSourceFrame: enqueue the seek job first and bail out with false (untouched elapsed/decoded state, no discarded in-flight read) if it can't be queued, instead of committing to the new position regardless. - SourceMP3::addReadJob / SourceWAV::addReadJob: same fix as SourceAAC's read path. - OutputAAC::addWriteJob: leave the buffer in freeBuffers on a failed enqueue instead of stranding it in a permanently-pending state (which could starve the encoder's free-buffer wait loops. OutputWAV's two addJob call sites already checked the return value correctly and needed no changes. Added tests/JobQueueContractSelfCheck.cpp: a structural scan proving every real Sched.addJob() call site checks the return value, plus a deterministic functional harness (mirroring the fixed addReadJob/processReadJob/seek state machine against an injectable fake scheduler) covering queue-full rejection + recovery for both read and seek jobs, and a bounded close/teardown wait. The harness is also run against a harness mirroring the pre-fix pattern to prove the check discriminates the reported defect (fails on old behavior, passes on fixed behavior). Re-ran all host self-checks (adts_timing_self_check, SpeedModifierSelfCheck, wav_header_selfcheck, and the new JobQueueContractSelfCheck) with -std=c++11 -Wall -Wextra -Werror -fsanitize=address,undefined -- all pass. Rebuilt the Arduino consumer firmware (cm:esp32:jayd): 1,026,806 bytes flash / 44,456 bytes static RAM, unchanged from the prior bb4c945 build (pure logic fix, no data size change). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> EOF ) --- src/AudioLib/OutputAAC.cpp | 4 +- src/AudioLib/SourceAAC.cpp | 26 ++- src/AudioLib/SourceMP3.cpp | 8 +- src/AudioLib/SourceWAV.cpp | 8 +- tests/JobQueueContractSelfCheck.cpp | 248 ++++++++++++++++++++++++++++ 5 files changed, 279 insertions(+), 15 deletions(-) create mode 100644 tests/JobQueueContractSelfCheck.cpp diff --git a/src/AudioLib/OutputAAC.cpp b/src/AudioLib/OutputAAC.cpp index 9d9b8b4..73e8768 100644 --- a/src/AudioLib/OutputAAC.cpp +++ b/src/AudioLib/OutputAAC.cpp @@ -181,13 +181,13 @@ void OutputAAC::addWriteJob(){ if(freeBuffers.empty()) return; uint8_t i = freeBuffers.front(); - Sched.addJob(new SDJob{ + if(!Sched.addJob(new SDJob{ .type = SDJob::SD_WRITE, .file = file, .size = outBuffers[i]->readAvailable(), .buffer = const_cast(outBuffers[i]->readData()), .result = &writeResult[i] - }); + })) return; // Queue full: leave the buffer in freeBuffers so it can be retried. freeBuffers.erase(freeBuffers.begin()); writePending[i] = true; diff --git a/src/AudioLib/SourceAAC.cpp b/src/AudioLib/SourceAAC.cpp index 6b8bdee..5213163 100644 --- a/src/AudioLib/SourceAAC.cpp +++ b/src/AudioLib/SourceAAC.cpp @@ -125,13 +125,17 @@ void SourceAAC::addReadJob(bool full){ return; } - Sched.addJob(new SDJob{ + if(!Sched.addJob(new SDJob{ .type = SDJob::SD_READ, .file = file, .size = size, .buffer = buf, .result = &readResult - }); + })){ + // Queue full: leave the read retryable rather than claiming data is in flight. + free(buf); + return; + } readJobPending = true; } @@ -388,6 +392,17 @@ bool SourceAAC::seekSourceFrame(uint64_t frame){ offset = frameIndex[index].offset; indexedFrame = frameIndex[index].sourceFrame; } + // Enqueue the seek before touching any state: if it can't be queued, the file + // position never moves, so leave elapsed/decoded state and any in-flight read + // untouched and fail cleanly so the caller can retry. + if(!Sched.addJob(new SDJob{ + .type = SDJob::SD_SEEK, + .file = file, + .size = offset, + .buffer = nullptr, + .result = nullptr + })) return false; + if(readJobPending && readResult != nullptr){ free(readResult->buffer); delete readResult; @@ -397,13 +412,6 @@ bool SourceAAC::seekSourceFrame(uint64_t frame){ }else if(readJobPending){ discardPendingRead = true; } - Sched.addJob(new SDJob{ - .type = SDJob::SD_SEEK, - .file = file, - .size = offset, - .buffer = nullptr, - .result = nullptr - }); portENTER_CRITICAL(&timingMux); elapsedSourceFrames = frame; portEXIT_CRITICAL(&timingMux); diff --git a/src/AudioLib/SourceMP3.cpp b/src/AudioLib/SourceMP3.cpp index 1692cd3..fdb1933 100644 --- a/src/AudioLib/SourceMP3.cpp +++ b/src/AudioLib/SourceMP3.cpp @@ -76,13 +76,17 @@ void SourceMP3::addReadJob(bool full){ buf = static_cast(ps_malloc(size)); } - Sched.addJob(new SDJob{ + if(!Sched.addJob(new SDJob{ .type = SDJob::SD_READ, .file = file, .size = size, .buffer = buf, .result = &readResult - }); + })){ + // Queue full: leave the read retryable rather than claiming data is in flight. + free(buf); + return; + } readJobPending = true; } diff --git a/src/AudioLib/SourceWAV.cpp b/src/AudioLib/SourceWAV.cpp index 0463ac6..a3fdcf8 100644 --- a/src/AudioLib/SourceWAV.cpp +++ b/src/AudioLib/SourceWAV.cpp @@ -72,13 +72,17 @@ void SourceWAV::addReadJob(bool full){ buf = static_cast(ps_malloc(size)); } - Sched.addJob(new SDJob{ + if(!Sched.addJob(new SDJob{ .type = SDJob::SD_READ, .file = file, .size = size, .buffer = buf, .result = &readResult - }); + })){ + // Queue full: leave the read retryable rather than claiming data is in flight. + free(buf); + return; + } readJobPending = true; } diff --git a/tests/JobQueueContractSelfCheck.cpp b/tests/JobQueueContractSelfCheck.cpp new file mode 100644 index 0000000..74d0580 --- /dev/null +++ b/tests/JobQueueContractSelfCheck.cpp @@ -0,0 +1,248 @@ +// Integration self-check for the SDScheduler::addJob() bool-return contract +// introduced by the recording-core work (queue-full => delete-on-full, +// return false) versus every job-issuing call site merged from the AAC/MP3/ +// WAV sources and the AAC/WAV outputs. +// +// It has two halves: +// 1. A structural scan of the real, shipped call sites proving none of them +// discard SDScheduler::addJob()'s return value as a bare statement (the +// exact defect class reported: SourceAAC ignored the bool at its read-job +// and seek-job call sites, so a full queue left readJobPending stuck true +// with readResult forever null, spinning processReadJob(true)/open() +// forever). +// 2. A deterministic functional harness mirroring the fixed addReadJob/ +// processReadJob/seek state machine against an injectable fake scheduler, +// covering: read-job queue-full rejection + recovery, seek rejection +// preserving state (no silent success) + recovery, and a bounded +// close/teardown wait that would only ever hang under the pre-fix +// unconditional-pending pattern (also exercised here, and shown to fail +// the same bounded wait, proving this check discriminates the bug). + +#include +#include +#include +#include + +namespace { + +std::string readFile(const char* path){ + std::ifstream f(path); + assert(f.good()); + return std::string((std::istreambuf_iterator(f)), std::istreambuf_iterator()); +} + +// True if every "Sched.addJob(" occurrence in `source` is used (checked with +// if/return/assignment, etc.) rather than discarded as a bare statement. +bool everyAddJobCallIsChecked(const std::string& source){ + size_t pos = 0; + bool found = false; + while((pos = source.find("Sched.addJob(", pos)) != std::string::npos){ + found = true; + size_t lineStart = source.rfind('\n', pos); + lineStart = (lineStart == std::string::npos) ? 0 : lineStart + 1; + size_t contentStart = source.find_first_not_of(" \t", lineStart); + if(contentStart == std::string::npos || contentStart >= pos) return false; + // A bare, discarded call looks like "Sched.addJob(" starting the + // statement. Any checked form (if(!..., if(..., return ..., + // x = ...) has other tokens before "Sched.addJob(" on that line. + if(source.compare(contentStart, pos - contentStart, "Sched.addJob(") == 0){ + return false; + } + pos += 1; + } + return found; +} + +// --- Functional harness: mirrors SourceAAC's fixed addReadJob/processReadJob --- + +struct FakeScheduler { + int capacity; + int inFlight = 0; + explicit FakeScheduler(int cap) : capacity(cap){} + bool addJob(){ + if(inFlight >= capacity) return false; + inFlight++; + return true; + } + void completeOne(){ if(inFlight > 0) inFlight--; } +}; + +struct ReadJobHarness { + FakeScheduler& sched; + bool pending = false; + bool resultReady = false; + int allocated = 0; + int freed = 0; + explicit ReadJobHarness(FakeScheduler& s) : sched(s){} + + // Fixed contract: only claim a job is in flight once it is actually queued. + void addReadJob(){ + if(pending) return; + allocated++; + if(!sched.addJob()){ + freed++; // caller must free its own buffer; addJob() never took ownership. + return; + } + pending = true; + } + + // Bounded stand-in for the real busy-wait: returns false ("would have + // hung") instead of spinning forever if a wait is requested on a result + // that will never arrive. + bool processReadJob(bool wait, int maxSpin){ + if(!pending) return true; + if(!resultReady){ + if(!wait) return true; + int i = 0; + for(; i < maxSpin && !resultReady; i++){} + if(!resultReady) return false; + } + resultReady = false; + pending = false; + return true; + } + + void deliverResult(){ resultReady = true; sched.completeOne(); } +}; + +// Pre-fix behaviour: pending is claimed unconditionally, regardless of +// whether the job was actually queued. Used to prove this check would have +// caught the reported defect. +struct BuggyReadJobHarness { + FakeScheduler& sched; + bool pending = false; + bool resultReady = false; + explicit BuggyReadJobHarness(FakeScheduler& s) : sched(s){} + void addReadJob(){ + if(pending) return; + sched.addJob(); // return value ignored, exactly like the reported bug + pending = true; + } + bool processReadJob(bool wait, int maxSpin){ + if(!pending) return true; + if(!resultReady){ + if(!wait) return true; + int i = 0; + for(; i < maxSpin && !resultReady; i++){} + if(!resultReady) return false; + } + resultReady = false; + pending = false; + return true; + } +}; + +struct SeekHarness { + FakeScheduler& sched; + uint64_t elapsedFrames; + explicit SeekHarness(FakeScheduler& s, uint64_t initial) : sched(s), elapsedFrames(initial){} + // Fixed contract: enqueue first, only commit state on success. + bool seek(uint64_t frame){ + if(!sched.addJob()) return false; + elapsedFrames = frame; + return true; + } +}; + +struct BuggySeekHarness { + FakeScheduler& sched; + uint64_t elapsedFrames; + explicit BuggySeekHarness(FakeScheduler& s, uint64_t initial) : sched(s), elapsedFrames(initial){} + bool seek(uint64_t frame){ + sched.addJob(); // return value ignored, exactly like the reported bug + elapsedFrames = frame; // silent "success" even if the seek was dropped + return true; + } +}; + +template +bool waitForIdle(Harness& h, int maxIterations){ + for(int i = 0; i < maxIterations; i++){ + if(!h.pending) return true; + } + return false; +} + +} // namespace + +int main(){ + // --- 1. Structural: every real call site must check addJob()'s return value. + const char* callers[] = { + "src/AudioLib/SourceAAC.cpp", + "src/AudioLib/SourceMP3.cpp", + "src/AudioLib/SourceWAV.cpp", + "src/AudioLib/OutputAAC.cpp", + "src/AudioLib/OutputWAV.cpp", + }; + for(const char* path : callers){ + assert(everyAddJobCallIsChecked(readFile(path))); + } + + // --- 2. Read job: queue-full rejection must not claim a pending job. + { + FakeScheduler sched(1); + assert(sched.addJob()); // fill the only slot with an unrelated job + ReadJobHarness h(sched); + h.addReadJob(); + assert(!h.pending); // fixed: rejection leaves the read retryable + assert(h.allocated == h.freed); // no leaked buffer on rejection + assert(h.processReadJob(true, 100000)); // no job pending: never spins + + // Recovery: free capacity, retry succeeds. + sched.completeOne(); + h.addReadJob(); + assert(h.pending); + h.deliverResult(); + assert(h.processReadJob(true, 100000)); + assert(!h.pending); + } + + // --- 2b. Same scenario against the pre-fix pattern must fail the bounded + // wait, proving this check discriminates the reported defect. + { + FakeScheduler sched(1); + assert(sched.addJob()); + BuggyReadJobHarness h(sched); + h.addReadJob(); + assert(h.pending); // bug: falsely claims the job is in flight + assert(!h.processReadJob(true, 1000)); // would spin forever in production + } + + // --- 3. Seek: rejection must not silently move the tracked position. + { + FakeScheduler sched(1); + assert(sched.addJob()); + SeekHarness h(sched, 42); + assert(!h.seek(1000)); + assert(h.elapsedFrames == 42); // state preserved for a coherent retry + + sched.completeOne(); + assert(h.seek(1000)); + assert(h.elapsedFrames == 1000); + } + { + FakeScheduler sched(1); + assert(sched.addJob()); + BuggySeekHarness h(sched, 42); + assert(h.seek(1000)); // bug: reports success despite the dropped seek + assert(h.elapsedFrames == 1000); // silent corruption: file was never seeked + } + + // --- 4. Close/teardown must not hang after a rejected job. + { + FakeScheduler sched(1); + assert(sched.addJob()); + ReadJobHarness h(sched); + h.addReadJob(); + assert(waitForIdle(h, 1000)); // fixed: already idle, returns immediately + } + { + FakeScheduler sched(1); + assert(sched.addJob()); + BuggyReadJobHarness h(sched); + h.addReadJob(); + assert(!waitForIdle(h, 1000)); // bug: stuck pending forever + } + + return 0; +} From 435ce90a0453af8bd85a05742a7e752dca116fca Mon Sep 17 00:00:00 2001 From: Ryan Trauntvein Date: Tue, 25 Aug 2026 15:32:03 -0700 Subject: [PATCH 11/15] Scope nonblocking SD enqueue to recording Preserve the scheduler contract used by existing decoders and encoders while keeping OutputWAV queue retries nonblocking. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> (cherry picked from commit ac6ba80245fabe18ac90f87bc36a5d7c203a8624) --- src/AudioLib/OutputWAV.cpp | 4 ++-- src/Services/SDScheduler.cpp | 8 +++++++- src/Services/SDScheduler.h | 3 ++- tests/wav_header_selfcheck.cpp | 11 +++++++++-- 4 files changed, 20 insertions(+), 6 deletions(-) diff --git a/src/AudioLib/OutputWAV.cpp b/src/AudioLib/OutputWAV.cpp index 24e84c1..49d869f 100644 --- a/src/AudioLib/OutputWAV.cpp +++ b/src/AudioLib/OutputWAV.cpp @@ -250,7 +250,7 @@ bool OutputWAV::addWriteJob(){ const size_t size = outBuffers[i]->readAvailable(); if(size == 0) return true; - if(!Sched.addJob(new SDJob{ + if(!Sched.tryAddJob(new SDJob{ .type = SDJob::SD_WRITE, .file = file, .size = size, @@ -295,7 +295,7 @@ bool OutputWAV::hasPendingWrites() const{ } bool OutputWAV::queueFinalizeJob(SDJob::Type type){ - return Sched.addJob(new SDJob { + return Sched.tryAddJob(new SDJob { .type = type, .file = file, .size = type == SDJob::SD_SEEK ? 0 : sizeof(WavHeader), diff --git a/src/Services/SDScheduler.cpp b/src/Services/SDScheduler.cpp index a743485..6b92b8a 100644 --- a/src/Services/SDScheduler.cpp +++ b/src/Services/SDScheduler.cpp @@ -10,7 +10,13 @@ SDScheduler::~SDScheduler(){ vQueueDelete(jobs); } -bool SDScheduler::addJob(SDJob *job){ +void SDScheduler::addJob(SDJob *job){ + if(job == nullptr) return; + if(xQueueSend(jobs, &job, portMAX_DELAY) == pdTRUE) return; + delete job; +} + +bool SDScheduler::tryAddJob(SDJob *job){ if(job == nullptr) return false; if(xQueueSend(jobs, &job, 0) == pdTRUE) return true; delete job; diff --git a/src/Services/SDScheduler.h b/src/Services/SDScheduler.h index 0fd7b87..3e034df 100644 --- a/src/Services/SDScheduler.h +++ b/src/Services/SDScheduler.h @@ -24,7 +24,8 @@ class SDScheduler : public LoopListener { SDScheduler(); ~SDScheduler(); - bool addJob(SDJob *job); + void addJob(SDJob *job); + bool tryAddJob(SDJob *job); void loop(uint micros) override; private: static constexpr uint8_t jobCapacity = 8; diff --git a/tests/wav_header_selfcheck.cpp b/tests/wav_header_selfcheck.cpp index dd02dad..af00f39 100644 --- a/tests/wav_header_selfcheck.cpp +++ b/tests/wav_header_selfcheck.cpp @@ -115,7 +115,14 @@ int main(){ (std::istreambuf_iterator(schedulerFile)), std::istreambuf_iterator() ); - assert(scheduler.find("xQueueSend(jobs, &job, 0)") != std::string::npos); - assert(scheduler.find("portMAX_DELAY") == std::string::npos); + const size_t tryAddBegin = scheduler.find("bool SDScheduler::tryAddJob("); + const size_t tryAddEnd = scheduler.find("void SDScheduler::loop(", tryAddBegin); + assert(tryAddBegin != std::string::npos); + assert(tryAddEnd != std::string::npos); + const std::string tryAdd = scheduler.substr(tryAddBegin, tryAddEnd - tryAddBegin); + assert(tryAdd.find("xQueueSend(jobs, &job, 0)") != std::string::npos); + assert(tryAdd.find("portMAX_DELAY") == std::string::npos); + assert(wavSource.find("Sched.tryAddJob(") != std::string::npos); + assert(wavSource.find("Sched.addJob(") == std::string::npos); return 0; } From 35b4f80a47f5e5eef029e0c57db76699a076df4a Mon Sep 17 00:00:00 2001 From: Ryan Trauntvein Date: Tue, 25 Aug 2026 15:57:47 -0700 Subject: [PATCH 12/15] Reconcile addJob bool-audit fix with restored blocking SDScheduler API PR #20 advanced to ac6ba80 ("Scope nonblocking SD enqueue to recording"), reverting SDScheduler::addJob() back to the legacy blocking void contract and adding a new nonblocking bool tryAddJob() scoped exclusively to OutputWAV's real-time recording write/finalize path. That makes the earlier 78a9f3d fix (which retrofitted bool-return checking onto every addJob() caller, matching an intermediate nonblocking-everywhere revision of #20) obsolete and non-compiling against the restored void signature. Revert SourceAAC::addReadJob/seekSourceFrame, SourceMP3::addReadJob, SourceWAV::addReadJob, and OutputAAC::addWriteJob to plain blocking Sched.addJob(...) calls with no return-value check, matching the restored contract. OutputWAV's two call sites already correctly use the new Sched.tryAddJob(...) (renamed by ac6ba80 from its prior addJob() bool usage) with the return value checked; no changes needed there. Queue-ownership review: Sched is drained both by the main sketch loop (LoopManager::addListener(&Sched)) and, at specific synchronous wait points, directly by the calling thread (e.g. MixSystem open/openChannel busy-wait via Sched.loop(0) while awaiting isReadReady()). The audio task's blocking addJob() calls (SourceAAC/MP3/WAV read jobs, OutputAAC write jobs) are drained by a different, independent thread/task, so a full queue blocks that call only until the drain thread services it; no self-deadlock. OutputWAV's write/finalize path runs on the same real-time audio task and must not block it, which is exactly why tryAddJob()'s nonblocking, checked, retry-on-false contract remains scoped there. Rewrite tests/JobQueueContractSelfCheck.cpp for the new dual-API contract: structural checks assert SourceAAC/SourceMP3/SourceWAV/ OutputAAC call only the blocking Sched.addJob and never Sched.tryAddJob, OutputWAV calls only Sched.tryAddJob with every call site's return checked and never the blocking Sched.addJob, and SDScheduler.h declares both signatures. A functional harness (mirroring OutputWAV::addWriteJob) proves queue-full rejection under tryAddJob() still leaves the buffer retryable with no leak/false pending state, plus a "buggy" counterpart proving the check discriminates a regression of the original defect class. All four host self-checks (adts_timing_self_check, run-speed-modifier-self-check.sh, wav_header_selfcheck, JobQueueContractSelfCheck) pass with -Wall -Wextra -Werror -fsanitize=address,undefined. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/AudioLib/OutputAAC.cpp | 4 +- src/AudioLib/SourceAAC.cpp | 26 +-- src/AudioLib/SourceMP3.cpp | 8 +- src/AudioLib/SourceWAV.cpp | 8 +- tests/JobQueueContractSelfCheck.cpp | 260 ++++++++++------------------ 5 files changed, 110 insertions(+), 196 deletions(-) diff --git a/src/AudioLib/OutputAAC.cpp b/src/AudioLib/OutputAAC.cpp index 73e8768..9d9b8b4 100644 --- a/src/AudioLib/OutputAAC.cpp +++ b/src/AudioLib/OutputAAC.cpp @@ -181,13 +181,13 @@ void OutputAAC::addWriteJob(){ if(freeBuffers.empty()) return; uint8_t i = freeBuffers.front(); - if(!Sched.addJob(new SDJob{ + Sched.addJob(new SDJob{ .type = SDJob::SD_WRITE, .file = file, .size = outBuffers[i]->readAvailable(), .buffer = const_cast(outBuffers[i]->readData()), .result = &writeResult[i] - })) return; // Queue full: leave the buffer in freeBuffers so it can be retried. + }); freeBuffers.erase(freeBuffers.begin()); writePending[i] = true; diff --git a/src/AudioLib/SourceAAC.cpp b/src/AudioLib/SourceAAC.cpp index 5213163..6b8bdee 100644 --- a/src/AudioLib/SourceAAC.cpp +++ b/src/AudioLib/SourceAAC.cpp @@ -125,17 +125,13 @@ void SourceAAC::addReadJob(bool full){ return; } - if(!Sched.addJob(new SDJob{ + Sched.addJob(new SDJob{ .type = SDJob::SD_READ, .file = file, .size = size, .buffer = buf, .result = &readResult - })){ - // Queue full: leave the read retryable rather than claiming data is in flight. - free(buf); - return; - } + }); readJobPending = true; } @@ -392,17 +388,6 @@ bool SourceAAC::seekSourceFrame(uint64_t frame){ offset = frameIndex[index].offset; indexedFrame = frameIndex[index].sourceFrame; } - // Enqueue the seek before touching any state: if it can't be queued, the file - // position never moves, so leave elapsed/decoded state and any in-flight read - // untouched and fail cleanly so the caller can retry. - if(!Sched.addJob(new SDJob{ - .type = SDJob::SD_SEEK, - .file = file, - .size = offset, - .buffer = nullptr, - .result = nullptr - })) return false; - if(readJobPending && readResult != nullptr){ free(readResult->buffer); delete readResult; @@ -412,6 +397,13 @@ bool SourceAAC::seekSourceFrame(uint64_t frame){ }else if(readJobPending){ discardPendingRead = true; } + Sched.addJob(new SDJob{ + .type = SDJob::SD_SEEK, + .file = file, + .size = offset, + .buffer = nullptr, + .result = nullptr + }); portENTER_CRITICAL(&timingMux); elapsedSourceFrames = frame; portEXIT_CRITICAL(&timingMux); diff --git a/src/AudioLib/SourceMP3.cpp b/src/AudioLib/SourceMP3.cpp index fdb1933..1692cd3 100644 --- a/src/AudioLib/SourceMP3.cpp +++ b/src/AudioLib/SourceMP3.cpp @@ -76,17 +76,13 @@ void SourceMP3::addReadJob(bool full){ buf = static_cast(ps_malloc(size)); } - if(!Sched.addJob(new SDJob{ + Sched.addJob(new SDJob{ .type = SDJob::SD_READ, .file = file, .size = size, .buffer = buf, .result = &readResult - })){ - // Queue full: leave the read retryable rather than claiming data is in flight. - free(buf); - return; - } + }); readJobPending = true; } diff --git a/src/AudioLib/SourceWAV.cpp b/src/AudioLib/SourceWAV.cpp index a3fdcf8..0463ac6 100644 --- a/src/AudioLib/SourceWAV.cpp +++ b/src/AudioLib/SourceWAV.cpp @@ -72,17 +72,13 @@ void SourceWAV::addReadJob(bool full){ buf = static_cast(ps_malloc(size)); } - if(!Sched.addJob(new SDJob{ + Sched.addJob(new SDJob{ .type = SDJob::SD_READ, .file = file, .size = size, .buffer = buf, .result = &readResult - })){ - // Queue full: leave the read retryable rather than claiming data is in flight. - free(buf); - return; - } + }); readJobPending = true; } diff --git a/tests/JobQueueContractSelfCheck.cpp b/tests/JobQueueContractSelfCheck.cpp index 74d0580..3722b88 100644 --- a/tests/JobQueueContractSelfCheck.cpp +++ b/tests/JobQueueContractSelfCheck.cpp @@ -1,22 +1,28 @@ -// Integration self-check for the SDScheduler::addJob() bool-return contract -// introduced by the recording-core work (queue-full => delete-on-full, -// return false) versus every job-issuing call site merged from the AAC/MP3/ -// WAV sources and the AAC/WAV outputs. +// Integration self-check for the SDScheduler dual-API contract restored by +// "Scope nonblocking SD enqueue to recording": legacy blocking +// void SDScheduler::addJob(SDJob*) for every decode/encode call site +// (SourceAAC, SourceMP3, SourceWAV, OutputAAC), plus a new nonblocking +// bool SDScheduler::tryAddJob(SDJob*) scoped exclusively to OutputWAV's +// real-time recording write/finalize path. +// +// This supersedes an earlier universal "every addJob() caller must check a +// bool return" contract, which was correct for one intermediate revision of +// the recording-core work but is no longer what ships: addJob() is void +// again, so a caller checking its return would not even compile, and a +// caller silently reusing tryAddJob() outside OutputWAV would reintroduce +// the false-pending queue-full class of bug this check exists to prevent. // // It has two halves: -// 1. A structural scan of the real, shipped call sites proving none of them -// discard SDScheduler::addJob()'s return value as a bare statement (the -// exact defect class reported: SourceAAC ignored the bool at its read-job -// and seek-job call sites, so a full queue left readJobPending stuck true -// with readResult forever null, spinning processReadJob(true)/open() -// forever). -// 2. A deterministic functional harness mirroring the fixed addReadJob/ -// processReadJob/seek state machine against an injectable fake scheduler, -// covering: read-job queue-full rejection + recovery, seek rejection -// preserving state (no silent success) + recovery, and a bounded -// close/teardown wait that would only ever hang under the pre-fix -// unconditional-pending pattern (also exercised here, and shown to fail -// the same bounded wait, proving this check discriminates the bug). +// 1. A structural scan of the real, shipped call sites proving: decode/ +// encode sources call only the blocking Sched.addJob(...) (never +// Sched.tryAddJob(...)), and OutputWAV calls only Sched.tryAddJob(...) +// (never the blocking Sched.addJob(...)) with every call site checking +// the bool return. +// 2. A deterministic functional harness mirroring OutputWAV's +// addWriteJob()/queueFinalizeJob() use of tryAddJob(): queue-full +// rejection must leave the buffer retryable (no leak, no false +// "in flight" state), and recovery once the queue has room must +// succeed cleanly. #include #include @@ -31,35 +37,39 @@ std::string readFile(const char* path){ return std::string((std::istreambuf_iterator(f)), std::istreambuf_iterator()); } -// True if every "Sched.addJob(" occurrence in `source` is used (checked with -// if/return/assignment, etc.) rather than discarded as a bare statement. -bool everyAddJobCallIsChecked(const std::string& source){ +bool contains(const std::string& source, const char* token){ + return source.find(token) != std::string::npos; +} + +// True if every occurrence of `token` (e.g. "Sched.tryAddJob(") in `source` +// is used (if/return/assignment, etc.) rather than discarded as a bare +// statement starting its line. +bool everyCallIsChecked(const std::string& source, const char* token){ + const size_t tokenLen = std::string(token).length(); size_t pos = 0; bool found = false; - while((pos = source.find("Sched.addJob(", pos)) != std::string::npos){ + while((pos = source.find(token, pos)) != std::string::npos){ found = true; size_t lineStart = source.rfind('\n', pos); lineStart = (lineStart == std::string::npos) ? 0 : lineStart + 1; size_t contentStart = source.find_first_not_of(" \t", lineStart); if(contentStart == std::string::npos || contentStart >= pos) return false; - // A bare, discarded call looks like "Sched.addJob(" starting the - // statement. Any checked form (if(!..., if(..., return ..., - // x = ...) has other tokens before "Sched.addJob(" on that line. - if(source.compare(contentStart, pos - contentStart, "Sched.addJob(") == 0){ + if(source.compare(contentStart, pos - contentStart, token) == 0){ return false; } - pos += 1; + pos += tokenLen; } return found; } -// --- Functional harness: mirrors SourceAAC's fixed addReadJob/processReadJob --- +// --- Functional harness: mirrors OutputWAV's fixed addWriteJob(), which +// only claims a buffer is in flight once tryAddJob() actually enqueues it. struct FakeScheduler { int capacity; int inFlight = 0; explicit FakeScheduler(int cap) : capacity(cap){} - bool addJob(){ + bool tryAddJob(){ if(inFlight >= capacity) return false; inFlight++; return true; @@ -67,181 +77,101 @@ struct FakeScheduler { void completeOne(){ if(inFlight > 0) inFlight--; } }; -struct ReadJobHarness { +struct WriteJobHarness { FakeScheduler& sched; bool pending = false; - bool resultReady = false; - int allocated = 0; - int freed = 0; - explicit ReadJobHarness(FakeScheduler& s) : sched(s){} - - // Fixed contract: only claim a job is in flight once it is actually queued. - void addReadJob(){ - if(pending) return; - allocated++; - if(!sched.addJob()){ - freed++; // caller must free its own buffer; addJob() never took ownership. - return; - } + bool bufferHeld = true; // starts owned by the free-buffer pool + explicit WriteJobHarness(FakeScheduler& s) : sched(s){} + + // Fixed contract: only hand the buffer to the scheduler once it is + // actually queued; a rejection leaves it retryable in the pool. + bool addWriteJob(){ + if(pending || !bufferHeld) return false; + if(!sched.tryAddJob()) return false; // buffer stays in the pool + bufferHeld = false; pending = true; + return true; } - // Bounded stand-in for the real busy-wait: returns false ("would have - // hung") instead of spinning forever if a wait is requested on a result - // that will never arrive. - bool processReadJob(bool wait, int maxSpin){ - if(!pending) return true; - if(!resultReady){ - if(!wait) return true; - int i = 0; - for(; i < maxSpin && !resultReady; i++){} - if(!resultReady) return false; - } - resultReady = false; + void deliverResult(){ + sched.completeOne(); pending = false; - return true; + bufferHeld = true; // returned to the pool once written } - - void deliverResult(){ resultReady = true; sched.completeOne(); } }; -// Pre-fix behaviour: pending is claimed unconditionally, regardless of -// whether the job was actually queued. Used to prove this check would have -// caught the reported defect. -struct BuggyReadJobHarness { +// Pre-fix behaviour: the buffer is handed off unconditionally regardless of +// whether tryAddJob() actually queued it. Used to prove this check would +// have caught the reported defect class if it recurred. +struct BuggyWriteJobHarness { FakeScheduler& sched; bool pending = false; - bool resultReady = false; - explicit BuggyReadJobHarness(FakeScheduler& s) : sched(s){} - void addReadJob(){ - if(pending) return; - sched.addJob(); // return value ignored, exactly like the reported bug + bool bufferHeld = true; + explicit BuggyWriteJobHarness(FakeScheduler& s) : sched(s){} + bool addWriteJob(){ + if(pending || !bufferHeld) return false; + sched.tryAddJob(); // return value ignored, exactly like the reported bug + bufferHeld = false; pending = true; - } - bool processReadJob(bool wait, int maxSpin){ - if(!pending) return true; - if(!resultReady){ - if(!wait) return true; - int i = 0; - for(; i < maxSpin && !resultReady; i++){} - if(!resultReady) return false; - } - resultReady = false; - pending = false; return true; } }; -struct SeekHarness { - FakeScheduler& sched; - uint64_t elapsedFrames; - explicit SeekHarness(FakeScheduler& s, uint64_t initial) : sched(s), elapsedFrames(initial){} - // Fixed contract: enqueue first, only commit state on success. - bool seek(uint64_t frame){ - if(!sched.addJob()) return false; - elapsedFrames = frame; - return true; - } -}; - -struct BuggySeekHarness { - FakeScheduler& sched; - uint64_t elapsedFrames; - explicit BuggySeekHarness(FakeScheduler& s, uint64_t initial) : sched(s), elapsedFrames(initial){} - bool seek(uint64_t frame){ - sched.addJob(); // return value ignored, exactly like the reported bug - elapsedFrames = frame; // silent "success" even if the seek was dropped - return true; - } -}; - -template -bool waitForIdle(Harness& h, int maxIterations){ - for(int i = 0; i < maxIterations; i++){ - if(!h.pending) return true; - } - return false; -} - } // namespace int main(){ - // --- 1. Structural: every real call site must check addJob()'s return value. - const char* callers[] = { + // --- 1. Structural: decode/encode sources use only the restored + // blocking API; OutputWAV uses only the nonblocking API, checked. + const char* blockingCallers[] = { "src/AudioLib/SourceAAC.cpp", "src/AudioLib/SourceMP3.cpp", "src/AudioLib/SourceWAV.cpp", "src/AudioLib/OutputAAC.cpp", - "src/AudioLib/OutputWAV.cpp", }; - for(const char* path : callers){ - assert(everyAddJobCallIsChecked(readFile(path))); + for(const char* path : blockingCallers){ + const std::string source = readFile(path); + assert(contains(source, "Sched.addJob(")); + assert(!contains(source, "Sched.tryAddJob(")); } - - // --- 2. Read job: queue-full rejection must not claim a pending job. { - FakeScheduler sched(1); - assert(sched.addJob()); // fill the only slot with an unrelated job - ReadJobHarness h(sched); - h.addReadJob(); - assert(!h.pending); // fixed: rejection leaves the read retryable - assert(h.allocated == h.freed); // no leaked buffer on rejection - assert(h.processReadJob(true, 100000)); // no job pending: never spins - - // Recovery: free capacity, retry succeeds. - sched.completeOne(); - h.addReadJob(); - assert(h.pending); - h.deliverResult(); - assert(h.processReadJob(true, 100000)); - assert(!h.pending); + const std::string source = readFile("src/AudioLib/OutputWAV.cpp"); + assert(!contains(source, "Sched.addJob(")); + assert(everyCallIsChecked(source, "Sched.tryAddJob(")); } - - // --- 2b. Same scenario against the pre-fix pattern must fail the bounded - // wait, proving this check discriminates the reported defect. { - FakeScheduler sched(1); - assert(sched.addJob()); - BuggyReadJobHarness h(sched); - h.addReadJob(); - assert(h.pending); // bug: falsely claims the job is in flight - assert(!h.processReadJob(true, 1000)); // would spin forever in production + const std::string header = readFile("src/Services/SDScheduler.h"); + assert(contains(header, "void addJob(SDJob *job);")); + assert(contains(header, "bool tryAddJob(SDJob *job);")); } - // --- 3. Seek: rejection must not silently move the tracked position. + // --- 2. Write job: queue-full rejection must not claim a pending job + // or strand the buffer outside the free-buffer pool. { FakeScheduler sched(1); - assert(sched.addJob()); - SeekHarness h(sched, 42); - assert(!h.seek(1000)); - assert(h.elapsedFrames == 42); // state preserved for a coherent retry + assert(sched.tryAddJob()); // fill the only slot with an unrelated job + WriteJobHarness h(sched); + assert(!h.addWriteJob()); // fixed: rejection leaves the write retryable + assert(!h.pending); + assert(h.bufferHeld); // buffer never left the pool + // Recovery: free capacity, retry succeeds. sched.completeOne(); - assert(h.seek(1000)); - assert(h.elapsedFrames == 1000); - } - { - FakeScheduler sched(1); - assert(sched.addJob()); - BuggySeekHarness h(sched, 42); - assert(h.seek(1000)); // bug: reports success despite the dropped seek - assert(h.elapsedFrames == 1000); // silent corruption: file was never seeked + assert(h.addWriteJob()); + assert(h.pending); + h.deliverResult(); + assert(!h.pending); + assert(h.bufferHeld); } - // --- 4. Close/teardown must not hang after a rejected job. - { - FakeScheduler sched(1); - assert(sched.addJob()); - ReadJobHarness h(sched); - h.addReadJob(); - assert(waitForIdle(h, 1000)); // fixed: already idle, returns immediately - } + // --- 2b. Same scenario against the pre-fix pattern must strand the + // buffer, proving this check discriminates the reported defect class. { FakeScheduler sched(1); - assert(sched.addJob()); - BuggyReadJobHarness h(sched); - h.addReadJob(); - assert(!waitForIdle(h, 1000)); // bug: stuck pending forever + assert(sched.tryAddJob()); + BuggyWriteJobHarness h(sched); + assert(h.addWriteJob()); // bug: reports success despite the drop + assert(h.pending); // bug: falsely claims the job is in flight + assert(!h.bufferHeld); // bug: buffer stranded outside the pool } return 0; From ad85f455d90d01ae1445e1bcb32f5a397d83eb8b Mon Sep 17 00:00:00 2001 From: Ryan Trauntvein Date: Tue, 25 Aug 2026 17:28:51 -0700 Subject: [PATCH 13/15] Add SourceAAC::Status and MixSystem status/gain accessors for guarded firmware The wireless-bringup guarded firmware's MixScreen (remoteStatus()/ remoteUpdateMasterGain(), used by an HTTP status/control server gated behind -DJAYD_WIRELESS_BRINGUP) needs a coarse per-channel decoder status and a way to re-apply master I2S gain after an out-of-band volume change. Neither existed in PR #22. - SourceAAC gains an enum class Status { CLOSED, DATA, STARVED, END_OF_STREAM, FAILED } and getStatus(), tracked purely observationally at open/close/generate() transitions (construct-fail, decoder-init-fail, starved-before-first-frame, per-call DATA/END_OF_STREAM). No existing control flow (ADTS parsing, frame index, seek, EOF one-shot reset, decode-discard) is touched. - MixSystem::getChannelStatus(channel) wraps source[c]->getStatus(), following the same sourceMutex/cleanupRetiredSources() locking pattern already used by hasChannel(). - MixSystem::updateGain() extracts the existing inline i2s->setGain(0.4f*volumeLevel/255.0f) expression from the constructor into a callable method, and the constructor now calls it too. All 4 host self-checks re-run clean (-Wall -Wextra -Werror -fsanitize=address,undefined). Standard Arduino consumer build: 1,026,890 B flash / 44,456 B RAM (+56 B flash, +0 B RAM vs 35b4f80). Guarded JayD-Firmware build (-DJAYD_WIRELESS_BRINGUP) against this branch: 1,834,298 B flash / 54,968 B RAM, MixScreen.cpp compiles clean. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/AudioLib/SourceAAC.cpp | 12 ++++++++++++ src/AudioLib/SourceAAC.h | 19 +++++++++++++++++++ src/AudioLib/Systems/MixSystem.cpp | 15 ++++++++++++++- src/AudioLib/Systems/MixSystem.h | 7 +++++++ 4 files changed, 52 insertions(+), 1 deletion(-) diff --git a/src/AudioLib/SourceAAC.cpp b/src/AudioLib/SourceAAC.cpp index 6b8bdee..8095c1e 100644 --- a/src/AudioLib/SourceAAC.cpp +++ b/src/AudioLib/SourceAAC.cpp @@ -39,6 +39,7 @@ void SourceAAC::open(fs::File file){ buildFrameIndex(); if(sourceSampleRate == 0){ Serial.println("SourceAAC: no valid ADTS frames"); + status = Status::FAILED; return; } file.seek(firstFrameOffset); @@ -46,9 +47,11 @@ void SourceAAC::open(fs::File file){ hAACDecoder = AACInitDecoder(); if(hAACDecoder == nullptr){ Serial.println("Decoder construct fail"); + status = Status::FAILED; return; } + status = Status::STARVED; addReadJob(true); } @@ -60,6 +63,10 @@ bool SourceAAC::isReadReady() const { return !readJobPending || readResult != nullptr; } +SourceAAC::Status SourceAAC::getStatus() const { + return status.load(); +} + void SourceAAC::close(){ if(readJobPending){ while(readResult == nullptr){ @@ -93,6 +100,7 @@ void SourceAAC::close(){ readEof = false; discardPendingRead = false; eofNotification.reset(); + status = Status::CLOSED; } SourceAAC::~SourceAAC(){ @@ -215,11 +223,13 @@ bool SourceAAC::prepareNextFrame(ADTSTiming::Header& header){ size_t SourceAAC::generate(int16_t* outBuffer){ if(!file){ Serial.println("file false"); + status = Status::FAILED; return 0; } if(!hAACDecoder){ Serial.println("Decoder false"); + status = Status::FAILED; return 0; } @@ -310,6 +320,7 @@ size_t SourceAAC::generate(int16_t* outBuffer){ } if(samples == 0){ + status = Status::END_OF_STREAM; if(readEof && eofNotification.take() && songDoneCallback != nullptr) { songDoneCallback(); } @@ -323,6 +334,7 @@ size_t SourceAAC::generate(int16_t* outBuffer){ rewindAttempted = false; } }else{ + status = Status::DATA; const uint32_t outputRate = sampleRate == 0 ? sourceSampleRate : sampleRate; const uint64_t numerator = elapsedFrameRemainder + uint64_t(samples) * sourceSampleRate; const uint64_t advanced = numerator / outputRate; diff --git a/src/AudioLib/SourceAAC.h b/src/AudioLib/SourceAAC.h index c0cdf83..b975ce1 100644 --- a/src/AudioLib/SourceAAC.h +++ b/src/AudioLib/SourceAAC.h @@ -12,10 +12,23 @@ #include #include #include "ADTSTiming.h" +#include class SourceAAC : public Source { public: + // Coarse decoder status for status-reporting consumers (e.g. remote/UI + // screens). Purely observational: no control-flow decision anywhere in + // this class depends on it, so it cannot change decode/timing/EOF + // behavior. + enum class Status : uint8_t { + CLOSED, + DATA, + STARVED, + END_OF_STREAM, + FAILED + }; + SourceAAC(); SourceAAC(fs::File file); ~SourceAAC(); @@ -49,9 +62,15 @@ class SourceAAC : public Source void setSongDoneCallback(void (*callback)()); bool isReadReady() const; + // Coarse, observational decoder status (see enum Status above). Thread-safe + // for the same audio-task/main-thread split as the rest of this class. + Status getStatus() const; + private: fs::File file; + std::atomic status{Status::CLOSED}; + float volume = 1.0f; RingBuffer readBuffer; diff --git a/src/AudioLib/Systems/MixSystem.cpp b/src/AudioLib/Systems/MixSystem.cpp index 30a1dad..7763ff1 100644 --- a/src/AudioLib/Systems/MixSystem.cpp +++ b/src/AudioLib/Systems/MixSystem.cpp @@ -38,8 +38,8 @@ MixSystem::MixSystem() : audioTask("MixAudio", audioThread, 16 * 1024, this), qu .use_apll = false }, i2s_pin_config, I2S_NUM_0); - i2s->setGain(0.4f*((float) Settings.get().volumeLevel) / 255.0f); i2s->setSource(mixer); + updateGain(); fsOut = new OutputWAV(); @@ -389,6 +389,15 @@ bool MixSystem::hasChannel(uint8_t c){ return loaded; } +SourceAAC::Status MixSystem::getChannelStatus(uint8_t c){ + if(c >= 2) return SourceAAC::Status::CLOSED; + cleanupRetiredSources(); + sourceMutex.lock(); + SourceAAC::Status status = source[c] != nullptr ? source[c]->getStatus() : SourceAAC::Status::CLOSED; + sourceMutex.unlock(); + return status; +} + uint8_t MixSystem::getVolume(uint8_t c){ return c < 2 ? volume[c] : 0; } @@ -397,6 +406,10 @@ uint8_t MixSystem::getMix(){ return mixer ? mixer->getMixRatio() : 128; } +void MixSystem::updateGain(){ + i2s->setGain(0.4f*((float) Settings.get().volumeLevel) / 255.0f); +} + void MixSystem::setVolume(uint8_t c, uint8_t volume){ if(c >= 2) return; this->volume[c] = volume; diff --git a/src/AudioLib/Systems/MixSystem.h b/src/AudioLib/Systems/MixSystem.h index 8914d5c..ff175a2 100644 --- a/src/AudioLib/Systems/MixSystem.h +++ b/src/AudioLib/Systems/MixSystem.h @@ -70,10 +70,17 @@ class MixSystem { bool hasChannel(uint8_t channel); uint8_t getVolume(uint8_t channel); uint8_t getMix(); + // Coarse decoder status for the given channel, or Status::CLOSED if no + // source is currently open on it (mirrors hasChannel's locking). + SourceAAC::Status getChannelStatus(uint8_t channel); void setVolume(uint8_t channel, uint8_t volume); void setMix(uint8_t ratio); + // Re-applies the master output gain from the current Settings volume + // level. Call after externally changing Settings.get().volumeLevel. + void updateGain(); + void addSpeed(uint8_t channel); void removeSpeed(uint8_t channel); void setSpeed(uint8_t channel, uint8_t speed); From 21d90a79383d975bd6bd5ff1d42ecbfd9ed584d9 Mon Sep 17 00:00:00 2001 From: Ryan Trauntvein Date: Tue, 25 Aug 2026 17:27:22 -0700 Subject: [PATCH 14/15] Add per-deck three-band isolator EQ Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> (cherry picked from commit 285b0160f239f674a6b2adee858b9ce7b3cdd886) --- src/AudioLib/Effects/ThreeBandEQ.cpp | 109 ++++++++++++++++++ src/AudioLib/Effects/ThreeBandEQ.h | 53 +++++++++ src/AudioLib/Systems/MixSystem.cpp | 28 ++++- src/AudioLib/Systems/MixSystem.h | 6 +- tests/ThreeBandEQSelfCheck.cpp | 155 ++++++++++++++++++++++++++ tests/run-three-band-eq-self-check.sh | 14 +++ 6 files changed, 361 insertions(+), 4 deletions(-) create mode 100644 src/AudioLib/Effects/ThreeBandEQ.cpp create mode 100644 src/AudioLib/Effects/ThreeBandEQ.h create mode 100644 tests/ThreeBandEQSelfCheck.cpp create mode 100755 tests/run-three-band-eq-self-check.sh diff --git a/src/AudioLib/Effects/ThreeBandEQ.cpp b/src/AudioLib/Effects/ThreeBandEQ.cpp new file mode 100644 index 0000000..3ae1b42 --- /dev/null +++ b/src/AudioLib/Effects/ThreeBandEQ.cpp @@ -0,0 +1,109 @@ +#include "ThreeBandEQ.h" +#include "../../AudioSetup.hpp" +#include +#include + +namespace { + +// Hardware calibration points: tune these against the Jay-D output path. +constexpr float LowCrossoverHz = 250.0f; +constexpr float HighCrossoverHz = 3000.0f; +constexpr float ButterworthQ = 0.70710678f; + +} + +static_assert(SAMPLE_RATE == 24000, "ThreeBandEQ is calibrated for 24 kHz audio"); +static_assert(NUM_CHANNELS == 1, "ThreeBandEQ is calibrated for mono audio"); +static_assert(BUFFER_SAMPLES == 256, "ThreeBandEQ is calibrated for 256-sample blocks"); +static_assert(BYTES_PER_SAMPLE == 2, "ThreeBandEQ requires 16-bit samples"); + +ThreeBandEQ::ThreeBandEQ(){ + for(uint8_t i = 0; i < 2; i++){ + configure(lowLowPass[i], false, LowCrossoverHz); + configure(lowHighPass[i], true, LowCrossoverHz); + configure(highLowPass[i], false, HighCrossoverHz); + configure(highHighPass[i], true, HighCrossoverHz); + } + configureAllPass(highAllPass, HighCrossoverHz); +} + +void ThreeBandEQ::applyEffect(int16_t* inBuffer, int16_t* outBuffer, size_t numSamples){ + for(size_t i = 0; i < numSamples; i++){ + const float input = inBuffer[i]; + float low = lowLowPass[1].process(lowLowPass[0].process(input)); + const float upper = lowHighPass[1].process(lowHighPass[0].process(input)); + const float mid = highLowPass[1].process(highLowPass[0].process(upper)); + const float high = highHighPass[1].process(highHighPass[0].process(upper)); + low = highAllPass.process(low); + + outBuffer[i] = saturate(low * gain[0] + mid * gain[1] + high * gain[2]); + } +} + +void ThreeBandEQ::setIntensity(uint8_t intensity){ + const uint8_t level = NeutralLevel - intensity; + setLevel(Band::Low, level); + setLevel(Band::Mid, level); + setLevel(Band::High, level); +} + +bool ThreeBandEQ::setLevel(Band band, uint8_t level){ + const uint8_t index = static_cast(band); + if(index >= static_cast(Band::Count)) return false; + + gain[index] = static_cast(level) / static_cast(NeutralLevel); + return true; +} + +void ThreeBandEQ::reset(){ + for(uint8_t i = 0; i < 2; i++){ + lowLowPass[i].clear(); + lowHighPass[i].clear(); + highLowPass[i].clear(); + highHighPass[i].clear(); + } + highAllPass.clear(); +} + +float ThreeBandEQ::Biquad::process(float input){ + const float output = b0 * input + z1; + z1 = b1 * input - a1 * output + z2; + z2 = b2 * input - a2 * output; + return output; +} + +void ThreeBandEQ::Biquad::clear(){ + z1 = 0.0f; + z2 = 0.0f; +} + +void ThreeBandEQ::configure(Biquad& filter, bool highPass, float frequency){ + const float omega = 2.0f * std::acos(-1.0f) * frequency / static_cast(SAMPLE_RATE); + const float cosine = std::cos(omega); + const float alpha = std::sin(omega) / (2.0f * ButterworthQ); + const float scale = 1.0f / (1.0f + alpha); + const float direction = highPass ? 1.0f : -1.0f; + + filter.b0 = 0.5f * (1.0f + direction * cosine) * scale; + filter.b1 = -(1.0f + direction * cosine) * scale; + filter.b2 = filter.b0; + if(!highPass) filter.b1 = -filter.b1; + filter.a1 = -2.0f * cosine * scale; + filter.a2 = (1.0f - alpha) * scale; +} + +void ThreeBandEQ::configureAllPass(Biquad& filter, float frequency){ + Biquad prototype; + configure(prototype, false, frequency); + filter.b0 = prototype.a2; + filter.b1 = prototype.a1; + filter.b2 = 1.0f; + filter.a1 = prototype.a1; + filter.a2 = prototype.a2; +} + +int16_t ThreeBandEQ::saturate(float sample){ + if(sample >= static_cast(INT16_MAX)) return INT16_MAX; + if(sample <= static_cast(INT16_MIN)) return INT16_MIN; + return static_cast(sample); +} diff --git a/src/AudioLib/Effects/ThreeBandEQ.h b/src/AudioLib/Effects/ThreeBandEQ.h new file mode 100644 index 0000000..7f08d98 --- /dev/null +++ b/src/AudioLib/Effects/ThreeBandEQ.h @@ -0,0 +1,53 @@ +#ifndef JAYD_LIBRARY_THREEBANDEQ_H +#define JAYD_LIBRARY_THREEBANDEQ_H + +#include +#include "../Effect.h" + +class ThreeBandEQ : public Effect { +public: + enum class Band : uint8_t { + Low, + Mid, + High, + Count + }; + + static constexpr uint8_t NeutralLevel = 255; + static constexpr uint8_t KillLevel = 0; + + ThreeBandEQ(); + + void applyEffect(int16_t* inBuffer, int16_t* outBuffer, size_t numSamples) override; + void setIntensity(uint8_t intensity) override; + + bool setLevel(Band band, uint8_t level); + void reset(); + +private: + struct Biquad { + float b0 = 0.0f; + float b1 = 0.0f; + float b2 = 0.0f; + float a1 = 0.0f; + float a2 = 0.0f; + float z1 = 0.0f; + float z2 = 0.0f; + + float process(float input); + void clear(); + }; + + Biquad lowLowPass[2]; + Biquad lowHighPass[2]; + Biquad highLowPass[2]; + Biquad highHighPass[2]; + Biquad highAllPass; + float gain[static_cast(Band::Count)] = { 1.0f, 1.0f, 1.0f }; + + static void configure(Biquad& filter, bool highPass, float frequency); + static void configureAllPass(Biquad& filter, float frequency); + static int16_t saturate(float sample); +}; + +#endif //JAYD_LIBRARY_THREEBANDEQ_H diff --git a/src/AudioLib/Systems/MixSystem.cpp b/src/AudioLib/Systems/MixSystem.cpp index 7763ff1..16f8a4b 100644 --- a/src/AudioLib/Systems/MixSystem.cpp +++ b/src/AudioLib/Systems/MixSystem.cpp @@ -22,6 +22,7 @@ MixSystem::MixSystem() : audioTask("MixAudio", audioThread, 16 * 1024, this), qu for(int j = 0; j < 3; j++){ effector[i]->addEffect(nullptr); } + effector[i]->addEffect(&eq[i]); mixer->addSource(effector[i]); } @@ -98,6 +99,7 @@ bool MixSystem::replaceSource(uint8_t c, SourceAAC* newSource){ } newSource->setVolume(volume[c]); auto oldSource = source[c]; + eq[c].reset(); source[c] = newSource; if(speed[c]){ speed[c]->setSource(newSource); @@ -256,6 +258,9 @@ void MixSystem::audioThread(Task* task){ case MixRequest::SET_EFFECT_INTENSITY: system->_setEffectIntensity(request.channel, request.slot, request.value); break; + case MixRequest::SET_EQ: + system->_setEQ(request.channel, static_cast(request.slot), request.value); + break; case MixRequest::SET_INFO: system->_setInfoGenerator(request.channel, reinterpret_cast(uintptr_t(request.value))); break; @@ -500,6 +505,15 @@ void MixSystem::setEffectIntensity(uint8_t channel, uint8_t slot, uint8_t intens enqueueRequest({ MixRequest::SET_EFFECT_INTENSITY, channel, slot, intensity }); } +bool MixSystem::setEQ(uint8_t channel, ThreeBandEQ::Band band, uint8_t level){ + if(channel >= 2 || static_cast(band) >= static_cast(ThreeBandEQ::Band::Count)){ + return false; + } + if(!out->isRunning()) return _setEQ(channel, band, level); + + return enqueueRequest({ MixRequest::SET_EQ, channel, static_cast(band), level }); +} + void MixSystem::_addSpeed(uint8_t c){ if(c >= 2 || !effector[c] || !source[c] || speed[c]) return; sourceMutex.lock(); @@ -549,6 +563,10 @@ void MixSystem::_setEffectIntensity(uint8_t c, uint8_t s, uint8_t intensity){ effector[c]->getEffect(s)->setIntensity(intensity); } +bool MixSystem::_setEQ(uint8_t c, ThreeBandEQ::Band band, uint8_t level){ + return c < 2 && eq[c].setLevel(band, level); +} + Effect* (* MixSystem::getEffect[])() = { []() -> Effect*{ return nullptr; }, // None []() -> Effect*{ return nullptr; }, // Speed @@ -606,7 +624,10 @@ bool MixSystem::seekChannelSourceFrame(uint8_t channel, uint64_t frame){ if(!out->isRunning()){ sourceMutex.lock(); const bool success = source[channel] && source[channel]->seekSourceFrame(frame); - if(success && speed[channel]) speed[channel]->reset(); + if(success){ + if(speed[channel]) speed[channel]->reset(); + eq[channel].reset(); + } sourceMutex.unlock(); return success; } @@ -634,8 +655,9 @@ void MixSystem::_seekChannel(uint8_t channel, uint64_t frame){ if(seekPending[channel] > 0) seekPending[channel]--; SourceAAC* channelSource = source[channel]; sourceMutex.unlock(); - if(channelSource && channelSource->seekSourceFrame(frame) && speed[channel]){ - speed[channel]->reset(); + if(channelSource && channelSource->seekSourceFrame(frame)){ + if(speed[channel]) speed[channel]->reset(); + eq[channel].reset(); } } diff --git a/src/AudioLib/Systems/MixSystem.h b/src/AudioLib/Systems/MixSystem.h index ff175a2..66c1bd5 100644 --- a/src/AudioLib/Systems/MixSystem.h +++ b/src/AudioLib/Systems/MixSystem.h @@ -11,6 +11,7 @@ #include "../Mixer.h" #include "../SourceWAV.h" #include "../EffectType.hpp" +#include "../Effects/ThreeBandEQ.h" #include "../SourceAAC.h" #include #include @@ -18,7 +19,7 @@ #include "../OutputWAV.h" struct MixRequest { - enum { ADD_SPEED, REMOVE_SPEED, SET_SPEED, SET_RATE, NUDGE_RATE, SET_EFFECT, SET_EFFECT_INTENSITY, SET_INFO, SET_SEEK, RECORD, OPEN } type; + enum { ADD_SPEED, REMOVE_SPEED, SET_SPEED, SET_RATE, NUDGE_RATE, SET_EFFECT, SET_EFFECT_INTENSITY, SET_EQ, SET_INFO, SET_SEEK, RECORD, OPEN } type; uint8_t channel; uint8_t slot; uint64_t value; @@ -89,6 +90,7 @@ class MixSystem { void nudgeRate(uint8_t channel, int32_t amount); void setEffect(uint8_t channel, uint8_t slot, EffectType type); void setEffectIntensity(uint8_t channel, uint8_t slot, uint8_t intensity); + bool setEQ(uint8_t channel, ThreeBandEQ::Band band, uint8_t level); void setOutInfo(InfoGenerator* outInfoGen); void setChannelInfo(uint8_t channel, InfoGenerator* channelInfoGen); @@ -127,6 +129,7 @@ class MixSystem { uint8_t volume[2] = { 255, 255 }; EffectProcessor* effector[2]; + ThreeBandEQ eq[2]; Mixer* mixer; OutputI2S* i2s; OutputWAV* fsOut; @@ -144,6 +147,7 @@ class MixSystem { void _nudgeRate(uint8_t channel, int32_t amount); void _setEffect(uint8_t channel, uint8_t slot, EffectType type); void _setEffectIntensity(uint8_t channel, uint8_t slot, uint8_t intensity); + bool _setEQ(uint8_t channel, ThreeBandEQ::Band band, uint8_t level); void _setInfoGenerator(uint8_t channel, InfoGenerator* generator); void _seekChannel(uint8_t channel, uint64_t frame); void _startRecording(); diff --git a/tests/ThreeBandEQSelfCheck.cpp b/tests/ThreeBandEQSelfCheck.cpp new file mode 100644 index 0000000..197f4ff --- /dev/null +++ b/tests/ThreeBandEQSelfCheck.cpp @@ -0,0 +1,155 @@ +#include "AudioLib/Effects/ThreeBandEQ.h" + +#include +#include +#include +#include +#include +#include + +namespace { + +constexpr double Pi = 3.14159265358979323846; +constexpr double SampleRate = 24000.0; +constexpr size_t BlockSamples = 256; +constexpr size_t WarmupSamples = 12000; +constexpr size_t MeasuredSamples = 24000; + +struct Levels { + uint8_t low; + uint8_t mid; + uint8_t high; +}; + +double currentResponse(ThreeBandEQ& eq, double frequency){ + std::array input = {}; + std::array output = {}; + const size_t totalSamples = WarmupSamples + MeasuredSamples; + double inputPower = 0.0; + double outputPower = 0.0; + + eq.reset(); + + for(size_t offset = 0; offset < totalSamples; offset += BlockSamples){ + const size_t count = std::min(BlockSamples, totalSamples - offset); + for(size_t i = 0; i < count; i++){ + const double phase = 2.0 * Pi * frequency * static_cast(offset + i) / SampleRate; + input[i] = static_cast(12000.0 * std::sin(phase)); + } + eq.applyEffect(input.data(), output.data(), count); + + for(size_t i = 0; i < count; i++){ + if(offset + i < WarmupSamples) continue; + inputPower += static_cast(input[i]) * input[i]; + outputPower += static_cast(output[i]) * output[i]; + } + } + + return std::sqrt(outputPower / inputPower); +} + +double response(ThreeBandEQ& eq, double frequency, Levels levels){ + assert(eq.setLevel(ThreeBandEQ::Band::Low, levels.low)); + assert(eq.setLevel(ThreeBandEQ::Band::Mid, levels.mid)); + assert(eq.setLevel(ThreeBandEQ::Band::High, levels.high)); + return currentResponse(eq, frequency); +} + +void checkNeutralAndKills(){ + ThreeBandEQ eq; + const std::array frequencies = {{ 60.0, 1000.0, 8000.0 }}; + const std::array bands = {{ + ThreeBandEQ::Band::Low, + ThreeBandEQ::Band::Mid, + ThreeBandEQ::Band::High + }}; + const Levels neutral = { 255, 255, 255 }; + + for(double frequency : frequencies){ + const double neutralGain = response(eq, frequency, neutral); + assert(std::abs(20.0 * std::log10(neutralGain)) <= 0.1); + } + + for(size_t selected = 0; selected < bands.size(); selected++){ + Levels isolated = { 0, 0, 0 }; + Levels killed = neutral; + if(selected == 0){ + isolated.low = 255; + killed.low = 0; + }else if(selected == 1){ + isolated.mid = 255; + killed.mid = 0; + }else{ + isolated.high = 255; + killed.high = 0; + } + + const double selectedGain = response(eq, frequencies[selected], isolated); + const double killedGain = response(eq, frequencies[selected], killed); + assert(selectedGain >= 0.95); + assert(killedGain <= 0.02); + + for(size_t neighbor = 0; neighbor < bands.size(); neighbor++){ + if(neighbor == selected) continue; + Levels neighborOnly = { 0, 0, 0 }; + if(neighbor == 0) neighborOnly.low = 255; + if(neighbor == 1) neighborOnly.mid = 255; + if(neighbor == 2) neighborOnly.high = 255; + assert(selectedGain >= 20.0 * response(eq, frequencies[selected], neighborOnly)); + } + } +} + +void checkGainCorners(){ + ThreeBandEQ eq; + const std::array frequencies = {{ 60.0, 1000.0, 8000.0 }}; + for(uint8_t mask = 0; mask < 8; mask++){ + const Levels levels = { + static_cast((mask & 1) ? 255 : 0), + static_cast((mask & 2) ? 255 : 0), + static_cast((mask & 4) ? 255 : 0) + }; + for(double frequency : frequencies){ + assert(response(eq, frequency, levels) <= 1.001); + } + } + + assert(!eq.setLevel(static_cast(255), 255)); + + std::array input; + std::array output; + input.fill(INT16_MAX); + eq.setIntensity(255); + eq.applyEffect(input.data(), output.data(), output.size()); + for(int16_t sample : output) assert(sample == 0); +} + +void checkReset(){ + ThreeBandEQ eq; + std::array input = {}; + std::array output = {}; + input[0] = INT16_MAX; + + assert(eq.setLevel(ThreeBandEQ::Band::Low, 0)); + assert(eq.setLevel(ThreeBandEQ::Band::Mid, 255)); + assert(eq.setLevel(ThreeBandEQ::Band::High, 255)); + eq.applyEffect(input.data(), output.data(), output.size()); + + eq.reset(); + input.fill(0); + eq.applyEffect(input.data(), output.data(), output.size()); + for(int16_t sample : output) assert(sample == 0); + + const double retainedKill = currentResponse(eq, 60.0); + assert(retainedKill <= 0.02); +} + +} + +int main(){ + checkNeutralAndKills(); + checkGainCorners(); + checkReset(); + std::cout << "ThreeBandEQ self-check passed\n"; + return 0; +} diff --git a/tests/run-three-band-eq-self-check.sh b/tests/run-three-band-eq-self-check.sh new file mode 100755 index 0000000..1d964b3 --- /dev/null +++ b/tests/run-three-band-eq-self-check.sh @@ -0,0 +1,14 @@ +#!/bin/sh +set -eu + +repo=$(CDPATH= cd -- "$(dirname -- "$0")/.." && pwd) +binary="${TMPDIR:-/tmp}/jayd-three-band-eq-self-check" +trap 'rm -f "$binary"' EXIT + +c++ -std=c++11 -Wall -Wextra -Werror \ + -fsanitize=address,undefined -fno-omit-frame-pointer \ + -I"$repo/tests/stubs" -I"$repo/src" \ + "$repo/tests/ThreeBandEQSelfCheck.cpp" \ + "$repo/src/AudioLib/Effects/ThreeBandEQ.cpp" \ + -o "$binary" +"$binary" From 1d5feb8df9a5d2faa4bfeb263bc0ecaaa93cb367 Mon Sep 17 00:00:00 2001 From: Ryan Trauntvein Date: Wed, 26 Aug 2026 10:51:02 -0700 Subject: [PATCH 15/15] Fix _openChannel() discarding prior pause state on hot-swap Coordinator reported a concrete device defect: a deck paused before a remoteLoad-triggered source replacement came back playing after the swap, even though the request succeeded and the deck had been deliberately paused. Root cause: _openChannel() computed wasPaused up front but then decided whether to resume with if(replaceSource(channel, newSource) || !wasPaused) mixer->resumeChannel(channel); Since replaceSource() returns true on any successful swap, the || made every successful replacement resume the channel regardless of wasPaused - the prior pause state was only ever consulted on failure. Fix: perform the replacement unconditionally, then resume only if the channel was not paused beforehand, independent of whether the replacement succeeded or failed. This preserves pause state end to end: a paused deck stays paused across a successful hot-swap, a playing deck keeps playing, and a failed replacement leaves the deck exactly as it was. Adds OpenChannelPauseSelfCheck: a structural check on the shipped _openChannel() text plus a functional harness mirroring its exact pause/resume decision, covering paused/playing x success/failure, and a negative case proving the pre-fix pattern would have been caught. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/AudioLib/Systems/MixSystem.cpp | 7 +- tests/OpenChannelPauseSelfCheck.cpp | 132 +++++++++++++++++++++ tests/run-open-channel-pause-self-check.sh | 13 ++ 3 files changed, 151 insertions(+), 1 deletion(-) create mode 100644 tests/OpenChannelPauseSelfCheck.cpp create mode 100755 tests/run-open-channel-pause-self-check.sh diff --git a/src/AudioLib/Systems/MixSystem.cpp b/src/AudioLib/Systems/MixSystem.cpp index 16f8a4b..3f41ac5 100644 --- a/src/AudioLib/Systems/MixSystem.cpp +++ b/src/AudioLib/Systems/MixSystem.cpp @@ -152,7 +152,12 @@ void MixSystem::_openChannel(uint8_t channel, SourceAAC* newSource){ const bool wasPaused = mixer->isChannelPaused(channel); mixer->pauseChannel(channel); - if(replaceSource(channel, newSource) || !wasPaused) mixer->resumeChannel(channel); + replaceSource(channel, newSource); + // Restore the caller's prior pause state regardless of whether the + // replacement succeeded: a paused deck must stay paused across a + // hot-swap, and a failed replacement must not leave a previously + // playing deck stuck paused. + if(!wasPaused) mixer->resumeChannel(channel); } int8_t MixSystem::reserveRequest(const MixRequest& request){ diff --git a/tests/OpenChannelPauseSelfCheck.cpp b/tests/OpenChannelPauseSelfCheck.cpp new file mode 100644 index 0000000..c8cfe45 --- /dev/null +++ b/tests/OpenChannelPauseSelfCheck.cpp @@ -0,0 +1,132 @@ +// Integration self-check for MixSystem::_openChannel()'s pause-state +// preservation across a hot-swap (remoteLoad/openChannel replacing a +// channel's source while the deck is playing or paused). +// +// Reported defect: `_openChannel()` unconditionally paused the channel +// before replacing its source, then resumed it whenever the replacement +// succeeded, regardless of whether the deck had been paused by the user +// beforehand: +// +// if(replaceSource(channel, newSource) || !wasPaused) mixer->resumeChannel(channel); +// +// Since replaceSource() returns true on any successful swap, a deck that +// was deliberately paused before a hot-swap request would be silently +// resumed by it (status API and firmware UI would then read +// paused=false for a deck the user had explicitly paused). +// +// Fixed: perform the replacement unconditionally, then resume only if the +// deck was not paused beforehand - restoring, not overriding, the prior +// pause state regardless of whether the replacement itself succeeded or +// failed: +// +// replaceSource(channel, newSource); +// if(!wasPaused) mixer->resumeChannel(channel); +// +// This has two halves: +// 1. A structural scan of the real, shipped _openChannel() proving the +// fixed pattern is present and the reported buggy pattern is gone. +// 2. A deterministic functional harness mirroring _openChannel()'s exact +// pause/resume decision, covering: successful replacement while +// paused stays paused; successful replacement while playing keeps +// playing; failed replacement preserves the prior state in both +// directions. + +#include +#include +#include +#include + +namespace { + +std::string readFile(const char* path){ + std::ifstream f(path); + assert(f.good()); + return std::string((std::istreambuf_iterator(f)), std::istreambuf_iterator()); +} + +bool contains(const std::string& source, const char* token){ + return source.find(token) != std::string::npos; +} + +// --- Functional harness: mirrors _openChannel()'s fixed decision, with a +// fake mixer/replaceSource standing in for the real hardware-backed ones. + +struct FakeMixer { + bool paused = false; + void pauseChannel(){ paused = true; } + void resumeChannel(){ paused = false; } + bool isChannelPaused() const { return paused; } +}; + +// Fixed contract: replace unconditionally, resume only if it wasn't +// paused beforehand - independent of whether the replacement succeeded. +void openChannelFixed(FakeMixer& mixer, bool replaceSucceeds){ + const bool wasPaused = mixer.isChannelPaused(); + mixer.pauseChannel(); + (void) replaceSucceeds; // replaceSource()'s own return no longer gates resume + if(!wasPaused) mixer.resumeChannel(); +} + +// Pre-fix behaviour: resumes whenever the replacement succeeds regardless +// of the prior pause state. Used to prove this check discriminates the +// reported defect class. +void openChannelBuggy(FakeMixer& mixer, bool replaceSucceeds){ + const bool wasPaused = mixer.isChannelPaused(); + mixer.pauseChannel(); + if(replaceSucceeds || !wasPaused) mixer.resumeChannel(); +} + +} // namespace + +int main(){ + // --- 1. Structural: the real _openChannel() no longer lets a + // successful replacement force a resume. + { + const std::string source = readFile("src/AudioLib/Systems/MixSystem.cpp"); + assert(contains(source, "replaceSource(channel, newSource);")); + assert(contains(source, "if(!wasPaused) mixer->resumeChannel(channel);")); + assert(!contains(source, "if(replaceSource(channel, newSource) || !wasPaused)")); + } + + // --- 2. Functional: successful replacement while paused must stay + // paused (the reported bug: this used to resume). + { + FakeMixer mixer; + mixer.paused = true; + openChannelFixed(mixer, /*replaceSucceeds=*/true); + assert(mixer.paused); + } + + // --- 3. Successful replacement while playing must keep playing. + { + FakeMixer mixer; + mixer.paused = false; + openChannelFixed(mixer, /*replaceSucceeds=*/true); + assert(!mixer.paused); + } + + // --- 4. Failed replacement must also preserve prior state, both ways. + { + FakeMixer mixer; + mixer.paused = true; + openChannelFixed(mixer, /*replaceSucceeds=*/false); + assert(mixer.paused); + } + { + FakeMixer mixer; + mixer.paused = false; + openChannelFixed(mixer, /*replaceSucceeds=*/false); + assert(!mixer.paused); + } + + // --- 5. The pre-fix pattern must fail case 2, proving this check + // would have caught the reported regression. + { + FakeMixer mixer; + mixer.paused = true; + openChannelBuggy(mixer, /*replaceSucceeds=*/true); + assert(!mixer.paused); // bug: silently resumed a paused deck + } + + return 0; +} diff --git a/tests/run-open-channel-pause-self-check.sh b/tests/run-open-channel-pause-self-check.sh new file mode 100755 index 0000000..1d4714e --- /dev/null +++ b/tests/run-open-channel-pause-self-check.sh @@ -0,0 +1,13 @@ +#!/bin/sh +set -eu + +repo=$(CDPATH= cd -- "$(dirname -- "$0")/.." && pwd) +binary="${TMPDIR:-/tmp}/jayd-open-channel-pause-self-check" +trap 'rm -f "$binary"' EXIT + +c++ -std=c++11 -Wall -Wextra -Werror \ + -fsanitize=address,undefined -fno-omit-frame-pointer \ + -I"$repo/tests/stubs" -I"$repo/src" \ + "$repo/tests/OpenChannelPauseSelfCheck.cpp" \ + -o "$binary" +(cd "$repo" && "$binary")