abarisain / dmix

A modern MPD Client for Android.
Apache License 2.0
590 stars 198 forks source link

Let's have a discussion about AlbumCache/CacheMPD. #653

Open avuton opened 9 years ago

avuton commented 9 years ago

I'm here to begin a discussion about the need for AlbumCache/CacheMPD to be radically different than it's current implementation. For the discussions sake, I'll refer to AlbumCache/CacheMPD as CacheMPD. The below is a draft of my thoughts.

Q) Why does it need to change??

A) Well, there are several answers here, in no particular order

Q) What should we do?

A) My thoughts: It's unnecessary. I accept that not everyone is going to believe me here, but the MPD protocol is rather nimble. If a user can't upload/download 10KB every now and again (smaller than the size of many web services in use), often over a local network, then the problem is not the protocol, nor the media server. I believe our problem is much more with how we implement the protocol (I'm working on this); possibly the speed of our devices. On my devices, on my network, the load is done in ms, when it's not it's indicative of a much more refined implementation bug.

Q) I understand your point of view, but I don't care, I want it anyhow!!1

A) I don't live in a vacuum: I realize that not everyone thinks the same as I do about this. That's OK. The problem with CacheMPD is in the implementation details, and here's my thoughts on that:

Q) Why all this talk, is something coming?

A) Depending on the outcome of this discussion, after the long awaited 1.07 final release, my intention is, to push the database refactor. This will break CacheMPD. Soon thereafter, MPD.java will become a singleton. If someone wants to step up and refactor around my refactoring that would be fine, but I would hope it would be a little more sustainable.

abarisain commented 9 years ago

Thanks for the writeup !

kingosticks commented 9 years ago

For what it's worth I completely agree. Clients are not supposed to cache the database and the mpd author has said this many times. On 30 Nov 2014 22:00, "Arnaud Barisain Monrose" notifications@github.com wrote:

Thanks for the writeup

— Reply to this email directly or view it on GitHub https://github.com/abarisain/dmix/issues/653#issuecomment-65002300.

hurzl commented 9 years ago

A) My thoughts: It's unnecessary

You never understood it and want to have it away. Go ahead.

kittybuttontin commented 9 years ago

While I understand the arguments against it, the reason I currently use the local cache is because without it some multi-album Artist screens can take up to 15 seconds to load (even wth decent hardware).

avuton commented 9 years ago

@hurzl :( You're right, but I don't need to understand it. It actually really pains me that this has become necessary. I'm hopeful that it makes a return, but I'm not sure how it could really be done in a light maintenance way. Please realize it's not personal at all, strictly technical.

avuton commented 9 years ago

@kittybuttontin I understand what you're saying as well. Have no doubt that I see all lag myself and will be doing everything in my ability to reduce any lag you see, but, the current cache implementation isn't really sustainable. I have a Nexus 6 now (which I see no lag at any time) BUT most of my testing still happens on my galaxy nexus, and my media server is on an old intel atom; I have MANY ideas on how to cut any seen lag, but it's (unfortunately) a long road to get there.

hurzl commented 9 years ago

I don't need to understand it

You should understand before saying it's unnecessary.

carnager commented 9 years ago

listallinfo is not be relevant for mpdroid itself.

But still I want to highlight why its used so often. there is a reason why people use listallinfo. That reason is mpds protocol itself, which is crippled in certain areas.

Try to get a formatted list of all albums in database. Not possible, The list command only returns exactly one tag (e.g. artist or album), nothing else

I requested a listinfo command, where you would be able to tell mpd what kind of tags it should transfer to the client, when working on a list, but there was no interest in it.

I know several client developers (including myself) who would love this feature and the only way to work around it is the listallinfo command.

UPDATE ok, it is possible to archive roughly the same doing a recursive search. its slower (by a factor of 3 here), but it works