Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

make compatible with cx_freeze in gui32 mode #69

Merged
merged 3 commits into from
Oct 9, 2014
Merged

Conversation

bobatsar
Copy link
Contributor

this should fix issues #67 and #68

Attention I also removed the IOError from conf.py as suggested in #66

@Zulko
Copy link
Owner

Zulko commented Sep 25, 2014

Thanks, I'll need to test and merge that when I have time (and just after I'll make that conf.py change to allow settings changes). These Popen changes are always a little dangerous. Can you confirm that everything works fine on your computer even when not using cx_freeze ? Can I ask why you use cx_freeze ? What kind of app are you making ?

@bobatsar
Copy link
Contributor Author

I am only using some basic operations (reading, converting and writing videos). This is working frozen (with cx_freeze) and without. I am unsing Win7.

I understand, that these Popen calls are the basic interface to the ffmpeg binary but the changes I made are very simple, just have a look at the commit.

We are using cx_freeze to build .exe files for customers in a win GUI application.
Our application is a sensor measurement system with video integration (sync video with other measurement data).

@bobatsar
Copy link
Contributor Author

Just one more thing to my pull request.
I am using Pycharm Community Edition as editor (in Win and Linux) and I like it very much. It removed all these unneded whitespaces from your files.
And also the github integration is very nice. This was my first github fork and pull request and it just worked.

It helps me to write much cleaner code and it suggested a lot of things to fix in moviepy code (according to PEP8).
I did not want to change too much in your code but perhaps this would help to make it more readable and more stable.

@Zulko Zulko merged commit 30a880b into Zulko:master Oct 9, 2014
@Zulko
Copy link
Owner

Zulko commented Oct 9, 2014

Merged :)
It's merged on the command line. As a consequence Github doesn't recognize you as a contributor (strange) but the commits history remembers you :)

Now I will put in place that "modifiable configuration" thing we talked about.

I don't like PyCharm because it's damn slow. I am now on Sublime but I inactivated the code cleaner.
I know my code needs more PEP8 but the linters often change too much of the code. I tend to disagree with some of the (very many) PEP8 rules. When my code becomes strict PEP8, sometimes I don't feel at home :)

@Zulko
Copy link
Owner

Zulko commented Oct 9, 2014

You added a threading option in write_videofile. Looks great. It seems to do nothing on my machine but I am only on dualcore. Does it make a great difference on your machine ?

@bobatsar
Copy link
Contributor Author

Yes it makes a difference. I am also on a dualcore machine and the cpu load raises from about 60% to 100% when using threads=2

On a laptop with an i7 with two cores and hyperthreading (I set the value to psutil.cpu_count() and this is here 4), the load raises from about 25% to 100%.

The ffmpeg doc says the thread option should use a good default, but it seems good to tweak it a bit.

But there is one issue to be aware of.

If on a windows 64bit version ffmpeg binaries with 32bit are used, the process crashes.
Win32 needs ffmpeg32 and Win64 needs ffmpeg64.

@bobatsar
Copy link
Contributor Author

I just rebased my git from your repo and realized, the threads parameter was lost. I just submitted a new pull request.

Wow, just realised you already merge the request. Quite fast :-)

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.

None yet

3 participants