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

feat(cli): auto detect resolution #158

Merged
merged 2 commits into from
Mar 16, 2023
Merged
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
15 changes: 8 additions & 7 deletions manim_slides/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@
import tempfile
from enum import Enum
from pathlib import Path
from typing import Dict, List, Optional, Set, Union
from typing import Dict, List, Optional, Set, Tuple, Union

from pydantic import BaseModel, FilePath, root_validator, validator
from pydantic import BaseModel, FilePath, PositiveInt, root_validator, validator
from PySide6.QtCore import Qt

from .defaults import FFMPEG_BIN
Expand All @@ -20,7 +20,7 @@ def merge_basenames(files: List[FilePath]) -> Path:
"""
logger.info(f"Generating a new filename for animations: {files}")

dirname = files[0].parent
dirname: Path = files[0].parent
ext = files[0].suffix

basenames = (file.stem for file in files)
Expand All @@ -31,7 +31,7 @@ def merge_basenames(files: List[FilePath]) -> Path:
# https://github.com/jeertmans/manim-slides/issues/123
basename = hashlib.sha256(basenames_str.encode()).hexdigest()

return dirname / (basename + ext)
return dirname.joinpath(basename + ext)


class Key(BaseModel): # type: ignore
Expand Down Expand Up @@ -149,21 +149,22 @@ def slides_slice(self) -> slice:
class PresentationConfig(BaseModel): # type: ignore
slides: List[SlideConfig]
files: List[FilePath]
resolution: Tuple[PositiveInt, PositiveInt] = (1920, 1080)
Comment on lines 149 to +152
Copy link
Owner Author

Choose a reason for hiding this comment

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

@Fairlight8 first, I added a new resolution field. Specifying a default value is very important for backward compatibility (i.e., what to do when no resolution was specified? Use the default!).


@root_validator
def animation_indices_match_files(
cls, values: Dict[str, Union[List[SlideConfig], List[FilePath]]]
) -> Dict[str, Union[List[SlideConfig], List[FilePath]]]:
files = values.get("files")
slides = values.get("slides")
files: List[FilePath] = values.get("files") # type: ignore
slides: List[SlideConfig] = values.get("slides") # type: ignore

if files is None or slides is None:
return values

n_files = len(files)

for slide in slides:
if slide.end_animation > n_files: # type: ignore
if slide.end_animation > n_files:
raise ValueError(
f"The following slide's contains animations not listed in files {files}: {slide}"
)
Expand Down
14 changes: 8 additions & 6 deletions manim_slides/convert.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
from .present import get_scenes_presentation_config


def open_with_default(file: Path):
def open_with_default(file: Path) -> None:
system = platform.system()
if system == "Darwin":
subprocess.call(("open", str(file)))
Expand Down Expand Up @@ -66,7 +66,7 @@ def load_template(self) -> str:
An empty string is returned if no template is used."""
return ""

def open(self, file: Path) -> bool:
def open(self, file: Path) -> Any:
"""Opens a file, generated with converter, using appropriate application."""
raise NotImplementedError

Expand Down Expand Up @@ -376,7 +376,7 @@ class Config:
use_enum_values = True
extra = "forbid"

def open(self, file: Path) -> bool:
def open(self, file: Path) -> None:
return open_with_default(file)

def convert_to(self, dest: Path) -> None:
Expand All @@ -389,7 +389,9 @@ def convert_to(self, dest: Path) -> None:

# From GitHub issue comment:
# - https://github.com/scanny/python-pptx/issues/427#issuecomment-856724440
def auto_play_media(media: pptx.shapes.picture.Movie, loop: bool = False):
def auto_play_media(
media: pptx.shapes.picture.Movie, loop: bool = False
) -> None:
el_id = xpath(media.element, ".//p:cNvPr")[0].attrib["id"]
el_cnt = xpath(
media.element.getparent().getparent().getparent(),
Expand Down Expand Up @@ -463,7 +465,7 @@ def callback(ctx: Context, param: Parameter, value: bool) -> None:

ctx.exit()

return click.option(
return click.option( # type: ignore
"--show-config",
is_flag=True,
help="Show supported options for given format and exit.",
Expand Down Expand Up @@ -491,7 +493,7 @@ def callback(ctx: Context, param: Parameter, value: bool) -> None:

ctx.exit()

return click.option(
return click.option( # type: ignore
"--show-template",
is_flag=True,
help="Show the template (currently) used for a given conversion format and exit.",
Expand Down
96 changes: 71 additions & 25 deletions manim_slides/present.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import click
import cv2
import numpy as np
from click import Context, Parameter
from pydantic import ValidationError
from PySide6.QtCore import Qt, QThread, Signal, Slot
from PySide6.QtGui import QCloseEvent, QIcon, QImage, QKeyEvent, QPixmap, QResizeEvent
Expand Down Expand Up @@ -70,8 +71,7 @@ class Presentation:
"""Creates presentation from a configuration object."""

def __init__(self, config: PresentationConfig) -> None:
self.slides: List[SlideConfig] = config.slides
self.files: List[str] = config.files
self.config = config

self.__current_slide_index: int = 0
self.current_animation: int = self.current_slide.start_animation
Expand All @@ -90,23 +90,41 @@ def __init__(self, config: PresentationConfig) -> None:
def __len__(self) -> int:
return len(self.slides)

@property
def slides(self) -> List[SlideConfig]:
"""Returns the list of slides."""
return self.config.slides

@property
def files(self) -> List[Path]:
"""Returns the list of animation files."""
return self.config.files

@property
def resolution(self) -> Tuple[int, int]:
"""Returns the resolution."""
return self.config.resolution

Comment on lines +103 to +107
Copy link
Owner Author

Choose a reason for hiding this comment

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

@Fairlight8 Second, add a property to Presentation so that Presentation.resolution maps to PresentationConfig.resolution

@property
def current_slide_index(self) -> int:
return self.__current_slide_index

@current_slide_index.setter
def current_slide_index(self, value: Optional[int]):
if value:
def current_slide_index(self, value: Optional[int]) -> None:
if value is not None:
if -len(self) <= value < len(self):
self.__current_slide_index = value
self.current_animation = self.current_slide.start_animation
logger.debug(f"Set current slide index to {value}")
else:
logger.error(
f"Could not load slide number {value}, playing first slide instead."
)

def set_current_animation_and_update_slide_number(self, value: Optional[int]):
if value:
def set_current_animation_and_update_slide_number(
self, value: Optional[int]
) -> None:
if value is not None:
n_files = len(self.files)
if -n_files <= value < n_files:
if value < 0:
Expand All @@ -116,6 +134,8 @@ def set_current_animation_and_update_slide_number(self, value: Optional[int]):
if value < slide.end_animation:
self.current_slide_index = i
self.current_animation = value

logger.debug(f"Playing animation {value}, at slide index {i}")
return

assert (
Expand Down Expand Up @@ -159,7 +179,7 @@ def load_animation_cap(self, animation: int) -> None:

self.release_cap()

file: str = self.files[animation]
file: str = str(self.files[animation])

if self.reverse:
file = "{}_reversed{}".format(*os.path.splitext(file))
Expand All @@ -178,7 +198,7 @@ def current_cap(self) -> cv2.VideoCapture:

def rewind_current_slide(self) -> None:
"""Rewinds current slide to first frame."""
logger.debug("Rewinding curring slide")
logger.debug("Rewinding current slide")
if self.reverse:
self.current_animation = self.current_slide.end_animation - 1
else:
Expand Down Expand Up @@ -216,9 +236,10 @@ def load_next_slide(self) -> None:

def load_previous_slide(self) -> None:
"""Loads previous slide."""
logger.debug("Loading previous slide")
logger.debug(f"Loading previous slide, current is {self.current_slide_index}")
self.cancel_reverse()
self.current_slide_index = max(0, self.current_slide_index - 1)
logger.debug(f"Loading slide index {self.current_slide_index}")
self.rewind_current_slide()

@property
Expand All @@ -229,7 +250,8 @@ def fps(self) -> int:
logger.warn(
f"Something is wrong with video file {self.current_file}, as the fps returned by frame {self.current_frame_number} is 0"
)
return max(fps, 1) # TODO: understand why we sometimes get 0 fps
# TODO: understand why we sometimes get 0 fps
return max(fps, 1) # type: ignore

def reset(self) -> None:
"""Rests current presentation."""
Expand Down Expand Up @@ -320,6 +342,7 @@ class Display(QThread): # type: ignore

change_video_signal = Signal(np.ndarray)
change_info_signal = Signal(dict)
change_presentation_sigal = Signal()
finished = Signal()

def __init__(
Expand Down Expand Up @@ -365,10 +388,12 @@ def current_presentation_index(self) -> int:
return self.__current_presentation_index

@current_presentation_index.setter
def current_presentation_index(self, value: Optional[int]):
if value:
def current_presentation_index(self, value: Optional[int]) -> None:
if value is not None:
if -len(self) <= value < len(self):
self.__current_presentation_index = value
self.current_presentation.release_cap()
self.change_presentation_sigal.emit()
else:
logger.error(
f"Could not load scene number {value}, playing first scene instead."
Expand All @@ -379,6 +404,11 @@ def current_presentation(self) -> Presentation:
"""Returns the current presentation."""
return self.presentations[self.current_presentation_index]

@property
def current_resolution(self) -> Tuple[int, int]:
"""Returns the resolution of the current presentation."""
return self.current_presentation.resolution

def run(self) -> None:
"""Runs a series of presentations until end or exit."""
while self.run_flag:
Expand Down Expand Up @@ -413,7 +443,7 @@ def run(self) -> None:
if self.record_to is not None:
self.record_movie()

logger.debug("Closing video thread gracully and exiting")
logger.debug("Closing video thread gracefully and exiting")
self.finished.emit()

def record_movie(self) -> None:
Expand Down Expand Up @@ -587,7 +617,6 @@ def __init__(
*args: Any,
config: Config = DEFAULT_CONFIG,
fullscreen: bool = False,
resolution: Tuple[int, int] = (1980, 1080),
hide_mouse: bool = False,
aspect_ratio: AspectRatio = AspectRatio.auto,
resize_mode: Qt.TransformationMode = Qt.SmoothTransformation,
Expand All @@ -599,7 +628,12 @@ def __init__(
self.setWindowTitle(WINDOW_NAME)
self.icon = QIcon(":/icon.png")
self.setWindowIcon(self.icon)
self.display_width, self.display_height = resolution

# create the video capture thread
kwargs["config"] = config
self.thread = Display(*args, **kwargs)

self.display_width, self.display_height = self.thread.current_resolution
self.aspect_ratio = aspect_ratio
self.resize_mode = resize_mode
self.hide_mouse = hide_mouse
Expand All @@ -619,9 +653,6 @@ def __init__(
self.label.setPixmap(self.pixmap)
self.label.setMinimumSize(1, 1)

# create the video capture thread
kwargs["config"] = config
self.thread = Display(*args, **kwargs)
# create the info dialog
self.info = Info()
self.info.show()
Expand All @@ -635,6 +666,7 @@ def __init__(
# connect signals
self.thread.change_video_signal.connect(self.update_image)
self.thread.change_info_signal.connect(self.info.update_info)
self.thread.change_presentation_sigal.connect(self.update_canvas)
self.thread.finished.connect(self.closeAll)
self.send_key_signal.connect(self.thread.set_key)

Expand Down Expand Up @@ -688,6 +720,14 @@ def update_image(self, cv_img: np.ndarray) -> None:

self.label.setPixmap(QPixmap.fromImage(qt_img))

@Slot()
def update_canvas(self) -> None:
"""Update the canvas when a presentation has changed."""
logger.debug("Updating canvas")
self.display_width, self.display_height = self.thread.current_resolution
if not self.isFullScreen():
self.resize(self.display_width, self.display_height)
Comment on lines +723 to +729
Copy link
Owner Author

Choose a reason for hiding this comment

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

@Fairlight8 Third, every time the presentation changes, call to resize.
There, you will need to call setStyleSheet(...).



@click.command()
@click.option(
Expand Down Expand Up @@ -757,7 +797,7 @@ def value_proc(value: Optional[str]) -> List[str]:
while True:
try:
scenes = click.prompt("Choice(s)", value_proc=value_proc)
return scenes
return scenes # type: ignore
except ValueError as e:
raise click.UsageError(str(e))

Expand Down Expand Up @@ -785,7 +825,9 @@ def get_scenes_presentation_config(
return presentation_configs


def start_at_callback(ctx, param, values: str) -> Tuple[Optional[int], ...]:
def start_at_callback(
ctx: Context, param: Parameter, values: str
) -> Tuple[Optional[int], ...]:
if values == "(None, None, None)":
return (None, None, None)

Expand Down Expand Up @@ -838,9 +880,8 @@ def str_to_int_or_none(value: str) -> Optional[int]:
"--resolution",
metavar="<WIDTH HEIGHT>",
type=(int, int),
default=(1920, 1080),
default=None,
Comment on lines -841 to +883
Copy link
Owner Author

Choose a reason for hiding this comment

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

@Fairlight8 Fourth, set the default to None

help="Window resolution WIDTH HEIGHT used if fullscreen is not set. You may manually resize the window afterward.",
show_default=True,
)
@click.option(
"--to",
Expand Down Expand Up @@ -931,7 +972,7 @@ def present(
start_paused: bool,
fullscreen: bool,
skip_all: bool,
resolution: Tuple[int, int],
resolution: Optional[Tuple[int, int]],
Comment on lines -934 to +975
Copy link
Owner Author

Choose a reason for hiding this comment

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

@Fairlight8 same as above, but for type hint

record_to: Optional[Path],
exit_after_last_slide: bool,
hide_mouse: bool,
Expand All @@ -956,9 +997,15 @@ def present(
if skip_all:
exit_after_last_slide = True

presentation_configs = get_scenes_presentation_config(scenes, folder)

if resolution is not None:
for presentation_config in presentation_configs:
presentation_config.resolution = resolution

presentations = [
Presentation(presentation_config)
for presentation_config in get_scenes_presentation_config(scenes, folder)
for presentation_config in presentation_configs
]

if config_path.exists():
Expand Down Expand Up @@ -994,7 +1041,6 @@ def present(
start_paused=start_paused,
fullscreen=fullscreen,
skip_all=skip_all,
resolution=resolution,
record_to=record_to,
exit_after_last_slide=exit_after_last_slide,
hide_mouse=hide_mouse,
Expand Down
Loading