Skip to content

Commit

Permalink
Updated deprecated test approach + minor changes (#89)
Browse files Browse the repository at this point in the history
* isort _io.py

* type fix: FFMEG -> FFMPEG

* removed unused variable i in for loops

* added better, more descriptive variable names

* fixed deprecated warns(None) syntax

* method docstrings, removed unnecessary i in loops

* isort all

* black all

* removed unused pytest.warns import

* replaced variable p with process
  • Loading branch information
lbreede authored Apr 10, 2023
1 parent 4edd3ef commit 2f3403a
Show file tree
Hide file tree
Showing 11 changed files with 79 additions and 52 deletions.
2 changes: 1 addition & 1 deletion imageio_ffmpeg/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,5 @@
# flake8: noqa

from ._definitions import __version__
from ._utils import get_ffmpeg_exe, get_ffmpeg_version
from ._io import count_frames_and_secs, read_frames, write_frames
from ._utils import get_ffmpeg_exe, get_ffmpeg_version
2 changes: 1 addition & 1 deletion imageio_ffmpeg/_definitions.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import platform
import sys
import struct
import sys

__version__ = "0.4.8"

Expand Down
39 changes: 20 additions & 19 deletions imageio_ffmpeg/_io.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,12 @@
import sys
import time
import pathlib
import subprocess
from functools import lru_cache
import sys
import time
from collections import defaultdict
from functools import lru_cache

from ._utils import get_ffmpeg_exe, _popen_kwargs, logger
from ._parsing import LogCatcher, parse_ffmpeg_header, cvsecs

from ._parsing import LogCatcher, cvsecs, parse_ffmpeg_header
from ._utils import _popen_kwargs, get_ffmpeg_exe, logger

ISWIN = sys.platform.startswith("win")

Expand Down Expand Up @@ -164,7 +163,9 @@ def count_frames_and_secs(path):
out = subprocess.check_output(cmd, stderr=subprocess.STDOUT, **_popen_kwargs())
except subprocess.CalledProcessError as err:
out = err.output.decode(errors="ignore")
raise RuntimeError("FFMEG call failed with {}:\n{}".format(err.returncode, out))
raise RuntimeError(
"FFMPEG call failed with {}:\n{}".format(err.returncode, out)
)

# Note that other than with the subprocess calls below, ffmpeg wont hang here.
# Worst case Python will stop/crash and ffmpeg will continue running until done.
Expand Down Expand Up @@ -264,15 +265,15 @@ def read_frames(
cmd += input_params + ["-i", path]
cmd += pre_output_params + output_params + ["-"]

p = subprocess.Popen(
process = subprocess.Popen(
cmd,
stdin=subprocess.PIPE,
stdout=subprocess.PIPE,
stderr=subprocess.PIPE,
**_popen_kwargs(prevent_sigint=True)
)

log_catcher = LogCatcher(p.stderr)
log_catcher = LogCatcher(process.stderr)

# Init policy by which to terminate ffmpeg. May be set to "kill" later.
stop_policy = "timeout" # not wait; ffmpeg should be able to quit quickly
Expand Down Expand Up @@ -302,8 +303,8 @@ def read_frames(

# ----- Read frames

w, h = meta["size"]
framesize_bits = w * h * bits_per_pixel
width, height = meta["size"]
framesize_bits = width * height * bits_per_pixel
framesize_bytes = framesize_bits / 8
assert (
framesize_bytes.is_integer()
Expand All @@ -316,7 +317,7 @@ def read_frames(
try:
bb = bytes()
while len(bb) < framesize_bytes:
extra_bytes = p.stdout.read(framesize_bytes - len(bb))
extra_bytes = process.stdout.read(framesize_bytes - len(bb))
if not extra_bytes:
if len(bb) == 0:
return
Expand Down Expand Up @@ -350,7 +351,7 @@ def read_frames(
log_catcher.stop_me()

# Make sure that ffmpeg is terminated.
if p.poll() is None:
if process.poll() is None:
# Ask ffmpeg to quit
try:
# I read somewhere that modern ffmpeg on Linux prefers a
Expand All @@ -364,8 +365,8 @@ def read_frames(
# 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()
process.stdout.close()
process.stdin.close()
# p.stderr.close() -> not here, the log_catcher closes it
except Exception as err: # pragma: no cover
logger.warning("Error while attempting stop ffmpeg (r): " + str(err))
Expand All @@ -374,16 +375,16 @@ def read_frames(
# Wait until timeout, produce a warning and kill if it still exists
try:
etime = time.time() + 1.5
while time.time() < etime and p.poll() is None:
while time.time() < etime and process.poll() is None:
time.sleep(0.01)
finally:
if p.poll() is None: # pragma: no cover
if process.poll() is None: # pragma: no cover
logger.warning("We had to kill ffmpeg to stop it.")
p.kill()
process.kill()

else: # stop_policy == "kill"
# Just kill it
p.kill()
process.kill()


def write_frames(
Expand Down
2 changes: 1 addition & 1 deletion imageio_ffmpeg/_parsing.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import re
import time
import threading
import time

from ._utils import logger

Expand Down
7 changes: 4 additions & 3 deletions imageio_ffmpeg/_utils.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
import os
import sys
import logging
import os
import subprocess
import sys
from functools import lru_cache

from pkg_resources import resource_filename

from ._definitions import get_platform, FNAME_PER_PLATFORM
from ._definitions import FNAME_PER_PLATFORM, get_platform

logger = logging.getLogger("imageio_ffmpeg")

Expand Down
7 changes: 4 additions & 3 deletions tasks.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
""" Invoke tasks for imageio-ffmpeg
"""

import importlib
import os
import sys
import shutil
import importlib
import subprocess
import sys
from urllib.request import urlopen

from invoke import task
Expand Down Expand Up @@ -138,7 +138,7 @@ def build(ctx):
# Get version and more
sys.path.insert(0, os.path.join(ROOT_DIR, "imageio_ffmpeg"))
try:
from _definitions import __version__, FNAME_PER_PLATFORM, WHEEL_BUILDS
from _definitions import FNAME_PER_PLATFORM, WHEEL_BUILDS, __version__
finally:
sys.path.pop(0)

Expand Down Expand Up @@ -236,6 +236,7 @@ def update_readme(ctx):
text = text.split("\n## API\n")[0] + "\n## API\n\n"

import inspect

import imageio_ffmpeg

for func in (
Expand Down
3 changes: 3 additions & 0 deletions tests/snippets.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@
# is now to wait for ffmpeg.

import os

import numpy as np

import imageio_ffmpeg

ims = [
Expand All @@ -30,6 +32,7 @@
# %% Behavior of KeyboardInterrupt / Ctrl+C

import os

import imageio_ffmpeg

filename = os.path.expanduser("~/.imageio/images/cockatoo.mp4")
Expand Down
29 changes: 20 additions & 9 deletions tests/test_io.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,27 @@
The main tests for the public API.
"""

import tempfile
import time
import types
import tempfile
import warnings

from pytest import raises, skip
from testutils import (
ensure_test_files,
no_warnings_allowed,
test_dir,
test_file1,
test_file2,
test_file3,
)

import imageio_ffmpeg
from imageio_ffmpeg._io import ffmpeg_test_encoder, get_compiled_h264_encoders
from imageio_ffmpeg._io import get_first_available_h264_encoder

from pytest import skip, raises, warns
from testutils import no_warnings_allowed
from testutils import ensure_test_files, test_dir, test_file1, test_file2, test_file3
from imageio_ffmpeg._io import (
ffmpeg_test_encoder,
get_compiled_h264_encoders,
get_first_available_h264_encoder,
)


def setup_module():
Expand Down Expand Up @@ -40,12 +50,13 @@ def test_read_frames_resource_warning():
todo: use pytest.does_not_warn() as soon as it becomes available
(see https://github.com/pytest-dev/pytest/issues/9404)
"""
with warns(None) as warnings:

with warnings.catch_warnings(record=True) as warnings_:
gen = imageio_ffmpeg.read_frames(test_file1)
next(gen)
gen.close()
# there should not be any warnings, but show warning messages if there are
assert not [w.message for w in warnings]
assert not [w.message for w in warnings_]


@no_warnings_allowed
Expand Down
4 changes: 2 additions & 2 deletions tests/test_special.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,10 @@
import queue
import threading

import imageio_ffmpeg

from testutils import ensure_test_files, test_file1

import imageio_ffmpeg


def setup_module():
ensure_test_files()
Expand Down
33 changes: 22 additions & 11 deletions tests/test_terminate.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,18 @@
"""

import gc
import sys
import subprocess
import sys

import imageio_ffmpeg

from testutils import no_warnings_allowed, get_ffmpeg_pids
from testutils import ensure_test_files, test_file1, test_file2
from testutils import (
ensure_test_files,
get_ffmpeg_pids,
no_warnings_allowed,
test_file1,
test_file2,
)

import imageio_ffmpeg

N = 2 # number of times to perform each test

Expand All @@ -30,7 +34,8 @@ def test_ffmpeg_version():

@no_warnings_allowed
def test_reader_done():
for i in range(N):
"""Test that the reader is done after reading all frames"""
for _ in range(N):
pids0 = get_ffmpeg_pids()
r = imageio_ffmpeg.read_frames(test_file1)
pids1 = get_ffmpeg_pids().difference(pids0) # generator has not started
Expand All @@ -47,7 +52,8 @@ def test_reader_done():

@no_warnings_allowed
def test_reader_close():
for i in range(N):
"""Test that the reader is done after closing it"""
for _ in range(N):
pids0 = get_ffmpeg_pids()
r = imageio_ffmpeg.read_frames(test_file1)
pids1 = get_ffmpeg_pids().difference(pids0) # generator has not started
Expand All @@ -63,7 +69,8 @@ def test_reader_close():

@no_warnings_allowed
def test_reader_del():
for i in range(N):
"""Test that the reader is done after deleting it"""
for _ in range(N):
pids0 = get_ffmpeg_pids()
r = imageio_ffmpeg.read_frames(test_file1)
pids1 = get_ffmpeg_pids().difference(pids0) # generator has not started
Expand All @@ -80,7 +87,8 @@ def test_reader_del():

@no_warnings_allowed
def test_write_close():
for i in range(N):
"""Test that the writer is done after closing it"""
for _ in range(N):
pids0 = get_ffmpeg_pids()
w = imageio_ffmpeg.write_frames(test_file2, (64, 64))
pids1 = get_ffmpeg_pids().difference(pids0) # generator has not started
Expand All @@ -97,7 +105,7 @@ def test_write_close():

@no_warnings_allowed
def test_write_del():
for i in range(N):
for _ in range(N):
pids0 = get_ffmpeg_pids()
w = imageio_ffmpeg.write_frames(test_file2, (64, 64))
pids1 = get_ffmpeg_pids().difference(pids0) # generator has not started
Expand All @@ -115,7 +123,10 @@ def test_write_del():

def test_partial_read():
# Case: https://github.com/imageio/imageio-ffmpeg/issues/69
template = "import sys; import imageio_ffmpeg; f=sys.argv[1]; print(f); r=imageio_ffmpeg.read_frames(f);"
template = (
"import sys; import imageio_ffmpeg; f=sys.argv[1]; print(f); "
"r=imageio_ffmpeg.read_frames(f);"
)
for i in range(4):
code = template + " r.__next__();" * i
cmd = [sys.executable, "-c", code, test_file1]
Expand Down
3 changes: 1 addition & 2 deletions tests/testutils.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,12 @@
import logging.handlers
import os
import tempfile
import logging.handlers
from urllib.request import urlopen

import psutil

import imageio_ffmpeg


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"
Expand Down

0 comments on commit 2f3403a

Please sign in to comment.