myMPD icon indicating copy to clipboard operation
myMPD copied to clipboard

Full partition support

Open jcorporation opened this issue 3 years ago • 25 comments

myMPD should control all MPD partitions with different jukebox, player, etc. settings.

  • [ ] Maintain one mpd connection per partition
  • [ ] Rework mympd_state struct
    • [ ] Move global mpd states to mpd_shared_state
    • [ ] Move partition specific states to mpd_state
    • [ ] Maintain one mpd_state struct per partition
  • [ ] Add partition parameter to api calls
  • [ ] Redesign switch partition to only switch webinterface
  • [ ] Save current partition in browser localStorage
  • [ ] Save partition specific settings
  • [ ] Add partition option to scripts
  • [ ] Add partition option to timers
  • [ ] Add partition option to trigger
  • [ ] Add partition to last_played list

jcorporation avatar Mar 07 '21 08:03 jcorporation

Currently, changing partition on one mympd client switches partitions for all other connected mympd clients. It would be great if the partition was stored in a browser-session cookie to allow multiple concurrent users on mympd, each managing their own partition.

tsunulukai avatar Aug 06 '21 19:08 tsunulukai

Exactly this was possible with this enhancement. In the actual implementation we have only one client instance connected to mpd and this instance can only control one partition at a time.

jcorporation avatar Aug 06 '21 20:08 jcorporation

Great ! Lokking forward to it :)

tsunulukai avatar Aug 07 '21 06:08 tsunulukai

I played around with mpd partitions. It seems mpd does not retain partition’s across restarts. I think more work for partition support is only worthwhile if mpd partitions are more major.

jcorporation avatar Apr 24 '22 11:04 jcorporation

Indeed, mpd looses state for all partitions except the default one across restarts, which is quite annoying ! For those who run their mpd daemon on a system that runs 24/7 that's mitigated by the fact one barely ever needs to restart the mpd daemon, but for other use cases when you'd only boot your device when you want to play music, that's a huge bummer !

On could argue it's a chicken and egg issue: partition support is not widespread and very few clients implement it, which results in limited feedback to mpd concerning that feature.

Implementing good partition support in mympd and reporting the issues back (such as the lack of state persistance across mpd restarts) to mpd's maintainers could trigger a virtuous circle for that feature's enhancement.

I'm definitely looking forward for a good partition support, but I'm not sure many do have a setup that can benefit from it.

In my case, I run mpd on a NAS that runs 24/7 and streams the audio using pulseaudio module-rtp-send & module-rtp-receive modules to low powered devices that are connected to speakers in different rooms. Partition support would greatly enhance the flexibility of the setup.

tsunulukai avatar Apr 24 '22 11:04 tsunulukai

On could argue it's a chicken and egg issue

Right, I opened a discussion in the mpd repository: https://github.com/MusicPlayerDaemon/MPD/discussions/1520

jcorporation avatar Apr 24 '22 11:04 jcorporation

I played around with partitions and found a deal breaker: https://github.com/MusicPlayerDaemon/MPD/discussions/1521

jcorporation avatar Apr 30 '22 11:04 jcorporation

I too am trying to use partitions to allow multiple users to play different audio in different parts of the house at the same time. This seems to work properly on the MPD side. It also seems to work on the MyMPD side, but MyMPD makes this very cumbersome to do: two users can't work in different partitions at the same time. It seems that this issue addresses exactly that.

I agree with the annoyance of partitions not being kept across mpd restarts (and the fact that mpc doesn't support them so I can't even script their re-creation), but it doesn't stop them from being usable once they're created. They're very easy to create, I only need a couple, and my system stays up 24x7 so I don't have to do it often. It's ugly, but it can stay mostly invisible to my users.

But the fact that MyMPD forcibly moves unrelated browser sessions to use the same partition makes this feature just about unusable. When two users are trying listen to something at the same time (the whole reason I need the partitions to begin with!), they simply fight with each other across the same interface. And seeing as you can't use more than one MyMPD per MPD server, I don't see a way of making this work smoothly.

I understand that there may be issues still with the protocol (https://github.com/MusicPlayerDaemon/MPD/discussions/1521) but even with that, MyMPD is working right now, even with those limitations. We just have to keep switching back and forth (and back and forth and...). I think @tsunulukai and I just wish that MyMPD would just maintain a copy of the state for each partition (and remember which browser goes with which partition). Both sets of states could simply process the events just like it would have if that were the only state: it doesn't seem that would be any different than what happens right now when these vague messages are being applied to a single "current partition" state, and that seems to work OK right now.

And if the switching absolutely is required (if, say, the communication with MyMPD truly needs to assume that everything is always from a silly idea of "the current partition"), it would be a lot nicer for the poor users if that switching were done within MyMPD, rather than forcing the users to repeatedly make that swtich. Think of it like operating system task-switching: sure, it's a bunch of extra work for the OS to maintain a bunch of state information for multiple processes and constantly switch tasks back and forth every few milliseconds, but it makes the computer worlds more useful to create the impression that there are multiple tasks simultanteously running seamlessly (and invisibly). We want the impression that there are multiple MyMPD's simultaneously running seamlessly (and invisibly) that we can switch between.

Besides, MPD's protocol isn't exactly above constantly polling the server. So who cares if it has to round-robin poll for each partition, switching from one to another between polls... :) Yeah, it's ugly, but at least it's hidden from the user..... :)

