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

Remove deprecated path parameter to VideoReader and made src mandatory #8125

Merged
merged 8 commits into from
Nov 20, 2023
8 changes: 8 additions & 0 deletions test/test_videoapi.py
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,14 @@ def test_keyframe_reading(self, test_video, config, backend):
for i in range(len(av_keyframes)):
assert av_keyframes[i] == approx(vr_keyframes[i], rel=0.001)

def test_src(self):
with pytest.raises(ValueError, match="src cannot be empty"):
VideoReader(src="")
with pytest.raises(ValueError, match="`src` must be either string"):
NicolasHug marked this conversation as resolved.
Show resolved Hide resolved
VideoReader(src=2)
with pytest.raises(TypeError, match="unexpected keyword argument"):
VideoReader(path="path")


if __name__ == "__main__":
pytest.main([__file__])
23 changes: 5 additions & 18 deletions torchvision/io/video_reader.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import io
import warnings

from typing import Any, Dict, Iterator, Optional
from typing import Any, Dict, Iterator

import torch

Expand Down Expand Up @@ -102,40 +102,27 @@ class VideoReader:
If Tensor, it is interpreted internally as byte buffer.
It must be one-dimensional, of type ``torch.uint8``.



stream (string, optional): descriptor of the required stream, followed by the stream id,
in the format ``{stream_type}:{stream_id}``. Defaults to ``"video:0"``.
Currently available options include ``['video', 'audio']``

num_threads (int, optional): number of threads used by the codec to decode video.
Default value (0) enables multithreading with codec-dependent heuristic. The performance
will depend on the version of FFMPEG codecs supported.


path (str, optional):
.. warning:
This parameter was deprecated in ``0.15`` and will be removed in ``0.17``.
Please use ``src`` instead.
"""

def __init__(
self,
src: str = "",
src: str,
stream: str = "video",
num_threads: int = 0,
path: Optional[str] = None,
) -> None:
_log_api_usage_once(self)
from .. import get_video_backend

self.backend = get_video_backend()
if isinstance(src, str):
if src == "":
if path is None:
raise TypeError("src cannot be empty")
src = path
warnings.warn("path is deprecated and will be removed in 0.17. Please use src instead")
if isinstance(src, str) and not src:
raise ValueError("src cannot be empty")
Copy link
Member Author

Choose a reason for hiding this comment

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

This isn't strictly needed, we could just rely on the error from the backend. This line just makes that error consistent across backends and thus is easier to test. If we remove this line, I'll just remove the test.

elif isinstance(src, bytes):
if self.backend in ["cuda"]:
raise RuntimeError(
Expand All @@ -154,7 +141,7 @@ def __init__(
"VideoReader cannot be initialized from Tensor object when using cuda or pyav backend."
)
else:
raise TypeError("`src` must be either string, Tensor or bytes object.")
raise ValueError(f"`src` must be either string, Tensor or bytes object. Got {type(src)}")
NicolasHug marked this conversation as resolved.
Show resolved Hide resolved

if self.backend == "cuda":
device = torch.device("cuda")
Expand Down
Loading