Saving/restoring partition state

[john-]

I am taking a look at what is involved in saving/restoring partition state. This is all behind the scenes and not exposed to the user.

For outputs, the current format is this:

AUDIO_DEVICE_STATE<value>:<device>

It seems reasonable to do this:

AUDIO_DEVICE_STATE<value>:<device>:<partition>

This will allow straightforward read of the current format (on first run) and not be too bad looking on save. For example:

audio_device_state:0:MyTestOutput:default
audio_device_state:1:MyTestOutput:TestPartition

I have the basics of that working just to prove it out but have not tested it fully.

However when it comes to the player state (and probably volume state) I am not sure what direction to go in as far as format.

Here is an example of current state:

state: pause
current: 0
time: 5.953
random: 0
repeat: 0
single: 0
consume: 0
crossfade: 0
mixrampdb: 0
mixrampdelay: -1
lastloadedplaylist:
playlist_begin
0:path/to/song.flac
playlist_end

Changing this by adding the label seems like it may be involved parsing/generating it. Any recommendations on what format should be used? I think it is going to be pretty involved for me technically so want to start on something at least close to final approach.

Of course, if the format of the whole state file should be revamped at this point that would be good to know too. There are some radically different options such as per partition state files or formats that segregate out the partitions from each other in a single file.

[MaxKellermann]

The most difficult part of your work will be implementing the backend, to actually make it work. The file format is the easiest part.
I suggest not changing the "audio_device_state" line because that would make state files not backwards-compatible. Instead, for each non-default partition, start a new "block", e.g. by writing a line:

partition: TestPartition

All that comes after this will be the state of that partition. That includes playback modes, seek position and the queue (current playlist) and so on. Additionally, each output assigned to that partition needs to be listed here.
With this structure, state files of MPD instances with just one partition remain the same, and only those with additional partitions will have an additional structure for those partitions.

[MaxKellermann]

This branch has a bunch of commits, but the following commits are just fixups. I don't want to review the mistakes you already found. Why don't you submit a PR with proper separation of commits?

[john-]

I am not sure what you mean by separating the commits. Would a PR with all commits squashed address what you are looking for?

[MaxKellermann]

If you believe everything belongs in one commit and there are no changes that belong anywhere else, then yes - but take care to have a proper commit message; don't just let git concatenate all existing work-in-progress commit messages.

Your TestFullIntegration would be worthwile to have as a separate commit, and the commit that implements your new feature would then add code to test this feature. But .... this test looks like a very ugly mess. This will take a few rounds of refactoring to bring it to a shape that's mergeable.

[john-]

I understand the need for separate commits to break up the work.

I was thinking a commit for test cases/framework, one for reading state and one for writing. I separated out the read/write based on the idea it would be easier to review. Both are required together for the functionality to work.

Also, is each commit to be in its own branch/pull request? Or are the separate commits in a single branch/pull request?

This is new to me and want to make sure I get it right.

On a possibly related note I saw reference to stgit in the developer documentation for MPD. Is that relevant to this breakdown?

[MaxKellermann]

Reading and writing belongs in the same commit. One doesn't make sense without the other.
But if you suggest adding a new unit test that can be used without your work, make it a separate commit.
Make separate commits, not branches. Ideally, your PR branch should have all these commits. (Maybe only two: one for the new unit test, and the second commit for your new feature.)
You can use any tool you like to make proper git commits. I use stgit, but I could never convince anybody to use it (I wrote the MPD dev documentation) - everybody just decides to use plain git and then they complain it's too difficult to make a proper PR branch - oh, really?