Skip to content

display progress status for both mplayer and mpv#47

Merged
np1 merged 2 commits intomps-youtube:popen-non-slavefrom
thomasleveil:popen-non-slave/mpv-status
Mar 9, 2014
Merged

display progress status for both mplayer and mpv#47
np1 merged 2 commits intomps-youtube:popen-non-slavefrom
thomasleveil:popen-non-slave/mpv-status

Conversation

@thomasleveil
Copy link
Copy Markdown
Contributor

warning: tested only with mplayer. Make sure to test it with mpv before merging

@np1
Copy link
Copy Markdown
Contributor

np1 commented Mar 9, 2014

Fantastic! So far it's working well with mpv audio and video. Will do a bit more testing. Well done!

The stderr redirection in your most recent commit work. I was making very similar changes but was setting stderr=subprocess.PIPE within Popen.subprocess(). Was that wrong? I see you set stderr=subprocess.STDOUT

Thanks again!

@thomasleveil
Copy link
Copy Markdown
Contributor Author

stderr=subprocess.STDOUT tells popen to redirect any stderr output into stdout.
stdout=subprocess.PIPE tells open to pipe the stdout ouput (which now receives both stdout and stderr)
Then we just have to keep track of whatever data we can read from the popen object .stdout property instead of having to follow both .stdout and .stderr

@thomasleveil thomasleveil reopened this Mar 9, 2014
np1 added a commit that referenced this pull request Mar 9, 2014
display progress status for both mplayer and mpv
@np1 np1 merged commit b093287 into mps-youtube:popen-non-slave Mar 9, 2014
@thomasleveil thomasleveil deleted the popen-non-slave/mpv-status branch March 9, 2014 21:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants