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

Video handle persistence #64

Merged
merged 6 commits into from
Sep 12, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 49 additions & 7 deletions sleap_io/io/video.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,18 +43,25 @@ class VideoBackend:
Attributes:
filename: Path to video file.
grayscale: Whether to force grayscale. If None, autodetect on first frame load.
keep_open: Whether to keep the video reader open between calls to read frames.
If False, will close the reader after each call. If True (the default), it
will keep the reader open and cache it for subsequent calls which may
enhance the performance of reading multiple frames.
"""

filename: str
grayscale: Optional[bool] = None
keep_open: bool = True
_cached_shape: Optional[Tuple[int, int, int, int]] = None
_open_reader: Optional[object] = None

@classmethod
def from_filename(
cls,
filename: str,
dataset: Optional[str] = None,
grayscale: Optional[bool] = None,
keep_open: bool = True,
**kwargs,
) -> VideoBackend:
"""Create a VideoBackend from a filename.
Expand All @@ -64,6 +71,10 @@ def from_filename(
dataset: Name of dataset in HDF5 file.
grayscale: Whether to force grayscale. If None, autodetect on first frame
load.
keep_open: Whether to keep the video reader open between calls to read
frames. If False, will close the reader after each call. If True (the
default), it will keep the reader open and cache it for subsequent calls
which may enhance the performance of reading multiple frames.

Returns:
VideoBackend subclass instance.
Expand All @@ -73,13 +84,17 @@ def from_filename(

if filename.endswith(MediaVideo.EXTS):
return MediaVideo(
filename, grayscale=grayscale, **_get_valid_kwargs(MediaVideo, kwargs)
filename,
grayscale=grayscale,
keep_open=keep_open,
**_get_valid_kwargs(MediaVideo, kwargs),
)
elif filename.endswith(HDF5Video.EXTS):
return HDF5Video(
filename,
dataset=dataset,
grayscale=grayscale,
keep_open=keep_open,
**_get_valid_kwargs(HDF5Video, kwargs),
)
else:
Expand Down Expand Up @@ -319,12 +334,25 @@ def _read_frame(self, frame_idx: int) -> np.ndarray:
`get_frame` method of the `VideoBackend` class instead.
"""
if self.plugin == "opencv":
reader = cv2.VideoCapture(self.filename)
if self.keep_open:
if self._open_reader is None:
self._open_reader = cv2.VideoCapture(self.filename)
reader = self._open_reader
else:
reader = cv2.VideoCapture(self.filename)
reader.set(cv2.CAP_PROP_POS_FRAMES, frame_idx)
_, img = reader.read()
else:
with iio.imopen(self.filename, "r", plugin=self.plugin) as vid:
elif self.plugin == "pyav" or self.plugin == "FFMPEG":
if self.keep_open:
if self._open_reader is None:
self._open_reader = iio.imopen(
self.filename, "r", plugin=self.plugin
)
reader = self._open_reader
img = vid.read(index=frame_idx)
else:
with iio.imopen(self.filename, "r", plugin=self.plugin) as vid:
img = vid.read(index=frame_idx)
return img

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The _read_frame method has been updated to use the cached reader if keep_open is True. This could potentially improve performance when reading multiple frames. However, it's important to ensure that the reader is properly closed when it's no longer needed to avoid resource leaks.

+            if self.keep_open:
+                if self._open_reader is None:
+                    self._open_reader = cv2.VideoCapture(self.filename)
+                reader = self._open_reader
+            else:
+                reader = cv2.VideoCapture(self.filename)

def _read_frames(self, frame_inds: list) -> np.ndarray:
Expand All @@ -341,7 +369,13 @@ def _read_frames(self, frame_inds: list) -> np.ndarray:
`get_frames` method of the `VideoBackend` class instead.
"""
if self.plugin == "opencv":
reader = cv2.VideoCapture(self.filename)
if self.keep_open:
if self._open_reader is None:
self._open_reader = cv2.VideoCapture(self.filename)
reader = self._open_reader
else:
reader = cv2.VideoCapture(self.filename)

reader.set(cv2.CAP_PROP_POS_FRAMES, frame_inds[0])
imgs = []
for idx in frame_inds:
Comment on lines 389 to 401
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar to the _read_frame method, the _read_frames method has also been updated to use the cached reader if keep_open is True. Again, make sure that the reader is properly closed when it's no longer needed.

+            if self.keep_open:
+                if self._open_reader is None:
+                    self._open_reader = cv2.VideoCapture(self.filename)
+                reader = self._open_reader
+            else:
+                reader = cv2.VideoCapture(self.filename)

Expand All @@ -352,9 +386,17 @@ def _read_frames(self, frame_inds: list) -> np.ndarray:
imgs.append(img)
imgs = np.stack(imgs, axis=0)

else:
with iio.imopen(self.filename, "r", plugin=self.plugin) as vid:
elif self.plugin == "pyav" or self.plugin == "FFMPEG":
if self.keep_open:
if self._open_reader is None:
self._open_reader = iio.imopen(
self.filename, "r", plugin=self.plugin
)
reader = self._open_reader
imgs = np.stack([vid.read(index=idx) for idx in frame_inds], axis=0)
else:
with iio.imopen(self.filename, "r", plugin=self.plugin) as vid:
imgs = np.stack([vid.read(index=idx) for idx in frame_inds], axis=0)
return imgs
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes in this hunk are similar to the ones in the _read_frame method. The same comments apply here.

+            if self.keep_open:
+                if self._open_reader is None:
+                    self._open_reader = iio.imopen(
+                        self.filename, "r", plugin=self.plugin
+                    )
+                reader = self._open_reader
+                imgs = np.stack([vid.read(index=idx) for idx in frame_inds], axis=0)
+            else:
+                with iio.imopen(self.filename, "r", plugin=self.plugin) as vid:
+                    imgs = np.stack([vid.read(index=idx) for idx in frame_inds], axis=0)



Expand Down
6 changes: 4 additions & 2 deletions tests/io/test_video_backends.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import numpy as np
from numpy.testing import assert_equal
import h5py
import pytest


def test_video_backend_from_filename(centered_pair_low_quality_path, slp_minimal_pkg):
Expand Down Expand Up @@ -55,9 +56,10 @@ def test_get_frame(centered_pair_low_quality_path):
assert_equal(backend[-3:-1], backend.get_frames(range(1097, 1099)))


def test_mediavideo(centered_pair_low_quality_path):
@pytest.mark.parametrize("keep_open", [False, True])
def test_mediavideo(centered_pair_low_quality_path, keep_open):
backend = VideoBackend.from_filename(
centered_pair_low_quality_path, plugin="FFMPEG"
centered_pair_low_quality_path, plugin="FFMPEG", keep_open=keep_open
)
assert type(backend) == MediaVideo
assert backend.filename == centered_pair_low_quality_path
Expand Down