willprice / python-omxplayer-wrapper

:tv: Control OMXPlayer, the Raspberry Pi media player, from Python
http://python-omxplayer-wrapper.readthedocs.io
GNU Lesser General Public License v3.0
253 stars 72 forks source link

Make redirecting stdout/stderr optional #205

Open matthijskooijman opened 4 years ago

matthijskooijman commented 4 years ago

Feature Request

Description

Currently, omxplayer is always started with stdout redirected to /dev/null and stderr left untouched (e.g. print to the calling process's stderr). This sometimes throws away valuable information including errors (which omxplayer currently prints to stdout.

It would be helpful if this redirection could be disabled, or redirected to a file or a pipe. Maybe the default should be changed to not redirect?

For completeness, allowing redirects of stderr would also be helpful.

Implementing this could be a matter of passing the stdin/stdout arguments from the OMXPlayer constructor all the way to subprocess.

matthijskooijman commented 4 years ago

I had a little thought about this. An obvious implementation for this would be to pass stdin/stdout/stderr arguments to the OMXPlayer constructor, store them in attributes and use them in load(). Values could be passed as-is to Popen.

PIPE is probably not useful unless you can access the resulting Popen object externaly, but I think you can always create a pipe pair externally and pass one end as stdin/stdout/stderr.

To make it easier to write to files, you could also support passing a Path object, which omxplayerwrapper could then open and pass the resulting file object to Popen. This would also allow using Path(os.devnull) as the default value for stdin/stdout to keep existing behaviour.

However, this does make me wonder when, if at all, file objects should be closed. If omxplayerwrapper opens a file (e.g. devnull), it should probably close it, but when? When the process exits? But what if another load() is called and a new player is started? Re-open the file? Or would you expect content to be appended among runs? And for file objects passed externally, should the caller arrange closing them? That might require additional bookkeeping to figure out who opened which files?

Another approach is to pass stdin/stderr/stdout directly to load(). The arguments passed to the constructor are then only used for the initial load() call, any subsequent load() calls should have the streams explicitly specified. This probably makes it easier for the caller to decide whether to reopen files in between or not (though thinking on it, wrt appending, I'm not sure if keeping a file open will actually cause appending, since the writes are done in a subprocess, so the FD is duplicated when omxplayer is started).

So, not so trivial as I had hoped (which kept me from implementing this now, also since I really should be getting other things done), but I wanted to at least remember and share my thoughts.