finamp icon indicating copy to clipboard operation
finamp copied to clipboard

Desktop Support

Open Chaphasilor opened this issue 9 months ago • 30 comments

I'm sick of merging the changes back into the desktop branch, so I'd like to merge them if possible.
Unless there are any critical issues? If some of you could give this PR some testing to make sure nothing broke, both on desktop and mobile, that would be great.

My plan is to merge this in for the next beta update, and then keep it around for one or two updates without an "official" desktop release to make sure the mobile version isn't negatively affected by any of the changes.
This time period can then also be used to a soft release, where all of the various stores and repositories can be prepared based on the redesign branch to make Finamp available everywhere.

CC @jmshrv @Komodo5197 @Maxr1998 @baarkerlounger @AverageHelper

It would also be nice to compile a list of how to build for each platform in the comments, as well as which files need to be updated when there is an update (e.g. bumping version number, etc.)

Chaphasilor avatar May 04 '24 13:05 Chaphasilor

I'll start with Windows:

Building

dart run msix:create

Updating

  1. Bump msix_version in msix_config within pubspec.yaml. https://github.com/jmshrv/finamp/blob/e2eb8c816e41fa3d8e541d731e23974b6a59d330/pubspec.yaml#L173
  2. Create a new build

Caveats:

  • It's not possible to update unless the version number was incremented. The last digit of the number can be incremented in case a re-release is needed

Stores/Repositories:

  • Microsoft Store: https://pub.dev/packages/msix#microsoft-store-icon-publishing-to-the-microsoft-store
  • winget: https://github.com/microsoft/winget-pkgs
    • winget also supports installing from the Microsoft Store, so this is optional

Chaphasilor avatar May 04 '24 13:05 Chaphasilor

For macOS, the process is described in https://docs.flutter.dev/deployment/macos#create-a-build-archive-with-xcode

For distribution, I'll put it on the Mac App Store and include a built .app for people who don't want to use the Mac App Store.

Currently setting up the macOS Xcode project, hopefully it's just copying stuff over from iOS

jmshrv avatar May 04 '24 18:05 jmshrv

Just going to post macOS nitpicks as I go here, will mostly be packaging stuff for me to fix

First off, the app icon needs fixing to fit in with macOS

macOS dock showing Finamp's icon being a perfect square instead of rounded

jmshrv avatar May 04 '24 19:05 jmshrv

Just pushed macOS metadata, first impressions are very good, seems to work as well as iOS :)

jmshrv avatar May 04 '24 19:05 jmshrv

Linux

The official deployment guide describes how to bundle a package for the snap store. As snaps aren't exactly popular in the general Linux community[1, 2], I'll instead describe the packaging process that I use for Arch Linux (which can likely be adapted for other solutions like deb or rpm).

Update: there's actually an official packaging guide for deb and rpm on Medium, which should be used preferably. Nonetheless, the guide below applies for Arch Linux and other niche distributions/package managers.

Building

First, you simply create a release build with the flutter tool:

flutter --suppress-analytics build linux --release

# Or, if you split dependency download and build,
# like the PKGBUILD (Arch buildscript) does:
flutter --suppress-analytics pub get
flutter --suppress-analytics build linux --release --no-pub

Packaging

To package everything, you can simply add the created files into your package archive. Things you'll want:

  • The release bundle (build/linux/x64/release/bundle). This should ultimately be copied to /opt/finamp during installation. Unfortunately, it's not possible to use the recommended system paths for executables (/usr/bin) and libs (/usr/lib or /lib), as the files from the bundle need to stay in the same folder relative to each other.
  • A desktop file to launch the app. There's currently one included in the finamp-git AUR repo (which should probably be moved to this repo eventually).
  • Desktop/application icons. These are included in this repo (assets/icon/linux/) and can be copied to the package to that they're installed to /usr/share/icons/hicolor/….
  • The LICENSE, depending on the package format and distribution. On Arch, it's installed to /usr/share/licenses/finamp.
  • Important: declare all required build & run dependencies (as determined by ldd and nampcap on Arch).

Updating

Currently, we only have a finamp-git AUR package on Arch, which grabs a version number automatically from the commit hash. No additional step is necessary.

Later on, I plan to also maintain a variant that builds the latest tagged release. As long as there's a git tag with a version number (used as the tag, for example), we're good. For new versions, the version number needs to manually be updated in the PKGBUILD by the maintainer (me).

