search

mopidy / mopidy-mpd

Mopidy extension for controlling playback from MPD clients
https://mopidy.com/ext/mpd/
Apache License 2.0
100 stars 21 forks source link

having issues with modern mpd clients #68

Open r3k2 opened 2 months ago

r3k2 commented 2 months ago

I have been using old ncmpcpp, but lately I wanted to switch to something that will work well with mopidy and MPD that has a modern TUI in rust or go and found one but for some reason is not compatible with mopidy-mpd? ?

Here is the issue for more context https://github.com/mierak/rmpc/issues/50

Is there a reason why mopidy-mpd has this problem?

thanks

adamcik commented 2 months ago

Yeah, mopidy-mpd is very far behind with respect to mpd compatibility. There are probably some open issues for part of the problem if memory serves.

This plugin needs someone to step up and implement the missing protocol features and ideally to own maintaining the plugin. But just fixing the issues at hand of course also helps 🙂

girst commented 2 months ago

closely related to https://github.com/mopidy/mopidy-mpd/issues/47. last time i looked, we only missed album art and revamped search (the latter of which the official mpd clients didn't even use at the time).

given that mopidy-mpd doesn't work at all for a substantial portion of users, maybe it's time to pretend to support a higher version, even if we're missing some stuff?

mierak commented 2 months ago

time i looked, we only missed album art and revamped search (the latter of which the official mpd clients didn't even use at the time).

from my (really) quick look at mopidy-mpd, the getvol command, for example, is missing as well, that's a 0.23.0 protocol version.

The albumart/readpicture commands are starting to get some adoption nowadays. From what I can see it was being worked on here https://github.com/mopidy/mopidy-mpd/pull/57 but was abandoned.

And there is also stuff like status command returning -1 for volume in some cases, which is a deprecated behaviour in mpd and most likely the reason for crash in the issue linked by OP as my client parses it into an unsigned value.

given that mopidy-mpd doesn't work at all for a substantial portion of users, maybe it's time to pretend to support a higher version, even if we're missing some stuff?

I do not think that reporting an incorrect version is the way to go as that would make clients unable to properly decide whether a given feature is supported by the protocol ahead of time. This would force these clients to instead rely on error responses for fallback behavior.

kingosticks commented 2 months ago

There is an issue somewhere that's trying to track what's missing, if you find something missing from that list then please add it. There was an issue with the implementation for albumart if I recall but maybe it can be rescued, I don't remember exactly. Someone (else) needs to take the lead on this.

leso-kn commented 3 weeks ago

Fyi, #57 is working fine and has been rebased onto master several times :) I do not know why it has not been merged yet, but I've been using it since two years with no issues

kingosticks commented 3 weeks ago

Feel free to address the numerous review comments. That's why it's not merged. just because it "works for me" doesn't mean it's code we can merge because it's us left maintaining it.