OliverF / mjpeg-relay

Relays any given MJPEG stream
MIT License
66 stars 23 forks source link

Better shutdown #14

Open Silex opened 7 years ago

Silex commented 7 years ago

Only the last commit is relevant here, as the others are from #13

I'll rebase this PR onces #13 is merged.

This basically removes all the foo.kill. It looks like "it works" but I didn't give it a lot of testing.

By the way, does https://github.com/Silex/mjpeg-relay/blob/217904d862cade494d031667d9e30884a9e1e34d/relay.py#L26 really make sense? I know it works, but theorically how can quit() access global variables here? I guess I don't know python enough :stuck_out_tongue_closed_eyes:

I think it's simpler to move the quit() body at the bottom of the try block, and remove the quit() calls. There's no real need to have multiple exit paths.

I was thinking of refactoring a bit more and replacing the foo.connected thing with a function instead, that way we can get rid of the variable and the check is always accurate (simply return the socket state).

Also, each of these "servers" should have a saner start/stop mechanism. At the moment the thread thing for each of them is a bit weird... not the thread in itself but its permanence. I think in general, "start" should start listening/open the socket and spawn a thread, and stop should close the socket/server and kill the thread.

I'll see what I can do, and ping you when I have something. Give me your thoughts in the meantime :wink:

p.s: you code on windows? I was a bit surprised by the windows line endings.

OliverF commented 7 years ago

Regarding foo.kill please see my comment on the code.

quit() works but I suppose it should mark the variables as global scope. A better solution is to just pass them as parameters!

Making foo.connected a function makes sense, that way we can get the true value of the socket like you say.

And I agree, the start/stop needs a re-think :)

P.S. yes I do - I love Linux and I'd switch in a heartbeat, but sadly a lot of my programs run on Windows only. One day :)