Skip to content

Commit

Permalink
AudioLogger add LockGuard
Browse files Browse the repository at this point in the history
  • Loading branch information
pschatzmann committed Jan 7, 2025
1 parent bca62a5 commit b4bdb1a
Show file tree
Hide file tree
Showing 3 changed files with 73 additions and 39 deletions.
4 changes: 0 additions & 4 deletions src/AudioTools/Concurrency/LockGuard.h
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
#pragma once
#include "AudioConfig.h"
#include "AudioTools/CoreAudio/AudioLogger.h"
#include "Mutex.h"

namespace audio_tools {
Expand All @@ -18,18 +17,15 @@ namespace audio_tools {
class LockGuard {
public:
LockGuard(MutexBase &mutex) {
TRACED();
p_mutex = &mutex;
p_mutex->lock();
}
LockGuard(MutexBase *mutex) {
TRACED();
p_mutex = mutex;
if (p_mutex != nullptr)
p_mutex->lock();
}
~LockGuard() {
TRACED();
if (p_mutex != nullptr)
p_mutex->unlock();
}
Expand Down
72 changes: 55 additions & 17 deletions src/AudioTools/Concurrency/RP2040/BufferRP2040.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ namespace audio_tools {
/**
* @brief Buffer implementation which is based on a RP2040 queue. This
* class is intended to be used to exchange data between the 2 different
* cores.
* cores. Multi-core and IRQ safe queue implementation!
* @ingroup buffers
* @ingroup concurrency
* @author Phil Schatzmann
Expand All @@ -38,8 +38,10 @@ class BufferRP2040T : public BaseBuffer<T> {
if (buffer_size_total_bytes != buffer_size_req_bytes) {
LOGI("resize %d -> %d", buffer_size_total_bytes, buffer_size_req_bytes);
assert(buffer_size > 0);
write_buffer.resize(buffer_size);
read_buffer.resize(buffer_size * 2);
if (is_blocking_write){
write_buffer.resize(buffer_size);
read_buffer.resize(buffer_size * 2);
}
// release existing queue
if (buffer_size_total_bytes > 0) {
queue_free(&queue);
Expand Down Expand Up @@ -71,6 +73,10 @@ class BufferRP2040T : public BaseBuffer<T> {
// reads multiple values
int readArray(T data[], int len) override {
LOGD("readArray: %d", len);
if (!is_blocking_write && read_buffer.size()==0){
// make sure that the read buffer is big enough
read_buffer.resize(len + buffer_size);
}
// handle unalloc;ated queue
if (buffer_size_total_bytes == 0) return 0;
if (isEmpty() && read_buffer.isEmpty()) return 0;
Expand All @@ -94,24 +100,17 @@ class BufferRP2040T : public BaseBuffer<T> {

int writeArray(const T data[], int len) override {
LOGD("writeArray: %d", len);
int result = 0;
// make sure that we have the data allocated
resize(buffer_size_req_bytes);

// blocking write: wait for available space
while (availableForWrite()<=len){
delay(5);
};

// fill the write buffer and when it is full flush it to the queue
for (int j = 0; j < len; j++) {
write_buffer.write(data[j]);
if (write_buffer.isFull()) {
LOGD("queue_add_blocking");
queue_add_blocking(&queue, write_buffer.data());
write_buffer.reset();
}
if (is_blocking_write) {
result = writeBlocking(data, len);
} else {
result = writeNonBlocking(data, len);
}
return len;

return result;
}

// checks if the buffer is full
Expand Down Expand Up @@ -150,13 +149,52 @@ class BufferRP2040T : public BaseBuffer<T> {

size_t size() { return buffer_size_req_bytes / sizeof(T); }

/// When we use a non blocking write, the write size must be identical with the buffer size
void setBlockingWrite(bool flag){
is_blocking_write = flag;
}

protected:
queue_t queue;
int buffer_size_total_bytes = 0;
int buffer_size_req_bytes = 0;
int buffer_size = 0;
SingleBuffer<T> write_buffer{0};
audio_tools::RingBuffer<T> read_buffer{0};
bool is_blocking_write = true;

int writeBlocking(const T data[], int len) {
LOGD("writeArray: %d", len);

if (len > buffer_size){
LOGE("write %d too big for buffer_size: %d", len, buffer_size);
return 0;
}

// fill the write buffer and when it is full flush it to the queue
for (int j = 0; j < len; j++) {
write_buffer.write(data[j]);
if (write_buffer.isFull()) {
LOGD("queue_add_blocking");
queue_add_blocking(&queue, write_buffer.data());
write_buffer.reset();
}
}
return len;
}

int writeNonBlocking(const T data[], int len) {
if (len != buffer_size){
LOGE("write %d must be buffer_size: %d", len, buffer_size);
return 0;
}

if (queue_try_add(&queue, write_buffer.data())){
return len;
}
return 0;
}

};

using BufferRP2040 = BufferRP2040T<uint8_t>;
Expand Down
36 changes: 18 additions & 18 deletions src/AudioTools/CoreAudio/AudioLogger.h
Original file line number Diff line number Diff line change
Expand Up @@ -4,18 +4,28 @@
#if defined(ARDUINO) && !defined(IS_MIN_DESKTOP)
# include "Print.h"
#endif

#if defined(RP2040)
# include "AudioTools/Concurrency/RP2040.h"
#elif defined(ESP32)
# include "AudioTools/Concurrency/RTOS.h"
#else
# include "AudioTools/Concurrency/LockGuard.h"
#endif

// Logging Implementation
#if USE_AUDIO_LOGGING

namespace audio_tools {

#if defined(ESP32) && defined(SYNCHRONIZED_LOGGING)
# include "freertos/FreeRTOS.h"
# include "freertos/task.h"
static portMUX_TYPE mutex_logger = portMUX_INITIALIZER_UNLOCKED;
#if defined(RP2040)
MutexRP2040 audio_logger_mutex;
#elif defined(ESP32)
MutexESP32 audio_logger_mutex;
#else
MutexBase audio_logger_mutex; // no locking
#endif


/**
* @brief A simple Logger that writes messages dependent on the log level
* @ingroup tools
Expand Down Expand Up @@ -53,7 +63,6 @@ class AudioLogger {
}

AudioLogger &prefix(const char* file, int line, LogLevel current_level){
lock();
printPrefix(file,line,current_level);
return *this;
}
Expand All @@ -65,7 +74,6 @@ class AudioLogger {
log_print_ptr->println(print_buffer);
#endif
print_buffer[0]=0;
unlock();
}

char* str() {
Expand Down Expand Up @@ -134,17 +142,6 @@ class AudioLogger {
return len;
}

void lock(){
#if defined(ESP32) && defined(SYNCHRONIZED_LOGGING)
portENTER_CRITICAL(&mutex_logger);
#endif
}

void unlock(){
#if defined(ESP32) && defined(SYNCHRONIZED_LOGGING)
portEXIT_CRITICAL(&mutex_logger);
#endif
}
};

static AudioLogger &AudioToolsLogger = AudioLogger::instance();
Expand Down Expand Up @@ -188,17 +185,20 @@ class CustomLogLevel {

//#define LOG_OUT(level, fmt, ...) {AudioLogger::instance().prefix(__FILE__,__LINE__, level);cont char PROGMEM *fmt_P=F(fmt); snprintf_P(AudioLogger::instance().str(), LOG_PRINTF_BUFFER_SIZE, fmt, ##__VA_ARGS__); AudioLogger::instance().println();}
#define LOG_OUT_PGMEM(level, fmt, ...) { \
LockGuard guard{audio_logger_mutex};\
AudioLogger::instance().prefix(__FILE__,__LINE__, level); \
snprintf(AudioLogger::instance().str(), LOG_PRINTF_BUFFER_SIZE, PSTR(fmt), ##__VA_ARGS__); \
AudioLogger::instance().println();\
}

#define LOG_OUT(level, fmt, ...) { \
LockGuard guard{audio_logger_mutex};\
AudioLogger::instance().prefix(__FILE__,__LINE__, level); \
snprintf(AudioLogger::instance().str(), LOG_PRINTF_BUFFER_SIZE, fmt, ##__VA_ARGS__); \
AudioLogger::instance().println();\
}
#define LOG_MIN(level) { \
LockGuard guard{audio_logger_mutex};\
AudioLogger::instance().prefix(__FILE__,__LINE__, level); \
AudioLogger::instance().println();\
}
Expand Down

2 comments on commit b4bdb1a

@martinroger
Copy link

@martinroger martinroger commented on b4bdb1a Jan 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pschatzmann FYI the addition of the LockGuard is creating all kind of issues when compiling on ESP32, despite not using the AudioLogger actively (but it is still included) : Image

Not sure if the implementation is supposed to be stable, but I thought I'ld let you know. Previous commit bca62a5 works just fine.

@pschatzmann
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I got the class name wrong for the ESP32 since I tested only an a RP2040.
The correction has been committed...

Please sign in to comment.