Skip to content
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

Make FlxSound#time more precise #3272

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

Sword352
Copy link
Contributor

It seems that directly seeking for channel.position is less prone to errors when seeking for sound.time.
Not sure where the issue actually comes from yet, but this fixed bunch of issues related to music syncing for me.

import flixel.FlxG;
import flixel.FlxState;
import flixel.sound.FlxSound;
import flixel.text.FlxText;

class PlayState extends FlxState
{
	var _instrumental:FlxSound;
	var _voices:Array<FlxSound> = [];

	var _resyncText:FlxText;
	var _resyncs:Int = 0;

	override public function create()
	{
		super.create();

		FlxG.fixedTimestep = false;

		_instrumental = FlxG.sound.load(AssetPaths.Inst__ogg).play();
		_voices.push(FlxG.sound.load(AssetPaths.Voices_Opponent__ogg).play());
		_voices.push(FlxG.sound.load(AssetPaths.Voices_Player__ogg).play());

		_resyncText = new FlxText(10, 10, 0, "0", 16);
		add(_resyncText);
	}

	override public function update(elapsed:Float)
	{
		for (voice in _voices)
		{
			if (Math.abs(voice.time - _instrumental.time) > 5)
			{
				voice.time = _instrumental.time;
				_resyncText.text = Std.string(++_resyncs);
			}
		}
	}
}

Here, the voices would resync often, causing audio stutters. The changes made by this PR seems to fix it, as well as other issues in my other project
Pr in draft as it may be better to fix the root issue instead

NOTE: this PR introduces behaviour change, originally sounds that aren't updated won't have their time value changing during playback. Here it changes regardless.

@Geokureli Geokureli marked this pull request as ready for review October 28, 2024 12:07
@Geokureli
Copy link
Member

Geokureli commented Nov 4, 2024

I have issues wit this. I feel like this is a part of a larger issue with FlxSound AND I think the behavior change is not insignificant.

My concern, here: Is there a benefit to setting _time in update? the only case I can imagine is where we do something like

doSomething(mySound.time); // `time` has some value
someProcessThatTakesAFewMilliseconds();
doSomethingElse(mySound.time); // May not be the same value as before

In the above case it may be expected, or important that mySound.time has the same value at both function calls, it will in the old code, but potentially not in the new code. The reason this is (sorta) important is that flixel is entirely based on the principle that "game time" stands still until the start of the next update, when game time advances by FlxG.elapsed. But even then sounds are a little different, they may not advance the same as everything else, and they are not effected by FlxG.timeScale, so I'm torn

I'm wondering if it's better to create a new field in FlxSound called position, eventually phase out time, and see where else we can remove the need for sounds to be updated in order to work.

Maybe something like this, where the documentation clearly states it behaves different from time:

inline function get_position():Float
{
	if (playing)
		_position  = _channel.position;
	
	return _position;
}

But with this would we also need to set _position on pause() calls or do paused sounds still have a _channel we can access? With just the above code, pausing a sound will keep the previous checked position, so if time passed between that check and a pause it may go out of sync, either way we'll definitely need to set _position on stop, startSound and set_time calls

@Sword352
Copy link
Contributor Author

Sword352 commented Nov 4, 2024

I feel like this is a part of a larger issue with FlxSound AND I think the behavior change is not insignificant.

yeah that's why the pr used to be in draft

@Sword352
Copy link
Contributor Author

Sword352 commented Nov 4, 2024

But even then sounds are a little different, they may not advance the same as everything else

Yeah it would make sense to make sounds not follow this rule since sound playback is entirely independant from the game loop

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants