lmms icon indicating copy to clipboard operation
lmms copied to clipboard

Remove song import global automation

Open Veratil opened this issue 6 years ago • 36 comments

Completes step 2 from #403

I've also removed the global automation <track> node from the templates. I noticed in default.mpt that there were <object> nodes, but no related id in any other nodes like there would normally be if there was a pattern connected. I believe this might be a bug in the automation editor.

Veratil avatar Oct 07 '19 16:10 Veratil

I think DataFile is a better place to handle the upgrade stuff rather than Song.

PhysSong avatar Oct 10 '19 00:10 PhysSong

I think DataFile is a better place to handle the upgrade stuff rather than Song.

I tried that initially, but it didn't do anything. I'll try playing with it some more.

Veratil avatar Oct 10 '19 01:10 Veratil

@PhysSong okay it took some playing around with finding nodes, but I finally managed to get it working in DataFile.

Veratil avatar Oct 10 '19 04:10 Veratil

@Veratil Could you look into factory project files to see if any of them has global automation tracks?

PhysSong avatar Oct 15 '19 02:10 PhysSong

@Veratil Could you look into factory project files to see if any of them has global automation tracks?

@PhysSong I believe that's already taken care of in the second commit, or are there other ones that I missed?

Veratil avatar Oct 15 '19 03:10 Veratil

It will be okay if you've checked all demo projects and confirmed that there are no global automation tracks that you need to remove.

PhysSong avatar Oct 15 '19 04:10 PhysSong

It will be okay if you've checked all demo projects and confirmed that there are no global automation tracks that you need to remove.

I've gone through and found that some of the demos are still adding tracks that aren't linked to anything. I've updated the check to look for more than one <time> node now which has prevented adding extraneous automation tracks which aren't linked to anything.

There is one demo that has an unlinked automation track which has mulitple time nodes.

@PhysSong Would you like me to go through and update all the demo files in this patch to the "newest" save file version then?

Veratil avatar Oct 16 '19 16:10 Veratil

@PhysSong Would you like me to go through and update all the demo files in this patch to the "newest" save file version then?

If you want to do so, I'd recommend not touching files without global automations. I'm also okay if you do that in a separate pull request.

PhysSong avatar Oct 17 '19 01:10 PhysSong

@PhysSong Would you like me to go through and update all the demo files in this patch to the "newest" save file version then?

If you want to do so, I'd recommend not touching files without global automations. I'm also okay if you do that in a separate pull request.

Is there an easy way to read the mmpz files? That would be the easiest to see if there is any automation patterns that should be imported.

Veratil avatar Oct 17 '19 03:10 Veratil

Is there an easy way to read the mmpz files?

lmms -d input.mmpz prints the decompressed version of input.mmpz.

PhysSong avatar Oct 23 '19 01:10 PhysSong

Is there an easy way to read the mmpz files?

lmms -d input.mmpz prints the decompressed version of input.mmpz.

Would it be okay to run lmms upgrade [in] [out] on all the files? 😄

Veratil avatar Apr 19 '20 23:04 Veratil

That makes sense, but Qt will mix the order of attributes in XML tags. You can write a simple program to compress/decompress .mmpz files with QByteArray::qCompress() and QByteArray::qUncompress().

PhysSong avatar Apr 20 '20 01:04 PhysSong

The result of qCompress() consists of the size of the uncompressed data(in 4byte big-endian integer) and the Zlib-compressed data. It means you can use some other software to (de)compress the project files.

PhysSong avatar Apr 20 '20 01:04 PhysSong

@Veratil Are you going to finish this PR?

JohannesLorenz avatar Nov 29 '20 11:11 JohannesLorenz

Seconded. It would be superb if we could get this and step 3 (remove from the GUI) done for 1.3, and step 3 should be a quick and safe fix.

Spekular avatar Nov 29 '20 12:11 Spekular

I need to go through all the demo files still.

Veratil avatar Nov 29 '20 14:11 Veratil

@Veratil Bump.

PhysSong avatar Jan 12 '21 10:01 PhysSong

  1. I rebased on master 😁
  2. I updated the update function to the new format.
  3. I removed the global automation save from Song's saveSettings, no more hidden automations will be saved after this at all.
  4. I went through each demo file and actually loaded them in lmms. Any that had an unused hidden automation tracks that actually created visible automation tracks I removed and saved over. Anything else I didn't touch. If they do have the block, it's either ignored or will get removed on future updates where we update the demos for whatever reason.

Veratil avatar Jan 17 '21 05:01 Veratil

Regarding global automation clips with only one point: In many cases, they are just holding the model's value at time 0, but it's not guaranteed that they always do.

PhysSong avatar Jan 29 '21 06:01 PhysSong

The rest looks good to me.

PhysSong avatar Jan 30 '21 07:01 PhysSong

Regarding global automation clips with only one point: In many cases, they are just holding the model's value at time 0, but it's not guaranteed that they always do.

That's where thing are weird. Every global automation track that had one node was either a leftover that wasn't removed (file bloat because there wasn't a way to delete the global track), or didn't actually affect the model's value anymore (that value was saved as an attribute instead or didn't actually connect anymore).

I can make the change to load it regardless, and I'll go through all the projects again to see if anything else pops up.

Veratil avatar Jan 30 '21 08:01 Veratil

I looked into this PR again. This PR will change some behaviors:

  • Global automation patterns have lower priority than ordinary ones, but this PR will reverse it due to the order of XML nodes
  • Global automation patterns with single point which "locks" the value of connected models will be removed
  • Global automation patterns created by user will never be saved

The first one sounds undesirable, but the rest don't sound too bad once #5230 is merged and https://github.com/LMMS/lmms/pull/5223#issuecomment-759225962 is addressed.

PhysSong avatar Jan 31 '22 02:01 PhysSong

Rebased on master and (hopefully) fixed conflicts (correctly). Tested build and it seemed to be working. I'll go back through all the demos to double check everything, those are the biggest changes for removing the hidden automation tracks.

Veratil avatar Jan 14 '23 06:01 Veratil

Looks like everything is still good in the demos. 👍

Veratil avatar Jan 14 '23 21:01 Veratil

Global automation patterns have lower priority than ordinary ones, but this PR will reverse it due to the order of XML nodes

I think this is now the only one which probably needs to be addressed. @Veratil any opinions?

PhysSong avatar Jul 02 '23 02:07 PhysSong

Global automation patterns have lower priority than ordinary ones, but this PR will reverse it due to the order of XML nodes

I think this is now the only one which probably needs to be addressed. @Veratil any opinions?

I'm honestly not sure what you mean by priority here. If two automation clips are connected to the same object the global wins?

Veratil avatar Jul 02 '23 04:07 Veratil

If two automation clips are connected to the same object the global wins?

Yes, I mean that case. https://github.com/LMMS/lmms/blob/1f30ffc5e4dddbd6ffb843bd8c56a8b4de859675/src/core/Song.cpp#L841-L844 Here you can see the global automation track is the first element before removal. You can also see that TrackContainer::automatedValuesFromTracks uses the value from the last track connected to an object.

PhysSong avatar Jul 02 '23 05:07 PhysSong

Okay I'll try to see what can be done for this to keep them in order. It never occurred to me that you could connect an object to multiple automations.

Veratil avatar Jul 02 '23 14:07 Veratil

Update?

Rossmaxx avatar Aug 24 '23 06:08 Rossmaxx

Update?

It's ready for review now.

PhysSong avatar Sep 11 '24 14:09 PhysSong