Maxr1998 avatar May 04 '24 23:05 Maxr1998

For Linux it'd be nice to have deb/rpm and Flatpak, that should cover most cases (and it's pretty standard for more niche distros to adapt deb/rpm packages (see most -bin AUR packages))

It'd also be nice to have arm64 builds (personally, for Asahi Linux, and a few people have mentioned Linux phones), but it looks like there may be a few blockers there based on discussion in #40.

This could all likely be done in GitHub Actions, saves someone a package to build lol

Also thanks @Maxr1998 for publishing an AUR package, it's super cool to see Finamp there :)

jmshrv avatar May 05 '24 07:05 jmshrv

macOS seems to run well enough! I like the new two-column layout on large window sizes.

Seems to have trouble playing OPUS, but that's also an issue on iOS IIRC, so that's probs fine for now.

It does seem like users can shrink the window a bit too much tho :P Screenshot 2024-05-06 at 09 23 01

AverageHelper avatar May 06 '24 15:05 AverageHelper

It looks like the player UI doesn't respond to settings changes until relayout, which gets extremely obvious in two-column view. This is a beta tho, so it's probably fine that way for now.

AverageHelper avatar May 06 '24 15:05 AverageHelper

Oh god yeah I made that assumption when initially developing the player screen, that'll be fun to fix

jmshrv avatar May 06 '24 17:05 jmshrv

Shouldn't be hard. _PlayerScreenContent is a ConsumerWidget since the lyrics PR, so we just need to watch the Finanmp settings provider 😁
I'll add that in a bit, and also take a look at a minimum size for the desktop window.

Chaphasilor avatar May 06 '24 20:05 Chaphasilor

Okay, player screen now updates whenever settings are changed.
I've also added the window_manager package to set a minimum width and get some additional goodies, and while that works, it seems to cause issues if used on unsupported platforms (like Android or iOS). So we'll need to be careful to only use it on supported platforms. Some kind of wrapper/proxy class might be useful here.

Would be nice if you could test if it also works fine on other platforms

Chaphasilor avatar May 06 '24 22:05 Chaphasilor

@Chaphasilor do you have a release bundle for me to point at? If so I'll prepare a PR to add Finamp to Flathub. Currently I'm pointing here but obviously pointing at this repo would be preferable.

baarkerlounger avatar May 07 '24 09:05 baarkerlounger

@baarkerlounger right now we manually build all artifacts, but we could try setting up some GitHub Actions for it. I could try setting up a linux build environment for manual builds too.
I'm guessing there's no way for Flathub to do the building when creating the flatpak?

Chaphasilor avatar May 07 '24 12:05 Chaphasilor

@Chaphasilor not as far as I'm aware for Flutter apps because of their requirements for offline builds/vendored dependencies (this is a flathub limitation, not a flatpak one).

baarkerlounger avatar May 07 '24 12:05 baarkerlounger

Seems like the landscape layout doesn't work great on desktop with split screen disabled:
image image

Do you think it would be better to revert to the portrait layout for now, or should I try to come up with a more optimized design?

Chaphasilor avatar May 07 '24 15:05 Chaphasilor

It might make sense to re-tune the cutoff to displays >20% wider than they are tall, or something like that. I probably wouldn’t put too much thought into it beyond that, because I imagine most people would use split screen, and everyone else can just choose a less stupid window size.

Komodo5197 avatar May 07 '24 15:05 Komodo5197

@Komodo5197 how about only applying landscape mode if the height is less than X px? I'd like to only use landscape mode if the height is relatively low.

Chaphasilor avatar May 07 '24 16:05 Chaphasilor

I also now managed to use GitHub Actions to build artifacts for all platforms (Android, iOS, Windows, Linux, macOS). I believe for everything except Android, only a single architecture (probably x64, or in the case of macOS Apple-Silicon) is supported.

@baarkerlounger are the artifacts found here of any use to you? It seems like they are always either up- or downloaded as zip, but they should contain the files you need. If only the download forces the zip, there might also be a way around that, to download the files directly.

Chaphasilor avatar May 07 '24 16:05 Chaphasilor

