rasmuslos / AmpFin

Native Jellyfin music player for iOS & iPadOS
Other
107 stars 10 forks source link

Various fixes for current main #34

Closed gnattu closed 3 months ago

gnattu commented 3 months ago

This PR addressed various issues during my own testing and daily usage to make it more production ready.

Notably:

gnattu commented 3 months ago

Addressed most issue, and added further fixed an issue that duplicated entries may show up in the albums/tracks page in offline provider because the list is not reseted on appear. Also added debounce for tracks searching as a quick typing may cause the last character change not start a new search.

I've noticed that we are building a very long list with placeholders and it will make the page less responsive even the supposed to be lazy-loaded entries are not loaded when the list is huge enough. Should we throttle the placeholder count so that the page performance is good unless the user really want to scroll to a place that enforces a list this long?

gnattu commented 3 months ago

I found that it is not a proper fix by resetting album/tracks view on appearance because this makes navigation back very broken as the scrolled position will be lost. Need to find better way to handle this.

Edit: fixed by only resetting when the item list is empty

rasmuslos commented 3 months ago

Addressed most issue, and added further fixed an issue that duplicated entries may show up in the albums/tracks page in offline provider because the list is not reseted on appear. Also added debounce for tracks searching as a quick typing may cause the last character change not start a new search.

I've noticed that we are building a very long list with placeholders and it will make the page less responsive even the supposed to be lazy-loaded entries are not loaded when the list is huge enough. Should we throttle the placeholder count so that the page performance is good unless the user really want to scroll to a place that enforces a list this long?

The main reason I added the placeholders was to make sure the scroll indicator gives a somewhat accurate position. If the performance is still fine the correct amount of placeholders should exist.

gnattu commented 3 months ago

Addressed most issue, and added further fixed an issue that duplicated entries may show up in the albums/tracks page in offline provider because the list is not reseted on appear. Also added debounce for tracks searching as a quick typing may cause the last character change not start a new search. I've noticed that we are building a very long list with placeholders and it will make the page less responsive even the supposed to be lazy-loaded entries are not loaded when the list is huge enough. Should we throttle the placeholder count so that the page performance is good unless the user really want to scroll to a place that enforces a list this long?

The main reason I added the placeholders was to make sure the scroll indicator gives a somewhat accurate position. If the performance is still fine the correct amount of placeholders should exist.

From my test, it is a problem for a huge library with more than 100K tracks. If you have that much tracks, even the placeholders along will make huge interaction delays, especially for searching and sorting. The UI will stop respond for like one second or two if you entered a character in the search bar or changed the sort order.

This upper limit can be very high, because such performance penalty only happens for a very huge library.

rasmuslos commented 3 months ago

Addressed most issue, and added further fixed an issue that duplicated entries may show up in the albums/tracks page in offline provider because the list is not reseted on appear. Also added debounce for tracks searching as a quick typing may cause the last character change not start a new search. I've noticed that we are building a very long list with placeholders and it will make the page less responsive even the supposed to be lazy-loaded entries are not loaded when the list is huge enough. Should we throttle the placeholder count so that the page performance is good unless the user really want to scroll to a place that enforces a list this long?

The main reason I added the placeholders was to make sure the scroll indicator gives a somewhat accurate position. If the performance is still fine the correct amount of placeholders should exist.

From my test, it is a problem for a huge library with more than 100K tracks. If you have that much tracks, even the placeholders along will make huge interaction delays, especially for searching and sorting. The UI will stop respond for like one second or two if you entered a character in the search bar or changed the sort order.

This upper limit can be very high, because such performance penalty only happens for a very huge library.

Ok, seem reasonable 👍

rasmuslos commented 3 months ago

Great! Thanks a lot! I will merge this tomorrow.

The only thing missing for the 2.4.0 release is the acknowledgement section, I should be able to add it tomorrow, so the release should be soon.

gnattu commented 3 months ago

The only thing missing for the 2.4.0 release is the acknowledgement section, I should be able to add it tomorrow, so the release should be soon.

Can we have an option to opt-out the spotlight integration? The current implementation will overload the server when the library is very huge and it will never success due to timeout. So the user can disable this for now until we can have a a better solution for this. I asked fellow Team members and they said ~15K items per minute is a safe fetch rate so we will want to add rate limit in our implementation, to batch like 100 items a time, wait half a second, then index the next 100.

One problem for this is that this indexing might take very long and might be interrupted, so our implementation also need to be modified to accommodate that so that it can be resumed from interruption safely, but if you want to draft a release very soon, we can just add an option as a workaround instead at the moment.


And the below is my personal opinion and some inputs from other team members of Jellyfin and I I'd like you to consider before the next release, but everything is totally up to you.

