Merge pull request #96572 from adamscott/fix-samples-leak

Fix leak when using audio samples instead of streams

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) {

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() {

scene/animation/animation_mixer.cpp

@@ -1636,6 +1636,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;

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() {

scene/resources/audio_stream_wav.cpp

@@ -475,6 +475,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() {}

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;

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_stream_playback_list_node(playback);
-					delete p->prev_bus_details;
-					delete p->bus_details.load();
-					p->stream_playback.unref();
-					delete p;
-				});
 				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()) {
+	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()) {
+	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);
+	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) {

servers/audio_server.h

@@ -297,6 +297,8 @@ private:
 
 	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;