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

fluid-synth 2.0.9 #46793

Closed
wants to merge 1 commit into from
Closed

fluid-synth 2.0.9 #46793

wants to merge 1 commit into from

Conversation

Bo98
Copy link
Member

@Bo98 Bo98 commented Nov 15, 2019

Created with brew bump-formula-pr.

@Bo98
Copy link
Member Author

Bo98 commented Nov 15, 2019

Looks like I need to completely replace the test because they forgot to update --version.

@SMillerDev
Copy link
Member

If you can make the test better that would be awesome. Regardless you might want to report it upstream.

@Bo98
Copy link
Member Author

Bo98 commented Nov 18, 2019

I've made it run a 8 second midi file with the volume set to 0 (to not spook the user). Because it's audio, it might well fail in the CI - we'll see. There's other audio-related formulae which have that issue.

@SMillerDev
Copy link
Member

=> /usr/local/Cellar/fluid-synth/2.0.9/bin/fluidsynth -acoreaudio -mcoremidi -i -g0 /usr/local/Cellar/fluid-synth/2.0.9/share/fluid-synth/sf2/VintageDreamsWaves-v2.sf2 Drum_sample.mid
fluidsynth: warning: Sample 'SineWave': ROM sample ignored
Killing child processes...
Error: fluid-synth: failed
An exception occurred within a child process:
  Timeout::Error: execution expired

@Bo98
Copy link
Member Author

Bo98 commented Nov 19, 2019

Yes, that only happens on the CI. Seems like audio playback hangs on there and I’m not sure if there’s much I can do or not. You can try it locally as well if you want.

@Bo98
Copy link
Member Author

Bo98 commented Nov 20, 2019

Looking to move this forward considering 2.0.8 is simply broken and we should update to 2.0.9 as soon as we can.

Considering the lack of audio support on the CI, I can't see any way to have a meaningful test that also works on the CI. Any thoughts anyone?

@bayandin
Copy link
Member

bayandin commented Dec 3, 2019

fluid-synth 2.1.0 has been released: https://github.com/FluidSynth/fluidsynth/archive/v2.1.0.tar.gz

@lilyinstarlight
Copy link
Contributor

Generating a .wav file like below would probably be an appropriate test. It wouldn't have caught #46643 since that was caused by undefined behaviour in the CoreAudio driver which apparently hangs in CI. I've only sparingly touched Ruby but taking inspiration from https://github.com/Homebrew/homebrew-core/blob/master/Formula/ffmpeg.rb#L106, the below works on my system. I'm happy to make a PR for v2.1.0 with this test unless @Bo98 wants to update this PR.

test do
  # Synthesize wav file from example midi
  resource("example_midi").stage testpath
  wavout = testpath/"Drum_sample.wav"
  system bin/"fluidsynth", "-F", wavout, pkgshare/"sf2/VintageDreamsWaves-v2.sf2", testpath/"Drum_sample.mid"
  assert_predicate wavout, :exist?
end

Additionally, should make check be run before make install like quite a few formulae already seem to have (e.g. https://github.com/Homebrew/homebrew-core/blob/master/Formula/gnupg.rb#L36)? That includes all of the built-in unit tests at https://github.com/FluidSynth/fluidsynth/tree/master/test and is recommended by FluidSynth at https://github.com/FluidSynth/fluidsynth/wiki/BuildingWithCMake#common-tips-for-compiling-from-source. If so, I'm happy to include this change in the v2.1.0 PR as well.

@lilyinstarlight lilyinstarlight mentioned this pull request Jan 10, 2020
5 tasks
@issyl0
Copy link
Member

issyl0 commented Jan 13, 2020

Superseded by #48863 which updates the version to the latest release.

@issyl0 issyl0 closed this Jan 13, 2020
@lock lock bot added the outdated PR was locked due to age label Feb 12, 2020
@lock lock bot locked as resolved and limited conversation to collaborators Feb 12, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated PR was locked due to age
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants