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

MoviePy fixes ? #3

Closed
Zulko opened this issue Jun 20, 2014 · 10 comments
Closed

MoviePy fixes ? #3

Zulko opened this issue Jun 20, 2014 · 10 comments

Comments

@Zulko
Copy link

Zulko commented Jun 20, 2014

Hi there,

First: bravo ! It's a very cool project, and it made my day !

I noticed that your script videogrep.py uses MoviePy in a way that could be improved, so here are a few remarks.

Remark 1

For every video segment assembled in create_supercut, you create a clip by calling VideoFileClip. This is bad, as every time you call VideoFileClip, MoviePÿ creates a new FFMPEG process to communicate with the file, and it takes 10-20 Mo of RAM. If you have 30 segments inside the same video file, you should call VideoFileClip one time only on this video file, then cut different subclips from that one instance.

Remark 2

The function concatenate ignores the starting and ending times specified by the set_start and set_end functions, actually it will recompute these times (like what you did with your cumulative 'time' variable).

Putting remarks 1 and 2 together, here is how I would have implemented your create_supercut (note that I separate the 'padding addition' from the actual video editing):

def create_supercut(composition, outputfile, padding):

    print ("Creating clips.")

    # add padding when necessary
    for (clip, nextclip) in in zip(composition, composition[1:]):
        if ( ( nextclip['file'] == clip['file'] ) and
             ( nextclip['start'] < clip['end'] )):
            nextclip['start'] += padding

    # put all clips together:
    all_filenames = set([c['file'] for c in composition)
    videofileclips = dict([(f, VideoFileClip(f)) for f in all_filenames])
    cut_clips = [videofileclips[ c['file']  ].subclip(c['start'], c['end'])
                       for c in composition]     
    final_clip = concatenate( cut_clips)
    final_clip.to_videofile(outputfile)

Now some minor remarks:

Minor remark 1

In my examples I often use

from moviepy.editor import *

but for long scripts it is a little frowned upon, your scripts we be better understandable if you state what you import

from moviepy.editor import VideoFileClip, concatenate

The moviepy.editor module is meant for editing videos "by hand" in an interactive way, so it loads a lot of things (and starts a PyGame session) and can slow down your program at the beginning. Write rather this (the program will only fetch what it needs):

from moviepy.video.io.VideoFileClip import VideoFileClip
from moviepy.video.compositing.concatenate import concatenate

Minor remark 2

Apparently you have been a victim of the bug that makes that every time you generate a video file it creates an unwanted log file for the audio. I wasn't aware of this bug (it has been there in the last two months), but yesterday someone commited a fix for it, so the last version of MoviePy doesn't have it (no logfile is generated by default). So your 'cleaning' function is no longer needed.

Minor non-moviepy-related remarks

Maybe you should consider making a proper Python module (with a setup.py). This would come with advantages, like easy 'pip' install, automatic installation of the dependencies 'pattern' and 'moviepy' during the installation of videogrep, and automatic installation of the executable scripts like videogrep.py, so that they could be launched from anywhere, not just the folder where the script is.

Also, you could have a look at the python module "docopt", it makes the command line interface very easy to code and debug.

I hope this helped ! If you have any questions or request concerning MoviePy let me know.

Cheers !

@antiboredom
Copy link
Owner

Thank you so much for all of this! Extremely helpful. And of course thank you for MoviePy - it's really wonderful.

On Jun 20, 2014, at 12:17 PM, Zulko notifications@github.com wrote:

Hi there,

First: bravo ! It's a very cool project, and it made my day !

I noticed that your script videogrep.py uses MoviePy in a way that could be improved, so here are a few remarks.

Remark 1

For every video segment assembled in create_supercut, you create a clip by calling VideoFileClip. This is bad, as every time you call VideoFileClip, MoviePÿ creates a new FFMPEG process to communicate with the file, and it takes 10-20 Mo of RAM. If you have 30 segments inside the same video file, you should call VideoFileClip one time only on this video file, then cut different subclips from that one instance.

Remark 2

The function concatenate ignores the starting and ending times specified by the set_start and set_end functions, actually it will recompute these times (like what you did with your cumulative 'time' variable).

Putting remarks 1 and 2 together, here is how I would have implemented your create_supercut (note that I separate the 'padding addition' from the actual video editing):

def create_supercut(composition, outputfile, padding):

print ("Creating clips.")

# add padding when necessary
for (clip, nextclip) in in zip(composition, composition[1:]):
    if ( ( nextclip['file'] == clip['file'] ) and
         ( nextclip['start'] < clip['end'] )):
        nextclip['start'] += padding
    duration = end - start

# put all clips together:
all_filenames = set([c['file'] for c in composition)
videofileclips = dict([(f, VideoFileClip(f)) for f in all_filenames])
cut_clips = [videofileclips[ c['file']  ].subclip(c['start'], c['end'])
                   for c in composition]     
final_clip = concatenate( cut_clips)
final_clip.to_videofile(outputfile)

Now some minor remarks:

Minor remark 1

In my examples I often use

from moviepy.editor import *
but for long scripts it is a little frowned upon, your scripts we be better understandable if you state what you import

from moviepy.editor import VideoFileClip, concatenate
The moviepy.editor module is meant for editing videos "by hand" in an interactive way, so it loads a lot of things (and starts a PyGame session) and can slow down your program at the beginning. Write rather this (the program will only fetch what it needs):

from moviepy.video.io.VideoFileClip import VideoFileClip
from moviepy.video.compositing.concatenate import concatenate
Minor remark 2

Apparently you have been a victim of the bug that makes that every time you generate a video file it creates an unwanted log file for the audio. I wasn't aware of this bug (it has been there in the last two months), but yesterday someone commited a fix for it, so the last version of MoviePy doesn't have it (no logfile is generated by default). So your 'cleaning' function is no longer needed.

Minor non-moviepy-related remarks

Maybe you should consider making a proper Python module (with a setup.py). This would come with advantages, like easy 'pip' install, automatic installation of the dependencies 'pattern' and 'moviepy' during the installation of videogrep, and automatic installation of the executable scripts like videogrep.py, so that they could be launched from anywhere, not just the folder where the script is.

Also, you could have a look at the python module "docopt", it makes the command line interface very easy to code and debug.

I hope this helped ! If you have any questions or request concerning MoviePy let me know.

Cheers !


Reply to this email directly or view it on GitHub.

@Zulko
Copy link
Author

Zulko commented Jun 22, 2014

Hey !

I see you are getting more Github stars in a few days than MoviePy over it's one year of existence. I would be jealous but since MoviePy clearly benefited from the success of Videogrep I'd rather thank you !

I couldn't resist, I wrote a blog post with some more videogreping. In the post, I try to grep not only subtitles blocks, but the whole sentences where the expression appears, possibly spanning several subtitles blocks: this makes cleaner and more interesting cuts. I also make a lame attempt at greping single words (that is, cutting the scene tightly to keep just the word we are looking for). I don't know if you already tried this, but I thought you could be interested :)

@ghost
Copy link

ghost commented Jun 22, 2014

I'd not heard of moviepy until I saw videogrep. Both are awesome tools. I've uploaded a python script called wordsworth, which does frequency analysis on text files. If you give it a subtitles file you can find out which are the most commonly occurring word, word pairs, triples and quads. I use it to find candidate search terms for videogrep.

@antiboredom
Copy link
Owner

Hey @Zulko - love the blog post! Really great stuff.

As far as the stars thing goes it's pretty nuts. Your work totally made videogrep possible - and I really hope that more and more people will learn about & use MoviePy. Also, if you're in NY (or pass through at some point) let me know. Beers on me.

@shawnr
Copy link

shawnr commented Jun 24, 2014

@antiboredom @autonomoid @Zulko I love finding new troves of projects. This is a great example. I'm also new to MoviePy, but it's going in my toolkit!

@ghost
Copy link

ghost commented Jun 24, 2014

I've just written a little tool that lets you check which codecs and file extensions your ffmpeg installation supports. Its currently in my fork. Let me know if this is something you want me to integrate into videogrep. This way you can do support checking at runtime.

https://github.com/autonomoid/videogrep/blob/master/ffmpeg_support.py

@antiboredom
Copy link
Owner

@autonomoid that sounds really useful - perhaps it could be integrated into MoviePy itself? What do you think @Zulko?

@Zulko
Copy link
Author

Zulko commented Jun 24, 2014

The best move would be to first change the default codec to 'libmp3lame' in Videogrep (and maybe in moviepy). Then yes you could integrate @automonoid's ffmpeg_support.py, that may come in handy if people still have issues.

On MoviePy's side, my focus it to clarify the errors returned when something goes wrong with FFMPEG. I still didn't push it online (getting error handling to work in a cross-platform and cross-version way is more difficult than it sounds in Python) but here is what you will get when it's done:

IOError: [Errno 32] Broken pipe
MoviePy error: FFMPEG encountered the following error while writing file testTEMP_MPY_to_videofile_SOUND.m4a:

Unknown encoder 'libfdk_aac'

MoviePy error: the audio export failed because FFMPEG didn't find the specified codec for audio  encoding (libfdk_aac). Please install this codec or change the codec when calling to_videofile or to_audiofile. For instance for mp3:
  >>> to_videofile('myvid.mp4', audio_codec='libmp3lame')

That should greatly help people understand the issue and fix the problems by themselves.

@shawnr
Copy link

shawnr commented Jun 24, 2014

That would be excellent error feedback, @Zulko!

@Zulko
Copy link
Author

Zulko commented Jul 5, 2014

The error feedbacks are now better in moviepy, it should make debugging easier. And the default codec for audio is now libmp3lame. I hope this will help.

And if you guys want some more automatic cutting, here is a ~20 lines script which summarizes soccer videos based on the reactions of the crowd :)

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

No branches or pull requests

3 participants