Skip to content

Commit

Permalink
Fix: properly know when a player is deleted from inside the stream.
Browse files Browse the repository at this point in the history
This eliminates crashes that happen because stream playbacks will call
_mix() for some reason even after their audio player is stopped and
deleted.
  • Loading branch information
stechyo committed Jan 12, 2024
1 parent aa97bf0 commit e9d15f7
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 6 deletions.
15 changes: 12 additions & 3 deletions src/player.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,11 @@ SteamAudioPlayer::~SteamAudioPlayer() {
iplAudioBufferFree(gs->ctx, &local_state.bufs.mono);
iplAudioBufferFree(gs->ctx, &local_state.bufs.refl_ambi);
iplAudioBufferFree(gs->ctx, &local_state.bufs.refl_out);

if (!pb.is_null()) {
auto playback = dynamic_cast<SteamAudioStreamPlayback *>(pb.ptr());
playback->parent = nullptr;
}
}

LocalSteamAudioState *SteamAudioPlayer::get_local_state() {
Expand Down Expand Up @@ -198,9 +203,9 @@ void SteamAudioPlayer::_ready() {
// The stream might be actually instantiated before Player::_ready().
// If so, transfer the necessary data directly to it.
if (this->is_playing() && !get_stream_playback().is_null()) {
auto pb = dynamic_cast<SteamAudioStreamPlayback *>(get_stream_playback().ptr());
pb->parent = this;
pb->set_stream(sub_stream);
auto playback = dynamic_cast<SteamAudioStreamPlayback *>(get_stream_playback().ptr());
playback->parent = this;
playback->set_stream(sub_stream);
}
}

Expand All @@ -216,6 +221,10 @@ void SteamAudioPlayer::_process(double delta) {
if (Engine::get_singleton()->is_editor_hint() && sub_stream.ptr() != get_stream().ptr()) {
set_stream(sub_stream);
}

if (is_playing() && !get_stream_playback().is_null()) {
pb = get_stream_playback();
}
}

Ref<AudioStream> SteamAudioPlayer::get_sub_stream() { return sub_stream; }
Expand Down
4 changes: 4 additions & 0 deletions src/player.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ class SteamAudioPlayer : public AudioStreamPlayer3D {
GDCLASS(SteamAudioPlayer, AudioStreamPlayer3D);

private:
// This ref is kept because at destruction we can't get the playback
// since the player has stopped (even though the playback still mixes...)
Ref<AudioStreamPlayback> pb;

Ref<AudioStream> sub_stream;
// TODO: we can probably move these values inside local state
// for cleanup and the ability to adjust them at runtime
Expand Down
4 changes: 2 additions & 2 deletions src/steam_audio.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
#include <godot_cpp/classes/audio_stream_player3d.hpp>
#include <godot_cpp/classes/node3d.hpp>
#include <godot_cpp/core/class_db.hpp>
#include <mutex>
#include <shared_mutex>

using namespace godot;

Expand Down Expand Up @@ -78,7 +78,7 @@ struct LocalSteamAudioState {
LocalSteamAudioBuffers bufs;
SteamAudioEffects fx;
SteamAudioSourceConfig cfg;
std::mutex mux;
std::shared_mutex mux;
};

inline int ambisonic_channels_from(int order) {
Expand Down
10 changes: 9 additions & 1 deletion src/stream.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ SteamAudioStreamPlayback::~SteamAudioStreamPlayback() {}

int32_t SteamAudioStreamPlayback::_mix(AudioFrame *buffer, double rate_scale, int32_t frames) {
if (parent == nullptr) {
SteamAudio::log(SteamAudio::log_error, "mixing: ERROR, parent is null!");
return frames;
}

Expand All @@ -58,6 +57,15 @@ int32_t SteamAudioStreamPlayback::_mix(AudioFrame *buffer, double rate_scale, in
}
std::unique_lock lock(ls->mux);

// Some extra checks because at this point parent may have been deleted
if (parent == nullptr) {
return frames;
}
ls = parent->get_local_state();
if (ls == nullptr || !ls->src.player) {
return frames;
}

PackedVector2Array mixed_frames = stream_playback->get_raw_audio(rate_scale, frames);
frames = int(mixed_frames.size());

Expand Down

0 comments on commit e9d15f7

Please sign in to comment.