rasmuslos / AmpFin

Native Jellyfin music player for iOS & iPadOS
Other
163 stars 13 forks source link

Add option to transcode (QoL improvement) #55

Closed Daria-E closed 3 months ago

Daria-E commented 4 months ago

First off - this is by far the most polished Jellyfin music app on iOS that I’ve come across (and I’ve been looking).

The one obvious missing feature, to me - transcoding. I keep my music library in FLAC for preservation, but the massive files chew through mobile data.

Finamp does support this, and since it’s MPL - no worries about learning from their implementation.

rasmuslos commented 3 months ago

It should be fairly straightforward because in the Jellyfin server should be able to handle transcoding for us. I think maxStreamingBitrate is the right one, it should default to direct play but transcode if the media-file exceeds the provided bitrate. Is this correct @gnattu?

gnattu commented 3 months ago

Our official clients construct different device profiles for transcoding and direct streaming.

We can do this better than just set maxStreamingBitrate. The server will perform some check to find out if the client is in the same LAN and that detection sometimes is incorrect because the client might behind some sort of VPN and that will confuse our server. To make our user's life easier, we can provide two sets of profiles: one on cheap network(like wifi) and one on expensive network (like cellular), and the user can choose highest audio quality for each (up to lossless or up to 256k aac). For a bonus point, we can even specify a lossless transcoding target like flac or alac for use in the "cheap" network. This will transcode the file that cannot be played directly (like opus or vorbis) lossless at the cost of increased file size, but that should not be a big concern for cheap network.

rasmuslos commented 3 months ago

I have encountered profiles once but couldn't find any documentation... Are the profiles just a set of query / post parameters or is there more going on?

gnattu commented 3 months ago

Are the profiles just a set of query / post parameters or is there more going on?

For music playback that's all you need to worry about. You just change the audio format and the max streaming bitrate to force a transcode.

For Video due to the complexity of the playback mode we are using different profiles for direct play(direct file), remux(change container, copy streams), direct streaming(copy video and transcode audio) and full transcode, and it actually need to send multiple profiles to the server to describe the codec support status in different containers. I do have a draft to add remux for audio: https://github.com/jellyfin/jellyfin/pull/11399, but that does not use multiple profiles in the request either in my current proposed method.

rasmuslos commented 3 months ago

@Daria-E can you send me an E-Mail to git@rfk.io from the E-Mail address you use for iTunes purchases? Then I could add you to a TestFlight build and you could the this

Daria-E commented 3 months ago

After a bit of testing - it seems there are a few issues with the audio quality toggles:

gnattu commented 3 months ago

It seems like the current usage of NWPathMonitor is wrong, which makes the constrained networking detection is always false. I'm investigating current server behavior because I noticed in certain circumstances the audio is not always transcoded even if it exceeds current set max streaming bitrate. A workaround for that is to remove all codecs from direct play profile to force transcoding.

Also, I suggest to remove the confusing "High quality" settings. These will be very confusing as the server won't transcode audios to this bitrate. For AAC, the highest bitrate the server will use is 256kbps. For lossless codecs, the bitrate does not reflect quality but how good the audio is compressed, and there is no way to enforce a quality for lossless anyway. In both cases, the quality+bitrate description is misleading. I suggest reduce it to only three tiers: lossless, 256kbps and 128kbps.

Daria-E commented 3 months ago

@gnattu I agree about the reduction in tiers.

Additionally, something I forgot to mention. Even when transcoding, according to the in-app indicators FLAC is still used (not AAC).

gnattu commented 3 months ago

Additionally, something I forgot to mention. Even when transcoding, according to the in-app indicators FLAC is still used (not AAC).

That indicator is wrong. It is generated by the client itself and does not reflect the actual profile the server is using.

gnattu commented 3 months ago

Alright, I found out a current limitation on Jellyfin Server: the UniversalAudioEndpoint will cache the transcoding result for 24hours with no way for in-server removal currently, which means the server will serve the original cache even after the client changed the bitrate limit setting, which makes our current intended usage partially broken. For example, if you want 256k on Wifi and 128k on Cellular, the server will serve you the 256k cache if you have listened to that song on Wifi in the previous 24 hours.

It is too late to introduce an api change to the server now, so the proper fix can only be landed in 10.10. In the meantime, I will submit a PR for proper network path detection and simplify the bitrate settings.

rasmuslos commented 3 months ago

With #56 everything should work now, @Daria-E I uploaded a new build, could you verify that everything works as expected now?

Daria-E commented 3 months ago

I can confirm all but one of the issues have been fixed - great job, @gnattu and @rasmuslos!

In-app indicator for downloaded music still shows “0 - AAC”. I’ve deleted the older downloads, redownloaded, and tried different albums.

gnattu commented 3 months ago

The downloaded audio's bitrate is divided by one more 1000:

image

Which rounds to zero because it is too small.

rasmuslos commented 3 months ago

When the media info was not retrieved from the server the bitrate was divided by thousand twice, should be fixed now 👍