Anyway, thank you very much for your work on MyMPD. I appreciate your time and attention.

MiM-TMassey avatar Jun 10 '22 19:06 MiM-TMassey

Partition UI suggestions:

  • In the partition list, clicking on the name of the partition should select the partition and close the window.
  • Given that, there would no longer need to be a checkmark at the far-right side, very small and dangerously close to the delete trash can for big, fat fingers to click,

That is exactly how the "Move an output to this partition" interface works, which seems logical as well.

Also, in the "Move an output to this partition", it might be nice to remove the outputs that are already assigned to that partition, or otherwise show which ones are already there and which ones are not (maybe using the highlight color?).

Update: it seems that when an output is enabled in a partition, it is removed from that list, but not when it's been added and not actually enabled. I'm not sure that this makes sense: if the output has been added to the partition, it doesn't seem like it should be in the "Move" list, even if it isn't yet enabled.

I would suggest that the "Move" list allow a user to select more than one output, highlighting it, then click a button such as "Save" at the bottom, or "Cancel" to not save. The list would show only outputs that have not been added to that partition, whether they have been actually enabled or not.

Thank you!

MiM-TMassey avatar Jun 10 '22 20:06 MiM-TMassey

Supporting multiple partition at the same time implies a big redesign of the myMPD backend. I hopped that a response from max points me to the right direction how to implement it properly.

It would be great if you also join the discussion at the MPD repository. If max notices that this feature is interesting for more people than he hopefully enhances the protocol.

I do not want to implement large workarounds that are obsolet after short time.

In the meanwhile you can run one myMPD instance for each partition. With scripting myMPD can switch to the right partition after connection or can create the partitions that you want.

The output management could be enhanced independently.

jcorporation avatar Jun 10 '22 22:06 jcorporation

Yeah, I can understand. Now MyMPD has to be a multi-state server itself instead of a single state client that simply gets a single stream of asynchronous input from multiple places.

It seems the intention was that multiple traditional MPD clients treat partitions as slightly extended normal individual servers, and you use a different client for each. That would work fine for me (though not ideal), if I could simply run multiple MyMPD instances on the same box and against the same local MPD server.

I would have set up multiple MyMPD’s already except for the doc saying not to: https://jcorporation.github.io/myMPD/additional-topics/multiple-myMPD-instances-on-same-host I just want to get confirmation: I can run two copies of MyMPD, using different ports and working directories (and cache directories?) against the same MPD server on the same box, using the same local socket and music file system paths?

Either way, I will add a comment over on the MPD side as well. Unfortunately, I won’t have much more than a “Me Too!” but if that helps, I’m all for it. For those of us with whole house audio and families, being able to seamlessly handle multiple playbacks dynamically across a variable set of outputs is pretty awesome. As rough as the interface is right now, my non-tech wife is still excited about it.

MiM-TMassey avatar Jun 11 '22 12:06 MiM-TMassey

I can run two copies of MyMPD, using different ports and working directories (and cache directories?) against the same MPD server on the same box, using the same local socket and music file system paths?

This should work. Some things like scrobble triggers, playcount are not working correctly because of the global idle events. That’s also the reason why the multi partition feature could not be implemented correctly without mpd protocol enhancements.

jcorporation avatar Jun 11 '22 14:06 jcorporation

In the partition list, clicking on the name of the partition should select the partition and close the window.

This is now implemented in the v9.4.0 branch

jcorporation avatar Jun 12 '22 20:06 jcorporation

Also, in the "Move an output to this partition", it might be nice to remove the outputs that are already assigned to that partition, or otherwise show which ones are already there and which ones are not (maybe using the highlight color?).

This was a bug. I thought the output ids from MPD are valid across partitions - this was not the case. Additionally in the default partition there are all outputs listed. Outputs that are moved to an other partition are assigned a "dummy" plugin.

This is now also fixed in the v9.4.0 branch.

jcorporation avatar Jun 12 '22 20:06 jcorporation

I would suggest that the "Move" list allow a user to select more than one output, highlighting it, then click a button such as "Save" at the bottom, or "Cancel" to not save.

Good idea, is now also implemented in the v9.4.0 branch.

jcorporation avatar Jun 12 '22 21:06 jcorporation

Wow, that is all awesome!

I’ve been playing with partitions, primarily selecting outputs, for the last few days. There are clearly some bugs. Hopefully, these are addressed above. I look forward to putting them through their paces.

Do you have any idea on timeframe? If I have to set up a compilation environment I will, but I’d rather just apply the RPM… :-)

