7

I'm having a great deal of difficulty coming up with code that reliably copies a CVPixelBuffer on any iOS device. My first attempt worked fine until I tried it on an iPad Pro:

extension CVPixelBuffer {
    func deepcopy() -> CVPixelBuffer? {
        let width = CVPixelBufferGetWidth(self)
        let height = CVPixelBufferGetHeight(self)
        let format = CVPixelBufferGetPixelFormatType(self)
        var pixelBufferCopyOptional:CVPixelBuffer?
        CVPixelBufferCreate(nil, width, height, format, nil, &pixelBufferCopyOptional)
        if let pixelBufferCopy = pixelBufferCopyOptional {
            CVPixelBufferLockBaseAddress(self, kCVPixelBufferLock_ReadOnly)
            CVPixelBufferLockBaseAddress(pixelBufferCopy, 0)
            let baseAddress = CVPixelBufferGetBaseAddress(self)
            let dataSize = CVPixelBufferGetDataSize(self)
            let target = CVPixelBufferGetBaseAddress(pixelBufferCopy)
            memcpy(target, baseAddress, dataSize)
            CVPixelBufferUnlockBaseAddress(pixelBufferCopy, 0)
            CVPixelBufferUnlockBaseAddress(self, kCVPixelBufferLock_ReadOnly)
        }
        return pixelBufferCopyOptional
    }
}

The above crashes on an iPad Pro because CVPixelBufferGetDataSize(self) is slightly larger than CVPixelBufferGetDataSize(pixelBufferCopy), so the memcpy writes to unallocated memory.

So I gave up with that and tried this:

func copy() -> CVPixelBuffer?
{
    precondition(CFGetTypeID(self) == CVPixelBufferGetTypeID(), "copy() cannot be called on a non-CVPixelBuffer")

    var _copy: CVPixelBuffer?

    CVPixelBufferCreate(
        nil,
        CVPixelBufferGetWidth(self),
        CVPixelBufferGetHeight(self),
        CVPixelBufferGetPixelFormatType(self),
        CVBufferGetAttachments(self, .shouldPropagate),
        &_copy)

    guard let copy = _copy else { return nil }

    CVPixelBufferLockBaseAddress(self, .readOnly)
    CVPixelBufferLockBaseAddress(copy, [])
    defer
    {
        CVPixelBufferUnlockBaseAddress(copy, [])
        CVPixelBufferUnlockBaseAddress(self, .readOnly)
    }

    for plane in 0 ..< CVPixelBufferGetPlaneCount(self)
    {
        let dest        = CVPixelBufferGetBaseAddressOfPlane(copy, plane)
        let source      = CVPixelBufferGetBaseAddressOfPlane(self, plane)
        let height      = CVPixelBufferGetHeightOfPlane(self, plane)
        let bytesPerRow = CVPixelBufferGetBytesPerRowOfPlane(self, plane)

        memcpy(dest, source, height * bytesPerRow)
    }

    return copy
}

That works on both my test devices, but it's just reached actual customers and it turns out it crashes on the iPad 6 (and only that device so far). It's an EXC_BAD_ACCESS on the call to memcpy() again.

Seems crazy that there isn't a simple API call for this given how hard it seems to be to make it work reliably. Or am I make it harder than it needs to be? Thanks for any advice!

Nestor
  • 2,753
  • 2
  • 29
  • 34
  • 1
    The second implementation looks quite solid. The only problem I can imagine is that a plane in the new pixel buffer is allocated with a different stride length (*bytes per row*). The stride length is based on *width × (bytes per pixel)* and then rounded up in an unspecified way to achieve optimal memory access. So check if `CVPixelBufferGetBytesPerRowOfPlane(self, plane) == CVPixelBufferGetBytesPerRowOfPlane(copy, plane)`. If not, copy the pixel plane row by row. – Codo Nov 03 '18 at 16:38
  • Thanks, do you want to post that as an answer? – Nestor Nov 04 '18 at 12:27
  • Does it solve the crashes? – Codo Nov 04 '18 at 13:10
  • I won't know until I get it out on the App Store unfortunately as it crashes on the iPad 6th gen and nothing else. – Nestor Nov 05 '18 at 09:21
  • It works! No crashes any more. – Nestor Nov 12 '18 at 23:12

2 Answers2

11

This questions and answer combo is solid gold. Let me add value with a slight refactor and some control flow to account for CVPixelBuffers that do not have planes.

