From 79f551c4638bf4d57dc6978b64d2d5bef14c95f2 Mon Sep 17 00:00:00 2001 From: Alexander Soare Date: Thu, 3 Oct 2024 13:41:07 +0100 Subject: [PATCH 1/6] Allow arbitrary camera frame dimensions --- .../common/robot_devices/cameras/opencv.py | 21 ++++++++----------- tests/test_cameras.py | 20 +++++++++++++++++- 2 files changed, 28 insertions(+), 13 deletions(-) diff --git a/lerobot/common/robot_devices/cameras/opencv.py b/lerobot/common/robot_devices/cameras/opencv.py index b066a451a..2ac4b21ee 100644 --- a/lerobot/common/robot_devices/cameras/opencv.py +++ b/lerobot/common/robot_devices/cameras/opencv.py @@ -4,6 +4,7 @@ import argparse import concurrent.futures +import logging import math import platform import shutil @@ -276,19 +277,17 @@ def connect(self): raise OSError( f"Can't set {self.fps=} for OpenCVCamera({self.camera_index}). Actual value is {actual_fps}." ) + resolution_info_fstring = ( + "The provided {name} ({value}) could not be set natively on your camera. The settings " + f"are now (width={int(actual_width)}, height={int(actual_height)}. This class will resize the " + "frames to your desired resolution before returning them." + ) if self.width is not None and self.width != actual_width: - raise OSError( - f"Can't set {self.width=} for OpenCVCamera({self.camera_index}). Actual value is {actual_width}." - ) + logging.info(resolution_info_fstring.format(name="width", value=self.width)) if self.height is not None and self.height != actual_height: - raise OSError( - f"Can't set {self.height=} for OpenCVCamera({self.camera_index}). Actual value is {actual_height}." - ) + logging.info(resolution_info_fstring.format(name="height", value=self.height)) self.fps = actual_fps - self.width = actual_width - self.height = actual_height - self.is_connected = True def read(self, temporary_color_mode: str | None = None) -> np.ndarray: @@ -324,9 +323,7 @@ def read(self, temporary_color_mode: str | None = None) -> np.ndarray: h, w, _ = color_image.shape if h != self.height or w != self.width: - raise OSError( - f"Can't capture color image with expected height and width ({self.height} x {self.width}). ({h} x {w}) returned instead." - ) + color_image = cv2.resize(color_image, (self.width, self.height)) # log the number of seconds it took to read the image self.logs["delta_timestamp_s"] = time.perf_counter() - start_time diff --git a/tests/test_cameras.py b/tests/test_cameras.py index 0d5d94425..11d9515cd 100644 --- a/tests/test_cameras.py +++ b/tests/test_cameras.py @@ -11,7 +11,11 @@ import pytest from lerobot import available_robots -from lerobot.common.robot_devices.cameras.opencv import OpenCVCamera, save_images_from_cameras +from lerobot.common.robot_devices.cameras.opencv import ( + OpenCVCamera, + OpenCVCameraConfig, + save_images_from_cameras, +) from lerobot.common.robot_devices.utils import RobotDeviceAlreadyConnectedError, RobotDeviceNotConnectedError from tests.utils import require_robot @@ -131,6 +135,20 @@ def test_camera(request, robot_type): del camera +@pytest.mark.parametrize("robot_type", available_robots) +@require_robot +def test_camera_resize(*_): + """Check that resizing to a resolution not supported by the camera still works.""" + height = 10 + width = 10 + camera = OpenCVCamera(CAMERA_INDEX, OpenCVCameraConfig(height=height, width=width)) + camera.connect() + assert camera.height == height + assert camera.width == width + img = camera.read() + assert img.shape[:2] == (height, width) + + @pytest.mark.parametrize("robot_type", available_robots) @require_robot def test_save_images_from_cameras(tmpdir, request, robot_type): From 860320f1cb2894dc6884c37c0ced1fc5b3da6902 Mon Sep 17 00:00:00 2001 From: Alexander Soare Date: Thu, 3 Oct 2024 14:46:45 +0100 Subject: [PATCH 2/6] add mock test --- tests/conftest.py | 13 +++++++++++++ tests/test_cameras.py | 31 +++++++++++++++++++++++++------ 2 files changed, 38 insertions(+), 6 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 52006f331..d2a269003 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -13,8 +13,10 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. +import platform import traceback +import cv2 import pytest from lerobot.common.utils.utils import init_hydra_config @@ -41,3 +43,14 @@ def is_robot_available(robot_type): traceback.print_exc() print(f"\nA {robot_type} robot is not available.") return False + + +@pytest.fixture +def is_camera_available(request: pytest.FixtureRequest): + camera_index = request.param + if platform.system() == "Linux": + tmp_camera = cv2.VideoCapture(f"/dev/video{camera_index}") + else: + tmp_camera = cv2.VideoCapture(camera_index) + + return tmp_camera.isOpened() diff --git a/tests/test_cameras.py b/tests/test_cameras.py index 11d9515cd..b276b66aa 100644 --- a/tests/test_cameras.py +++ b/tests/test_cameras.py @@ -7,6 +7,9 @@ ``` """ +from contextlib import nullcontext +from unittest.mock import patch + import numpy as np import pytest @@ -135,14 +138,30 @@ def test_camera(request, robot_type): del camera -@pytest.mark.parametrize("robot_type", available_robots) -@require_robot -def test_camera_resize(*_): - """Check that resizing to a resolution not supported by the camera still works.""" +# Note: indirect=True passes the CAMERA_INDEX to the `is_camera_available` fixture, then this returns the +# `is_camera_available` parameter for the test. +@pytest.mark.parametrize("is_camera_available", [CAMERA_INDEX], indirect=True) +def test_camera_resize(is_camera_available: bool): + """ + Check that, even if the requested resolution is not supported natively, the `OpenCVCamera.read` method + returns an image with the requested resolution. + + When `is_camera_available=True` this test also checks that the `OpenCVCamera.connect` works with natively + unsupported resolutions, otherwise if `is_camera_available=False` we have to use a mock patch which + unfortunately can't test this. + """ height = 10 width = 10 - camera = OpenCVCamera(CAMERA_INDEX, OpenCVCameraConfig(height=height, width=width)) - camera.connect() + camera = OpenCVCamera(0, OpenCVCameraConfig(height=height, width=width)) + with nullcontext() if is_camera_available else patch("cv2.VideoCapture") as mock_video_capture: + if not is_camera_available: + # Set an output resolution that is different from the requested one. + mock_video_capture.return_value.read.return_value = ( + True, + np.zeros((2 * height, 2 * width), dtype=np.uint8), + ) + mock_video_capture.return_value.isOpened.return_value = True + camera.connect() assert camera.height == height assert camera.width == width img = camera.read() From a2625522ead63b824b720072f9e91de1c43eb58a Mon Sep 17 00:00:00 2001 From: Alexander Soare Date: Thu, 3 Oct 2024 14:52:07 +0100 Subject: [PATCH 3/6] nit type hint --- tests/conftest.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/conftest.py b/tests/conftest.py index d2a269003..f96708d2a 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -46,7 +46,7 @@ def is_robot_available(robot_type): @pytest.fixture -def is_camera_available(request: pytest.FixtureRequest): +def is_camera_available(request: pytest.FixtureRequest) -> bool: camera_index = request.param if platform.system() == "Linux": tmp_camera = cv2.VideoCapture(f"/dev/video{camera_index}") From cfafc88580fed5991c29f635796fdd14204a57b1 Mon Sep 17 00:00:00 2001 From: Alexander Soare Date: Thu, 3 Oct 2024 14:58:40 +0100 Subject: [PATCH 4/6] improve test coverage --- tests/test_cameras.py | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/tests/test_cameras.py b/tests/test_cameras.py index b276b66aa..d9c164f4d 100644 --- a/tests/test_cameras.py +++ b/tests/test_cameras.py @@ -140,8 +140,14 @@ def test_camera(request, robot_type): # Note: indirect=True passes the CAMERA_INDEX to the `is_camera_available` fixture, then this returns the # `is_camera_available` parameter for the test. +@pytest.mark.parametrize("request_resolution", [(10, 10)]) +# This parameter is only used when the camera is not available. Checks for various configurations of +# (mis)match between the requested resolution and the target resolution. +@pytest.mark.parametrize("read_resolution", [(10, 10), (10, 20), (20, 10), (20, 20)]) @pytest.mark.parametrize("is_camera_available", [CAMERA_INDEX], indirect=True) -def test_camera_resize(is_camera_available: bool): +def test_camera_resize( + request_resolution: tuple[int, int], read_resolution: tuple[int, int], is_camera_available: bool +): """ Check that, even if the requested resolution is not supported natively, the `OpenCVCamera.read` method returns an image with the requested resolution. @@ -150,22 +156,20 @@ def test_camera_resize(is_camera_available: bool): unsupported resolutions, otherwise if `is_camera_available=False` we have to use a mock patch which unfortunately can't test this. """ - height = 10 - width = 10 - camera = OpenCVCamera(0, OpenCVCameraConfig(height=height, width=width)) + camera = OpenCVCamera(0, OpenCVCameraConfig(height=request_resolution[0], width=request_resolution[1])) with nullcontext() if is_camera_available else patch("cv2.VideoCapture") as mock_video_capture: if not is_camera_available: # Set an output resolution that is different from the requested one. mock_video_capture.return_value.read.return_value = ( True, - np.zeros((2 * height, 2 * width), dtype=np.uint8), + np.zeros(read_resolution, dtype=np.uint8), ) mock_video_capture.return_value.isOpened.return_value = True camera.connect() - assert camera.height == height - assert camera.width == width + assert camera.height == request_resolution[0] + assert camera.width == request_resolution[1] img = camera.read() - assert img.shape[:2] == (height, width) + assert img.shape[:2] == request_resolution @pytest.mark.parametrize("robot_type", available_robots) From db529c65cfd8c56425f303fa9e37820e3fb80d2e Mon Sep 17 00:00:00 2001 From: Alexander Soare Date: Thu, 3 Oct 2024 14:59:59 +0100 Subject: [PATCH 5/6] reunite comment with its loc --- tests/test_cameras.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_cameras.py b/tests/test_cameras.py index d9c164f4d..a6eb75906 100644 --- a/tests/test_cameras.py +++ b/tests/test_cameras.py @@ -138,12 +138,12 @@ def test_camera(request, robot_type): del camera -# Note: indirect=True passes the CAMERA_INDEX to the `is_camera_available` fixture, then this returns the -# `is_camera_available` parameter for the test. @pytest.mark.parametrize("request_resolution", [(10, 10)]) # This parameter is only used when the camera is not available. Checks for various configurations of # (mis)match between the requested resolution and the target resolution. @pytest.mark.parametrize("read_resolution", [(10, 10), (10, 20), (20, 10), (20, 20)]) +# Note: indirect=True passes the CAMERA_INDEX to the `is_camera_available` fixture, then this returns the +# `is_camera_available` parameter for the test. @pytest.mark.parametrize("is_camera_available", [CAMERA_INDEX], indirect=True) def test_camera_resize( request_resolution: tuple[int, int], read_resolution: tuple[int, int], is_camera_available: bool From 0a6d65daba00e2f13c3a7d007ce574311f8386c2 Mon Sep 17 00:00:00 2001 From: Alexander Soare Date: Thu, 3 Oct 2024 15:06:08 +0100 Subject: [PATCH 6/6] typo --- lerobot/common/robot_devices/cameras/opencv.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lerobot/common/robot_devices/cameras/opencv.py b/lerobot/common/robot_devices/cameras/opencv.py index 2ac4b21ee..b5ec13cea 100644 --- a/lerobot/common/robot_devices/cameras/opencv.py +++ b/lerobot/common/robot_devices/cameras/opencv.py @@ -279,7 +279,7 @@ def connect(self): ) resolution_info_fstring = ( "The provided {name} ({value}) could not be set natively on your camera. The settings " - f"are now (width={int(actual_width)}, height={int(actual_height)}. This class will resize the " + f"are now (width={int(actual_width)}, height={int(actual_height)}). This class will resize the " "frames to your desired resolution before returning them." ) if self.width is not None and self.width != actual_width: