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 bug reported in issue 727 #729

Closed
wants to merge 4 commits into from
Closed

Fix bug reported in issue 727 #729

wants to merge 4 commits into from

Conversation

jjceresa
Copy link
Collaborator

@jjceresa jjceresa commented Dec 30, 2020

This PR addresses issues reported in #727

calling fluid_synth_free_voice_by_kill_LOCAL(), two problems occur:

 1)The value returned by fluid_synth_get_active_voice_count() never
   returns back to 0.
 2)SoundFont samples are not unref'd properly, and therefore if an attempt is
   made to unload the SoundFont, the deferred unload timer is started, and
   fluid_synth_sfunload_callback() unsuccessfully tries	to unload the SoundFont forever.
 These 2 issues are fixed by this commit.
…s().

initialize voice->sample to NULL in fluid_voice_stop().
Copy link
Member

@derselbst derselbst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That looks surprisingly simple. I haven't tested it yet, I may have an opportunity later to do so.

src/synth/fluid_voice.c Show resolved Hide resolved
@@ -4629,6 +4628,9 @@ fluid_synth_check_finished_voices(fluid_synth_t *synth)
else if(synth->voice[j]->overflow_rvoice == fv)
{
fluid_voice_overflow_rvoice_finished(synth->voice[j]);

/* Decrement voice count */
synth->active_voice_count--;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For consistency I think it would be better to move this call into fluid_voice_overflow_rvoice_finished().

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But I see you did it earlier and the unit tests failed. Perhaps you could just add a small comment to explain why the decrement must be put here.

@sonarcloud
Copy link

sonarcloud bot commented Dec 30, 2020

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@derselbst
Copy link
Member

Closed in favor of #736 which is based on 2.1.x branch.

@derselbst derselbst closed this Jan 2, 2021
@derselbst derselbst deleted the fix-issue-727 branch January 2, 2021 16:12
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