search

BradLarson / GPUImage2

GPUImage 2 is a BSD-licensed Swift framework for GPU-accelerated video and image processing.
BSD 3-Clause "New" or "Revised" License
4.87k stars 609 forks source link

[Pipeline] Fixed framebuffers not being properly locked (overrelease warnings), added remove(target:) support, fixed exc_bad_access crashes, PictureInput throw support, etc. #243

Open joshbernfeld opened 6 years ago

joshbernfeld commented 6 years ago 
public func updateTargetsWithFramebuffer(_ framebuffer:Framebuffer) {
    // This retrieves the count as it is currently
    // not factoring in any targets that are waiting in the target serial queue to be added
    // and may include targets which have since been released
    if targets.count == 0 {
        framebuffer.lock()
        framebuffer.unlock()
    } else {
        for _ in targets {
            framebuffer.lock()// This does not get called when it should get called
        }
    }
    // This jumps onto the target serial queue before iterating
    // So if there were any targets that had not yet been added they are added now and iterated through
    // But this is a problem because we didn't account for locking the frame buffer above for these new targets since the count did not reflect them
    // This is why when we retrieve the targets count, it should only be after any targets in the target serial queue are finished being added
    // Even if we did that, its still not comprehensive enough because there is still the risk that another target could be added
    // in between retrieving the count and iterating over them here.
    // So we need to make sure that all of the work we do here is done on the target serial queue in order to guarantee its done atomically.
    // That or we should guarantee only one operation is needed, which is what the new solution does.
    for (target, index) in targets {
        target.newFramebufferAvailable(framebuffer, fromSourceIndex:index)
    }
}

The solution was to retrieve a single list of targets that will not change during the execution of the function. Since we have to iterate through the targets in this solution before retrieving the count it also accounts for any targets which have since been released (which count did not do before). In addition, the count and target variables have been made private to prevent future errors.

public func updateTargetsWithFramebuffer(_ framebuffer:Framebuffer) {
    var foundTargets = [(ImageConsumer, UInt)]()
    for target in targets {
        foundTargets.append(target)
    }

    if foundTargets.count == 0 { // Deal with the case where no targets are attached by immediately returning framebuffer to cache
        framebuffer.lock()
        framebuffer.unlock()
    } else {
        // Lock first for each output, to guarantee proper ordering on multi-output operations
        for _ in foundTargets {
            framebuffer.lock()
        }
    }
    for (target, index) in foundTargets {
        target.newFramebufferAvailable(framebuffer, fromSourceIndex:index)
    }
}

Related issue: https://github.com/BradLarson/GPUImage2/pull/84

For anyone looking to debug future framebuffer overelease warnings you can replace the lock() and unlock() functions with the following functions and then set a breakpoint on the warning and inspect the contents of the call stack history / retain count history variables in your debugger.

var callLockStackHistory = [[String]]()
var retainCountLockHistory = [Int]()
func lock() {
    framebufferRetainCount += 1

    retainCountLockHistory.append(framebufferRetainCount)
    callLockStackHistory.append(Thread.callStackSymbols)
}

func resetRetainCount() {
    framebufferRetainCount = 0
}

var callUnlockStackHistory = [[String]]()
var retainCountUnlockHistory = [Int]()
public func unlock() {
    framebufferRetainCount -= 1

    retainCountUnlockHistory.append(framebufferRetainCount)
    callUnlockStackHistory.append(Thread.callStackSymbols)

    if (framebufferRetainCount < 1) {
        if ((framebufferRetainCount < 0) && (cache != nil)) {
            print("WARNING: Tried to overrelease a framebuffer")
        }
        framebufferRetainCount = 0
        cache?.returnToCache(self)
    }
}
Jane930525 commented 6 years ago

@joshbernfeld Receiving this error when I try to initialize Camera and apply filter.This would eventually lead to crash at method 'clearFramebufferWithColor' in OpenGLRendering.swift. Please help. ` Warning: tried to add target beyond target's input capacity --> Pipeline.swift: addTarget(:atTargetIndex:): 43 Warning: tried to add target beyond target's input capacity --> Pipeline.swift: addTarget(:atTargetIndex:): 43 Warning: tried to add target beyond target's input capacity --> Pipeline.swift: addTarget(_:atTargetInde

Thanks

joshbernfeld commented 6 years ago

@Jane930525 can you attach a sample project that reproduces the issue?