Audio visualizer in C++

Question

I have completed my first attempt of a simple music visualizer app for windows in C++ using SDL. It takes system audio and outputs the soundwave in real-time. I'd appreciate some feedback on how I can improve the code, especially the OOP design and where I can use modern C++ features.

main.cpp:

#include <chrono>
#include <thread>

#include "visualizer.h"

constexpr int sleep_time = 20;

int main(int argc, char* argv[])
{
    Visualizer generator;
    bool continue_app = generator.init_successful();

    while (continue_app)
    {
        continue_app = generator.update();
        std::this_thread::sleep_for(std::chrono::milliseconds(sleep_time));
    }

    return 0;
}

audio_sink.h:

#pragma once

// Abstract base class to copy data from an AudioRecorder
class AudioSink
{
public:
    // Copy a packet of data from the audio recorder
    // param: data - pointer to data values
    // param: channels - the number of audio channels
    // param: frames - the number of frames of data
    virtual void copy_data(float * data, int channels, int frames) = 0;
};

audio_recorder.h:

#pragma once

#include <atomic>

#include <Audioclient.h>
#include <Audiopolicy.h>
#include <Mmdeviceapi.h>

#include "audio_sink.h"

// Class for recording system audio via WASAPI Loopback
// 
class AudioRecorder {
public:
    AudioRecorder();
    ~AudioRecorder();

    // The status of the initialization process
    // return: bool - whether the initialization was successful
    bool init_successful() const;

    // Record audio data from the system indefinitely
    // param: audio_sink - class which copies the recorded packets
    // param: exit_flag - flag to stop recording (passed by ref so it can be stopped externally)
    void record(AudioSink * audio_sink, std::atomic_bool &exit_flag) const;

private:
    mutable HRESULT m_hr;
    IMMDeviceEnumerator * m_device_enumerator = nullptr;
    IMMDevice * m_audio_device_endpoint = nullptr;
    IAudioClient * m_audio_client = nullptr;
    IAudioCaptureClient *m_capture_client = nullptr;
    int m_num_channels;

    static const int sleep_time = 10; // Time spent sleeping tp wait for new packets
};

audio_recorder.cpp:

#include "audio_recorder.h"

#include <comdef.h>
#include <Audioclient.h>
#include <Audiopolicy.h>
#include <Mmdeviceapi.h>

#include <chrono>
#include <iostream>
#include <thread>

// Define IIDs for initialization
const CLSID CLSID_MMDeviceEnumerator = __uuidof(MMDeviceEnumerator);
const IID IID_IMMDeviceEnumerator = __uuidof(IMMDeviceEnumerator);
const IID IID_IAudioClient = __uuidof(IAudioClient);
const IID IID_IAudioCaptureClient = __uuidof(IAudioCaptureClient);


AudioRecorder::AudioRecorder() {
    m_hr = S_OK;
    m_audio_client = nullptr;

    // Initialize audio device endpoint
    CoInitialize(nullptr);
    m_hr = CoCreateInstance(
        CLSID_MMDeviceEnumerator, nullptr,
        CLSCTX_ALL, IID_IMMDeviceEnumerator,
        (void**)&m_device_enumerator);

    if (m_hr == S_OK && m_device_enumerator) {
        m_hr = m_device_enumerator->GetDefaultAudioEndpoint(eRender, eConsole, &m_audio_device_endpoint);
    }

    // init audio client
    WAVEFORMATEX *pwfx = nullptr;
    REFERENCE_TIME hns_requested_duration = 100000000;

    if (m_hr == S_OK && m_audio_device_endpoint) {
        m_hr = m_audio_device_endpoint->Activate(IID_IAudioClient, CLSCTX_ALL, nullptr, (void**)&m_audio_client);
    }

    if (m_hr == S_OK && m_audio_client) {
        m_hr = m_audio_client->GetMixFormat(&pwfx);

        if (m_hr == S_OK && pwfx) {
            if (pwfx->wFormatTag != WAVE_FORMAT_EXTENSIBLE
                || reinterpret_cast<WAVEFORMATEXTENSIBLE *>(pwfx)->SubFormat != KSDATAFORMAT_SUBTYPE_IEEE_FLOAT) {
                std::cout << "Error: the audio format is not supported" << std::endl;
                m_hr = S_FALSE;
            }
        }

        if (m_hr == S_OK) {
            m_hr = m_audio_client->Initialize(AUDCLNT_SHAREMODE_SHARED, AUDCLNT_STREAMFLAGS_LOOPBACK, hns_requested_duration, 10, pwfx, nullptr);
        }

        if (m_hr == S_OK && pwfx) {
            m_num_channels = pwfx->nChannels;
        }

        if (m_hr == S_OK) {
            m_hr = m_audio_client->GetService(IID_IAudioCaptureClient, (void**)&m_capture_client);
        }

        if (m_hr == S_OK && m_capture_client) {
            m_hr = m_audio_client->Start();  // Start recording.
        }
    }

    if (m_hr != S_OK) {
        std::cout << "Error: During AudioRecorder intialization - " << _com_error(m_hr).ErrorMessage() << std::endl;

    }
};

