fulldecent/FDWaveformView

Crash -[AVAssetReader setTimeRange:] cannot be called after reading has started'

PavelKandziuba opened this issue · 7 comments

It happens after fixing that issue i think
#76

That crash occurs in case with tableView with audio cells (7-8 cells with short audio)

func sliceAsset(withRange slice: Range, andDownsampleTo targetSamples: Int, done: (_ samples: [CGFloat], _ sampleMax: CGFloat) -> Void) {
guard slice.count > 0 else { return }
guard let asset = asset else { return }
guard let assetTrack = assetTrack else { return }
guard let assetReader = assetReader else { return }

    assetReader.timeRange = CMTimeRange(start: CMTime(value: Int64(slice.lowerBound), timescale: asset.duration.timescale), duration: CMTime(value: Int64(slice.count), timescale: asset.duration.timescale)) <<------ here is crash

here is screenshot
http://joxi.ru/RmzYddWcoLzkrO

Hmmm, interesting. Thank you for reporting.

I am having a little trouble debugging this.

Could you please help to create a minimally viable test example, possibly using this technique: http://stackoverflow.com/questions/33367609/how-to-use-cocoapods-with-playground

@fulldecent : I'm able to reproduce this issue fairly easily. If you load the waveform in a view, and then that view's bounds change, it will call layoutSubviews again inside FDWaveformView. It will then call renderAsset() because cacheIsDirty() returns true. And finally, from there it obviously calls sliceAsset where it attempts to specify the assetReader timeRange. I've attached the stack trace so you can see what I'm seeing in more detail.

screen shot 2017-04-26 at 2 08 14 am

After doing some reading here and here it seems that the workaround is to recreate the assetReader every time you need to change the timeRange. I tried not setting the timeRange after the first slice, but you'll get similar errors down the line regarding not being able to do things after the reading has started.

My workaround at the moment is to move the assetReader initialization from the audioURL set handler to sliceAsset(). Here is my new sliceAsset() function:

func sliceAsset(withRange slice: Range<Int>, andDownsampleTo targetSamples: Int, done: (_ samples: [CGFloat], _ sampleMax: CGFloat) -> Void) {
        guard slice.count > 0 else { return }
        guard let asset = asset else { return }
        guard let assetTrack = assetTrack else { return }
        //guard let assetReader = assetReader else { return }
        
        assetReader = try? AVAssetReader(asset: asset)
        guard assetReader != nil else {
            NSLog("FDWaveformView failed to load AVAssetReader")
            return
        }
        
        assetReader!.timeRange = CMTimeRange(start: CMTime(value: Int64(slice.lowerBound), timescale: asset.duration.timescale), duration: CMTime(value: Int64(slice.count), timescale: asset.duration.timescale))
        let outputSettingsDict: [String : Any] = [
            AVFormatIDKey: Int(kAudioFormatLinearPCM),
            AVLinearPCMBitDepthKey: 16,
            AVLinearPCMIsBigEndianKey: false,
            AVLinearPCMIsFloatKey: false,
            AVLinearPCMIsNonInterleaved: false
        ]

        let readerOutput = AVAssetReaderTrackOutput(track: assetTrack, outputSettings: outputSettingsDict)
        readerOutput.alwaysCopiesSampleData = false
        assetReader!.add(readerOutput)

        var channelCount = 1
        let formatDesc = assetTrack.formatDescriptions
        for item in formatDesc {
            guard let fmtDesc = CMAudioFormatDescriptionGetStreamBasicDescription(item as! CMAudioFormatDescription) else { return }    // TODO: Can the forced downcast in here be safer?
            channelCount = Int(fmtDesc.pointee.mChannelsPerFrame)
        }

        var sampleMax = noiseFloor
        let samplesPerPixel = max(1, channelCount * slice.count / targetSamples)
        let filter = [Float](repeating: 1.0 / Float(samplesPerPixel), count: samplesPerPixel)

        var outputSamples = [CGFloat]()
        var sampleBuffer = Data()

        // 16-bit samples
        assetReader!.startReading()

        while assetReader!.status == .reading {
            guard let readSampleBuffer = readerOutput.copyNextSampleBuffer(),
                let readBuffer = CMSampleBufferGetDataBuffer(readSampleBuffer) else {
                    break
            }

            // Append audio sample buffer into our current sample buffer
            var readBufferLength = 0
            var readBufferPointer: UnsafeMutablePointer<Int8>?
            CMBlockBufferGetDataPointer(readBuffer, 0, &readBufferLength, nil, &readBufferPointer)
            sampleBuffer.append(UnsafeBufferPointer(start: readBufferPointer, count: readBufferLength))
            CMSampleBufferInvalidate(readSampleBuffer)

            let totalSamples = sampleBuffer.count / MemoryLayout<Int16>.size
            let downSampledLength = totalSamples / samplesPerPixel
            let samplesToProcess = downSampledLength * samplesPerPixel

            guard samplesToProcess > 0 else { continue }
            
            processSamples(fromData: &sampleBuffer,
                           sampleMax: &sampleMax,
                           outputSamples: &outputSamples,
                           samplesToProcess: samplesToProcess,
                           downSampledLength: downSampledLength,
                           samplesPerPixel: samplesPerPixel,
                           filter: filter)
        }
        
        // Process the remaining samples at the end which didn't fit into samplesPerPixel
        let samplesToProcess = sampleBuffer.count / MemoryLayout<Int16>.size
        if samplesToProcess > 0 {
            let downSampledLength = 1
            let samplesPerPixel = samplesToProcess
            let filter = [Float](repeating: 1.0 / Float(samplesPerPixel), count: samplesPerPixel)
            
            processSamples(fromData: &sampleBuffer,
                           sampleMax: &sampleMax,
                           outputSamples: &outputSamples,
                           samplesToProcess: samplesToProcess,
                           downSampledLength: downSampledLength,
                           samplesPerPixel: samplesPerPixel,
                           filter: filter)
        }
        
        // if (reader.status == AVAssetReaderStatusFailed || reader.status == AVAssetReaderStatusUnknown)
        // Something went wrong. Handle it.
        if assetReader!.status == .completed {
            done(outputSamples, sampleMax)
        } else {
            print(assetReader!.status)
        }
    }

Let me know if you need any more info.

Ah! That makes sense, very nice.

The reason I had originally moved it was to fix #76

Currently we seemed to be trapped between two conditions:

  1. If we initialize in the instance class that crashes because the timeslice is immutable, and we need to make new timeslices.
  2. If we initialize in the rendering sequence this crashes because if the underlying asset is changed too quickly then the asset reading process is interrupted and apparently asset reading cannot be aborted while in progress.

Do you have any advice on solving both of these at the same time? Or can you confirm that #76 is somehow not regressed in your implementation?

@fulldecent : This may be something that you've already looked into, but there is a cancelReading() function in AVAssetReader. I'm just spit-balling here but couldn't something like this be done?

   //create a semaphore so we can wait if assetReader is reading
    var shouldAbort = false
    let semaphore = DispatchSemaphore(value: 0)
    /// Read the asset and create create a lower resolution set of samples
    func sliceAsset(withRange slice: Range<Int>, andDownsampleTo targetSamples: Int, done: (_ samples: [CGFloat], _ sampleMax: CGFloat) -> Void) {
        guard slice.count > 0 else { return }
        guard let asset = asset else { return }
        guard let assetTrack = assetTrack else { return }
        //guard let assetReader = assetReader else { return }
        
        //we're currently reading. Initializing a new assetReader here would normally cause a crash.
        //let's wait until the reading process is aborted to continue.
        if assetReader != nil {
            if assetReader!.status == .reading {
                shouldAbort = true
                _ = semaphore.wait(timeout: .distantFuture)
            }
        }
        
        assetReader = try? AVAssetReader(asset: asset)
        guard assetReader != nil else {
            NSLog("FDWaveformView failed to load AVAssetReader")
            return
        }

        print("slice count: \(slice.count)")
        print("slice lowerbound: \(slice.lowerBound)")
        
        assetReader!.timeRange = CMTimeRange(start: CMTime(value: Int64(slice.lowerBound), timescale: asset.duration.timescale), duration: CMTime(value: Int64(slice.count), timescale: asset.duration.timescale))
        let outputSettingsDict: [String : Any] = [
            AVFormatIDKey: Int(kAudioFormatLinearPCM),
            AVLinearPCMBitDepthKey: 16,
            AVLinearPCMIsBigEndianKey: false,
            AVLinearPCMIsFloatKey: false,
            AVLinearPCMIsNonInterleaved: false
        ]

        let readerOutput = AVAssetReaderTrackOutput(track: assetTrack, outputSettings: outputSettingsDict)
        readerOutput.alwaysCopiesSampleData = false
        assetReader!.add(readerOutput)

        var channelCount = 1
        let formatDesc = assetTrack.formatDescriptions
        for item in formatDesc {
            guard let fmtDesc = CMAudioFormatDescriptionGetStreamBasicDescription(item as! CMAudioFormatDescription) else { return }    // TODO: Can the forced downcast in here be safer?
            channelCount = Int(fmtDesc.pointee.mChannelsPerFrame)
        }

        var sampleMax = noiseFloor
        let samplesPerPixel = max(1, channelCount * slice.count / targetSamples)
        let filter = [Float](repeating: 1.0 / Float(samplesPerPixel), count: samplesPerPixel)

        var outputSamples = [CGFloat]()
        var sampleBuffer = Data()

        // 16-bit samples
        assetReader!.startReading()

        while assetReader!.status == .reading {
            
            //check to see if we can abort
            if shouldAbort {
                //cancel reading, signal the new assetReader initialization to take place, and return
                assetReader!.cancelReading()
                shouldAbort = false
                semaphore.signal()
                return
            }
            
            guard let readSampleBuffer = readerOutput.copyNextSampleBuffer(),
                let readBuffer = CMSampleBufferGetDataBuffer(readSampleBuffer) else {
                    break
            }

            // Append audio sample buffer into our current sample buffer
            var readBufferLength = 0
            var readBufferPointer: UnsafeMutablePointer<Int8>?
            CMBlockBufferGetDataPointer(readBuffer, 0, &readBufferLength, nil, &readBufferPointer)
            sampleBuffer.append(UnsafeBufferPointer(start: readBufferPointer, count: readBufferLength))
            CMSampleBufferInvalidate(readSampleBuffer)

            let totalSamples = sampleBuffer.count / MemoryLayout<Int16>.size
            let downSampledLength = totalSamples / samplesPerPixel
            let samplesToProcess = downSampledLength * samplesPerPixel

            guard samplesToProcess > 0 else { continue }
            
            processSamples(fromData: &sampleBuffer,
                           sampleMax: &sampleMax,
                           outputSamples: &outputSamples,
                           samplesToProcess: samplesToProcess,
                           downSampledLength: downSampledLength,
                           samplesPerPixel: samplesPerPixel,
                           filter: filter)
        }
        
        // Process the remaining samples at the end which didn't fit into samplesPerPixel
        let samplesToProcess = sampleBuffer.count / MemoryLayout<Int16>.size
        if samplesToProcess > 0 {
            let downSampledLength = 1
            let samplesPerPixel = samplesToProcess
            let filter = [Float](repeating: 1.0 / Float(samplesPerPixel), count: samplesPerPixel)
            
            processSamples(fromData: &sampleBuffer,
                           sampleMax: &sampleMax,
                           outputSamples: &outputSamples,
                           samplesToProcess: samplesToProcess,
                           downSampledLength: downSampledLength,
                           samplesPerPixel: samplesPerPixel,
                           filter: filter)
        }
        
        // if (reader.status == AVAssetReaderStatusFailed || reader.status == AVAssetReaderStatusUnknown)
        // Something went wrong. Handle it.
        if assetReader!.status == .completed {
            done(outputSamples, sampleMax)
        } else {
            print(assetReader!.status)
        }
    }

We have just checked in a big refactor #80. Could you please download the latest version and let us know what you see? I think this may be fixed.

@fulldecent diving into it now. One of the first things I noticed is that if you don't specify a value for zoomSamples, the waveform won't load at all because you initialize zoomSamples as empty:

zoomSamples = 0 ..< 0

I fixed this as such:

   /// Current audio context to be used for rendering
    private var audioContext: FDAudioContext? {
        didSet {
            waveformImage = nil
            zoomSamples = 0 ..< totalSamples

This makes it load fully zoomed out. I will let you know what else I find.

Edit: highlightedSamples calculation in handlePanGesture/handleTapGesture is broken as well:

highlightedSamples = 0 ..< zoomSamples.startIndex + Int(CGFloat(zoomSamples.startIndex) * recognizer.location(in: self).x / bounds.width)

I've changed it to this:

let rangeSamples = CGFloat(zoomSamples.endIndex - zoomSamples.startIndex)
highlightedSamples = 0 ..< Int((CGFloat(zoomSamples.startIndex) + rangeSamples * recognizer.location(in: self).x / bounds.width))

Also note that the highlighted waveform and the background waveform become misaligned when panning/pinching. Seems similar to #83.

The error from this issue seems to be fixed, though.

Applied this fix in 943d2cd, I believe this fixes the issue. Thank you! Please comment or reopen if this is NOT fixed.