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

Comments

@ghisvail
Copy link

commented Jul 30, 2018

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

This comment has been minimized.

Copy link
Contributor

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

This comment has been minimized.

Copy link
Contributor

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

This comment has been minimized.

Copy link
Member

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

This comment has been minimized.

Copy link
Contributor

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

This comment has been minimized.

Copy link
Author

commented Aug 1, 2018

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

@nielsmh

This comment has been minimized.

Copy link
Contributor

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

This comment has been minimized.

Copy link
Author

commented Aug 2, 2018

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

@nielsmh

This comment has been minimized.

Copy link
Contributor

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

This comment has been minimized.

Copy link
Author

commented Aug 13, 2018

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

This comment has been minimized.

Copy link
Author

commented Aug 14, 2018

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

This comment has been minimized.

Copy link
Contributor

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

This comment has been minimized.

Copy link
Author

commented Aug 14, 2018

Try setting musicdriver = fluidsynth in openttd.cfg.

I get: Error: No such music driver: fluidsynth

@nielsmh

This comment has been minimized.

Copy link
Contributor

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

This comment has been minimized.

Copy link
Member

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

This comment has been minimized.

Copy link
Author

commented Aug 14, 2018

@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

This comment has been minimized.

Copy link
Author

commented Aug 14, 2018

@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

This comment has been minimized.

Copy link
Author

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

This comment has been minimized.

Copy link

commented Sep 22, 2018

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

This comment has been minimized.

Copy link
Member

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

This comment has been minimized.

Copy link
Author

commented Sep 23, 2018

@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

This comment has been minimized.

Copy link
Member

commented Sep 23, 2018

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

@nielsmh

This comment has been minimized.

Copy link
Contributor

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 Jan 5, 2019

@ghisvail

This comment has been minimized.

Copy link
Author

commented Jan 5, 2019

@nielsmh

This comment has been minimized.

Copy link
Contributor

commented Jan 5, 2019

Usually a new release is made in April.

@ghisvail

This comment has been minimized.

Copy link
Author

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

This comment has been minimized.

Copy link
Contributor

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

This comment has been minimized.

Copy link
Author

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

This comment has been minimized.

Copy link
Author

commented Apr 16, 2019

Sadly, I am still unsuccessful with this :-(

ghisvail added a commit to ghisvail/OpenTTD that referenced this issue Apr 18, 2019

@ghisvail

This comment has been minimized.

Copy link
Author

commented Apr 18, 2019

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

This comment has been minimized.

Copy link

commented Apr 18, 2019

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

@LordAro

This comment has been minimized.

Copy link
Member

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

@LordAro LordAro closed this Aug 17, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.