codelucas / vsummarize

[OUT OF DATE] I only made this repo public since I'm out of Github credit, don't use it.
MIT License
20 stars 5 forks source link

Hello Zulko! Here is a message for you. #1

Open codelucas opened 10 years ago

codelucas commented 10 years ago

Hello @Zulko! I couldn't find your email or even your identity on your Github/blog! So i'm contacting you through this. I was the guy who posted the issue about .mp4 audio not working.

I loved your moviepy library! I wanted to write a video summarization library which uses it.

If you feel like you have anything to add on, feel free to tell me! Otherwise, if you are busy, please don't feel the need to waste your time on this :p

Zulko commented 10 years ago

That's a good thing you can't find my identity :)

Here is my feedback: the code of summarize as I would have written it, with some comments:

# moviepy.editor is meant to be used in a console/interactive session
# moviepy.editor will load many unwanted things and open PyGame
# Instead, just load what you need.

from moviepy.video.io.VideoFileClip import VideoFileClip
from moviepy.video.VideoClip import TextClip
from moviepy.video.compositing.concatenate import concatenate
from moviepy.video.compositing.CompositeVideoClip import CompositeVideoClip

def summarize(filepath, new_filename, hotclips):
    """
    Inputs a filepath for a video and generates a new shorter video
    in that same filepath.
    """

    # Only open the file once !
    video = VideoFileClip(filepath)

    # making subclips should never raise errors.
    # No need to try-except
    chunks = [ video.subclip(start, end)
               for (start, end) in hotclips]

    summarized_video = concatenate(chunks)

    txt_clip = ( TextClip("Generated by vSummarize",
                          fontsize=20, color='white')
                 .set_pos('bottom')
                 .set_duration(5))

    final_clip = CompositeVideoClip([summarized_video,
                                     txt_clip])

    try:
        # Use the to_videofile default codec, libx264
        # libx264 is much better than mpeg4, and still writes .mp4
        # Use the fps of the original video.
        final_clip.to_videofile(new_filename,
                                fps= video.fps,
                                audio_codec='mp3')
    except Exception, e:
        # Dangerous ? I believe this masks the call stack.
        # So it makes debugging more complicated.
        print 'ERROR when generating output video', str(e)

# EXAMPLE (WORKS)
summarize("./../videos/frozen_trailer.mp4",
          "sumtest.mp4",
          hotclips = [(1,5), (15,20), (35,40)]) 
codelucas commented 10 years ago

Thanks so much! I'll be sure to leave you credits in this repo before open sourcing it!

Zulko commented 10 years ago

Hey !

I have fixed a few bugs in moviepy that I think are important regarding vsummarize: for instance when you concatenated clips the audio tended to leak a little from one clip to another, it is now fixed.

In the readme you say "moviepy manipulates your terminal console environment.. it's a bit funky.". I have cleaned a little the terminal interface. Have you tried to do to_videofile('test.mp4', verbose=False) ? In the current version this should only show the progress bars (and they are removed once the file is written).

Still in the readme you say that moviepy requires PyGame, but PyGame is now completely optional, especially if moviepy is used through a program that does all the work like vsummarize. PyGame is only required if you want to edit your module 'by hand'.

Cheers !

codelucas commented 10 years ago

Thanks a ton for this update! I will be sure to update my moviepy tonight and tell you how things run afterwards.

I have not tried the new to_videofile(...) method, I will also try that tonight and update the README.md if things are better.

I will remove PyGame as a requirement, sorry for that. I did not read the documentation for your module carefully enough :)

codelucas commented 10 years ago

Hmm, sorry but I have one last issue:

When I am running the contents of video.py: python2.7 video.py which was written with help from you, I seem to be getting a very weird error where the program finishes encoding audio and ALMOST finishes the video part before stalling forever and never terminating. As a result, I have to ctrl-c exit and a TEMP_MPY_to_videofile.mp4 and TEMP_MPY_to_videofile_SOUND.mp3 are left in the working directory.

selection

This issue resides on my OSX machine with ffmpeg installed via homebrew. The most unusual thing about all of this is that 1-2 weeks ago when I tried this on OSX, it worked fine. But after I made some changes (and also added a few updates from you) and re-updated moviepy. This weird stalling error happened.

Do you have any ideas of what may be the cause?

Here is another image of the stalling occurring, this time without the output being piped into a log file. (This was 20mins after running the command). screen shot 2014-02-28 at 11 47 19 am

Thanks for your help once again.

Zulko commented 10 years ago

As always, thanks for the feedback.

Subprocess is a nightmare ! I had added this proc.wait command for Python3 compatibility and now it blocks in other platforms.

I just updated it (on Github) to replace wait by communicate(), as is suggested at many places on the web. Now it seems to work still fine on my computer with both Python2 and Python3, and there are (great) chances that it will work on your computer.

Could you update using the Github version and tell me if it works now?

Somewhat unrelated : apparently you printed the out.log here, but do you also get these strange progress bars after one another in a console ? This is strange, they are supposed to replace each other instead.

codelucas commented 10 years ago

The progress bars are normal when the output isn't piped into a log file! No worries, I think it's some weird piping mechanism that stacks them like that.

When I uninstalled moviepy via pip and cloned and installed the github version, it still stalls but it seems to stall on a different line:

Here is the output after I press ctrl-c: screen shot 2014-02-28 at 2 37 33 pm

Zulko commented 10 years ago

Ok, I don't know... all I can suggest for know is that you go into file moviepy/video/io/ffmpeg_writer.py and you remove the line 78:

out, err = self.proc.communicate() # proc.wait()

This should work fine with Python2.7 (not Python3).

What is your system already ? And was your video particularly large-sized ?

codelucas commented 10 years ago

This is tough, after commenting out those lines, different errors popped up. I think I may resort to my library installing an older version of moviepy (before all of those py3 lines were added).

screen shot 2014-02-28 at 4 30 37 pm

I did this, but moviepy required pygame as a dependency, even when just importing the library. That's OK but not ideal.

I am currently debugging this on both OSX & Ubuntu. OSX stalls on proc.wait(), as we are currently trying to debug it. Ubuntu has some weird IO error which "closes the pipe". I wanna finish debugging the issue for OSX first though.

Thanks for your help so far.

Zulko commented 10 years ago

Ok, it seems that communicate() is not the solution. I have a few more ideas. Another user has this hanging and he suggested that the cause was the stderr. And indeed I don't close the stderr, maybe that is the issue. Could you try this form for the close method in moviepy/video/io/ffmpeg_writer.py around line 78 ?

def close(self):
    self.proc.stdin.close()
    self.proc.stderr.close()
    self.proc.wait()

If it is not this it might be a buffer issue, you should try to add bufsize=1000000 to the Popen command in the FFMPEG_VideoWriter.

Just to keep you in the light, here is what is happening (I think !) :

To make a movie MoviePy proceeds in 3 steps: write the audio, write the video, merge them. The problem is that the audio and video files can only be merged together when both are properly finished being written and the pipe is closed, especially in Python3 for some reason. So I added this wait command which makes it work fine on my computer but hangs when other people use it for some reason.

codelucas commented 10 years ago

Hmm, it still hangs on proc.wait(). Is there a fix I can perform to just simply make it not compatible for python 3 but have it work? When I remove proc.wait(), subprocess_call() gives me an IO error.

I can't revert back to a previous version of moviepy because it requires pygame and i'd rather not have users install that. The current version of moviepy (minus this hanging issue) is great though,