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 #1033 Broken sound playback in AppImages #1341

Merged
merged 1 commit into from
May 6, 2020

Conversation

J5lx
Copy link
Member

@J5lx J5lx commented Apr 30, 2020

Took me long enough to figure out, but it’s finally here. To avoid insane amounts (~30MB) of bloat I had to pick the GStreamer plugins to distribute by hand, so there’s a chance I missed something, but I think most cases should be taken care of. Nonetheless, I think the most valuable “review” that can be done on this is trying it out on any nearby Linux systems. A build is available here.

In addition to the sound stuff I also did some rework of the Travis setup, mostly by relocating the post-build instructions into a separate script (à la AppVeyor). The Doxygen packages I removed are required according to the deploy script, but personally I’m quite skeptical about that. The documentation certainly built fine without them for me locally.

One change that proved unexpectedly effective is the switch to bsdtar on macOS. I originally did this only because I disliked that the script used several different tools for handling archives. However, as it turns out, it resulted in noticeably smaller (~40%) files because the zip tool that was previously used apparently doesn’t support symlinks, so symlinked files had three identical copies included in the archive. Of course, since the zip tool doesn’t support symlinks, there is a chance that other tools don’t support them either, so it’d be great if one of you macOS guys could grab the build here and check if whatever Apple’s default ZIP extraction tool is can handle them correctly.

Copy link
Member

@MrStevns MrStevns left a comment

Choose a reason for hiding this comment

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

LGTM, also tested the provided mac build and extracts without problem.

I don't have a linux test env so I won't merge before someone else can confirm sound working on their linux env using AppImage.

@MrStevns MrStevns self-assigned this May 2, 2020
@Jose-Moreno
Copy link
Member

@J5lx I tested the provided Linux build on Ubuntu 19.04 and it worked properly. AFter importing a WAV audio file, the sound loaded immediately. Exporting the sound for an MP4 target also was exported properly even though the OS didn't have the codecs for proper playback.

Thanks a lot!

@MrStevns
Copy link
Member

MrStevns commented May 6, 2020

I'll take Jose's word that it works, merging.

Well done and thank you @J5lx 👍

@MrStevns MrStevns merged commit 5bded97 into pencil2d:master May 6, 2020
@J5lx J5lx deleted the appimage-sound branch May 6, 2020 16:13
Copy link
Member

@scribblemaniac scribblemaniac left a comment

Choose a reason for hiding this comment

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

Great job! This was a very important issue in my eyes, and I knew this was going to be a pain to implement. The refactoring is fine too. However, there are still some issue playing mp3s for me so this will require a bit more work.

.travis.yml Show resolved Hide resolved
app/src/main.cpp Show resolved Hide resolved
.travis.yml Show resolved Hide resolved
util/after-build.sh Show resolved Hide resolved
@Jose-Moreno Jose-Moreno added this to the 0.6.5 milestone May 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Broken sound playback in AppImages
4 participants