reaper-oss / sws

The SWS extension is a collection of features that seamlessly integrate into REAPER, the Digital Audio Workstation (DAW) software by Cockos, Inc
https://www.sws-extension.org/
MIT License
455 stars 85 forks source link

sends envelopes not recalled correctly with snapshots #537

Closed Jeff0S closed 5 years ago

Jeff0S commented 10 years ago

From nofishonfriday on December 09, 2012 18:31:33

What steps will reproduce the problem? see attached example project it contains one snapshot with send automation, which was saved with the send automated (exactly as you see it when you open the project) now recall this snapshot, send automation is gone (envelope is at 0db)

What version of SWS extension are you using? (Extensions -> About SWS...)

2.3.0 #4 What is your Reaper version number and CPU architecture (e.g. Reaper 3.75 x64)? 4.32 RC1 What operating system are you running? (e.g. Windows XP/7, OSX 10.5.8 etc, 64bit?) WinXP32 SP3

Attachment: snapshots send automation bug test.RPP

Original issue: http://code.google.com/p/sws-extension/issues/detail?id=537

Jeff0S commented 10 years ago

From nofishonfriday on December 09, 2012 09:36:52

forgot to mention, this is reproduceable every time here ie create a new snapshot with the automated send, recall, automation is gone

Thanks for having a look.

Jeff0S commented 10 years ago

From considin...@gmail.com on December 11, 2012 11:41:01

I can confirm that (although my SWS build is a touch stale)

Jeff0S commented 10 years ago

From swstim on December 11, 2012 12:19:55

I hear ya. This is a non-trivial upgrade to snapshots, please bear with me as I figure it out in my miniscule free time.

Status: Accepted
Owner: swstim

Jeff0S commented 10 years ago

From nofishonfriday on December 11, 2012 12:47:30

Thank you.

Jeff0S commented 10 years ago

From considin...@gmail.com on March 05, 2013 02:49:39

Any news on this one? If nothing has been done, I could have a look at it (though I'm slow at figuring out other peoples code). Snapshots are important to me these days and this is a nasty flaw.

Jeff0S commented 10 years ago

From swstim on March 05, 2013 07:07:50

I've looked at it a little, and it's not an easy fix at all. I don't know that I'm going to be able to do it.

The snapshots feature is fragile in the sense that it's tied very closely to Reaper's internal workings, and when things change (or get added in this case) in Reaper it breaks things. I can't keep up with these changes.

Jeff0S commented 10 years ago

From nofishonfriday on March 05, 2013 13:25:41

Thanks for having a look Tim. An idea (suggestion):

For me the situation is, I have automated send envelopes in nearly all off my projects but actually I wouldn't need them to be 'snapshotted'. So how about (an option) for now to simply ignore them when doing full track mix snapshots ?

(This would be totally fine with me at least)

Jeff0S commented 10 years ago

From swstim on March 06, 2013 13:39:35

Sounds easy, but it's not. 2 issues: 1) The envelope in in the "XML" of the track, which means parsing the XML and keeping the envelope there to ignore it. If we got that much working, it'd be trivial to add the full functionality. 2) Sends are stored as receives, so the data isn't where you'd expect it to be - any time you modify the sending track, you need to find all its receives and update all the references to the sending track, etc. There's already mechanism in the code to do this, but it needs to be extended to work with envelopes, too.

Jeff0S commented 10 years ago

From considin...@gmail.com on March 13, 2013 06:01:51

Tim, perhaps a warning could be given so this doesn't bite the unwary?

Jeff0S commented 10 years ago

From considin...@gmail.com on March 30, 2013 12:41:48

Tim, I've been hacking at this and I've managed to get send envelopes stored and restored seemingly okay but there's probably a ton of bugs waiting to show up. I'm thinking that since you thought you might not be able to do it, it's probably way more complex than I imagine. Can you think of any issues that I might need to deal with beyond just storing and restoring envelopes?

Also, I don't want to commit what I've done as I could easily have broken something without realising. Can I send you a patch or something?

Jeff0S commented 10 years ago

From considin...@gmail.com on March 31, 2013 05:44:55

Never mind, I get it now. It's all about the chunk. Tricky.

Jeff0S commented 10 years ago

From swstim on March 31, 2013 17:41:40

Thanks for taking a look! If you ever want to "audition" code just make yourself a branch and check it in there.