MiM-TMassey avatar Jun 13 '22 00:06 MiM-TMassey

No, there is no timeframe for the v9.4.0 release. Next month probably.

jcorporation avatar Jun 13 '22 07:06 jcorporation

And if the switching absolutely is required (if, say, the communication with MyMPD truly needs to assume that everything is always from a silly idea of "the current partition")

This is the case. I think myMPD must use one connection per partition to work correctly, because some functions could not use a command list to prevent switching in the middle of processing an api request.

The biggest problem is the global nature of some idle events. It is not possible to detect from which partition an idle event is triggered. myMPD functions like jukebox, triggers, player states and playcounters must be run partition specific.

jcorporation avatar Jun 18 '22 08:06 jcorporation

Idle events and partitions are much more clearer now for me. I Think I can start implementing this feature for the next major release myMPD v10.0.0.

jcorporation avatar Jul 13 '22 13:07 jcorporation

Looking forward to trying all of this out. Will there be a 9.4, or are you going straight to 10? Like I said, I’ve avoided compiling my own MyMPD, but the partition support in 9.3 is pretty rough and I’d love to take advantage of these improvements. I’m still here and ready to test.

Now if we can just get per-output volume control in MPD…. :)

MiM-TMassey avatar Jul 13 '22 13:07 MiM-TMassey

v9.4.0 will be released this week hopefully. There are only some unit tests outstanding. All other tasks to improve partition support are done for the v10.0.0 version.

jcorporation avatar Jul 13 '22 14:07 jcorporation

Yay for 9.4, but now I gotta wait for 10.0? :) I’ll keep an eye out: I’m looking forward to it. Running multiple MyMPD’s has worked OK. Volume control and selecting outputs in in partitions is really messed up. Once I get 9.4 I’ll see what’s fixed and what’s not and I’ll create a new issue if necessary.

Again, thank you for your efforts. My wife uses the whole-house audio twice as much, now that she doesn’t have to worry about messing with my settings or a queue of music that’s not hers. So at least one family is grateful for your hard work! :)

MiM-TMassey avatar Jul 13 '22 14:07 MiM-TMassey

Upgraded to 9.4.1. Thoughts:

Thank you for changing the UI for selecting partitions: works much better. For move outputs to partition: the list seems to be correct now: show all outputs that aren't already in that partition. Again, thank you. A small request for the UI for moving outputs to a partition: you have to click on the round icons on the left, instead of simply clicking on the name. The name is not a link. Normally, I would expect to be able to click anywhere in the line (which is how the partitions UI works) even on a multi-select interface like outputs.

The volume control is still very messed up for partitions. Changing the volume requires clicking somewhere in the slider range. The output volumes change audibly, but the GUI does NOT change: it keeps the slider and numeric percent display right at whatever number it's showing. I imagine this is part of the basic partition improvements for 10. Looking forward to it.

And just throwing this out into the ether: I would kill (or even fund development!) of independent volume control outputs! Seeing as that was "fixed" (grr) in MPD itself, I've had to downgrade MPD, and I know I won't be able to stay downlevel forever.

MiM-TMassey avatar Jul 23 '22 18:07 MiM-TMassey

The volume control is still very messed up for partitions. Changing the volume requires clicking somewhere in the slider range. The output volumes change audibly, but the GUI does NOT change: it keeps the slider and numeric percent display right at whatever number it's showing.

This works for me and is independent of partitions or I misunderstand you..

jcorporation avatar Jul 24 '22 07:07 jcorporation

The volume control is still very messed up for partitions. Changing the volume requires clicking somewhere in the slider range. The output volumes change audibly, but the GUI does NOT change: it keeps the slider and numeric percent display right at whatever number it's showing.

MPD bug: https://github.com/MusicPlayerDaemon/MPD/issues/1583

jcorporation avatar Aug 01 '22 06:08 jcorporation

The basic infrastructure for concurrent partition support is now implemented but needs further testing. Timer and trigger frontends still need to be adjusted.

jcorporation avatar Aug 21 '22 14:08 jcorporation

Shall we test out of devel or another specific branch ?

tsunulukai avatar Aug 21 '22 17:08 tsunulukai

v10.0.0 is the right branch. Testing is welcome, but keep in mind that some code is missing.

jcorporation avatar Aug 22 '22 07:08 jcorporation

The syntax of the last_played file has changed. You can convert it with sed -i -r 's/^(.*)::(.*)/{"LastPlayed":\1,"partition":"default","uri":"\2"}/g' /var/lib/mympd/state/last_played

Concurrent partition support is now roughly tested and works in my test cases.

I store the browser to partition association in the localStorage. This keeps the partition selection across browser restarts, but different browser windows with different partitions overwrite this associations mutual.

https://github.com/jcorporation/myMPD/blob/v10.0.0/docs/references/partitions.md

jcorporation avatar Aug 24 '22 11:08 jcorporation

All features are now implemented and basically tested. Testing in other real world setup would be great!

jcorporation avatar Aug 25 '22 20:08 jcorporation