@Chaphasilor looks like they'll be fine for an x86 package yes, just need them to be uploaded against a release or similar as build artifacts aren't accessible without being logged into Github and I think only retained temporarily?

baarkerlounger avatar May 07 '24 16:05 baarkerlounger

Yes, we could then use these release artifacts and attach them to a release, just like we currently do with APKs.
I guess in this case we'll silently attach them to an upcoming release so you can try if everything works and then make your PR to Flathub, yes?

Another unrelated thing, on Windows I can only have one instance of Finamp running, because there's a lock on the Hive settings file. Sometimes after closing the app this lock is also kept open, seemingly because some background process didn't properly close.
Is that similar on other platforms as well?

Chaphasilor avatar May 07 '24 16:05 Chaphasilor

@Chaphasilor_ that would be perfect, thanks! I've not seen the hive lock issue so far.

baarkerlounger avatar May 07 '24 16:05 baarkerlounger

@Chaphasilor Why? If the window is 2:1 or something, no matter how big it is, I feel like portrait will look stupid. Also, the portrait layout algorithm will probably start hiding control elements if the aspect ratio gets too wide, even with fairly large windows.

Komodo5197 avatar May 07 '24 16:05 Komodo5197

Yeah I did notice the controls being shown even though there was enough space. But regarding the layout, I'd argue that stretched portrait mode actually looks better than the landscape mode with the huge gaps between the sections on the right side.
But if there's no consensus, I'd say we wait with any major changes for now (aside from not hiding controls if there's enough space, maybe) until we receive some further user feedback.

Chaphasilor avatar May 08 '24 12:05 Chaphasilor

It seems like transcoding isn't working at all on desktop/windows? I'll have to check with a debug build, but right now nothing plays, and the player can't be paused (it always shows the pause button). I'm guessing it works fine on macOS because just_audio supports it out-of-the-box, and it's probably a media-kit issue.

I also noticed that on Windows at least (but probably also on Linux), playback of FLAC files with 16 bit resolution and 48 kHz sampling rate doesn't sound great. There's some weird distortion going on, similar to what happens when changing the speed. On Android the track sounds much better.

Chaphasilor avatar May 08 '24 13:05 Chaphasilor

I also noticed that on Windows at least (but probably also on Linux), playback of FLAC files with 16 bit resolution and 48 kHz sampling rate doesn't sound great. There's some weird distortion going on, similar to what happens when changing the speed. On Android the track sounds much better.

Sounds like it's being (improperly) resampled. What audio settings do you have on your computer at the moment? Also, on Linux, at least pipewire (a more modern audio stack that's slowly replacing PulseAudio on a few distros) has a default sample rate of 48 kHz already, so that should be fine. I also didn't notice any issues when testing it a while back with 24/96 files.

Maxr1998 avatar May 08 '24 13:05 Maxr1998

Yeah seems like it's a problem with my bluetooth headphones on Windows, they're locked to 44.1 kHz

@Komodo5197 I noticed when opening the app / unlocking the screen after the track has changed, a white cover is shown until the actual cover loaded. Seems like at least a blurhash should be shown instead, but the cover should actually already be prefetched...
I'll try to reproduce this better. Mind taking a look?

Chaphasilor avatar May 10 '24 15:05 Chaphasilor

None of the currentAlbumImage paths were using blurhashes. I've rectified that. Regarding the prefetching, I believe that doesn't fire in the background due to none of the widgets building, so if you advanced more than three tracks this is expected. If not, I don't know what's up.

Komodo5197 avatar May 10 '24 19:05 Komodo5197

Yeah, it could be that I ran into the three tracks limit, which would explain why I wasn't able to reproduce it before.
I've managed to reproduce it now, by waiting a few minutes and skipping ahead multiple tracks. Will try your changes in a bit!

We could probably move the watching of the metadata provider out of the now playing bar into some kind of widget-less service, unless that's a bad idea?

Chaphasilor avatar May 10 '24 21:05 Chaphasilor

I don't know if watching providers outside of widgets or other providers is actually possible. I'm also not sure if avoiding a slight load on reopen is worth continually fetching album covers in the background when they aren't being used.

Komodo5197 avatar May 10 '24 21:05 Komodo5197

I'll see how it looks with the blurhash, but the blank screen definitely looks a bit jarring and unfinished...

Chaphasilor avatar May 10 '24 21:05 Chaphasilor