// safely release and nullify pointers (in destructor)
template<class T> inline void safe_release(T* &p_object) {
    if (p_object) {
        p_object->Release();
        p_object = nullptr;
    }
}

AudioRecorder::~AudioRecorder() {
    if (m_audio_client) {
        m_audio_client->Stop();  // Stop recording.
    }

    safe_release(m_device_enumerator);
    safe_release(m_audio_device_endpoint);
    safe_release(m_audio_client);
    safe_release(m_capture_client);
}

bool AudioRecorder::init_successful() const {
    return (m_hr == S_OK) && m_audio_client && m_capture_client;
}

void AudioRecorder::record(AudioSink * audio_sink, std::atomic_bool &exit_flag) const {
    m_hr = S_OK;
    UINT32 packet_length = 0;
    BYTE *data = nullptr;
    UINT32 num_frames_available;
    DWORD flags;

    while (!exit_flag) {
        m_hr = m_capture_client->GetNextPacketSize(&packet_length);
        while (packet_length != 0 && !exit_flag) // while there are available packets
        {
            // Get the available data in the shared buffer.
            data = nullptr;
            m_hr = m_capture_client->GetBuffer(
                &data,
                &num_frames_available,
                &flags, nullptr, nullptr);

            if (flags & AUDCLNT_BUFFERFLAGS_SILENT) {
                data = nullptr;  // data pointer is null for silence
            }
            audio_sink->copy_data((float*)data, m_num_channels, num_frames_available);
            
            m_hr = m_capture_client->ReleaseBuffer(num_frames_available);
            m_hr = m_capture_client->GetNextPacketSize(&packet_length);
        }
        std::this_thread::sleep_for(std::chrono::milliseconds(sleep_time));
    }
}

visualizer.h:

#pragma once
#include <atomic>
#include <mutex>
#include <vector>
#include <thread>

#include "audio_recorder.h"
#include "audio_sink.h"
#include "SDL.h"

// Visualizer class
// This class owns the SDL window and performs visual updates based on system audio
class Visualizer: public AudioSink {
public:

    Visualizer();
    ~Visualizer();

    // The status of the initialization process
    // return: bool - whether the initialization was successful
    bool init_successful() const;

    // Update the visuals based on the most recent packet
    // return: bool - whether the update was successful
    bool update();

    // Copy a packet of data from the audio recorder
    // Must override the method from AudioSink
    // param: data - pointer to data values
    // param: channels - the number of audio channels
    // param: frames - the number of frames of data
    void copy_data(float * data, int channels, int frames) override;

private:
    SDL_Window* window = nullptr;
    SDL_Renderer* renderer = nullptr;
    SDL_Event current_event;
    int window_width = 1000;
    int window_height = 600;
    bool mFullScreen = false;
    bool mMinimized = false;


    AudioRecorder recorder;
    std::thread recording_thread;
    std::atomic_bool exit_recording_thread_flag = false;
    std::mutex packet_buffer_mutex;
    typedef std::vector<float> packet;
    packet packet_buffer; // Must use mutex to access

    // Handle SDL window events
    // param: e - the SDL window event to handle
    void handle_event(const SDL_Event & e);

    // Draw a horizontal soundwave with the most recent packet data
    // param: start - the leftmost starting pixel of the wave
    // param: length - the length in pixels of the wave
    // param: pixel_amplitude - the maximum amplitude of the wave
    // param: color - the color of the wave
    void draw_wave(const SDL_Point &start, int length, int pixel_amplitude, const SDL_Color & color);
};

visualizer.cpp:

#pragma once
#include "visualizer.h"

#include <iostream>
#include <thread>
#include <vector>

#include "SDL.h"

Visualizer::Visualizer(): recorder() {
    if (SDL_Init(SDL_INIT_VIDEO) != 0) {
        std::cout << "Unable to initialize SDL: " << SDL_GetError() << std::endl;
        return;
    }

    window = SDL_CreateWindow("Visualizer", SDL_WINDOWPOS_UNDEFINED, SDL_WINDOWPOS_UNDEFINED, window_width, window_height, SDL_WINDOW_SHOWN | SDL_WINDOW_RESIZABLE);
    if (window) {
        renderer = SDL_CreateRenderer(window, -1, SDL_RENDERER_ACCELERATED | SDL_RENDERER_PRESENTVSYNC);
    }

    if (renderer) {
        SDL_SetRenderDrawColor(renderer, 0, 0, 0, SDL_ALPHA_OPAQUE);
        SDL_RenderClear(renderer);
    }

    if (recorder.init_successful()) {
        recording_thread = std::thread(&AudioRecorder::record, &recorder, (AudioSink *)this, std::ref(exit_recording_thread_flag));
    }
}

