-
Notifications
You must be signed in to change notification settings - Fork 52
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactor audio handling #277
Open
ryze312
wants to merge
42
commits into
uowuo:master
Choose a base branch
from
ryze312:audio_refactor
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
42 commits
Select commit
Hold shift + click to select a range
b33704b
Refactor voice audio handling
ryze312 cf64646
Apply suggestions
ryze312 eb1451a
Implement RAII mutex
ryze312 8e1d681
Implement slice
ryze312 0992139
Implement data channel
ryze312 555a3cd
Implement thread pool
ryze312 dbef57a
Fix RemoveSSRC never getting called
ryze312 9b550ed
Reimplement audio engine
ryze312 b7151f4
Sync with upstream
ryze312 39011b6
Fix typo: u_int8_t -> uint8_t
ryze312 145057f
Make constructors public for in-place constructor to work
ryze312 f4679b4
Apply suggestions
ryze312 c59c496
Fix switching devices
ryze312 6221b60
Use functors for deleters
ryze312 896714b
Split VoiceBuffer miniaudio ringbuffer
ryze312 f235bcd
Use VoiceClient and DiscordClient signals in VoiceAudio
ryze312 631eb03
Wrap ma_log
ryze312 09cff96
Remove no longer used stuff in AudioManager
ryze312 dffe44e
Clean up AudioManager and ifdef conditions, expose VoiceAudio
ryze312 486fec9
Make PeakMeter decay itself
ryze312 3d89d24
Wrap MaEngine
ryze312 2a1150a
Implement SystemAudio
ryze312 5e0b8f6
Make notification device start on demand
ryze312 cb44e05
More voice sounds
ryze312 030f5da
Why snake case?
ryze312 bc4b6d1
Add voice sounds
ryze312 72e2248
Play connect/disconnect sounds only for the current channel
ryze312 c5fbe39
Don't copy in capture signal
ryze312 d9373a2
i can't type
ryze312 a762623
i can't type 2
ryze312 0558de1
Make ringbuffer more readable and use std::copy_n
ryze312 0930bd7
Fix wrong max bitrate and default EncodingApplication
ryze312 bd45254
Implement separate sources for voice
ryze312 9d5eb73
Fix includes, include audio sources only when needed
ryze312 12e701c
Disable LTO on Windows
ryze312 98306b2
Document separate_sources option in README
ryze312 2d96b38
Add missing headers
ryze312 c3cf374
Merge branch 'master' into audio_refactor
ryze312 06fb2a9
Update RTP stripping
ryze312 8d4384e
Fix left denoised channel not being written
ryze312 d6ae366
Sync with upstream
ryze312 e48e487
Remove duplicate definition of GetPayloadOffset
ryze312 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,90 @@ | ||
#include "audio_device.hpp" | ||
|
||
namespace AbaddonClient::Audio { | ||
|
||
AudioDevice::AudioDevice(Context &context, ma_device_config &&config, std::optional<ma_device_id> &&device_id) noexcept : | ||
m_context(context), | ||
m_config(std::move(config)), | ||
m_device_id(std::move(device_id)) | ||
{ | ||
SyncDeviceID(); | ||
} | ||
|
||
bool AudioDevice::Start() noexcept { | ||
if (m_started) { | ||
return true; | ||
} | ||
|
||
m_device = Miniaudio::MaDevice::Create(m_context.GetRaw(), m_config); | ||
if (!m_device) { | ||
return false; | ||
} | ||
|
||
m_started = m_device->Start(); | ||
if (!m_started) { | ||
m_device.reset(); | ||
} | ||
|
||
return m_started; | ||
} | ||
|
||
bool AudioDevice::Stop() noexcept { | ||
if (!m_started) { | ||
return true; | ||
} | ||
|
||
m_started = !m_device->Stop(); | ||
|
||
// If we're still running something went wrong | ||
if (m_started) { | ||
return false; | ||
} | ||
|
||
m_device.reset(); | ||
return true; | ||
} | ||
|
||
bool AudioDevice::ChangeDevice(const ma_device_id &device_id) noexcept { | ||
m_device_id = device_id; | ||
|
||
return RefreshDevice(); | ||
} | ||
|
||
void AudioDevice::SyncDeviceID() noexcept { | ||
if (!m_device_id) { | ||
return; | ||
} | ||
|
||
auto& device_id = *m_device_id; | ||
|
||
switch (m_config.deviceType) { | ||
case ma_device_type_playback: { | ||
m_config.playback.pDeviceID = &device_id; | ||
} break; | ||
|
||
case ma_device_type_capture: { | ||
m_config.capture.pDeviceID = &device_id; | ||
} break; | ||
|
||
case ma_device_type_duplex: { | ||
m_config.playback.pDeviceID = &device_id; | ||
m_config.capture.pDeviceID = &device_id; | ||
} | ||
|
||
case ma_device_type_loopback: { | ||
m_config.capture.pDeviceID = &device_id; | ||
} | ||
} | ||
} | ||
|
||
bool AudioDevice::RefreshDevice() noexcept { | ||
m_device.reset(); | ||
if (m_started) { | ||
m_started = false; | ||
return Start(); | ||
} | ||
|
||
return true; | ||
} | ||
|
||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,30 @@ | ||
#pragma once | ||
|
||
#include "context.hpp" | ||
|
||
#include "miniaudio/ma_device.hpp" | ||
|
||
namespace AbaddonClient::Audio { | ||
|
||
class AudioDevice { | ||
public: | ||
AudioDevice(Context& context, ma_device_config &&config, std::optional<ma_device_id> &&device_id) noexcept; | ||
|
||
bool Start() noexcept; | ||
bool Stop() noexcept; | ||
|
||
bool ChangeDevice(const ma_device_id &device_id) noexcept; | ||
private: | ||
void SyncDeviceID() noexcept; | ||
bool RefreshDevice() noexcept; | ||
|
||
bool m_started = false; | ||
|
||
Context &m_context; | ||
std::optional<Miniaudio::MaDevice> m_device; | ||
|
||
ma_device_config m_config; | ||
std::optional<ma_device_id> m_device_id; | ||
}; | ||
|
||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,53 @@ | ||
#include "audio_engine.hpp" | ||
|
||
#include <glibmm/main.h> | ||
#include <spdlog/spdlog.h> | ||
|
||
namespace AbaddonClient::Audio { | ||
|
||
AudioEngine::AudioEngine(Context &context) noexcept : | ||
m_context(context) {} | ||
|
||
AudioEngine::~AudioEngine() noexcept { | ||
StopTimer(); | ||
} | ||
|
||
bool AudioEngine::PlaySound(std::string_view file_path) noexcept { | ||
StopTimer(); | ||
|
||
const auto result = m_engine.LockScope([this, &file_path](std::optional<Miniaudio::MaEngine> &engine) { | ||
if (!engine) { | ||
engine = Miniaudio::MaEngine::Create(m_context.GetEngineConfig()); | ||
if (!engine) { | ||
return false; | ||
} | ||
} | ||
|
||
return engine->PlaySound(file_path); | ||
}); | ||
|
||
StartTimer(); | ||
return result; | ||
} | ||
|
||
void AudioEngine::StartTimer() noexcept { | ||
// NOTE: I am not using g_timeout_add_seconds_once here since we want to own the source and destroy it manually | ||
// g_source_remove throws an error on destroyed source | ||
m_timer_source = g_timeout_source_new_seconds(5); | ||
|
||
g_source_set_callback(m_timer_source, GSourceFunc(AudioEngine::TimeoutEngine), this, nullptr); | ||
g_source_attach(m_timer_source, Glib::MainContext::get_default()->gobj()); | ||
} | ||
|
||
void AudioEngine::StopTimer() noexcept { | ||
if (m_timer_source != nullptr) { | ||
g_source_destroy(m_timer_source); | ||
g_source_unref(m_timer_source); | ||
} | ||
} | ||
|
||
void AudioEngine::TimeoutEngine(AudioEngine &engine) noexcept { | ||
engine.m_engine.Lock()->reset(); | ||
} | ||
|
||
} |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
im guessing this is meant to kill the engine if no sounds are played for 5 seconds? might it just be simpler to keep it around anyways? or maybe we can just call
ma_engine_stop
to avoid having to create it over and over.also im pretty sure there are glibmm bindings for this but thats not a big deal
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was trying to match the behavior of the official client, it spawns source as needed with the timeout of 5 seconds.
ma_engine_stop
seems to just stop the device's callbacks internally, it still keeps the source intact and connected.glibmm has
SignalTimeout::connect_seconds_once
, but it doesn't seem to provide a way to destroy/reset the timeout.