Jeff0S commented 10 years ago

From considin...@gmail.com on April 01, 2013 13:04:42

Okay, will do. Looks to me like the snapshot code could use a bit of an overhaul. I see there's a bunch of chunk processing stuff (from Jeffos?) that could perhaps be brought used, so maybe I'll tinker with that in my own little branch. I'd like this to be working well and somewhat future-proof if possible.

Jeff0S commented 10 years ago

From jeffos...@gmail.com on April 02, 2013 04:56:49

Hey Phil, sorry I'm not really sure I understand what you said (English..) so my answer might be totally off!

Snapshots do not use my "chunk state tool" (SNM_ChunkParserPatcher). Well, it is just used it in ..\ObjectState\TrackFX.cpp: I did that to fix snapshots that were not supporting frozen tracks - the SNM_ChunkParserPatcher does. (BTW, there are useless WDL_TypedBuf <-> WDL_FastString conversions in there: it would be a 1st great optimiz given the massive amount data being dealt with)

Just like Tim, I also see what should be done to fix this issue but I don't have the time to help ATM, sorry. However, what I can say is that the SNM_ChunkParserPatcher can indeed help to extract and push envelopes back in a fast and safe way. If this is what you meant in your previous post IX (?) and if you have any Qs about it do not hesitate to ping me! Note: there is a v2 of this tool with more comprehensible interfaces but it is not part of the open source yet.

I think the "tricky" part Tim was thinking about is not this one though, it will be to persist snaps and related envs of other tracks in RPP files. And this stuff should resist frozen tracks, track deletion + undo, etc... Well, that should be fun :) Thanks a lot for having a look!

Jeff0S commented 10 years ago

From swstim on April 03, 2013 07:10:30

Jeff understands the complexity here.

It's possible my existing framework for sends/receives is good enough to be able to be extended to work with chunks for the envelope data, but maybe that's wishful thinking.

Maybe I should petition hard for Cockos to get this native because it's very useful, but every new little feature breaks us.

Jeff0S commented 10 years ago

From considin...@gmail.com on April 03, 2013 11:19:43

Native would be so much better.

Jeffos, I apologise for my bad English as some of what I said doesn't make much sense due to bad editing. I was thinking your parser/patcher could be used to improve the way that the snapshots code handles chunks. I'll have a good look at it when I get some time and I probably will need to question you.

Jeff0S commented 10 years ago

From jeffos...@gmail.com on April 04, 2013 09:22:25

Well, sorry for my bad English IX! You're welcome.. I also agree that getting this feature native would be the best.

bLUiVORY commented 10 years ago

Any movement on this? Is this an issue with earlier versions?

nofishonfriday commented 6 years ago

I had a look into this issue and came up with a a rather 'naive' approach using Jeffos' ChunkParserPatcher which so far with simple testing it looks like it may work, but I'm thinking from the comments here it's probably not as straightforward as I'm assuming...

Test (click to enlarge): snapshots

@devs Here's the patch: https://github.com/nofishonfriday/sws/commit/e59f34e3f3ed854e40589d6d2fc40f3981e14bf7 Would like to hear your thoughts on this...

swstim commented 6 years ago

Wow, that is a really simple patch.... My first instinct is it can't be that simple, but I haven't checked it out yet. :) Looks like you add at most one send per track as you moved the m_sends.Add out of the loops. Does that work when restoring complicated routing snapshots? I will have a chance to dig in later but wanted you to know I had a first glance.

nofishonfriday commented 6 years ago

Thanks for looking into it, appreciated. Would be nice if we could finally squash this bugger. :)

Looks like you add at most one send per track as you moved the m_sends.Add out of the loops

Hm, now you got me wondering...

My understanding is that what it does in the loops is iterating through each track send, find the destination track and store the relevant "AUXRECV... chunk bit (and now also the env. points).

As for every send there's only one receive chunk (?) I'd think it should work this way, but you may know better as you've coded it originally I assume, no ? Anyway, maybe I test the wrong scenarios but couldn't break it here so far.

Even if it's not 100% foolproof it may at least be an improvement to before where the send env's completely vanished when recalling a snapshot ?

Curious about your findings when you have a chance to dig in. :)

edit: I notice loading projects which have stored snapshots using this patch gives problems (loading hangs). :(

edit2: