-
Notifications
You must be signed in to change notification settings - Fork 351
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
RFC: Visualization Output Plugin #1449
base: master
Are you sure you want to change the base?
Conversation
Hey, I'd very much like you to continue that work! Was it me who suggested this to you on IRC? I once had another idea to make everything simpler: what if we stream the FFT data on the regular MPD connection? A client could periodically say "hey give me more FFT data since the last time I asked you", and the MPD could send a binary response. That makes the MPD protocol somewhat more complex, but on the other hand, it would "just work" everywhere; maybe even without having to configure an output plugin, and without an extra listener. People who control MPD over a SSH tunnel just need to tunnel that one port. What do you think? |
Man, you are fast! Thanks for the quick review. Not sure who suggested this on IRC. What's your handle? Rasi for sure, but there was someone else whose name escapes me at the moment. I like your general approach of KISS (Keep It Simple, Stupid) and adding complexity gradually and only as needed. As I understand your first proposal, what I'm proposing go into the "Client Hello" message would be moved into the output plugin configuration, and all clients just get those settings. That means no handshake; they just connect, get a header, then get streaming sound information. I have two concerns:
Your second suggestion, of just moving all of this into the MPD client protocol, is interesting. People who tunnel into their MPD server (such as myself, at times) would indeed find it easier. There's already a So... what, the client says |
libmpdclient supports binary responses since version 2.17, here's an example: https://github.com/MusicPlayerDaemon/libmpdclient/blob/master/src/example.c#L110 |
Yes, but the code to which you link reads the binary data & writes it to typedef struct sound_info_s {
char *pcm;
char *fft;
size_t num_samp;
// et cetera
} sound_info_t;
...
const struct sound_info_t*
mpd_get_sound_info(...); |
I'm not sure if such a special feature deserves special handling in libmpdclient; after all, libmpdclient has no JPEG decoder for parsing cover art. If you write a client, implement the response parser there for now (and libmpdclient is just the transport for the opaque binary response), and later, we can decide to move it into libmpdclient if we think it's better to have "one true implementation". |
Great new rfc! I could help implementing the client side of this protocol. |
Thanks, @jcorporation tho we're now talking about just making this part of the MPD client protocol; i.e. the client would say something like |
Streaming from an outplug plugin seems easier for me to handle on clientside than getting timeframes and parsing binary responses. My idea is to build a output visualizer like cava on top of it for my myMPD client. |
Well, that was one of the things I liked about the original proposal: the client could simply wait on a socket & receive updates when it was time to render a frame. Other than parsing the messages, it's almost no work for them. |
I have feedback from two contributors at this point, one that wants this feature implemented as part of the client protocol & the other as an output plugin with its own network protocol. |
I wish it wasn't that way, but I'm the only permanent MPD developer (I picked up the existing but almost-dead MPD project 14 years ago and failed to recruit more long-time developers), so everything ends up being decided by me.
I suggest doing a separate listener first; that's simpler to implement, and gets you to a working solution more quickly. After that, we can discuss how to implement tunneling through the MPD protocol, which is something that may be interesting for other features as well, so we may end up with some generic reusable tunneling code. |
OK.
That's a long time to maintain a project solo (esp. one so widely-used). As a long-time user, I'm happy to contribute what I can back to the project. |
Request for review only: do not merge!
This plugin started from a conversation on the #mpd IRC channel. I asked about the best way to implement a music visualizer as a remote MPD client. All the current MPD visualizers of which I'm aware use the fifo output plugin and hence must be run on the same host as the MPD daemon.
The response I got was a suggestion that I write an output plugin that would just stream the data needed to implement a remote visualizer. I've begun work on such a plugin, but before I spend too much time implementing it I would like to lay out my proposal & solicit feedback.
The codebase uses doxygen-style comments, so I'm presenting this RFC as a few doxygen pages in the first files I'd be adding to the project. The overview is in file
VisualizationOutputPlugin.hxx
in this PR, and the proposed protocol is detailed inVisualizationOutputPlugin.cxx
.