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

Fixed symlink bug which made FFmpeg unable to load the video #7

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
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
215 changes: 121 additions & 94 deletions assets/install_ffmpeg.py

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@
"Programming Language :: Python :: 3.8",
"Programming Language :: Python :: 3.9",
"Programming Language :: Python :: 3.10",
"Programming Language :: Python :: 3.11",
"Programming Language :: Python :: 3.11",
"Programming Language :: Python :: Implementation :: CPython",
],
project_urls={
Expand Down
20 changes: 12 additions & 8 deletions tests/test_framesextractor.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,24 +15,28 @@ def test_all():
video_length = 52.3
interval = 1
ffmpeg_path = None
FramesExtractor(video_path, output_dir, video_length,
interval=interval, ffmpeg_path=ffmpeg_path)
FramesExtractor(
video_path, output_dir, video_length, interval=interval, ffmpeg_path=ffmpeg_path
)

with pytest.raises(FileNotFoundError):
video_path = os.path.join(script_path, "thisvideodoesnotexist.mp4")
output_dir = create_and_return_temporary_directory()
FramesExtractor(video_path, output_dir, video_length, interval=1,
ffmpeg_path=None)
FramesExtractor(
video_path, output_dir, video_length, interval=1, ffmpeg_path=None
)

with pytest.raises(FFmpegNotFound):
video_path = os.path.join(script_path, os.path.pardir, "assets", "rocket.mkv")
output_dir = create_and_return_temporary_directory()
ffmpeg_path = os.path.join(output_dir, "ffmpeg")
FramesExtractor(video_path, output_dir, video_length, interval=1,
ffmpeg_path=ffmpeg_path)
FramesExtractor(
video_path, output_dir, video_length, interval=1, ffmpeg_path=ffmpeg_path
)

with pytest.raises(FramesExtractorOutPutDirDoesNotExist):
video_path = os.path.join(script_path, "../assets/rocket.mkv")
output_dir = os.path.join(script_path, "thisdirdoesnotexist/")
FramesExtractor(video_path, output_dir, video_length, interval=1,
ffmpeg_path=None)
FramesExtractor(
video_path, output_dir, video_length, interval=1, ffmpeg_path=None
)
2 changes: 1 addition & 1 deletion tests/test_videoduration.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,4 +34,4 @@ def test_video_duration():
create_and_return_temporary_directory(),
("thisdirdoesnotexist" + os.path.sep),
)
video_duration(url="https://example.com", path=storage_path)
video_duration(url="https://example.com", path=storage_path)
3 changes: 2 additions & 1 deletion tests/test_videohash.py
Original file line number Diff line number Diff line change
Expand Up @@ -144,5 +144,6 @@ def __init__(self, hash=None):
)
VideoHash(path=path)


if __name__ == "__main__":
test_all()
test_all()
6 changes: 6 additions & 0 deletions videohash2/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,3 +66,9 @@ class FFmpegFailedToExtractFrames(FFmpegError):
"""FFmpeg failed to extract any frame at all. Maybe the input video is damaged or corrupt."""

pass


class FFmpegUnableToGetDuration(FFmpegError):
"""FFmpeg failed to get the duration. Maybe the input video is damaged or corrupt."""

pass
15 changes: 9 additions & 6 deletions videohash2/framesextractor.py
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ def detect_crop(
video_path: Optional[str] = None,
frames: int = 3,
ffmpeg_path: Optional[str] = None,
video_length: float = 2
video_length: float = 2,
) -> list:
"""
Detects the the amount of cropping to remove black bars.
Expand Down Expand Up @@ -148,7 +148,6 @@ def detect_crop(
7200,
14400,
]


crop_list = []

Expand All @@ -162,7 +161,9 @@ def detect_crop(

command = f'"{ffmpeg_path}" -ss {start_time} -i "{video_path}" -vframes {frames} -vf cropdetect -f null -'

process = Popen(shlex.split(command), stdin=DEVNULL, stdout=PIPE, stderr=PIPE)
process = Popen(
shlex.split(command), stdin=DEVNULL, stdout=PIPE, stderr=PIPE
)

output, error = process.communicate()

Expand Down Expand Up @@ -205,8 +206,10 @@ def extract(self) -> None:
output_dir = shlex.quote(self.output_dir)

crop = FramesExtractor.detect_crop(
video_path=video_path, frames=3, ffmpeg_path=ffmpeg_path,
video_length=video_length
video_path=video_path,
frames=3,
ffmpeg_path=ffmpeg_path,
video_length=video_length,
)

command = [
Expand All @@ -218,7 +221,7 @@ def extract(self) -> None:
"144x144",
"-r",
str(self.interval),
str(output_dir)+"video_frame_%07d.jpeg",
str(output_dir) + "video_frame_%07d.jpeg",
]

process = Popen(command, stdin=DEVNULL, stdout=PIPE, stderr=PIPE)
Expand Down
4 changes: 3 additions & 1 deletion videohash2/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ def does_path_exists(path: str) -> bool:
# it's file
return False


def create_and_return_temporary_directory() -> str:
"""
create a temporary directory where we can store the video, frames and the
Expand All @@ -48,6 +49,7 @@ def create_and_return_temporary_directory() -> str:
Path(path).mkdir(parents=True, exist_ok=True)
return path


def _get_task_uid() -> str:
"""
Returns an unique task id for the instance. Task id is used to
Expand All @@ -68,4 +70,4 @@ def _get_task_uid() -> str:
"ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789"
)
for _ in range(20)
)
)
65 changes: 35 additions & 30 deletions videohash2/videocopy.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,22 @@
from typing import Optional
from .exceptions import DidNotSupplyPathOrUrl, StoragePathDoesNotExist
from .downloader import Download
from .utils import (get_list_of_all_files_in_dir,
does_path_exists,
create_and_return_temporary_directory,
_get_task_uid)
from .utils import (
get_list_of_all_files_in_dir,
does_path_exists,
create_and_return_temporary_directory,
_get_task_uid,
)


def _copy_video_to_video_dir(
video_dir: str,
video_download_dir: str,
do_not_copy: Optional[bool] = True,
download_worst: bool = False,
url: Optional[str] = None,
path: Optional[str] = None) -> str:
video_dir: str,
video_download_dir: str,
do_not_copy: Optional[bool] = True,
download_worst: bool = False,
url: Optional[str] = None,
path: Optional[str] = None,
) -> str:
"""
Copy the video from the path to the video directory.

Expand Down Expand Up @@ -51,9 +55,9 @@ def _copy_video_to_video_dir(
video_path = os.path.join(video_dir, (f"video.{extension}"))

if do_not_copy:
os.symlink(path, video_path)
os.symlink(os.path.abspath(path), video_path)
else:
shutil.copyfile(path, video_path)
shutil.copyfile(os.path.abspath(path), video_path)

if url:

Expand All @@ -74,16 +78,17 @@ def _copy_video_to_video_dir(
video_path = f"{video_dir}video.{extension}"

if do_not_copy:
os.symlink(downloaded_file, video_path)
os.symlink(os.path.abspath(downloaded_file), video_path)
else:
shutil.copyfile(downloaded_file, video_path)
shutil.copyfile(os.path.abspath(downloaded_file), video_path)

return video_path


def _create_required_dirs_and_check_for_errors(
url: Optional[str] = None,
path: Optional[str] = None,
storage_path: Optional[str] = None
url: Optional[str] = None,
path: Optional[str] = None,
storage_path: Optional[str] = None,
) -> tuple:
"""
Creates important directories before the main processing starts.
Expand Down Expand Up @@ -119,22 +124,16 @@ def _create_required_dirs_and_check_for_errors(
if not storage_path:
storage_path = create_and_return_temporary_directory()
if not does_path_exists(storage_path):
raise StoragePathDoesNotExist(
f"Storage path '{storage_path}' does not exist."
)
raise StoragePathDoesNotExist(f"Storage path '{storage_path}' does not exist.")

os_path_sep = os.path.sep

storage_path = os.path.join(
storage_path, (f"{_get_task_uid()}{os_path_sep}")
)
storage_path = os.path.join(storage_path, (f"{_get_task_uid()}{os_path_sep}"))

video_dir = os.path.join(storage_path, (f"video{os_path_sep}"))
Path(video_dir).mkdir(parents=True, exist_ok=True)

video_download_dir = os.path.join(
storage_path, (f"downloadedvideo{os_path_sep}")
)
video_download_dir = os.path.join(storage_path, (f"downloadedvideo{os_path_sep}"))
Path(video_download_dir).mkdir(parents=True, exist_ok=True)

frames_dir = os.path.join(storage_path, (f"frames{os_path_sep}"))
Expand All @@ -149,8 +148,14 @@ def _create_required_dirs_and_check_for_errors(
horizontally_concatenated_image_dir = os.path.join(
storage_path, (f"horizontally_concatenated_image{os_path_sep}")
)
Path(horizontally_concatenated_image_dir).mkdir(
parents=True, exist_ok=True
Path(horizontally_concatenated_image_dir).mkdir(parents=True, exist_ok=True)

return (
storage_path,
video_dir,
video_download_dir,
frames_dir,
tiles_dir,
collage_dir,
horizontally_concatenated_image_dir,
)

return storage_path, video_dir, video_download_dir, frames_dir, tiles_dir, collage_dir, horizontally_concatenated_image_dir
35 changes: 19 additions & 16 deletions videohash2/videoduration.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,21 +2,23 @@
import shutil
from subprocess import PIPE, Popen
from typing import Optional
from .exceptions import DidNotSupplyPathOrUrl
from .videocopy import (_create_required_dirs_and_check_for_errors,
_copy_video_to_video_dir)
from .exceptions import DidNotSupplyPathOrUrl, FFmpegUnableToGetDuration
from .videocopy import (
_create_required_dirs_and_check_for_errors,
_copy_video_to_video_dir,
)

# Module to determine the length of video.
# The length is found by the FFmpeg, the output of video_duration is in seconds.


def video_duration(url: Optional[str] = None,
path: Optional[str] = None,
storage_path: Optional[str] = None,
do_not_copy: Optional[bool] = True,
ffmpeg_path: Optional[str] = None
) -> float:
def video_duration(
url: Optional[str] = None,
path: Optional[str] = None,
storage_path: Optional[str] = None,
do_not_copy: Optional[bool] = True,
ffmpeg_path: Optional[str] = None,
) -> float:
"""
Retrieve the exact video duration as echoed by the FFmpeg and return
the duration in seconds. Maximum duration supported is 999 hours, above
Expand All @@ -41,7 +43,7 @@ def video_duration(url: Optional[str] = None,
raise DidNotSupplyPathOrUrl(
"You must specify either a path or an URL of the video."
)

if path and url:
raise ValueError("Specify either a path or an URL and NOT both.")

Expand All @@ -50,16 +52,15 @@ def video_duration(url: Optional[str] = None,

if url:
video_dir, video_download_dir = _create_required_dirs_and_check_for_errors(
url=url,
storage_path=storage_path
)[0:2]
url=url, storage_path=storage_path
)[0:2]

path = _copy_video_to_video_dir(
video_dir,
video_download_dir,
do_not_copy=do_not_copy,
download_worst=True,
url=url
url=url,
)

command = f'"{ffmpeg_path}" -i "{path}"'
Expand All @@ -73,11 +74,13 @@ def video_duration(url: Optional[str] = None,

if match:
duration_string = match.group(1)
else:
raise FFmpegUnableToGetDuration()

hours, minutes, seconds = duration_string.strip().split(":")

if url and path:
cutPath = path[:path.find("/temp_storage_dir")]
cutPath = path[: path.find("/temp_storage_dir")]

try:
shutil.rmtree(cutPath)
Expand Down
41 changes: 21 additions & 20 deletions videohash2/videohash.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,12 @@
create_and_return_temporary_directory,
does_path_exists,
get_list_of_all_files_in_dir,
_get_task_uid
_get_task_uid,
)
from .videocopy import (
_create_required_dirs_and_check_for_errors,
_copy_video_to_video_dir,
)
from .videocopy import (_create_required_dirs_and_check_for_errors,
_copy_video_to_video_dir)
from .videoduration import video_duration


Expand Down Expand Up @@ -79,27 +81,26 @@ def __init__(

self.task_uid = _get_task_uid()

(self.storage_path,
self.video_dir,
self.video_download_dir,
self.frames_dir,
self.tiles_dir,
self.collage_dir,
self.horizontally_concatenated_image_dir
(
self.storage_path,
self.video_dir,
self.video_download_dir,
self.frames_dir,
self.tiles_dir,
self.collage_dir,
self.horizontally_concatenated_image_dir,
) = _create_required_dirs_and_check_for_errors(
url=self.url,
path=self.path,
storage_path=self.storage_path
url=self.url, path=self.path, storage_path=self.storage_path
)

self.video_path = _copy_video_to_video_dir(
video_dir=self.video_dir,
video_download_dir=self.video_download_dir,
do_not_copy=self.do_not_copy,
download_worst=self.download_worst,
url=self.url,
path=self.path
)
video_dir=self.video_dir,
video_download_dir=self.video_download_dir,
do_not_copy=self.do_not_copy,
download_worst=self.download_worst,
url=self.url,
path=self.path,
)

self.video_duration = video_duration(path=self.video_path)

Expand Down