Jellyfin currently lacks a robust music player solution for Apple devices (iOS, iPadOS, macOS). Our official clients, the Expo client struggles to reliably playback music in the background due to its heavy rely on the webview, and the Swiftfin does not support Music Playback at all at the moment. Also the other existing third-party options prioritize non-Apple platforms and works poorly on iPads.

That's why I put so much effort on this project as I see a good potential of AmpFin being a native client using first-classs Apple frameworks. I really want this to be the recommended music playback solution on Apple platforms for Jellyfin and list it as a recommended client on our website.

However, when I discussed this with other team members, they are concerned about this has a price tag on the App Store, and they told me that the infuse is the only exception, because we have no working tvOS client at the moment, and we are planing to remove infuse from the recommended list once we have fixed Swfitfin on Apple TV. I'm personally fine with the current $3.99 pricing, but some core team members told me that they don't want to make another exception like infuse, and the client need to be free and open-source to be listed.

The "open-source" part is negotiable, as the "source-available" model using a Common Clause license seems agreeable to most team members, but only the pricing, makes a lot of team members rejecting the request to list this as recommended, and some of them even argued that paid app should not even being listed on our website at all.

If you do accept the request to make AmpFin free on the App Store, then I will try my best to persuade my team members to list this as a recommended iOS client, but I cannot have any guarantee for you at the moment now as this need the approval of the project leaders and most of our team members need to agree on this, but I think this can be listed at least in the "all clients" section if this is free.

Ultimately, the pricing model is entirely your decision and this is your project. We respect your choice and appreciate your hard work on Ampfin. Regardless of the pricing structure, I'm personally committed to providing ongoing support and advocating for Ampfin within the Jellyfin community, because I really like this project and it does what I think an Apple native music player should do.

Also, regarding the Common Clause, am I allowed to send the compiled binaries to selected persons like my team members for testing purposes? I think this isn't considered "selling," right? Some of my team members will have unusual libraries that may trigger edge cases, which could be difficult to replicate on my end. Therefore, sharing the binaries may be necessary to triage the bugs. If it's not allowed, that's also fine, as I can simply instruct them to open issues here and work directly with you.

rasmuslos commented 3 months ago

We can just use the getTracks endpoint with a limit of 1 to get the total amount of tracks and disable the spotlight integration if it exceeds a certain number.

This could also be improved by relying on the koi sync plugin, I did not look into how it works, but I imagine it provides it allows clients to only fetch changes after a certain point, so only changes would have to be fetched from the server after initial sync.

The reason why I made AmpFin a paid app is that as a student I don't have much disposable income. I would not call myself poor by any stretch of the word but I am still operating on a shoestring budget. The app is and will be free for anyone willing to side-load. But I could not justify paying 100€ per year to Apple, to get the app into the App Store, essentially just burning the money.

I don't like monetized open-source software myself, but I think I struck a good balance with the current model because it does not rely on in-app-purchases or subscriptions and the app itself does not have any paid components, so you are not forced to buy it or waste your time removing stupid restrictions put in place to get you to buy it. The lack of CarPlay / Siri support for the side-loaded version is entirely caused by Apple and it can be reenabled if you have a paid developer account.

With all of this being said, the app did not perform particularly well in the AppStore, understandably, considering the also very much usable free alternatives. I had considered making the app free before, but I had two main gripes:

The whopping 25 people who bought the app will most likely and understandably be pretty mad that they paid for the app and a few weeks later it became free. I could use the money the app made to get a designer to make an additional app icon for them, but I don't particularly like that solution. Also, ShelfPlayer will remain a paid app, mostly because I am still throwing 100€ at Apple and some giving additional money to my postal office for a PO Box because of new EU regulations. There isn't really a logical point to be made here, but it would feel kind of off if one app would be paid and one is free.

But I think AmpFin should end up on the all clients lists with the release of 10.9, I did make a PR and it got approved: https://github.com/jellyfin/jellyfin.org/pull/900

If I understand the legal ramblings correctly distributing the binary in any form, even free, is prohibited, but I will grant anyone the right to distribute the app for development purposes / to friends and family without restriction. Adding this to the Readme should suffice.

I added the commons clause because I wanted to sell the app, if I make it free we can remove it, it should be fairly simple as we are the only contributors at the moment.

I have to think about this, if you have any input feel free to share it.

gnattu commented 3 months ago

But I could not justify paying 100€ per year to Apple, to get the app into the App Store, essentially just burning the money.

This is not what we want to see either, and unfortunately we cannot help much as you have another project that is on the App Store that is not related to Jellyfin, so even we can help you to release this, we cannot reduce your cost as you still need to pay for another one.

it does not rely on in-app-purchases or subscriptions and the app itself does not have any paid components

Actually we will reject the request to list the client if it has any, and Infuse is the only exception.

The whopping 25 people who bought the app will most likely and understandably be pretty mad that they paid for the app and a few weeks later it became free. I could use the money the app made to get a designer to make an additional app icon for them, but I don't particularly like that solution.

