xat / chromecast-player

simple chromecast player
MIT License
99 stars 20 forks source link

handle close-events correctly #5

Open xat opened 9 years ago

xat commented 9 years ago
JSteunou commented 9 years ago

I might be concern with this issue. I tried to made a simple web ui for my own needs, with 2 routes, one launching a cast on stream url, the other stopping the player.

It goes like this

var debug = require('debug')('webui');
var express = require('express');
var Player = require('chromecast-player');
var router = express.Router();

router.get('/launch', function(req, res, next) {
    var media = req.query.media;
    if (!media) return next(new Error('Missing media query param'));

    var options = {
        path: media,
        type: req.query.type
    };

    var player = new Player();
    player.attach(function(err, p) {
        if (err) return next(err);

        debug('launch player %o', options);
        p.load(options, function(err) {
            if (err) return next(err);
            res.status(200).send();
            // process.exit();
        });

    });
});

router.get('/stop', function(req, res, next) {

    var player = new Player();
    player.attach(function(err, p) {
        if (err) return next(err);

        debug('stop player');
        p.stop(function(err) {
            if (err) return next(err);
            res.status(200).send('ok');
            // process.exit();
        });
    });

});

module.exports = router;

But if I call my routes twice (like launch then stop) I got this error

device not found

Error: device not found
    at null._onTimeout (/home/jsteunou/git/castnow-webui/node_modules/chromecast-player/node_modules/chromecast-scanner/index.js:34:10)
    at Timer.listOnTimeout [as ontimeout] (timers.js:112:15)

You can see I tried to find a way around the error by killing the process after each success, letting my watchdog starting back the server. But that does not work well either.

What can I do to help?

xat commented 9 years ago

The player instance now can be destroyed like this:

player.launch('<url>', function(err, p, ctx) {
    setTimeout(function() {
        ctx.shutdown();
    }, 5000);

    ctx.on('closed', function() {
        console.log('player got shutdown');
    });
});

You should now also be able to launch the player multiple times, as long as you shutdown the instance which was running before.

JSteunou commented 9 years ago

thanks a lot, I'll try that!

JSteunou commented 9 years ago

Ok my code is a lot cleaner now, I did not have to call that shutdown method, or kill my process like before, and it works beautifully well!

var debug = require('debug')('webui');
var express = require('express');
var Player = require('chromecast-player');
var router = express.Router();

router.get('/launch', function(req, res, next) {
    var media = req.query.media;
    if (!media) return next(new Error('Missing media query param'));

    var options = {
        path: media,
        type: req.query.type
    };

    debug('launch player with options: %o', options);
    var player = new Player();
    player.launch(options, function(err, p, ctxt) {
        if (err) return next(err);

        p.once('playing', function(info) {
            debug('playing %o', info);
            res.status(200).send('ok');
        });

    });
});

router.get('/stop', function(req, res, next) {
    debug('attach to player');
    var player = new Player();
    player.attach(function(err, p, ctxt) {
        if (err) return next(err);

        debug('stop player');
        p.stop(function(err, info) {
            if (err) return next(err);
            debug('stopped %o', info);
            res.status(200).send('ok');
        });
    });
});

module.exports = router;

again thank you.

xat commented 9 years ago

Nice, but you should run the shutdown() method even if it works without. shutdown() will unlisten eventListeners and stop the setInterval created by the timeline helper. Otherwise it will leek memory.

JSteunou commented 9 years ago

ho ok thanks for the advice.