public extension CVPixelBuffer {
    func copy() throws -> CVPixelBuffer {
        precondition(CFGetTypeID(self) == CVPixelBufferGetTypeID(), "copy() cannot be called on a non-CVPixelBuffer")

        var _copy: CVPixelBuffer?

        let width = CVPixelBufferGetWidth(self)
        let height = CVPixelBufferGetHeight(self)
        let formatType = CVPixelBufferGetPixelFormatType(self)
        let attachments = CVBufferGetAttachments(self, .shouldPropagate)

        CVPixelBufferCreate(nil, width, height, formatType, attachments, &_copy)

        guard let copy = _copy else {
            throw PixelBufferCopyError.allocationFailed
        }

        CVPixelBufferLockBaseAddress(self, .readOnly)
        CVPixelBufferLockBaseAddress(copy, [])

        defer {
            CVPixelBufferUnlockBaseAddress(copy, [])
            CVPixelBufferUnlockBaseAddress(self, .readOnly)
        }

        let pixelBufferPlaneCount: Int = CVPixelBufferGetPlaneCount(self)


        if pixelBufferPlaneCount == 0 {
            let dest = CVPixelBufferGetBaseAddress(copy)
            let source = CVPixelBufferGetBaseAddress(self)
            let height = CVPixelBufferGetHeight(self)
            let bytesPerRowSrc = CVPixelBufferGetBytesPerRow(self)
            let bytesPerRowDest = CVPixelBufferGetBytesPerRow(copy)
            if bytesPerRowSrc == bytesPerRowDest {
                memcpy(dest, source, height * bytesPerRowSrc)
            }else {
                var startOfRowSrc = source
                var startOfRowDest = dest
                for _ in 0..<height {
                    memcpy(startOfRowDest, startOfRowSrc, min(bytesPerRowSrc, bytesPerRowDest))
                    startOfRowSrc = startOfRowSrc?.advanced(by: bytesPerRowSrc)
                    startOfRowDest = startOfRowDest?.advanced(by: bytesPerRowDest)
                }
            }

        }else {
            for plane in 0 ..< pixelBufferPlaneCount {
                let dest        = CVPixelBufferGetBaseAddressOfPlane(copy, plane)
                let source      = CVPixelBufferGetBaseAddressOfPlane(self, plane)
                let height      = CVPixelBufferGetHeightOfPlane(self, plane)
                let bytesPerRowSrc = CVPixelBufferGetBytesPerRowOfPlane(self, plane)
                let bytesPerRowDest = CVPixelBufferGetBytesPerRowOfPlane(copy, plane)

                if bytesPerRowSrc == bytesPerRowDest {
                    memcpy(dest, source, height * bytesPerRowSrc)
                }else {
                    var startOfRowSrc = source
                    var startOfRowDest = dest
                    for _ in 0..<height {
                        memcpy(startOfRowDest, startOfRowSrc, min(bytesPerRowSrc, bytesPerRowDest))
                        startOfRowSrc = startOfRowSrc?.advanced(by: bytesPerRowSrc)
                        startOfRowDest = startOfRowDest?.advanced(by: bytesPerRowDest)
                    }
                }
            }
        }
        return copy
    }
}

Valid with Swift 5. To provide a little more background... There are many formats that AVCaptureVideoDataOutput .videoSettings property can take. Not all of them have planes especially ones that ML Models might need.

Jon Vogel
  • 5,244
  • 1
  • 39
  • 54
7

The second implementation looks quite solid. The only problem I can imagine is that a plane in the new pixel buffer is allocated with a different stride length (bytes per row). The stride length is based on width × (bytes per pixel) and then rounded up in an unspecified way to achieve optimal memory access.

So check if:

CVPixelBufferGetBytesPerRowOfPlane(self, plane) == CVPixelBufferGetBytesPerRowOfPlane(copy, plane

If not, copy the pixel plane row by row:

for plane in 0 ..< CVPixelBufferGetPlaneCount(self)
{
    let dest            = CVPixelBufferGetBaseAddressOfPlane(copy, plane)
    let source          = CVPixelBufferGetBaseAddressOfPlane(self, plane)
    let height          = CVPixelBufferGetHeightOfPlane(self, plane)
    let bytesPerRowSrc  = CVPixelBufferGetBytesPerRowOfPlane(self, plane)
    let bytesPerRowDest = CVPixelBufferGetBytesPerRowOfPlane(copy, plane)

    if bytesPerRowSrc == bytesPerRowDest {
        memcpy(dest, source, height * bytesPerRowSrc)
    } else {
        var startOfRowSrc = source
        var startOfRowDest = dest
        for _ in 0..<height {
            memcpy(startOfRowDest, startOfRowSrc, min(bytesPerRowSrc, bytesPerRowDest))
            startOfRowSrc += bytesPerRowSrc
            startOfRowDest += bytesPerRowDest
        }
    }
}
Codo
  • 75,595
  • 17
  • 168
  • 206
  • works nicely for iPad 6th gen. it seems that you made a typo on line 7 though: bytesPerRowDest should be using copy and not self, otherwise you will always end up in the first block of the if test. – dgmz Jun 20 '19 at 12:22