thibauts / node-upnp-mediarenderer-client

An UPnP/DLNA MediaRenderer client
MIT License
125 stars 30 forks source link

.destroy(cb) #16

Closed jaruba closed 8 years ago

jaruba commented 9 years ago

How about a method to efficiently destroy the node-upnp-mediarenderer-client instance?

thibauts commented 9 years ago

What would it release and what would it exactly do ?

jaruba commented 9 years ago

If I cast a video and playback ends, the normal reaction should be to stop or restart (for a playlist) node-upnp-mediarenderer-client with a different video url.

So I would need a way to destroy the previous instance, I guess it should probably do client.stop(); (if needed) then disconnect, maybe remove listeners..

thibauts commented 9 years ago

There shouldn't be any listeners hanging, or memory leak when you dereference the client, by design. You only have to remove the listeners you have registered and everything will go fine. Maybe a close method that calls this.removeAllListeners for all possibly registered events would make it. If you submit a PR in this respect I'll happily merge it.

jaruba commented 9 years ago

how about client.server.close()?

thibauts commented 9 years ago

Why not a straight client.close() ?

jaruba commented 9 years ago

I meant that client also creates a server (it's actually created by upnp-device-client). So I guess that needs to be closed too.

Does this look correct to you?

MediaRendererClient.prototype.close = function(callback) {
    this.stop();
    this.removeAllListeners();
    if (this.server) this.server.close(callback || noop);
    else if (typeof callback === 'function') callback();
};

I also added this.stop() because the server can't be closed unless it stops being busy so without stopping the stream before running server.close() it will keep node-upnp-mediarenderer-client alive until the video ends and only then it will call the callback function.

Also, I know checking if this.server exists does not tell me if the server is actually running, but tbh I haven't found anything that can check if a server is active except listening for the listening event which doesn't help in this scenario..

Unfortunately I can't test this code until I get back to a Samsung TV which should be in a couple of days.

thibauts commented 9 years ago

The server is released as soon as you have unsubscribed from all events you had registered to. See this code