Repository: Zulko/moviepy

Issue:
FFMPEG VideoWriter throws TypeError on error when logged.
First of all, thanks for a great project! It's really useful :+1: 

Here's a small bug that was actually already raised in #593. Seems that the error checking assumes the output is still in the subprocess stream even when it's been redirected elsewhere.

### Expected Behavior
Errors out with a relevant exception message if ffmpeg has a problem (e.g. with parameters), and no more.

### Actual Behavior
Errors out, and _During handling of the above exception, another exception occurred_

```
Traceback (most recent call last):
  File "/home/nick/workspace/PYTHON/synchronar/src/slides.py", line 86, in <module>
    write_frames(ffmvw, DURATION * FPS)
  File "/home/nick/workspace/PYTHON/synchronar/src/slides.py", line 61, in write_frames
    ffmvw.write_frame(frame)
  File "/home/nick/.virtualenvs/synchronar-Hr0bFsh9/lib/python3.6/site-packages/moviepy/video/io/ffmpeg_writer.py", line 143, in write_frame
    if b"Unknown encoder" in ffmpeg_error:
TypeError: argument of type 'NoneType' is not iterable
```

### Steps to Reproduce the Problem
Redirect the logging output, and cause an error. 

#### Minimal Code
```
import sys
import numpy as np
from moviepy.video.io.ffmpeg_writer import FFMPEG_VideoWriter as FVW

writer = FVW('out.avi', (320, 200), 30, logfile=sys.stderr, codec='INVALID')
writer.write_frame(np.zeros((320,200,3))
```

### Specifications

  - Python Version: 3.6
  - Moviepy Version: 0.2.3.5
  - Platform Name: Ubuntu Linux
  - Platform Version: 18.10


Discussion:
coveralls: 
[![Coverage Status](https://coveralls.io/builds/28666773/badge)](https://coveralls.io/builds/28666773)

Coverage increased (+0.02%) to 65.029% when pulling **082f1d2caf8d8272b1c172a2274bab27c6d822b1 on declension:fix-logged-ffmpeg-type-error** into **f670d1ab0c1621b39910d506772ee36de064eb5e on Zulko:master**.

declension: @Overdrivr Sorry - I never saw the feedback. I've updated as suggested...
mgaitan: @declension this is ok for me and I would like to merge it. Could you fix the conflicts? 
mgaitan: maybe we should rebase it against `v2` @tburrows13 ?
tburrows13: That's the conflicts fixed (just through the web editor). If the tests pass, feel free to merge.

Patch:
diff --git a/README.rst b/README.rst
index b1f79ef..62003b4 100644
--- a/README.rst
+++ b/README.rst
@@ -125,7 +125,7 @@ Running `build_docs` has additional dependencies that require installation.
 
 .. code:: bash
 
-    $ (sudo) pip install moviepy[docs]
+    $ (sudo) pip install moviepy[doc]
 
 The documentation can be generated and viewed via:
 
diff --git a/moviepy/video/io/ffmpeg_writer.py b/moviepy/video/io/ffmpeg_writer.py
index e82f53f..ff464a8 100644
--- a/moviepy/video/io/ffmpeg_writer.py
+++ b/moviepy/video/io/ffmpeg_writer.py
@@ -81,7 +81,7 @@ class FFMPEG_VideoWriter:
 
         if logfile is None:
             logfile = sp.PIPE
-
+        self.logfile = logfile
         self.filename = filename
         self.codec = codec
         self.ext = self.filename.split(".")[-1]
@@ -137,7 +137,12 @@ class FFMPEG_VideoWriter:
         try:
             self.proc.stdin.write(img_array.tobytes())
         except IOError as err:
+            logs = self.logfile.name
             _, ffmpeg_error = self.proc.communicate()
+            if not ffmpeg_error:
+                with open(logs, "rb") as f:
+                    ffmpeg_error = f.read()
+
             error = str(err) + (
                 "\n\nMoviePy error: FFMPEG encountered "
                 "the following error while writing file %s:"


Test patch:
diff --git a/tests/test_VideoClip.py b/tests/test_VideoClip.py
index c9e4d93..a9b5683 100644
--- a/tests/test_VideoClip.py
+++ b/tests/test_VideoClip.py
@@ -27,6 +27,17 @@ def test_check_codec():
     close_all_clips(locals())
 
 
+def test_errors_with_redirected_logs():
+    """Checks error cases return helpful messages even when logs redirected
+    See https://github.com/Zulko/moviepy/issues/877"""
+    clip = VideoFileClip("media/big_buck_bunny_432_433.webm")
+    location = os.path.join(TMP_DIR, "logged-write.mp4")
+    with pytest.raises(IOError) as e:
+        clip.write_videofile(location, codec="nonexistent-codec", write_logfile=True)
+    assert "Unknown encoder 'nonexistent-codec'" in str(e.value)
+    close_all_clips(locals())
+
+
 def test_save_frame():
     clip = VideoFileClip("media/big_buck_bunny_432_433.webm")
     location = os.path.join(TMP_DIR, "save_frame.png")