Fix Invalid Argument and Broken Pipe warnings, and more (#36)

* allow ffmpeg_timeout to be None

* tests check that no warnings are produces

* forgot to add

* add a test, put some test-snippets in there

* lint

* Fix Invalid Argument and Broken Pipe warnings

* reduce amount of downloads during tests

* verbose tests

Author: almarklein

imageio_ffmpeg/_io.py

@@ -238,9 +238,11 @@ def read_frames(
                 # So let's do similar to what communicate does, but without
                 # reading stdout (which may block). It looks like only closing
                 # stdout is enough (tried Windows+Linux), but let's play safe.
-                p.stdin.write(b"q")
-                p.stdin.close()
+                # Found that writing to stdin can cause "Invalid argument" on
+                # Windows # and "Broken Pipe" on Unix.
+                # p.stdin.write(b"q")  # commented out in v0.4.1
                 p.stdout.close()
+                p.stdin.close()
             except Exception as err:  # pragma: no cover
                 logger.warning("Error while attempting stop ffmpeg (r): " + str(err))
 
@@ -271,7 +273,7 @@ def write_frames(
     codec=None,
     macro_block_size=16,
     ffmpeg_log_level="warning",
-    ffmpeg_timeout=0,
+    ffmpeg_timeout=None,
     input_params=None,
     output_params=None,
 ):
@@ -306,7 +308,7 @@ def write_frames(
             to 1 to avoid block alignment, though this is not recommended.
         ffmpeg_log_level (str): The ffmpeg logging level. Default "warning".
         ffmpeg_timeout (float): Timeout in seconds to wait for ffmpeg process
-            to finish. Value of 0 will wait forever (default). The time that
+            to finish. Value of 0 or None will wait forever (default). The time that
             ffmpeg needs depends on CPU speed, compression, and frame size.
         input_params (list): Additional ffmpeg input command line parameters.
         output_params (list): Additional ffmpeg output command line parameters.
@@ -335,6 +337,7 @@ def write_frames(
     ffmpeg_log_level = ffmpeg_log_level or "warning"
     input_params = input_params or []
     output_params = output_params or []
+    ffmpeg_timeout = ffmpeg_timeout or 0
 
     floatish = float, int
     if isinstance(size, (tuple, list)):

tasks.py

@@ -27,7 +27,7 @@
 def test(ctx, cover=False):
     """Perform unit tests. Use --cover to open a webbrowser to show coverage.
     """
-    cmd = [sys.executable, "-m", "pytest", "tests"]
+    cmd = [sys.executable, "-m", "pytest", "tests", "-v"]
     cmd += ["--cov=" + LIBNAME, "--cov-report=term", "--cov-report=html"]
     ret_code = subprocess.call(cmd, cwd=ROOT_DIR)
     if ret_code:

tests/snippets.py

@@ -0,0 +1,50 @@
+"""
+Snippets of code that are hard to bring under test, but that can be
+used to manually test the behavior of imageip-ffmpeg in certain
+use-cases. Some may depend on imageio.
+"""
+
+# %%  Write a series of large frames
+
+# In earlier versions of imageio-ffmpeg, the ffmpeg process was given a timeout
+# to complete, but this timeout must be longer for longer movies. The default
+# is now to wait for ffmpeg.
+
+import os
+import numpy as np
+import imageio_ffmpeg
+
+ims = [
+    np.random.uniform(0, 255, size=(1000, 1000, 3)).astype(np.uint8) for i in range(10)
+]
+
+filename = os.path.expanduser("~/Desktop/foo.mp4")
+w = imageio_ffmpeg.write_frames(filename, (1000, 1000), ffmpeg_timeout=0)
+w.send(None)
+for i in range(200):
+    w.send(ims[i % 10])
+    print(i)
+w.close()
+
+
+# %% Behavior of KeyboardInterrupt / Ctrl+C
+
+import os
+import imageio_ffmpeg
+
+filename = os.path.expanduser("~/.imageio/images/cockatoo.mp4")
+reader = imageio_ffmpeg.read_frames(filename)
+
+meta = reader.__next__()
+
+try:
+    input("Do a manual KeyboardInterrupt now [Ctrl]+[c]")
+    # Note: Raising an error with code won't trigger the original error.
+except BaseException as err:
+    print(err)
+    print("out1", len(reader.__next__()))
+    print("out2", len(reader.__next__()))
+
+print("closing")
+reader.close()
+print("closed")

tests/test_io.py

@@ -1,49 +1,37 @@
-import os
+"""
+The main tests for the public API.
+"""
+
+import time
 import types
 import tempfile
-import time
-from urllib.request import urlopen
-
-from pytest import skip, raises
 
 import imageio_ffmpeg
 
-
-if os.getenv("TRAVIS_OS_NAME") == "windows":
-    skip(
-        "Skip this on the Travis Windows run for now, see #408", allow_module_level=True
-    )
-
-
-test_dir = tempfile.gettempdir()
-test_url1 = "https://raw.githubusercontent.com/imageio/imageio-binaries/master/images/cockatoo.mp4"
-test_url2 = "https://raw.githubusercontent.com/imageio/imageio-binaries/master/images/realshort.mp4"
-test_file1 = os.path.join(test_dir, "cockatoo.mp4")
-test_file2 = os.path.join(test_dir, "test.mp4")
-test_file3 = os.path.join(test_dir, "realshort.mp4")
+from pytest import skip, raises
+from testutils import no_warnings_allowed
+from testutils import ensure_test_files, test_dir, test_file1, test_file2, test_file3
 
 
 def setup_module():
-    bb = urlopen(test_url1, timeout=5).read()
-    with open(test_file1, "wb") as f:
-        f.write(bb)
-    bb = urlopen(test_url2, timeout=5).read()
-    with open(test_file3, "wb") as f:
-        f.write(bb)
+    ensure_test_files()
 
 
+@no_warnings_allowed
 def test_ffmpeg_version():
     version = imageio_ffmpeg.get_ffmpeg_version()
     print("ffmpeg version", version)
     assert version > "3.0"
 
 
+@no_warnings_allowed
 def test_read_nframes():
     nframes, nsecs = imageio_ffmpeg.count_frames_and_secs(test_file1)
     assert nframes == 280
     assert 13.80 < nsecs < 13.99
 
 
+@no_warnings_allowed
 def test_reading1():
 
     # Calling returns a generator
@@ -67,6 +55,7 @@ def test_reading1():
     assert count == 280
 
 
+@no_warnings_allowed
 def test_reading2():
     # Same as 1, but using other pixel format
 
@@ -83,6 +72,7 @@ def test_reading2():
     assert count == 280
 
 
+@no_warnings_allowed
 def test_reading3():
     # Same as 1, but using other fps
 
@@ -99,6 +89,7 @@ def test_reading3():
     assert 50 < count < 100  # because smaller fps, same duration
 
 
+@no_warnings_allowed
 def test_reading4():
     # Same as 1, but wrong, using an insane bpp, to invoke eof halfway a frame
 
@@ -113,6 +104,7 @@ def test_reading4():
     assert "ffmpeg version" in msg  # The log is included
 
 
+@no_warnings_allowed
 def test_reading5():
     # Same as 1, but using other pixel format and bits_per_pixel
     bits_per_pixel = 12
@@ -137,6 +129,7 @@ def test_reading5():
     assert count == 36
 
 
+@no_warnings_allowed
 def test_reading_invalid_video():
     """
     Check whether invalid video is
@@ -156,6 +149,7 @@ def test_reading_invalid_video():
     assert end - start < 1, "Metadata extraction hangs"
 
 
+@no_warnings_allowed
 def test_write1():
 
     for n in (1, 9, 14, 279, 280, 281):
@@ -184,6 +178,7 @@ def test_write1():
         assert count == n
 
 
+@no_warnings_allowed
 def test_write_pix_fmt_in():
 
     sizes = []
@@ -204,6 +199,7 @@ def test_write_pix_fmt_in():
     assert sizes[0] <= sizes[1] <= sizes[2]
 
 
+@no_warnings_allowed
 def test_write_pix_fmt_out():
 
     sizes = []
@@ -224,6 +220,7 @@ def test_write_pix_fmt_out():
     assert sizes[0] < sizes[1]
 
 
+@no_warnings_allowed
 def test_write_wmv():
     # Switch to MS friendly codec when writing .wmv files
 
@@ -240,6 +237,7 @@ def test_write_wmv():
         assert meta["codec"].startswith(codec)
 
 
+@no_warnings_allowed
 def test_write_quality():
 
     sizes = []
@@ -260,6 +258,7 @@ def test_write_quality():
     assert sizes[0] < sizes[1] < sizes[2]
 
 
+@no_warnings_allowed
 def test_write_bitrate():
 
     # Mind that we send uniform images, so the difference is marginal
@@ -282,6 +281,7 @@ def test_write_bitrate():
     assert sizes[0] < sizes[1] < sizes[2]
 
 
+# @no_warnings_allowed --> will generate warnings abiut macro block size
 def test_write_macro_block_size():
 
     frame_sizes = []
@@ -304,6 +304,7 @@ def test_write_macro_block_size():
     assert frame_sizes[1] == (40, 50)
 
 
+# @no_warnings_allowed --> Will generate a warning about killing ffmpeg
 def test_write_big_frames():
     """Test that we give ffmpeg enough time to finish."""
     try:
@@ -343,6 +344,10 @@ def _write_frames(pixfmt, bpp, tout):
     nframes, nsecs = imageio_ffmpeg.count_frames_and_secs(test_file2)
     assert nframes == n
 
+    _write_frames("rgba", 4, None)  # the default os to wait (since v0.4.0)
+    nframes, nsecs = imageio_ffmpeg.count_frames_and_secs(test_file2)
+    assert nframes == n
+
 
 if __name__ == "__main__":
     setup_module()

tests/test_parsing.py

@@ -1,18 +1,11 @@
 # styletest: ignore E501
-""" Tests specific to parsing ffmpeg header.
+"""
+Tests specific to parsing ffmpeg header.
 """
 
-import os
-from pytest import skip
 from imageio_ffmpeg._parsing import cvsecs, limit_lines, parse_ffmpeg_header
 
 
-if os.getenv("TRAVIS_OS_NAME") == "windows":
-    skip(
-        "Skip this on the Travis Windows run for now, see #408", allow_module_level=True
-    )
-
-
 def dedent(text, dedent=8):
     lines = [line[dedent:] for line in text.splitlines()]
     text = "\n".join(lines)

tests/test_special.py

@@ -0,0 +1,45 @@
+""" Tests more special use-cases.
+"""
+
+import gc
+import queue
+import threading
+
+import imageio_ffmpeg
+
+from testutils import ensure_test_files, test_file1
+
+
+def setup_module():
+    ensure_test_files()
+
+
+def test_threading():
+    # See issue #20
+
+    num_threads = 16
+    num_frames = 5
+
+    def make_iterator(q, n):
+        for i in range(n):
+            gen = imageio_ffmpeg.read_frames(test_file1)
+            gen.__next__()  # meta data
+            q.put(gen.__next__())  # first frame
+
+    q = queue.Queue()
+    threads = []
+    for i in range(num_threads):
+        t = threading.Thread(target=make_iterator, args=(q, num_frames))
+        t.daemon = True
+        t.start()
+        threads.append(t)
+
+    for i in range(num_threads * num_frames):
+        print(i, end=" ")
+        q.get()
+        gc.collect()  # this seems to help invoke the segfault earlier
+
+
+if __name__ == "__main__":
+    # setup_module()
+    test_threading()