Wait for the player process to exit #236

Closed
wants to merge 4 commits into
from

Conversation

Projects
None yet
2 participants
@vadmium

vadmium commented Dec 9, 2013

Here’s a one-line fix to always wait for the player process, as discussed in Issue #234. Also included is a fix to close a non-Windows named pipe to avoid deadlocking, and a better version of one of my earlier ResourceWarning fixes.

I suspect no equivalent fix for the Windows named pipe is needed, but I haven’t tested it. However, reading the documentation it looks like you should probably be calling FlushFileBuffers() before DisconnectNamedPipe(), and CloseHandle() afterwards.

Martin Panter added some commits Nov 28, 2013

Martin Panter
cli: Close the file descriptor as well as deleting the pipe file.
This avoids a ResourceWarning, and explicitly signals the end of the stream
to the player, which will avoid a deadlock if we want to wait for it to exit.
Martin Panter
cli: Always wait for the player process to exit.
The “--player-no-close” option would previously let the player continue to
run when Livestreamer exited. This meant that control could return to the
shell with the player still running in a background process. When
Livestreamer has finished sending the stream, it now blocks until the player
exits.

This avoids awkward behaviours with M Player and MPV, which do not work well
in some cases when running as a background process. I noticed that if their
standard input was _not_ used to stream the media (e.g. “--player-http”
mode), then they would change the terminal settings after Livestreamer
exited. This would interfere with Bash and Readline.

Apparently also fixes #234, avoiding a Screen session from terminating too
early.
@chrippa

This comment has been minimized.

Show comment
Hide comment
@chrippa

chrippa Dec 9, 2013

Owner

Thanks, but please use rebase instead of merge to bring your tree up to sync next time. I don't want your merge commits in my tree. :)

Owner

chrippa commented Dec 9, 2013

Thanks, but please use rebase instead of merge to bring your tree up to sync next time. I don't want your merge commits in my tree. :)

@chrippa chrippa closed this Dec 9, 2013

@vadmium vadmium deleted the vadmium:wait branch Feb 13, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment