Fix leak when using audio samples instead of streams

akien-mga · Sep 4, 2024 · d3ddce6 · d3ddce6
1 parent e2dd56b
commit d3ddce6
Show file tree

Hide file tree

Showing 8 changed files with 59 additions and 12 deletions.
diff --git a/modules/minimp3/audio_stream_mp3.cpp b/modules/minimp3/audio_stream_mp3.cpp
@@ -159,6 +159,9 @@ Ref<AudioSamplePlayback> AudioStreamPlaybackMP3::get_sample_playback() const {
 
 void AudioStreamPlaybackMP3::set_sample_playback(const Ref<AudioSamplePlayback> &p_playback) {
 	sample_playback = p_playback;
+	if (sample_playback.is_valid()) {
+		sample_playback->stream_playback = Ref<AudioStreamPlayback>(this);
+	}
 }
 
 void AudioStreamPlaybackMP3::set_parameter(const StringName &p_name, const Variant &p_value) {

diff --git a/modules/vorbis/audio_stream_ogg_vorbis.cpp b/modules/vorbis/audio_stream_ogg_vorbis.cpp
@@ -390,6 +390,9 @@ Ref<AudioSamplePlayback> AudioStreamPlaybackOggVorbis::get_sample_playback() con
 
 void AudioStreamPlaybackOggVorbis::set_sample_playback(const Ref<AudioSamplePlayback> &p_playback) {
 	sample_playback = p_playback;
+	if (sample_playback.is_valid()) {
+		sample_playback->stream_playback = Ref<AudioStreamPlayback>(this);
+	}
 }
 
 AudioStreamPlaybackOggVorbis::~AudioStreamPlaybackOggVorbis() {

diff --git a/scene/animation/animation_mixer.cpp b/scene/animation/animation_mixer.cpp
@@ -1637,6 +1637,9 @@ void AnimationMixer::_blend_process(double p_delta, bool p_update_only) {
 						}
 
 						if (t_obj->call(SNAME("get_is_sample"))) {
+							if (t->audio_stream_playback->get_sample_playback().is_valid()) {
+								AudioServer::get_singleton()->stop_sample_playback(t->audio_stream_playback->get_sample_playback());
+							}
 							Ref<AudioSamplePlayback> sample_playback;
 							sample_playback.instantiate();
 							sample_playback->stream = stream;

diff --git a/scene/resources/audio_stream_polyphonic.cpp b/scene/resources/audio_stream_polyphonic.cpp
@@ -247,6 +247,11 @@ AudioStreamPlaybackPolyphonic::ID AudioStreamPlaybackPolyphonic::play_stream(con
 				sp->volume_vector.write[2] = AudioFrame(linear_volume, linear_volume);
 				sp->volume_vector.write[3] = AudioFrame(linear_volume, linear_volume);
 				sp->bus = p_bus;
+
+				if (streams[i].stream_playback->get_sample_playback().is_valid()) {
+					AudioServer::get_singleton()->stop_playback_stream(sp);
+				}
+
 				streams[i].stream_playback->set_sample_playback(sp);
 				AudioServer::get_singleton()->start_sample_playback(sp);
 			}
@@ -315,6 +320,9 @@ Ref<AudioSamplePlayback> AudioStreamPlaybackPolyphonic::get_sample_playback() co
 
 void AudioStreamPlaybackPolyphonic::set_sample_playback(const Ref<AudioSamplePlayback> &p_playback) {
 	sample_playback = p_playback;
+	if (sample_playback.is_valid()) {
+		sample_playback->stream_playback = Ref<AudioStreamPlayback>(this);
+	}
 }
 
 void AudioStreamPlaybackPolyphonic::_bind_methods() {

diff --git a/scene/resources/audio_stream_wav.cpp b/scene/resources/audio_stream_wav.cpp
@@ -479,6 +479,9 @@ Ref<AudioSamplePlayback> AudioStreamPlaybackWAV::get_sample_playback() const {
 
 void AudioStreamPlaybackWAV::set_sample_playback(const Ref<AudioSamplePlayback> &p_playback) {
 	sample_playback = p_playback;
+	if (sample_playback.is_valid()) {
+		sample_playback->stream_playback = Ref<AudioStreamPlayback>(this);
+	}
 }
 
 AudioStreamPlaybackWAV::AudioStreamPlaybackWAV() {}

diff --git a/servers/audio/audio_stream.h b/servers/audio/audio_stream.h
@@ -48,6 +48,7 @@ class AudioSamplePlayback : public RefCounted {
 
 public:
 	Ref<AudioStream> stream;
+	Ref<AudioStreamPlayback> stream_playback;
 
 	float offset = 0.0f;
 	float pitch_scale = 1.0;

diff --git a/servers/audio_server.cpp b/servers/audio_server.cpp
@@ -501,12 +501,7 @@ void AudioServer::_mix_step() {
 		switch (playback->state.load()) {
 			case AudioStreamPlaybackListNode::AWAITING_DELETION:
 			case AudioStreamPlaybackListNode::FADE_OUT_TO_DELETION:
-				playback_list.erase(playback, [](AudioStreamPlaybackListNode *p) {
-					delete p->prev_bus_details;
-					delete p->bus_details.load();
-					p->stream_playback.unref();
-					delete p;
-				});
+				_delete_stream_playback_list_node(playback);
 				break;
 			case AudioStreamPlaybackListNode::FADE_OUT_TO_PAUSE: {
 				// Pause the stream.
@@ -697,6 +692,23 @@ AudioServer::AudioStreamPlaybackListNode *AudioServer::_find_playback_list_node(
 	return nullptr;
 }
 
+void AudioServer::_delete_stream_playback(Ref<AudioStreamPlayback> p_playback) {
+	ERR_FAIL_COND(p_playback.is_null());
+	AudioStreamPlaybackListNode *playback_node = _find_playback_list_node(p_playback);
+	if (playback_node) {
+		_delete_stream_playback_list_node(playback_node);
+	}
+}
+
+void AudioServer::_delete_stream_playback_list_node(AudioStreamPlaybackListNode *p_playback_node) {
+	playback_list.erase(p_playback_node, [](AudioStreamPlaybackListNode *p) {
+		delete p->prev_bus_details;
+		delete p->bus_details.load();
+		p->stream_playback.unref();
+		delete p;
+	});
+}
+
 bool AudioServer::thread_has_channel_mix_buffer(int p_bus, int p_buffer) const {
 	if (p_bus < 0 || p_bus >= buses.size()) {
 		return false;
@@ -1227,8 +1239,12 @@ void AudioServer::stop_playback_stream(Ref<AudioStreamPlayback> p_playback) {
 	ERR_FAIL_COND(p_playback.is_null());
 
 	// Handle sample playback.
-	if (p_playback->get_is_sample() && p_playback->get_sample_playback().is_valid()) {
-		AudioServer::get_singleton()->stop_sample_playback(p_playback->get_sample_playback());
+	if (p_playback->get_is_sample()) {
+		if (p_playback->get_sample_playback().is_valid()) {
+			AudioServer::get_singleton()->stop_sample_playback(p_playback->get_sample_playback());
+		} else {
+			_delete_stream_playback(p_playback);
+		}
 		return;
 	}
 
@@ -1370,8 +1386,12 @@ void AudioServer::set_playback_highshelf_params(Ref<AudioStreamPlayback> p_playb
 bool AudioServer::is_playback_active(Ref<AudioStreamPlayback> p_playback) {
 	ERR_FAIL_COND_V(p_playback.is_null(), false);
 
-	if (p_playback->get_is_sample() && p_playback->get_sample_playback().is_valid()) {
-		return sample_playback_list.has(p_playback->get_sample_playback());
+	if (p_playback->get_is_sample()) {
+		if (p_playback->get_sample_playback().is_valid()) {
+			return sample_playback_list.has(p_playback->get_sample_playback());
+		} else {
+			return false;
+		}
 	}
 
 	AudioStreamPlaybackListNode *playback_node = _find_playback_list_node(p_playback);
@@ -1845,8 +1865,12 @@ void AudioServer::start_sample_playback(const Ref<AudioSamplePlayback> &p_playba
 
 void AudioServer::stop_sample_playback(const Ref<AudioSamplePlayback> &p_playback) {
 	ERR_FAIL_COND_MSG(p_playback.is_null(), "Parameter p_playback is null.");
-	AudioDriver::get_singleton()->stop_sample_playback(p_playback);
-	sample_playback_list.erase(p_playback);
+	if (sample_playback_list.has(p_playback)) {
+		sample_playback_list.erase(p_playback);
+		AudioDriver::get_singleton()->stop_sample_playback(p_playback);
+		p_playback->stream_playback->set_sample_playback(nullptr);
+		stop_playback_stream(p_playback->stream_playback);
+	}
 }
 
 void AudioServer::set_sample_playback_pause(const Ref<AudioSamplePlayback> &p_playback, bool p_paused) {

diff --git a/servers/audio_server.h b/servers/audio_server.h
@@ -297,6 +297,8 @@ class AudioServer : public Object {
 
 	SafeList<AudioStreamPlaybackListNode *> playback_list;
 	SafeList<AudioStreamPlaybackBusDetails *> bus_details_graveyard;
+	void _delete_stream_playback(Ref<AudioStreamPlayback> p_playback);
+	void _delete_stream_playback_list_node(AudioStreamPlaybackListNode *p_node);
 
 	// TODO document if this is necessary.
 	SafeList<AudioStreamPlaybackBusDetails *> bus_details_graveyard_frame_old;