From dc73f53bd99fd0d0e7bff2bbc3cae3cb6e4b39f3 Mon Sep 17 00:00:00 2001 From: Alexander Refsum Jensenius Date: Sun, 23 Aug 2026 14:44:22 +0200 Subject: [PATCH] Guard three more lazily opened writers, and document what ffmpeg_cmd returns #350's tail continued: mypy goes from 64 errors in 15 files to 52. Nothing here is a new behaviour, only failures made reachable to the reader. *Three writers with the same shape as the one already fixed in `_video.py`.* `video_out = None`, opened on the first frame, and closed unconditionally after the loop. A video that decodes to no frames therefore reached `None.stdin.close()`. Guarded, with a message that says what happened rather than naming an attribute of None. *`ffmpeg_cmd` had no return annotation at all,* and returns four different things depending on its `pipe` argument: a Popen to read from, a Popen to write to, the CompletedProcess of a finished run, or nothing. Annotating that precisely would need overloads keyed on the value of a string, which would describe the signature more exactly than it deserves, so it is `Any` with a docstring naming the four cases. This closed no errors on its own and the contract was undocumented, which is reason enough. *`target_name_video` is resolved to a str under one `if save_video:` and used under a later one.* mypy does not correlate two tests of the same flag, so it still saw the declared Optional. Asserted at the use sites, pointing at where the resolution happens. *A row of pose coordinates was inferred `list[int]`* from its first element, a timestamp, and then fed floats. 682 tests pass, 4 skipped. Co-Authored-By: Claude Opus 5 --- musicalgestures/_motionvideo.py | 3 +++ musicalgestures/_pose.py | 7 ++++++- musicalgestures/_utils.py | 14 ++++++++++++-- 3 files changed, 21 insertions(+), 3 deletions(-) diff --git a/musicalgestures/_motionvideo.py b/musicalgestures/_motionvideo.py index 1dd422b7..98da0034 100644 --- a/musicalgestures/_motionvideo.py +++ b/musicalgestures/_motionvideo.py @@ -248,6 +248,7 @@ def mg_motion( if save_video: if video_out is None: + assert target_name_video is not None # resolved under `if save_video` above cmd =['ffmpeg', '-y', '-s', '{}x{}'.format(motion_frame.shape[1], motion_frame.shape[0]), '-r', str(self.fps), '-f', 'rawvideo', '-pix_fmt', 'bgr24', '-vcodec', 'rawvideo', '-i', '-', '-vcodec', 'libx264', '-pix_fmt', 'yuv420p', target_name_video] @@ -265,6 +266,8 @@ def mg_motion( # Terminate the processes if save_video: + # the writer opens on the first frame, so this only holds for a video that had frames + assert video_out is not None, "no frames were written: the video decoded to nothing" video_out.stdin.close() video_out.wait() process.terminate() diff --git a/musicalgestures/_pose.py b/musicalgestures/_pose.py index c03f06df..a7c9d8b8 100644 --- a/musicalgestures/_pose.py +++ b/musicalgestures/_pose.py @@ -565,6 +565,7 @@ def pose( if save_video: if video_out is None: + assert target_name_video is not None # resolved under `if save_video` above cmd =['ffmpeg', '-y', '-s', '{}x{}'.format(frame.shape[1], frame.shape[0]), '-r', str(self.fps), '-f', 'rawvideo', '-pix_fmt', 'bgr24', '-vcodec', 'rawvideo', '-i', '-', '-vcodec', 'libx264', '-pix_fmt', 'yuv420p', target_name_video] @@ -579,6 +580,8 @@ def pose( # Terminate the processes if save_video: + # the writer opens on the first frame, so this only holds for a video that had frames + assert video_out is not None, "no frames were written: the video decoded to nothing" video_out.stdin.close() video_out.wait() # Check if the original video fil has audio @@ -1205,7 +1208,7 @@ def _pose_mediapipe( # Collect data row: time + normalised (x, y) for every landmark. # Always collected so the images and the keypoint cache are available. time_ms = frame2ms(ii, self.fps) - row = [time_ms] + row: list[float] = [time_ms] for i in range(len(MEDIAPIPE_LANDMARK_NAMES)): x, y, vis = keypoints[i] if vis >= threshold: @@ -1260,6 +1263,8 @@ def _pose_mediapipe( estimator.close() if save_video: + # the writer opens on the first frame, so this only holds for a video that had frames + assert video_out is not None, "no frames were written: the video decoded to nothing" video_out.stdin.close() video_out.wait() if self.has_audio: diff --git a/musicalgestures/_utils.py b/musicalgestures/_utils.py index 0b6600a9..85e6885e 100644 --- a/musicalgestures/_utils.py +++ b/musicalgestures/_utils.py @@ -1,7 +1,7 @@ from __future__ import annotations import warnings -from typing import Union, Tuple, TYPE_CHECKING, cast +from typing import Any, Union, Tuple, TYPE_CHECKING, cast if TYPE_CHECKING: # numpy is imported lazily inside functions (import-speed optimization); make the name @@ -1845,7 +1845,7 @@ def __init__(self, message): self.message = message -def ffmpeg_cmd(command: list, total_time: float, pb_prefix: str = 'Progress', print_cmd: bool = False, stream: bool = True, pipe: str | None = None): +def ffmpeg_cmd(command: list, total_time: float, pb_prefix: str = 'Progress', print_cmd: bool = False, stream: bool = True, pipe: str | None = None) -> Any: """ Run an ffmpeg command in a subprocess and show progress using an MgProgressbar. @@ -1857,6 +1857,16 @@ def ffmpeg_cmd(command: list, total_time: float, pb_prefix: str = 'Progress', pr stream (bool, optional): Whether to have a continuous output stream or just (the last) one. Defaults to True (continuous stream). pipe (str, optional): Whether to pipe video frames from FFmpeg to numpy array. Possible to read the video frame by frame with pipe='read', to load video in memory with pipe='load', or to write the frames of a numpy array to a video file with pipe='write'. Defaults to None. + Returns: + Any: What comes back depends on `pipe`, and the caller knows which it + asked for. `'read'` and `'write'` give a `subprocess.Popen` to read + frames from or write frames to, `'load'` gives the + `subprocess.CompletedProcess` of a finished run, and the default of + None runs the command to completion behind a progress bar and + returns nothing. Annotating that precisely would need overloads on + the value of a string argument, which would describe the signature + more exactly than it deserves. + Raises: KeyboardInterrupt: If the user stops the process. FFmpegError: If the ffmpeg process was unsuccessful.