From bdcc728ce76ac7d9588b0e5177bd34b04d4fae45 Mon Sep 17 00:00:00 2001 From: Chouffe Date: Thu, 4 Jun 2026 16:05:11 +0200 Subject: [PATCH] fix(encode): pad odd dimensions so H.264 encode produces web-ready video aris-encode failed on odd-sized ARIS clips (e.g. 924x1765) because libx264 with yuv420p requires even width and height. The encoder failed to open and wrote a 0-byte file, yet the CLI still reported success and exited 0. - Pad each dimension up to the next even number before libx264 (adds at most a 1px border, no rescaling). - Set pix_fmt=yuv420p and movflags=+faststart for reliable browser playback. - Re-raise ffmpeg errors instead of swallowing them (the docstring already documents this), and fix the malformed error log call. - Make the encode CLI count failures and exit non-zero, mirroring aris-convert. - Add a regression test that encodes a real odd-dimension (63x65) clip. --- src/aris/scripts/encode_video_with_codec.py | 22 +++++- src/aris/video/utils.py | 22 +++++- tests/unit/test_video_utils.py | 75 +++++++++++++++++++++ 3 files changed, 113 insertions(+), 6 deletions(-) diff --git a/src/aris/scripts/encode_video_with_codec.py b/src/aris/scripts/encode_video_with_codec.py index 2427ca4..c4af071 100644 --- a/src/aris/scripts/encode_video_with_codec.py +++ b/src/aris/scripts/encode_video_with_codec.py @@ -89,7 +89,7 @@ def process_video_filepath( video_codec: str, logger: Logger, force: bool = False, -) -> None: +) -> bool: """ Process a single video file by encoding it with the specified codec. @@ -104,6 +104,9 @@ def process_video_filepath( video_codec (str): The codec to use for encoding the video. logger (Logger): The logger instance to record processing information. force (bool): If True, will overwrite existing files without checking. + + Returns: + bool: True if the video was encoded (or already present), False otherwise. """ filepath_video_with_new_codec = dir_save / filepath_video.name logger.info(f"filepath_video_with_new_codec: {filepath_video_with_new_codec}") @@ -115,6 +118,7 @@ def process_video_filepath( logger.info( f"Skipping because the video is already generated in {filepath_video_with_new_codec}" ) + return True elif video_codec == "h264": filepath_video_with_new_codec.parent.mkdir(parents=True, exist_ok=True) video_utils.encode_video_with_h264_codec( @@ -122,8 +126,10 @@ def process_video_filepath( filepath_output=filepath_video_with_new_codec, ) logger.info(f"Done with video filepath {filepath_video}") + return True else: logger.error(f"Codec conversion {video_codec} not yet implemented") + return False def main(): @@ -154,16 +160,26 @@ def main(): logger.info(f"Saving results in {dir_save}") dir_save.mkdir(parents=True, exist_ok=True) + failed_count = 0 for fp_video in tqdm(filepaths_videos_to_process_shuffled): try: - process_video_filepath( + if not process_video_filepath( filepath_video=fp_video, dir_save=dir_save, logger=logger, video_codec=video_codec, - ) + ): + failed_count += 1 except Exception as e: logger.error(f"Error processing {fp_video}: {e}") + failed_count += 1 + + if failed_count > 0: + logger.error( + f"Failed to encode {failed_count}/" + f"{len(filepaths_videos_to_process_shuffled)} videos" + ) + exit(1) logger.info("Done ✅") diff --git a/src/aris/video/utils.py b/src/aris/video/utils.py index fe08da6..d0e5b42 100644 --- a/src/aris/video/utils.py +++ b/src/aris/video/utils.py @@ -41,14 +41,30 @@ def encode_video_with_h264_codec(filepath_input: Path, filepath_output: Path): try: ( ffmpeg.input(str(filepath_input)) - .output(str(filepath_output), vcodec="libx264", preset="medium") + .output( + str(filepath_output), + vcodec="libx264", + preset="medium", + # libx264 with yuv420p requires even width AND height. ARIS + # sonar frames are frequently odd (e.g. 924x1765), which makes + # the encoder fail to open and write a 0-byte file. Pad each + # dimension up to the next even number (adds at most a 1px + # border, no rescaling). + vf="pad=ceil(iw/2)*2:ceil(ih/2)*2", + # yuv420p + faststart make the output playable in web browsers + # (moov atom is moved to the front for progressive streaming). + pix_fmt="yuv420p", + movflags="+faststart", + ) .run(capture_stdout=True, capture_stderr=True) ) logging.info("Video encoded successfully.") except ffmpeg.Error as e: - logging.error("An error occurred while encoding the video.") - logging.error("Error message:", e.stderr.decode()) + logging.error( + "An error occurred while encoding the video: %s", e.stderr.decode() + ) + raise def get_average_frame( diff --git a/tests/unit/test_video_utils.py b/tests/unit/test_video_utils.py index e61f6da..acd634e 100644 --- a/tests/unit/test_video_utils.py +++ b/tests/unit/test_video_utils.py @@ -5,9 +5,14 @@ metadata reading, and video encoding operations. """ +import io +import subprocess +from pathlib import Path + import cv2 import numpy as np import pytest +from PIL import Image from aris.video.utils import ( encode_video_with_h264_codec, @@ -19,6 +24,43 @@ ) +def _write_odd_dimension_video( + path: Path, width: int, height: int, n_frames: int = 12, fps: int = 15 +) -> None: + """Write an mp4 with odd dimensions, the way pyARIS does. + + cv2.VideoWriter silently rounds odd dimensions down to even, so we pipe + MJPEG frames (which permit odd dimensions) into an mpeg4 mp4 to faithfully + reproduce ARIS-style odd-sized clips. + """ + command = [ + "ffmpeg", + "-y", + "-f", + "image2pipe", + "-vcodec", + "mjpeg", + "-r", + str(fps), + "-i", + "-", + "-an", + "-vcodec", + "mpeg4", + "-q:v", + "5", + str(path), + ] + pipe = subprocess.Popen(command, stdin=subprocess.PIPE, stderr=subprocess.DEVNULL) + for i in range(n_frames): + frame = np.full((height, width, 3), (i * 20) % 255, dtype=np.uint8) + buffer = io.BytesIO() + Image.fromarray(frame).save(buffer, format="JPEG") + pipe.stdin.write(buffer.getvalue()) + pipe.stdin.close() + pipe.wait() + + class TestGetAverageFrame: """Tests for get_average_frame() function.""" @@ -248,3 +290,36 @@ def test_creates_parent_directories(self, sample_video_file, tmp_path): assert output_path.exists() assert output_path.parent.exists() + + def test_encodes_odd_dimension_video(self, tmp_path): + """Regression: odd-dimension input must be padded to even, not dropped. + + libx264 with yuv420p requires even width AND height. ARIS sonar clips + are frequently odd (e.g. 924x1765), which previously made the encoder + fail to open and write a 0-byte file. The input here mirrors how pyARIS + produces video: MJPEG frames (which allow odd dimensions) piped into an + mpeg4 mp4 -- cv2.VideoWriter cannot create odd dimensions, it silently + rounds them down to even. + """ + odd_video = tmp_path / "odd_dimensions.mp4" + _write_odd_dimension_video(odd_video, width=63, height=65) + + # Sanity check: the input really is odd-dimensioned. + cap = cv2.VideoCapture(str(odd_video)) + assert int(cap.get(cv2.CAP_PROP_FRAME_WIDTH)) % 2 == 1 + assert int(cap.get(cv2.CAP_PROP_FRAME_HEIGHT)) % 2 == 1 + cap.release() + + output_path = tmp_path / "encoded_odd.mp4" + encode_video_with_h264_codec(odd_video, output_path) + + # The output must be a real, non-empty file (not a 0-byte stub) with + # both dimensions padded up to the next even number. + assert output_path.exists() + assert output_path.stat().st_size > 0 + cap = cv2.VideoCapture(str(output_path)) + width = int(cap.get(cv2.CAP_PROP_FRAME_WIDTH)) + height = int(cap.get(cv2.CAP_PROP_FRAME_HEIGHT)) + cap.release() + assert width % 2 == 0 + assert height % 2 == 0