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
448 stars 85 forks source link

BR_Envelope loses some automation item related settings - easy fix #1153

Open Breeder opened 5 years ago

Breeder commented 5 years ago

It seems BR_Envelope class doesn't take into account new token added to envelope chunk. The fix seems easy enough though

https://github.com/reaper-oss/sws/blob/master/Breeder/BR_EnvelopeUtil.cpp#L1698-L1702 Here you also need to read a second int token in ACT line

and make sure it gets written back here: https://github.com/reaper-oss/sws/blob/master/Breeder/BR_EnvelopeUtil.cpp#L1821-L1822

and also make sure that EnvProperties class inside BR_Envelope class has a new variable that will store that new token added here: https://github.com/reaper-oss/sws/blob/master/Breeder/BR_EnvelopeUtil.cpp#L1849-L1925

Breeder commented 5 years ago

I don't have VS installed so I would be super grateful if one of you fine gentleman who contribute to SWS these days could throw this fix while committing some of your stuff.

Thanks

nofishonfriday commented 5 years ago

Will do as I'm currently working on adding AI support in SWS anways. For the record, the purpose of the 2nd ACT token is documented here: https://github.com/Ultraschall/ultraschall-and-reaper-docs/blob/master/Docs/Reaper-Envelope-StateChunk-Doku.txt#L28

Breeder commented 5 years ago

Awesome - btw, that documentation is a bit misleading

I'm too adding support to some of my scripts for AI (will share on the forums when it's done) and it seems this particular setting (second ACT token) isn't 1 = something, 2 = something etc...instead, it's using bits. So in our case, 3rd bit determines if underlying envelope is bypassed or not etc... Only -1 means to read project settings instead. If it's not -1, you need to read the bits inside the value!

Same goes for project-wide setting of the same kind.

A lot of configurations in REAPER use bits of the value instead of the value itself. You can search in SWS for these functions to see what I mean: https://github.com/reaper-oss/sws/blob/master/Breeder/BR_Util.h#L71-L75

I personally simply try out all the settings I want to test, write down numbers for those settings and then use Window Calculator in programmer mode to see what bit changes for what setting to determine which bit is responsible for every particular setting.

For example, this is a lua function I'm using to determine if underlying envelope should even be processed - works perfectly here:

local function IsUnderlyingEnvelopeBypassed (envelope)
    local isUnderlyingEnvelopeBypassed = false
    local _, envChunk = reaper.GetEnvelopeStateChunk(envelope, "", false)
    for line in envChunk:gmatch("([^\n]*)\n?") do
        local lineTokens = {}
        for token in line:gmatch("[^%s]+") do
            lineTokens[#lineTokens + 1] = token
        end
        if #lineTokens > 0 and lineTokens[1] == "ACT" then
            local automationToken = tonumber(lineTokens[3])
            function GetBit (val, bit) return (val & 1 << bit) ~= 0 end
            if automationToken == -1 then
                if GetBit(reaper.SNM_GetIntConfigVar("POOLEDENVATTACH", "0"), 2) then
                    isUnderlyingEnvelopeBypassed = true
                end
            else
                if GetBit(automationToken, 2) then
                    isUnderlyingEnvelopeBypassed = true
                end
            end
            break
        end
    end
    return isUnderlyingEnvelopeBypassed
end
Breeder commented 5 years ago

One more bug which could potentially do a lot of damage: https://github.com/reaper-oss/sws/blob/master/Breeder/BR_EnvelopeUtil.cpp#L654-L655

Envelope_Evaluate() will return wrong values for underlying envelope if automation items are over it. I think it would be smartest to simply remove envelope_evaluate all together and use those formulas to do the work. Or just use envelope_evaluate() only when there are no automation items in the envelope at all...in any case, prevent it from messing things up!

Even for AI items envelope_evaluate() will return wrong values if AI are stacked one over the other.

Again, easy fix...just use this part only: https://github.com/reaper-oss/sws/blob/master/Breeder/BR_EnvelopeUtil.cpp#L658-L763

and comment out envelope_Evaluate() while leaving some kind of explanation in the comments there.

This should be a basic premise when dealing with AI: 1) Treat underlying envelope the same way it was treated before - that means, always process it unless the underlying envelope is bypassed 2) Treat each AI as a separate entity...so for example the action "SWS/BR: Set selected envelope points to first selected point's value" will get first selected point in each AI - not the one in underlying envelope or other AIs

