AnimationPlayer.advance() doesn't advance audio

[Jayman2000]

Godot version:

v3.3.1.stable.official

OS/device including version:

OS: Arch Linux (I don't know how to provide a version for this)

Issue description:

I have an AnimationPlayer with multiple tracks. One of them is an audio playback track. When I call the advance() method, I expect the audio to jump ahead. It doesn't do that.

Workaround
Call AudioStreamPlayer.seek() after calling AnimationPlayer.advance().

Steps to reproduce:

1. Create an AnimationPlayer.
2. Give it an animation named "example".
3. Add an audio playback track to that animation.
4. Add a 2 minute long sound clip to that audio playback track.
5. Make sure that the animation is at least 2 minutes long.
6. Attach the following script to the AnimationPlayer:

extends AnimationPlayer


func _ready():
	play("example")
	advance(60)

	# Workaround:
	#$AudioStreamPlayer.seek(60)

7. Run the scene.

Minimal reproduction project:

animation_player_does_not_advance_audio.zip

[alessandrofama]

Very new to Godot's source but trying to get started and sharing my observations. I think the relevant code (master) is here:

godot/scene/animation/animation_player.cpp

Lines 611 to 712 in db028ac

	case Animation::TYPE_AUDIO: {
	if (!nc->node) {
	continue;
	}
	if (p_delta == 0) {
	continue;
	}
	if (p_seeked) {
	//find whatever should be playing
	int idx = a->track_find_key(i, p_time);
	if (idx < 0) {
	continue;
	}
	Ref<AudioStream> stream = a->audio_track_get_key_stream(i, idx);
	if (!stream.is_valid()) {
	nc->node->call("stop");
	nc->audio_playing = false;
	playing_caches.erase(nc);
	} else {
	float start_ofs = a->audio_track_get_key_start_offset(i, idx);
	start_ofs += p_time - a->track_get_key_time(i, idx);
	float end_ofs = a->audio_track_get_key_end_offset(i, idx);
	float len = stream->get_length();
	if (start_ofs > len - end_ofs) {
	nc->node->call("stop");
	nc->audio_playing = false;
	playing_caches.erase(nc);
	continue;
	}
	nc->node->call("set_stream", stream);
	nc->node->call("play", start_ofs);
	nc->audio_playing = true;
	playing_caches.insert(nc);
	if (len && end_ofs > 0) { //force an end at a time
	nc->audio_len = len - start_ofs - end_ofs;
	} else {
	nc->audio_len = 0;
	}
	nc->audio_start = p_time;
	}
	} else {
	//find stuff to play
	List<int> to_play;
	a->track_get_key_indices_in_range(i, p_time, p_delta, &to_play);
	if (to_play.size()) {
	int idx = to_play.back()->get();
	Ref<AudioStream> stream = a->audio_track_get_key_stream(i, idx);
	if (!stream.is_valid()) {
	nc->node->call("stop");
	nc->audio_playing = false;
	playing_caches.erase(nc);
	} else {
	float start_ofs = a->audio_track_get_key_start_offset(i, idx);
	float end_ofs = a->audio_track_get_key_end_offset(i, idx);
	float len = stream->get_length();
	nc->node->call("set_stream", stream);
	nc->node->call("play", start_ofs);
	nc->audio_playing = true;
	playing_caches.insert(nc);
	if (len && end_ofs > 0) { //force an end at a time
	nc->audio_len = len - start_ofs - end_ofs;
	} else {
	nc->audio_len = 0;
	}
	nc->audio_start = p_time;
	}
	} else if (nc->audio_playing) {
	bool loop = a->has_loop();
	bool stop = false;
	if (!loop && p_time < nc->audio_start) {
	stop = true;
	} else if (nc->audio_len > 0) {
	float len = nc->audio_start > p_time ? (a->get_length() - nc->audio_start) + p_time : p_time - nc->audio_start;
	if (len > nc->audio_len) {
	stop = true;
	}
	}
	if (stop) {
	//time to stop
	nc->node->call("stop");
	nc->audio_playing = false;
	playing_caches.erase(nc);
	}
	}
	}
	} break;

I noticed that when you call AnimationPlayer.seek() it works correctly (p_seeked will be true) and it starts playing the audio again at the right start_ofs.
When calling AnimationPlayer.advance(), p_seeked is false and it runs the code inside the else statement below.
The size of the to_play list is 0 and so it skips that part and checks if there no audio at the position you shifted to and stops the stream in that case (I guess).

If you put another audio file on the same track and call advance(130) (example) it will play the second stream:

The size of the to_play list will be 1 in this case, so it doesn't skip this code inside the else statement.

I noticed that the value of p_delta is like 0.016666667535901070 (this is from Node::get_process_delta_time ( ) ?) when randomly setting a breakpoint, but will be set to the delta argument of AnimationPlayer.advance() when that function is called.

As a test, by doing something like

if (p_seeked || p_delta > 0.1) {
//find whatever should be playing
[...]

it will correctly jump to the right audio position, as p_delta > 0.1 will only be true when you call AnimationPlayer.advance(delta) with delta > 0.1, but p_delta will be smaller all the other times. But that does not seem like a clean solution to me.

It feels to me AnimationPlayer.advance() should behave as AnimationPlayer.seek() for audio, but the only value that differs in the code to check if advance() was called seems to be p_delta.
Not sure what the optimal solution to this problem would be right now. In any case looking forward to read and understand more of the AnimationPlayer code :-)

[alessandrofama]

One thing I've tried to do is adding a advanced bool to the Playback struct:

	struct Playback {
		List<Blend> blend;
		PlaybackData current;
		StringName assigned;
		bool seeked = false;
		bool started = false;
		bool advanced = false;
	} playback;

and setting it to true when calling AnimationPlayer::advance():

void AnimationPlayer::advance(float p_time) {
	playback.advanced = true;
	_animation_process(p_time);
}

then passing it to AnimationPlayer::_animation_process_data() in AnimationPlayer::_animation_process2():

_animation_process_data(c.current, p_delta, 1.0f, c.seeked && p_delta != 0, p_started, c.advanced);

and consequently also to AnimationPlayer::_animation_process_animation():

_animation_process_animation(cd.from, cd.pos, delta, p_blend, &cd == &playback.current, p_seeked, p_started, p_advanced);

In the if statement inside the Animation::TYPE_AUDIO switch case, where it is checking if it seeked, then also checking if it advanced:

if (p_seeked || p_advanced) {
	//find whatever should be playing
 [...]

That's similar to how Playback.seeked is being set in AnimationPlayer::seek(). Works fine and solves the issue, but not sure if it is the right approach, since introducing Playback.advanced would only be useful for this particular audio problem.
Testing it here: Changes, branch.