Visualizer::~Visualizer()
{
    // Stop recording thread before implicitly destroying AudioRecorder
    if (recording_thread.joinable()) {
        exit_recording_thread_flag = true;
        recording_thread.join();
    }

    // Destroy SDL window
    SDL_DestroyRenderer(renderer);
    SDL_DestroyWindow(window);
    SDL_Quit();
}

bool Visualizer::init_successful() const {
    return (renderer && recording_thread.joinable());
}


void Visualizer::handle_event(const SDL_Event & e) {
    //Window event occured
    if (e.type == SDL_WINDOWEVENT) {
        switch (e.window.event) {
        //Get new dimensions and repaint on window size change
        case SDL_WINDOWEVENT_SIZE_CHANGED:
            window_width = e.window.data1;
            window_height = e.window.data2;
            SDL_RenderPresent(renderer);
            break;

        //Repaint on exposure
        case SDL_WINDOWEVENT_EXPOSED:
            SDL_RenderPresent(renderer);
            break;

        case SDL_WINDOWEVENT_MINIMIZED:
            mMinimized = true;
            break;

        case SDL_WINDOWEVENT_MAXIMIZED:
            mMinimized = false;
            break;

        case SDL_WINDOWEVENT_RESTORED:
            mMinimized = false;
            break;
        }

    }
    //Enter exit full screen on return key
    else if (e.type == SDL_KEYDOWN && e.key.keysym.sym == SDLK_RETURN) {
        if (mFullScreen) {
            SDL_SetWindowFullscreen(window, SDL_FALSE);
            mFullScreen = false;
        }
        else {
            SDL_SetWindowFullscreen(window, SDL_TRUE);
            mFullScreen = true;
            mMinimized = false;
        }
    }
}

void Visualizer::copy_data(float * data, int channels, int frames) {
    std::lock_guard<std::mutex>read_guard(packet_buffer_mutex);

    if (data) {
        packet_buffer = packet(data, data + channels * frames);
    }
    else {
        // use an empty vector if there is no data (silence)
        packet_buffer = packet();
    }
}


bool Visualizer::update() {
    //Handle events on queue
    while (SDL_PollEvent(&current_event) != 0) {
        if (current_event.type == SDL_QUIT) {
            return false;
        }
        handle_event(current_event);
    }

    // Do not render if minimized
    if (mMinimized) {
        return true;
    }

    // Render visualizer
    SDL_SetRenderDrawColor(renderer, 0, 0, 0, SDL_ALPHA_OPAQUE);
    SDL_RenderClear(renderer);
    draw_wave(SDL_Point{ 0, window_height / 2 }, window_width, window_height, SDL_Color{ 255,255,255,255 });
    SDL_RenderPresent(renderer);

    return true;
}

void Visualizer::draw_wave(const SDL_Point &start, int length, int pixel_amplitude, const SDL_Color & color) {
    std::vector<SDL_Point> points;

    std::unique_lock<std::mutex>read_guard(packet_buffer_mutex);
    if (!packet_buffer.empty()) {
        // use smallest possible step so soundwave fills window
        int step = (int)ceil((double)length / (double)packet_buffer.size());
        auto amplitude = packet_buffer.begin();
        for (int i = 0; i < length; i+=step) {
            points.push_back(SDL_Point{ start.x + i, (int)(start.y + (*amplitude) * pixel_amplitude) });
            ++amplitude;
        }
    }
    else { // silence
        points.push_back(SDL_Point{ start.x, start.y });
        points.push_back(SDL_Point{ start.x + length - 1, start.y });
        
    }
    read_guard.unlock();

    SDL_SetRenderDrawColor(renderer, color.r, color.g, color.b, color.a);
    SDL_RenderDrawLines(renderer, &points[0], (int)points.size());
}

Answer 1

The only thing I think worth calling out is your init_successful pattern. This is somewhat antithetical to the way that object-oriented languages with exception handling encourage us to think about construction. The way to "make a constructor fail" is to throw an exception, which in turn effectively prevents us from retaining a constructed object at all. With your method, there is an intermediate state possible where an object exists in memory, its members can be called, but it's invalid. That's bad.

If you need special logic to handle a failed construction, catch the exception you throw from the constructor.