Let me consider a compensation plan, but I don't have a definitive one in mind yet. The lowest amount for an App Store gift card purchasable on their website is $10, so unfortunately, we cannot use this option. Additionally, it seems like Apple does not allow developers to issue refunds for the app themselves. If you want to refund the customers, you may need to write a formal letter and direct them to contact Apple directly for a refund.

I can personally reimburse you $100 to cover the refunds, or I can provide compensation in other forms such as a gift card if you prefer not to share personal financial information. Please let me know if you think this is necessary.

Furthermore, we can offer non-monetary compensation, such as listing their names in this project as "supporters" or "sponsors," similar to many other open-source projects. For instance, Jellyfin has an Open Collective page for backers: https://opencollective.com/jellyfin, and you can list these early adopters as backers. Additionally, you can list the Apple Developer program as an expense. This way, others will understand why you need financial support to keep this project running.

We can also provide them with TestFlight seats to access new features earlier. These are my suggestions, but please feel free to propose alternatives if you do not like these options.

There isn't really a logical point to be made here, but it would feel kind of off if one app would be paid and one is free.

I agree it looks weird, but it won't be that bad because you have to pay for your developer account, and I think it is an understandable choice for everyone knows that you need a paid account to list this on the App Store.

But I think AmpFin should end up on the all clients lists with the release of 10.9, I did make a PR and it got approved

It needs further approval from one of the team leaders so I'm not sure if we can have this merged with current paid status. I'm personally fine with that, but at least one of the team leaders needs to be fine with it as well.

rasmuslos commented 3 months ago

I can personally reimburse you $100 to cover the refunds

That's really nice of you, but not necessary. You did not financially benefit from this, so you should not have to cover the costs. I haven't even received any money from apple yet, so it's not like this cuts a hole in my budget.

I don't think there is a good way to communicate with the people that have bought the app before. I don't have any names or email addresses and I am quite sure that apple will reject anything that notifies them about the change. I would create a pinned issue with instructions on how to get a refund or alternatively an option to get included in the acknowledgments.

I am not planning on setting up anything like open collective, so this isn't really an option. And while a TestFlight would be nice I don't think it's a good idea, mainly because most it would only contain unstable alpha builds, and using them is more of a punishment than anything good. If it would contain finished builds this would essentially just mean that features are paywalled for a few weeks before they are released, some apps like e.g. AltStore just have highly requested features in the "beta" for years, essentially requiring you to pay, and I don't want to do that.

To reduce the impact of this I have temporarily made the app unavailable for purchase, but I did not decide yet. But I do have a question: What does becoming a recommended client actually mean?

Also this may be nitpicking, but if monetization is a deciding factor for the clients list I think it should be included in the docs

gnattu commented 3 months ago

But I do have a question: What does becoming a recommended client actually mean?

It means this client will be listed on the recommended page, a page typically reserved for our official clients. Being listed on this page signifies that the team agrees that the client is of high quality and can be considered a "default option" on the platform. This is uncommon for third-party clients unless there is no alternative available on the selected platform. However, in the case of AmpFin, I believe it fits a specific use case (music playing) that is not currently addressed by our official clients. Therefore, in my opinion, it could serve as the default option for Jellyfin's iOS users who want to listen to music due to limitations in our official app.

Also this may be nitpicking, but if monetization is a deciding factor for the clients list I think it should be included in the docs

I agree this should be documented clearer. In the Jellyfin Community Standards, we have listed it in the mission statement:

Jellyfin aims to be the best free and open-source media streaming platform possible, without any proprietary/locked features or unreasonable centralization.

This implicitly means that clients aligned with the Jellyfin Community Standards should be free and open-source. The only exception is when there are no free and open-source alternatives available, as is the case with our current situation on tvOS and infuse is an accepted exception.

I've already asked team members if we should edit this guideline, but there has been no response yet. It's possible that nobody has an idea about whether we should rewrite it to be more clear.

rasmuslos commented 3 months ago

After giving this a lot of thought, I just can't bring myself to make the app free right now. There are other good apps available for iOS, they may lack some of the bells and whistles like Siri integration, but work pretty well regardless.

Also, I have some really important exams during the next month and I really should start learning for them, and dealing with potentially many more users and feature requests is something I just don't have the time for at the moment.

I am sorry that this is probably not what you hoped for...

gnattu commented 3 months ago

After giving this a lot of thought, I just can't bring myself to make the app free right now. There are other good apps available for iOS, they may lack some of the bells and whistles like Siri integration, but work pretty well regardless.

Also, I have some really important exams during the next month and I really should start learning for them, and dealing with potentially many more users and feature requests is something I just don't have the time for at the moment.

I am sorry that this is probably not what you hoped for...

This is fine and I totally understand your reason. If you changed your mind in the future feel free to contact me and I will try my best to help you for the transition.