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

Address issue with subprocess and stdint #99

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ venv/
ENV/
env.bak/
venv.bak/
.idea*

# Spyder project settings
.spyderproject
Expand Down
2 changes: 1 addition & 1 deletion videohash/__version__.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
)

__url__ = "https://akamhy.github.io/videohash/"
__version__ = "3.0.1"
__version__ = "3.0.2"
__status__ = "production"
__author__ = "Akash Mahanty"
__author_email__ = "[email protected]"
Expand Down
2 changes: 1 addition & 1 deletion videohash/collagemaker.py
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ def make(self) -> None:

# scale the opened frame images
frame.thumbnail(
(scaled_frame_image_width, scaled_frame_image_height), Image.ANTIALIAS
(scaled_frame_image_width, scaled_frame_image_height), Image.Resampling.LANCZOS
)

# set the value of x to that of i's value.
Expand Down
66 changes: 43 additions & 23 deletions videohash/framesextractor.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
import re
import shlex
from shutil import which
from subprocess import PIPE, Popen, check_output
from subprocess import PIPE, DEVNULL, Popen, check_output
from typing import Optional, Union

from .exceptions import (
Expand All @@ -29,6 +29,7 @@ def __init__(
output_dir: str,
interval: Union[int, float] = 1,
ffmpeg_path: Optional[str] = None,
save_logs_path: Optional[str] = None,
) -> None:
"""
Raises Exeception if video_path does not exists.
Expand All @@ -51,6 +52,8 @@ def __init__(

:param ffmpeg_path: path of the ffmpeg software if not in path.

:param save_logs_path: path to directory for storing ffmpeg logs.

"""
self.video_path = video_path
self.output_dir = output_dir
Expand All @@ -71,7 +74,11 @@ def __init__(

self._check_ffmpeg()

self.ffmpeg_output = None
self.ffmpeg_error = None
self.extract()
if save_logs_path is not None:
self.save_ffmpeg_output_error(save_logs_path)

def _check_ffmpeg(self) -> None:
"""
Expand Down Expand Up @@ -152,12 +159,12 @@ def detect_crop(

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

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

output, error = process.communicate()

matches = re.findall(
r"crop\=[0-9]{1,4}:[0-9]{1,4}:[0-9]{1,4}:[0-9]{1,4}",
r"crop\=[1-9][0-9]{0,3}:[1-9][0-9]{0,3}:[0-9]{1,4}:[0-9]{1,4}",
(output.decode() + error.decode()),
)

Expand All @@ -168,9 +175,9 @@ def detect_crop(
if len(crop_list) > 0:
mode = max(crop_list, key=crop_list.count)

crop = " "
crop = []
if mode:
crop = f" -vf {mode} "
crop = ["-vf", mode]

return crop

Expand All @@ -197,29 +204,42 @@ def extract(self) -> None:
video_path=video_path, frames=3, ffmpeg_path=ffmpeg_path
)

command = (
f'"{ffmpeg_path}"'
+ " -i "
+ f'"{video_path}"'
+ f"{crop}"
+ " -s 144x144 "
+ " -r "
+ str(self.interval)
+ " "
+ '"'
+ output_dir
+ "video_frame_%07d.jpeg"
+ '"'
)
command = [
str(ffmpeg_path),
"-i",
str(video_path),
*crop,
"-s",
"144x144",
"-r",
str(self.interval),
str(output_dir)+"video_frame_%07d.jpeg",
]

process = Popen(command, shell=True, stdout=PIPE, stderr=PIPE)
process = Popen(command, stdin=DEVNULL, stdout=PIPE, stderr=PIPE)
output, error = process.communicate()

ffmpeg_output = output.decode()
ffmpeg_error = error.decode()
self.ffmpeg_output = output.decode()
self.ffmpeg_error = error.decode()

if len(os.listdir(self.output_dir)) == 0:

raise FFmpegFailedToExtractFrames(
f"FFmpeg could not extract any frames.\n{command}\n{ffmpeg_output}\n{ffmpeg_error}"
f"FFmpeg could not extract any frames.\n{command}\n{self.ffmpeg_output}\n{self.ffmpeg_error}"
)

def save_ffmpeg_output_error(self, directory, stdout_filename='ffmpeg_stdout.log', stderr_filename='ffmpeg_stderr.log') -> None:
"""
Saves the stdout and stderr from ffmpeg if they were created to the provided directory

:return: None

:rtype: NoneType
"""
if self.ffmpeg_output:
with open(os.path.join(directory, stdout_filename), 'w') as outfile:
outfile.write(self.ffmpeg_output)

if self.ffmpeg_error:
with open(os.path.join(directory, stderr_filename), 'w') as outfile:
outfile.write(self.ffmpeg_error)
15 changes: 3 additions & 12 deletions videohash/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,25 +18,16 @@ def get_list_of_all_files_in_dir(directory: str) -> List[str]:

def does_path_exists(path: str) -> bool:
"""
If a directory is supplied then check if it exists.
If a file is supplied then check if it exists.

Directory ends with "/" on posix or "\" in windows and files do not.

If directory/file exists returns True else returns False

:return: True if dir or file exists else False.

:rtype: bool
"""
if path.endswith("/") or path.endswith("\\"):
# it's directory
return os.path.isdir(path)

if os.path.isdir(path) or os.path.isfile(path):
return os.path.exists(path)
else:
# it's file
return os.path.isfile(path)

return False

def create_and_return_temporary_directory() -> str:
"""
Expand Down
18 changes: 15 additions & 3 deletions videohash/videohash.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ def __init__(
storage_path: Optional[str] = None,
download_worst: bool = False,
frame_interval: Union[int, float] = 1,
do_not_copy: bool = False,
save_logs: bool = False
) -> None:
"""
:param path: Absolute path of the input video file.
Expand All @@ -61,6 +63,9 @@ def __init__(
Smaller frame_interval implies fewer frames and
vice-versa.

:param save_logs: If set to True, also save the logs from ffmpeg
into the storage_path.


:return: None

Expand All @@ -75,6 +80,7 @@ def __init__(

self._storage_path = self.storage_path
self.download_worst = download_worst
self.do_not_copy = do_not_copy
self.frame_interval = frame_interval

self.task_uid = VideoHash._get_task_uid()
Expand All @@ -83,7 +89,7 @@ def __init__(

self._copy_video_to_video_dir()

FramesExtractor(self.video_path, self.frames_dir, interval=self.frame_interval)
FramesExtractor(self.video_path, self.frames_dir, interval=self.frame_interval, save_logs_path=self.storage_path if save_logs else None)

self.collage_path = os.path.join(self.collage_dir, "collage.jpg")

Expand Down Expand Up @@ -290,7 +296,10 @@ def _copy_video_to_video_dir(self) -> None:

self.video_path = os.path.join(self.video_dir, (f"video.{extension}"))

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

if self.url:

Expand All @@ -310,7 +319,10 @@ def _copy_video_to_video_dir(self) -> None:

self.video_path = f"{self.video_dir}video.{extension}"

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

def _create_required_dirs_and_check_for_errors(self) -> None:
"""
Expand Down