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

Jukebox not working in the flatpak version #6873

Closed
ghisvail opened this issue Jul 30, 2018 · 31 comments
Closed

Jukebox not working in the flatpak version #6873

ghisvail opened this issue Jul 30, 2018 · 31 comments
Labels
OS: Linux Issues specific to Linux builds

Comments

@ghisvail
Copy link
Contributor

A user of the flatpak version of OpenTTD reported an issue with the jukebox not playing any music. Indeed, the jukebox just keeps cycling through the original playlist without any playback.

Any ideas what could be causing this? How are the respective path to the individual tracks composing the playlist managed in the application?

@nielsmh
Copy link
Contributor

nielsmh commented Jul 30, 2018

When the jukebox shows track names and not an empty playlist, it indicates that the music files are found and are readable without issue. (The baseset search routine hashes all the requested files, i.e. the MIDI files, and compares the hash to a target in the baseset definition. If the files can't be read or the hashes don't match, loading a baseset fails.)

If the jukebox just cycles between tracks quickly, it indicates that the MIDI driver isn't able to start playback. It appears you're including libtimidity and freepats in your package, which ought to work, but I haven't personally tried any of the music code on non-Windows systems. The other potential failure mode would be changing slowly, as if the tracks are playing but without producing sound.

Try starting OpenTTD with commandline parameters -d driver=1 and watch the console. The libtimidity music driver should print "successfully initialised timidity" on startup, to indicate that it could correctly init the library and load the config. It would print "Could not open music file" or "Invalid MIDI file" if there are problems loading the music files.

@nielsmh nielsmh added OS: Linux Issues specific to Linux builds music labels Jul 30, 2018
@nielsmh
Copy link
Contributor

nielsmh commented Jul 31, 2018

I'm not sure if this is the final verdict, but my current result testing this is that music/libtimidity.cpp doesn't even get built, despite LIBTIMIDITY being defined and -ltimidity being linked in. Note that this is tested on current master, not 1.8.0 tag.

After hacking the build to include libtimidity.cpp (not a proper fix) it appears that it doesn't get correct file names to the MIDI files, but that's a different issue probably introduced in my reworking of the music code, and shouldn't be present in 1.8.0 or earlier.

So: Can you confirm whether music/libtimidity.cpp is printed during make?

@PeterN
Copy link
Member

PeterN commented Aug 1, 2018

libtimidity has never been built in the Linux builds, it was added for the PSP port only.

Linux builds use extmidi, which by default is set up to spawn timidity directly. It's likely enough that this doesn't work properly in a flatpak environment which I understand is sandboxed in some way.

@nielsmh
Copy link
Contributor

nielsmh commented Aug 1, 2018

That probably explains the issue then. The flatpak package tries to build with libtimidity but it doesn't get included since it's not designed to be included in non-PSP builds. So when the game runs, it instead tries to use extmidi to call an external timidity command to play, which fails because there isn't access to any command for playback.

@ghisvail
Copy link
Contributor Author

ghisvail commented Aug 1, 2018

Could we patch the build system to include it regardless? Would that solve the issue?

@nielsmh
Copy link
Contributor

nielsmh commented Aug 2, 2018

As a note, #6611 has a patch for a Fluidsynth driver, which might be more reliable. At least in my testing, Timidity is difficult to get working.

@ghisvail
Copy link
Contributor Author

ghisvail commented Aug 2, 2018

Do you want me to try a build with the patch and drop timidity altogether?

@nielsmh
Copy link
Contributor

nielsmh commented Aug 2, 2018

