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

Fix #9117, 04ce1f07: [Fluidsynth] Infinite wait when stopping song #9181

Merged
merged 1 commit into from May 3, 2021

Conversation

rubidium42
Copy link
Contributor

@rubidium42 rubidium42 commented May 2, 2021

Motivation / Problem

FluidSynth 2.2.0 hangs when stopping a song.

Description

In FluidSynth 2.2.0 an extra state was added to denote stopping. To transition from this state to a stopped state the rendering needs to be running. Since 04ce1f0 locking was added that skipped the rendering when something else held a lock, so the state would never get to stopped and join would never return.
Now the join will be performed while the lock is not held, so the other thread can transition the state. After that, the lock is regained and the rest of the cleanup is performed.

Limitations

Does not solve the issue when the audio driver does not have its own thread (e.g. Allegro). Don't have a good idea how to solve that nicely at the moment either. Maybe forcing all audio drivers to use a thread? I'll leave that issue to some other PR, however this closes #9117.

I have no idea why the try_lock was used in the sound rendering, so I have no idea what the repercussions of this change are. It seems to work for me. Since nielsmh added that, I've requested him to take a look at it.

Checklist for review

Some things are not automated, and forgotten often. This list is a reminder for the reviewers.

  • The bug fix is important enough to be backported? (label: 'backport requested')
  • This PR affects the save game format? (label 'savegame upgrade')
  • This PR affects the GS/AI API? (label 'needs review: Script API')
    • ai_changelog.hpp, gs_changelog.hpp need updating.
    • The compatibility wrappers (compat_*.nut) need updating.
  • This PR affects the NewGRF API? (label 'needs review: NewGRF')

@rubidium42 rubidium42 requested a review from nielsmh May 2, 2021 22:17
@nielsmh
Copy link
Contributor

nielsmh commented May 3, 2021

The basic idea of the try_lock is to avoid blocking the rest of the sound effects rendering if the music player is busy.

In FluidSynth 2.2.0 an extra state was added to denote stopping. To transition
from this state to a stopped state the rendering needs to be running. Since
04ce1f0 locking was added that skipped the rendering when something else held
a lock, so the state would never get to stopped and join would never return.
@rubidium42 rubidium42 added the backport requested This PR should be backport to current release (RC / stable) label May 3, 2021
@rubidium42 rubidium42 marked this pull request as ready for review May 3, 2021 14:27
@LordAro LordAro merged commit 6bd7f88 into OpenTTD:master May 3, 2021
@rubidium42 rubidium42 deleted the issue-9117 branch May 3, 2021 15:46
LordAro pushed a commit to LordAro/OpenTTD that referenced this pull request May 3, 2021
…ong (OpenTTD#9181)

In FluidSynth 2.2.0 an extra state was added to denote stopping. To transition
from this state to a stopped state the rendering needs to be running. Since
04ce1f0 locking was added that skipped the rendering when something else held
a lock, so the state would never get to stopped and join would never return.
LordAro pushed a commit that referenced this pull request May 3, 2021
)

In FluidSynth 2.2.0 an extra state was added to denote stopping. To transition
from this state to a stopped state the rendering needs to be running. Since
04ce1f0 locking was added that skipped the rendering when something else held
a lock, so the state would never get to stopped and join would never return.
@LordAro LordAro added backported This PR is backported to a current release (RC / stable) and removed backport requested This PR should be backport to current release (RC / stable) labels May 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backported This PR is backported to a current release (RC / stable)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

New game fails to start after upgrading FluidSynth to version 2.2.0
3 participants