Skip to content

Commit

Permalink
fix(present): black video flashes and other bugs linked to Qt (#465)
Browse files Browse the repository at this point in the history
* Relies on Pixel Format instead of Size

* chore(lib): remove deprecated warning

* chore(deps): update lockfiles

* chore(lib): cleanup code

* chore(ci): run fmt

* chore: update changelog

* chore: typo

* chore(docs): remove ref. to black screen

* fix(deps): issue on Windows

---------

Co-authored-by: PeculiarProgrammer <179261820+PeculiarProgrammer@users.noreply.github.com>
  • Loading branch information
jeertmans and PeculiarProgrammer authored Aug 26, 2024
1 parent 3b23c77 commit 5b6f5eb
Show file tree
Hide file tree
Showing 7 changed files with 215 additions and 206 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
[#429](https://github.com/jeertmans/manim-slides/pull/429)
- Fixed whitespace issue in default RevealJS template.
[#442](https://github.com/jeertmans/manim-slides/pull/442)
- Fixed black screen issue on recent Qt versions and device loss detected,
thanks to [@PeculiarProgrammer](https://github.com/PeculiarProgrammer)!
[#465](https://github.com/jeertmans/manim-slides/pull/465)

(unreleased-removed)=
### Removed
Expand Down
5 changes: 1 addition & 4 deletions docs/source/installation.md
Original file line number Diff line number Diff line change
Expand Up @@ -113,10 +113,7 @@ using optional dependencies:
and does not work with ManimGL;
- `manim` and `manimgl`, for installing the corresponding
dependencies;
- `pyqt6` to include PyQt6 Qt bindings. Those bindings are available
on most platforms and Python version, but produce a weird black
screen between slide with `manim-slides present`,
see [#QTBUG-118501](https://bugreports.qt.io/browse/QTBUG-118501);
- `pyqt6` to include PyQt6 Qt bindings;
- `pyqt6-full` to include `full` and `pyqt6`;
- `pyside6` to include PySide6 Qt bindings. Those bindings are available
on most platforms and Python version, except on Python 3.12[^2];
Expand Down
19 changes: 0 additions & 19 deletions manim_slides/present/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,23 +11,6 @@
from ..config import Config, PresentationConfig
from ..logger import logger

PREFERRED_QT_VERSIONS = ("6.5.1", "6.5.2")


def warn_if_non_desirable_pyside6_version() -> None:
from qtpy import API, QT_VERSION

if sys.version_info < (3, 12) and (
API != "pyside6" or QT_VERSION not in PREFERRED_QT_VERSIONS
):
logger.warn(
f"You are using {API = }, {QT_VERSION = }, "
"but we recommend installing 'PySide6==6.5.2', mainly to avoid "
"flashing screens between slides, "
"see issue https://github.com/jeertmans/manim-slides/issues/293. "
"You can do so with `pip install 'manim-slides[pyside6]'`."
)


@click.command()
@folder_path_option
Expand Down Expand Up @@ -302,8 +285,6 @@ def present(
if start_at[1]:
start_at_slide_number = start_at[1]

warn_if_non_desirable_pyside6_version()

from qtpy.QtCore import Qt
from qtpy.QtGui import QScreen

Expand Down
44 changes: 31 additions & 13 deletions manim_slides/present/player.py
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,8 @@ def __init__(
self.icon = QIcon(":/icon.png")
self.setWindowIcon(self.icon)

self.frame = QVideoFrame()

self.audio_output = QAudioOutput()
self.video_widget = QVideoWidget()
self.video_sink = self.video_widget.videoSink()
Expand All @@ -240,26 +242,14 @@ def __init__(
self.presentation_changed.connect(self.presentation_changed_callback)
self.slide_changed.connect(self.slide_changed_callback)

old_frame = None

def frame_changed(frame: QVideoFrame) -> None:
nonlocal old_frame

if old_frame and (frame.size() != old_frame.size()):
self.video_sink.setVideoFrame(old_frame)
frame = old_frame

self.info.video_sink.setVideoFrame(frame)
old_frame = frame

self.info = Info(
full_screen=full_screen,
aspect_ratio_mode=aspect_ratio_mode,
screen=info_window_screen,
)
self.info.close_event.connect(self.closeEvent)
self.info.key_press_event.connect(self.keyPressEvent)
self.video_sink.videoFrameChanged.connect(frame_changed)
self.video_sink.videoFrameChanged.connect(self.frame_changed)
self.hide_info_window = hide_info_window

# Connecting key callbacks
Expand Down Expand Up @@ -555,6 +545,34 @@ def hide_mouse(self) -> None:
else:
self.setCursor(Qt.BlankCursor)

def frame_changed(self, frame: QVideoFrame) -> None:
"""
Slot to handle possibly invalid frames.
This slot cannot be decorated with ``@Slot`` as
the video sinks are handled in different threads.
As of Qt>=6.5.3, the last frame of every video is "flushed",
resulting in a short black screen between each slide.
To avoid this issue, we check every frame, and avoid playing
invalid ones.
References
----------
1. https://github.com/jeertmans/manim-slides/issues/293
2. https://github.com/jeertmans/manim-slides/pull/464
:param frame: The most recent frame.
"""
if frame.isValid():
self.frame = frame
else:
self.video_sink.setVideoFrame(self.frame) # Reuse previous frame

self.info.video_sink.setVideoFrame(self.frame)

def closeEvent(self, event: QCloseEvent) -> None: # noqa: N802
self.close()

Expand Down
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ full = [
magic = ["manim-slides[manim]", "ipython>=8.12.2"]
manim = ["manim>=0.18.0"]
manimgl = ["manimgl>=1.6.1;python_version<'3.12'"]
pyqt6 = ["pyqt6>=6.6.1"]
pyqt6 = ["pyqt6>=6.7.0"]
pyqt6-full = ["manim-slides[full,pyqt6]"]
pyside6 = ["pyside6>=6.6.1"]
pyside6-full = ["manim-slides[full,pyside6]"]
Expand Down
Loading

0 comments on commit 5b6f5eb

Please sign in to comment.