Bugfix retain on pluginMediaStreamTrack will not allow camera/mic to be freed

[oNaiPs]

No description provided.

[oNaiPs]

I noticed CPU was spinning at 20% after i getUserMedia and stop the stream. This fixes the issue, as the lambda was retaining the pluginMediaStreamTrack

[AntonioCurci]

@oNaiPs so no more PacerThread thread running after deallocating iosrtc/Cordova?

[oNaiPs]

I'm not sure if that's the thread, but yeah it stops at least one thread
from Webrtc and another one from AV foundation
On Wed, Feb 3, 2016 at 12:40 PM SirSeymour notifications@github.com wrote:

@oNaiPs https://github.com/oNaiPs so no more PacerThread thread running
after deallocating iosrtc/Cordova?

—
Reply to this email directly or view it on GitHub
#126 (comment)
.

[AntonioCurci]

Thank you for your response. So basically now you saw that from 20% to ~0% cpu usage?

[oNaiPs]

Yes indeed. The bug is reproducible with:

cordova.plugins.iosrtc.getUserMedia({
    audio: true,
}, function(stream) {
    console.log('Got stream.');
    stream.stop();
}, function(e) {
    console.log(e);
})

When stream stops, the retention would keep the camera running indefinitely

[ibc]

Amazing! May you please explain why the track was not freed before and why it is freed using weak var?

May be more places in which weak var should be used?

[oNaiPs]

Yes there will be more places for that to be used. It looks like swift would benefit from other syntax though.

Take a look at https://developer.apple.com/library/prerelease/ios/documentation/Swift/Conceptual/Swift_Programming_Language/AutomaticReferenceCounting.html, mainly on "Resolving Strong Reference Cycles for Closures"

[ibc]

Is Swift a joke? The problem of "Strong Reference Cycles" is resolved in so many languages with GC...

[ibc]

@oNaiPs should then things like this also be fixed?:

    func MediaStreamTrack_setEnabled(command: CDVInvokedUrlCommand) {
        NSLog("iosrtcPlugin#MediaStreamTrack_setEnabled()")

        let id = command.argumentAtIndex(0) as! String
        let value = command.argumentAtIndex(1) as! Bool
        let pluginMediaStreamTrack = self.pluginMediaStreamTracks[id]

        if pluginMediaStreamTrack == nil {
            NSLog("iosrtcPlugin#MediaStreamTrack_setEnabled() | ERROR: pluginMediaStreamTrack with id=\(id) does not exist")
            return;
        }

        dispatch_async(self.queue) {
            pluginMediaStreamTrack!.setEnabled(value)
        }
    }

to:

    func MediaStreamTrack_setEnabled(command: CDVInvokedUrlCommand) {
        NSLog("iosrtcPlugin#MediaStreamTrack_setEnabled()")

        let id = command.argumentAtIndex(0) as! String
        let value = command.argumentAtIndex(1) as! Bool
        weak var pluginMediaStreamTrack = self.pluginMediaStreamTracks[id]

        if pluginMediaStreamTrack == nil {
            NSLog("iosrtcPlugin#MediaStreamTrack_setEnabled() | ERROR: pluginMediaStreamTrack with id=\(id) does not exist")
            return;
        }

        dispatch_async(self.queue) {
            pluginMediaStreamTrack!.setEnabled(value)
        }
    }

[ibc]

I'm preparing a commit that replaces all the let xxxx = somecontainer[id] with weak var.

[oNaiPs]

@ibc yes, it'll apply to those variables that are used inside the lambda block afterwards.

[ibc]

Thanks. I've set weak var more aggressively in all the references retrieved from containers (for consistency):

ad7a0e3