I believe this approach is the easiest to code because:

1) you just have to repeat the same operations over multiple set of points (one set it underlying envelope, the second set first AI, the third set second AI etc...) 2) It is the same way native envelope actions work - for example "Envelope: Insert 4 envelope points at time selection" will insert 4 points both at the AI and underlying envelope behind the AI. This is the same way I coded my private fix of the "SWS/BR: Insert 2 envelope points at time selection" action

-- Globals -------------------------------------------------------------------------------------------------------------
local ENVELOPE_DELTA = 0.00001

-- Misc functions ------------------------------------------------------------------------------------------------------
local function CheckBounds (val, min, max)
    return (val >= min and val <= max)
end

local function IsUnderlyingEnvelopeBypassed (envelope)
    local isUnderlyingEnvelopeBypassed = false
    local _, envChunk = reaper.GetEnvelopeStateChunk(envelope, "", false)
    for line in envChunk:gmatch("([^\n]*)\n?") do
        local lineTokens = {}
        for token in line:gmatch("[^%s]+") do
            lineTokens[#lineTokens + 1] = token
        end
        if #lineTokens > 0 and lineTokens[1] == "ACT" then
            local automationToken = tonumber(lineTokens[3])
            function GetBit (val, bit) return (val & 1 << bit) ~= 0 end
            if automationToken == -1 then
                if GetBit(reaper.SNM_GetIntConfigVar("POOLEDENVATTACH", "0"), 2) then
                    isUnderlyingEnvelopeBypassed = true
                end
            else
                if GetBit(automationToken, 2) then
                    isUnderlyingEnvelopeBypassed = true
                end
            end
            break
        end
    end
    return isUnderlyingEnvelopeBypassed
end

-- Main ----------------------------------------------------------------------------------------------------------------
function Main ()
    local envelope = reaper.GetSelectedEnvelope(0)
    if envelope ~= nil then

        -- Get take envelope data and default pont shape
        local takeEnvelopeItemStart = 0
        local defaultPointShape     = 0
        local br_envelope = reaper.BR_EnvAlloc(envelope, false)
        _, _, _, _ , _, defaultPointShape = reaper.BR_EnvGetProperties(br_envelope)

        local take = reaper.BR_EnvGetParentTake(br_envelope)
        if take ~= nil then
            takeEnvelopeItemStart = reaper.GetMediaItemInfo_Value(reaper.GetMediaItemTake_Item(take), "D_POSITION")
        end
        reaper.BR_EnvFree(envelope, false)

        -- Get time selection information
        local timeSel = false
        local timeSelStart, timeSelEnd = reaper.GetSet_LoopTimeRange(false, false, 0, 0, false)
        if timeSelStart ~= timeSelEnd then
            timeSel = true
        end
        if timeSel == true and (timeSelStart + ENVELOPE_DELTA >= timeSelEnd) then
            timeSel =  false
        end

        -- Insert envelope points
        if timeSel == true then

            -- Start
            reaper.PreventUIRefresh(1)
            local doUndo  = false

            -- Process envelope points
            if IsUnderlyingEnvelopeBypassed(envelope) == false then
                if doUndo == false then
                    doUndo = true
                    reaper.Undo_BeginBlock()
                end
                -- I think Envelope_Evaluate won't work here because automation item may be over underlying envelope but this SWS action takes care of it so just call it instead
                -- Note that it will also insert stretch markers for inserted tempo markers if dealing with tempo envelope so it's one more reason to use it here
                reaper.Main_OnCommand(reaper.NamedCommandLookup("_BR_INSERT_2_ENV_POINT_TIME_SEL"), 0) -- SWS/BR: Insert 2 envelope points at time selection
            end

            -- Process points in automation items
            local startPointsToAdd   = {}
            local endPointsToAdd     = {}
            local sampleRate = reaper.SNM_GetIntConfigVar("projsrate", 48000)
            for i = 0, reaper.CountAutomationItems(envelope)-1 do
                local selected = reaper.GetSetAutomationItemInfo(envelope, i, "D_UISEL", 0, false)
                local automationStart  = reaper.GetSetAutomationItemInfo(envelope, i, "D_POSITION", 0, false)
                local automationEnd    = reaper.GetSetAutomationItemInfo(envelope, i, "D_LENGTH", 0, false) + automationStart

                if (selected > 0 or selected == 0) then
                    local noTimeSelFound = 0
                    for h = 0, 1 do
                        local          newPointPosition = timeSelStart
                        if h == 1 then newPointPosition = timeSelEnd end

                        local          tableToStorePoints = startPointsToAdd
                        if h == 1 then tableToStorePoints = endPointsToAdd end

                        if CheckBounds(newPointPosition, automationStart, automationEnd) then
                            local prevId    = reaper.GetEnvelopePointByTimeEx(envelope, i, newPointPosition)
                            local nextId    = reaper.GetEnvelopePointByTimeEx(envelope, i, newPointPosition + ENVELOPE_DELTA)
                            local prevTime  = newPointPosition - ENVELOPE_DELTA * 10
                            local nextTime  = newPointPosition + ENVELOPE_DELTA * 10
                            if prevId >= 0 then
                                _, prevTime = reaper.GetEnvelopePointEx(envelope, i, prevId)
                            end
                            if nextId >= 0 then
                                _, nextTime = reaper.GetEnvelopePointEx(envelope, i, nextId)
                            end

                            tableToStorePoints[#tableToStorePoints + 1]          = {}
                            tableToStorePoints[#tableToStorePoints].automationId = i
                            tableToStorePoints[#tableToStorePoints].id           = -1
                            tableToStorePoints[#tableToStorePoints].position     = newPointPosition
                            tableToStorePoints[#tableToStorePoints].value        = ({reaper.Envelope_Evaluate(envelope, newPointPosition, sampleRate, 1)})[2]
                            if prevId >= 0 and CheckBounds(newPointPosition, prevTime, prevTime + ENVELOPE_DELTA) then                  
                                tableToStorePoints[#tableToStorePoints].id    = prevId
                            elseif nextId >= 0 and CheckBounds(newPointPosition, nextTime - ENVELOPE_DELTA, nextTime) then
                                tableToStorePoints[#tableToStorePoints].id    = nextId
                            end
                            if tableToStorePoints[#tableToStorePoints].position  <= automationStart + ENVELOPE_DELTA / 100 then
                                tableToStorePoints[#tableToStorePoints].position  = automationStart + ENVELOPE_DELTA / 100                                  
                            end
                            if tableToStorePoints[#tableToStorePoints].position  >= automationEnd - ENVELOPE_DELTA / 100 then
                                tableToStorePoints[#tableToStorePoints].position  = automationEnd - ENVELOPE_DELTA / 100                            
                            end
                        end
                    end
                end
            end
            if #startPointsToAdd > 0 or #endPointsToAdd > 0 then

                -- First unselect all points in all AI items
                for i = 0, reaper.CountAutomationItems(envelope)-1 do
                    local processedItem = false
                    for j = 0, reaper.CountEnvelopePointsEx(envelope, i)-1 do
                        local _, _, _, _, _, s = reaper.GetEnvelopePointEx(envelope, i, j)
                        if s == true then
                            if doUndo == false then                                 
                                doUndo = true
                                reaper.Undo_BeginBlock()
                            end
                            reaper.SetEnvelopePointEx(envelope, i, j, nil, nil, nil, nil, false, true)
                            processedItem = true
                        end
                    end
                    if processedItem == true then
                        reaper.Envelope_SortPointsEx(envelope, i)
                    end
                end

                -- Insert/edit 2 points in AI items
                for _, point in ipairs(startPointsToAdd) do
                    if doUndo == false then                                 
                        doUndo = true
                        reaper.Undo_BeginBlock()
                    end
                    if point.id == -1 then
                        reaper.InsertEnvelopePointEx(envelope, point.automationId, point.position, point.value, defaultPointShape, 0, true, true)
                    else
                        reaper.SetEnvelopePointEx(envelope, point.automationId, point.id, point.position, nil, nil, nil, true, true)
                    end
                end
                for _, point in ipairs(endPointsToAdd) do
                    if doUndo == false then                                 
                        doUndo = true
                        reaper.Undo_BeginBlock()
                    end
                    if point.id == -1 then
                        reaper.InsertEnvelopePointEx(envelope, point.automationId, point.position, point.value, defaultPointShape, 0, true, true)
                    else
                        reaper.SetEnvelopePointEx(envelope, point.automationId, point.id, point.position, nil, nil, nil, true, true)
                    end
                end
                for _, point in ipairs(startPointsToAdd) do reaper.Envelope_SortPointsEx(envelope, point.automationId) end
                for _, point in ipairs(endPointsToAdd)   do reaper.Envelope_SortPointsEx(envelope, point.automationId) end
            end

            -- Finish up
            if doUndo == true then
                reaper.Undo_EndBlock("Insert 2 envelope points at time selection", 1 | 4)
            end
            reaper.UpdateArrange()
            reaper.PreventUIRefresh(-1)
        end
    end
end

Main()
function NoUndoPoint () end -- Makes sure there is no unnecessary undo point created, see more
reaper.defer(NoUndoPoint)   -- here: http://forum.cockos.com/showpost.php?p=1523953&postcount=67
Breeder commented 5 years ago

Then again, this action:

SWS/BR: Convert selected envelope's curve in time selection to CC events in last clicked CC lane in last active MIDI editor

needs envelope_evaluate() to properly replicate the envelope curve so I guess in case of that action, you would simply use envelope_evaluate() inside the action.

This is all quite touchy, I hope you make it through to the other side, lol

Breeder commented 5 years ago

In the last script I pasted, it's only the luck

reaper.Main_OnCommand(reaper.NamedCommandLookup("_BR_INSERT_2_ENV_POINT_TIME_SEL"), 0) -- SWS/BR: Insert 2 envelope points at time selection

that this works on underlying envelope...because that specific action skips envelope_evaluate() probably due to some condition in BR_Envelope (points already edited? fast mode requested etc...?)

This is a real world example why envelope_evaluate() needs to be completely commented out from BR_Envelope() and used only in specific actions where required.

nofishonfriday commented 5 years ago

@Breeder As suggestion, wouldn't it make the most sense if you do the changes that you think are necessary? I mean as you coded these originally you probably know best what to do. :) If you want you could send me the files and I could test, check if they build and submit PRs, as for your Win32 ReaScript actions.

Breeder commented 5 years ago

I'm not going back to SWS, I simply don't have the time anymore. I would love to be able to fix this "blind" without installing VS and that would be possible if only BR_Envelope needs updating, but this is a huge change and every action that's using BR_Envelope needs to be thoroughly checked.

What I am doing is recreating lua versions of this stuff and that I will share ;D It won't be a full set, but after years of using these actions I found a good combo that works for most envelope editing tasks.

The only problem is that my lua versions rely on current way SWS is working right now. So if it gets fixed, it may break lua stuff unless it's done perfectly so it can replace my lua. Music is my job and the worst thing that can happen while you're on a job is that stuff that used to work, starts breaking. We all hate that feeling and no need to go through it twice. I think my lua pack will be enough so I feel it's even better this remains unfixed until somebody decides to do a complete and proper fixup (I think it would take around a month of coding to test for all corner-cases while fixing it...I would love if I could afford that time, but I simply cannot...what I can do is offer advice because this stuff is everywhere, from items (take envelopes) to tempo map (tempo envelope))

IMHO, it's much easier to recreate this using lua so I would just leave it like that and wait for my set of scripts. Still, this ACT token does need fixing so I would appreciate if you could throw it in while you're doing your thing, thanks ;D

Breeder commented 5 years ago

Done (the scripts)! https://forum.cockos.com/showthread.php?t=221082