python-video-converter icon indicating copy to clipboard operation
python-video-converter copied to clipboard

Windows compatible?

Open sbmthd opened this issue 13 years ago • 12 comments

I'm trying to use the module on Windows 7 64bit with python 2.7 and get this error when trying to invoke the convert method of the Converter class -

File "C:\python27\lib\subprocess.py", line 637, in init raise ValueError("close_fds is not supported on Windows " ValueError: close_fds is not supported on Windows platforms if you redirect stdi n/stdout/stderr

Is there a work around for this or is the module simply incompatible with windows?

sbmthd avatar Jul 03 '12 14:07 sbmthd

I haven't tried it on Windows, but it should be compatible in general.

The particular error you mention is due to the way I used subprocess.Popen here: https://github.com/senko/python-video-converter/blob/master/converter/ffmpeg.py#L263

If you can test whether it works on Windows if close_fds argument is removed, I'd gladly accept a pull request.

senko avatar Jul 03 '12 21:07 senko

as I need it for a current project I am working on a windows compatible version supporting current ffmpeg build from http://ffmpeg.zeranoe.com/builds/# and as soon as its finished I will send you a pull request

Marvin avatar Nov 16 '12 09:11 Marvin

Great, thanks!

senko avatar Nov 16 '12 09:11 senko

hi guys,does it work on windows properly now ? i means i just download the latest version and test it.i am petty sure i was already install ffmpeg.but when i run it occur a error said not found ffmpeg.so.i dig into souces and change the detect function.as the below pictures.

qq20140113170632

but still raise ValueError("close_fds is not supported on Windows ") any help will be thankful :smile:

xalexchen avatar Jan 13 '14 09:01 xalexchen

I have a patch to port to Windows. In addition to the above mentioned post, the SIGALRM is not supported on Windows (see doc for signal.alarm), so I have turned off timeout on Win.

Does anyone know how to turn off markdown while posting something? I am trying to share a patch file I generated. But I can't attach it (unsupported format), nor can I copy paste the contents, as github is messing up the formatting. I tried wrapping the text with html div tags, but that is removing the newline characters, making it worse. I'll be glad to email the patch to the author if you can give me the address.

karthikveeramani avatar Jan 08 '15 06:01 karthikveeramani

OK I figured out how to post my patch without formatting issues.

Authors, could you please merge this for everyone's benefit?

At a later time, someone (or I, if I find the time) could code the support for timeout on Windows, perhaps using a thread that sleeps for the timeout period and calls the alarm function. For now, since it doesn't work anyway on Windows (throws exception), I've turned it off.

converter/ffmpeg.py | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/converter/ffmpeg.py b/converter/ffmpeg.py
index 6b14133..b40c2e1 100644
--- a/converter/ffmpeg.py
+++ b/converter/ffmpeg.py
@@ -7,11 +7,13 @@ import signal
 from subprocess import Popen, PIPE
 import logging
 import locale
+import platform

 logger = logging.getLogger(__name__)

 console_encoding = locale.getdefaultlocale()[1] or 'UTF-8'

+windows = platform.system() == 'Windows'

 class FFMpegError(Exception):
     pass
@@ -321,21 +323,21 @@ class FFMpeg(object):

         def which(name):
             path = os.environ.get('PATH', os.defpath)
-            for d in path.split(':'):
+            for d in path.split(os.pathsep):
                 fpath = os.path.join(d, name)
                 if os.path.exists(fpath) and os.access(fpath, os.X_OK):
                     return fpath
             return None

         if ffmpeg_path is None:
-            ffmpeg_path = 'ffmpeg'
+            ffmpeg_path = 'ffmpeg.exe' if windows else 'ffmpeg'

         if ffprobe_path is None:
-            ffprobe_path = 'ffprobe'
+            ffprobe_path = 'ffprobe.exe' if windows else 'ffprobe'

-        if '/' not in ffmpeg_path:
+        if os.path.sep not in ffmpeg_path:
             ffmpeg_path = which(ffmpeg_path) or ffmpeg_path
-        if '/' not in ffprobe_path:
+        if os.path.sep not in ffprobe_path:
             ffprobe_path = which(ffprobe_path) or ffprobe_path

         self.ffmpeg_path = ffmpeg_path
@@ -351,7 +353,7 @@ class FFMpeg(object):
     def _spawn(cmds):
         logger.debug('Spawning ffmpeg with command: ' + ' '.join(cmds))
         return Popen(cmds, shell=False, stdin=PIPE, stdout=PIPE, stderr=PIPE,
-                     close_fds=True)
+                     close_fds=(not windows))

     def probe(self, fname, posters_as_video=True):
         """
@@ -422,6 +424,9 @@ class FFMpeg(object):
         cmds.extend(opts)
         cmds.extend(['-y', outfile])

+        # Windows does not support SIGALRM - need another mechanism to timeout
+        if windows: timeout = None
+
         if timeout:
             def on_sigalrm(*_):
                 signal.signal(signal.SIGALRM, signal.SIG_DFL)

karthikveeramani avatar Jan 08 '15 06:01 karthikveeramani

Bump

MJJMeijerink avatar Apr 14 '17 14:04 MJJMeijerink

It's still not Windows compatible

xcom169 avatar Aug 29 '19 18:08 xcom169

So, someone posted this up onto the python package manager thing, and i just discovered, after doing most of this fixing legwork shown above in one evening myself, that, this repo is the original source which the author jamieoglindsey never pointed to but just pushed up into https://pypi.org/project/PythonVideoConverter/ without credit? Not sure, but it looks that way. So I'm tempted to make a patch, and try apply it to this repo and PR it here.

I have emailed the pypi thingy package owner, if they don't respond, will test and raise a PR in a few days. Not guaranteeing the ported changes i have is 100% handling all error cases, but happy to address any as they come.

zaphodikus avatar Nov 10 '21 18:11 zaphodikus

Hey @zaphodikus thanks for the note and apologies for the delayed response!

I've really dropped the ball on this project a few years ago and am not maintaining (or using) it currently. I would love if someone actively using it would take over the ownership. Looking at the GitLab repo, it seems like that fork had a lot of additional work done and if Jaime and team are interested in actively maintaining it, I would have no objection (other than asking to re-add original authors back to AUTHORS.txt alongside new contributors).

senko avatar Dec 11 '21 13:12 senko

Ah, just noticed your response, the gitlab repo fork, where is that? Yes I'm still using it every so often. Will try reach out to Jamie again to be sure to credit people and work out how to merge any easier looking PRs waiting here.

I'll have to set up so I can run the tests on a quick unix-like box and add loads more test cases first, then yes I can help to maintain it @senko .

zaphodikus avatar Jan 25 '22 21:01 zaphodikus

@zaphodikus looks like this is the repo: https://gitlab.com/jamieoglindsey0/python-video-converter/

senko avatar Mar 10 '22 16:03 senko