It's worth a try. Note that patch won't work against current master, but should work against tag 1.8.0. Otherwise, just build with extmidi (that's default on Linux builds) and either include a Timidity++ executable that can get called, or configure with --with-midi= and point it at a Fluidsynth executable, that should also be possible.

@ghisvail
Copy link
Contributor Author

To sum up, the options are:

  1. Build with extmidi (default) and ensure Timidity++ is in PATH at runtime.
  2. Build with extmidi (default), point --with-midi to Fluidsynth and ensure Fluidsynth is in PATH at runtime
  3. Build with Fluidsynth by applying the patch and using the --with-fluidsynth option.

With option 3 likely becoming the default choice for OpenTTD on Linux in the future, correct?

@ghisvail
Copy link
Contributor Author

I applied the following patch (the original one did not apply cleanly on top of v1.8.0):

fluidsynth-support.patch.txt

And the build seems to acknowledge the presence of fluidsynth:

[...]
checking libtimidity... not found
checking fluidsynth... found
[...]
using CFLAGS... -O2 -fomit-frame-pointer  -Wall -Wno-multichar -Wsign-compare -Wundef -Wwrite-strings -Wpointer-arith -W -Wno-unused-parameter -Wredundant-decls -Wformat=2 -Wformat-security -Wno-unused-variable -Wno-unused-but-set-variable -Wno-unused-but-set-parameter -Winit-self -fno-strict-aliasing -Wcast-qual -fno-strict-overflow -Wnon-virtual-dtor -Wno-free-nonheap-object -rdynamic -DUNIX -D_FORTIFY_SOURCE=2 -DWITH_SSE -DWITH_SDL  -D_REENTRANT -I/app/include/SDL -DWITH_ZLIB   -DWITH_LZMA   -DWITH_LZO -DWITH_XDG_BASEDIR   -D_SQ64 -I/run/build/openttd/src/3rdparty/squirrel/include -DWITH_PNG -I/usr/include/libpng16  -DWITH_FONTCONFIG -I/usr/include/uuid -I/usr/include/freetype2 -I/usr/include/libpng16 -I/usr/include/harfbuzz -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include  -DWITH_FREETYPE -I/usr/include/freetype2 -I/usr/include/libpng16 -I/usr/include/harfbuzz -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include  -DWITH_ICU_LAYOUT   -DWITH_ICU_SORT   -DFLUIDSYNTH   -DENABLE_NETWORK -DNDEBUG -DWITH_PERSONAL_DIR -DPERSONAL_DIR=\\".openttd\\" -DGLOBAL_DATA_DIR=\\"/app/share/games/openttd\\"
using CXXFLAGS...  -flifetime-dse=1 -std=gnu++14
using LDFLAGS... -lstdc++ -lpthread -lc -L/app/lib -Wl,-rpath,/app/lib -lSDL -lpthread -lz  -llzma  -llzo2 -L/app/lib -lxdg-basedir  -lpng16  -lfontconfig -lfreetype  -lfreetype  -L/app/lib -liculx -licule -licuuc -licudata  -L/app/lib -licui18n -licuuc -licudata  -L/app/lib -lfluidsynth  -L/app/lib  -rdynamic

But the jukebox is not working yet. Do I need the fluidsynth executable, or just the shared library?

@nielsmh
Copy link
Contributor

nielsmh commented Aug 14, 2018

First thing to check is which music driver actually gets selected, or force the game to use fluidsynth in the config file. Try setting musicdriver = fluidsynth in openttd.cfg.

@ghisvail
Copy link
Contributor Author

Try setting musicdriver = fluidsynth in openttd.cfg.

I get: Error: No such music driver: fluidsynth

@nielsmh
Copy link
Contributor

nielsmh commented Aug 14, 2018

Well there's your answer for now, the driver isn't actually being compiled in. Unfortunately I don't have time to look at the fluidsynth patch myself, right now.

@LordAro
Copy link
Member

LordAro commented Aug 14, 2018

Unless I've misunderstood the code, fluidsynth.cpp/h isn't actually made use of anywhere in that patch - it's not included or used by any existing code. Missing part of the patch maybe?

@ghisvail
Copy link
Contributor Author

@LordAro here is the original patch. The latter did not apply cleanly on top of 1.8.0 due to conflicts in the config.lib and configure files, the rest is untouched.

I suppose fluidsynth support is dynamically loaded instead of statically included?

@ghisvail
Copy link
Contributor Author

@nielsmh To answer your earlier question:

Can you confirm whether music/libtimidity.cpp is printed during make?

It's not, despite libtimidity being detected and the CFLAGS / LDFLAGS are set properly.

@ghisvail
Copy link
Contributor Author

ghisvail commented Sep 3, 2018

@nielsmh @LordAro Any idea what else I could try?

I have a feeling fluidsynth is the solution here, just wondering whether I am missing some configuration options? The original OpenTTD forum thread mentioned SoundFont, which I am not sure what it is.

@lcarlier
Copy link

I also had this issue on my Ubuntu PC 16.04.
I installed the openttd package from my distro and then I noticed the following package where also installed
esound-common libaudiofile1 libesd0 libxdg-basedir1 openttd-data
openttd-opengfx openttd-openmsx timidity timidity-daemon
Most likely installing timidy and timidity-daemon fixed the issue.

Hope this can help

@LordAro
Copy link
Member

LordAro commented Sep 23, 2018

@ghisvail Dunno I'm afraid - you might want try the #6901 PR, which works for me, at the very least :)

@ghisvail
Copy link
Contributor Author

@LordAro what's the difference between the patch proposed in #6901 and in #6611?

Glad you confirmed it worked for you, that's a good sign :-)

@LordAro
Copy link
Member

LordAro commented Sep 23, 2018

Nothing functionally, other than some minor updates to make it compile with master

@nielsmh
Copy link
Contributor

nielsmh commented Jan 5, 2019

With Fluidsynth support added in #6901 and #7012 I think this is fixed for the next release.

@nielsmh nielsmh closed this as completed Jan 5, 2019
@ghisvail
Copy link
Contributor Author

ghisvail commented Jan 5, 2019 via email

@nielsmh
Copy link
Contributor

nielsmh commented Jan 5, 2019

Usually a new release is made in April.

@ghisvail
Copy link
Contributor Author

ghisvail commented Apr 6, 2019

I have updated the flatpak packaging with the new release of OpenTTD and enabled compilation with Fluidsynth. Still, no jukebox output unfortunately. Could you please reopen this bug whilst I work on fixing it?

Also, is the C library alone enough for MIDI support via Fluidsynth, or is the fluidsynth executable also required at runtime?

@nielsmh nielsmh reopened this Apr 6, 2019
@nielsmh
Copy link
Contributor

nielsmh commented Apr 6, 2019

The C library should be enough, nothing in the OpenTTD Fluidsynth music driver code calls out to an external program. The driver is written against Fluidsynth 1.11, I think it's API compatible with Fluidsynth 2 but not entirely sure.
I think there might be some configuration issues with Fluidsynth, especially how it selects the soundfont to use. Their documentation isn't really clear on which config files are maybe loaded automatically, I just know that when I use a regular system-wide installation the library doesn't need me to specify any configuration at all.

@ghisvail
Copy link
Contributor Author

ghisvail commented Apr 8, 2019

Thanks @nielsmh for your pointers. I started off by downgrading the version of FluidSynth from 2.0.4 to 1.1.11. We'll see how it goes.

@ghisvail
Copy link
Contributor Author

Sadly, I am still unsuccessful with this :-(

ghisvail added a commit to ghisvail/OpenTTD that referenced this issue Apr 18, 2019
@ghisvail
Copy link
Contributor Author

I finally got it working by installing the missing soundfont and patching the code to inject the Flatpak specific path. Please consider merging #7522 eventually.

@takluyver
Copy link

Thanks for your time to figure this out and fix it - I've been missing the music. 🙂

@LordAro
Copy link
Member

LordAro commented Aug 17, 2019

Going to close this as it looks like there's a solution in #7522. Feel free to request a reopen if this isn't the case

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OS: Linux Issues specific to Linux builds
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants