refactor(cameras): fps, width and height are optional at camera level, these 3 are now moved to the camera base class, the width and height specified in the config is now the one output by read() methods

lerobot/common/cameras/camera.py

@@ -18,11 +18,16 @@ import abc
 
 import numpy as np
 
-from .configs import ColorMode
+from .configs import CameraConfig, ColorMode
 
 
 # NOTE(Steven): Consider something like configure() if makes sense for both cameras
 class Camera(abc.ABC):
+    def __init__(self, config: CameraConfig):
+        self.fps: int | None = config.fps
+        self.width: int | None = config.width
+        self.height: int | None = config.height
+
     @property
     @abc.abstractmethod
     def is_connected(self) -> bool:

lerobot/common/cameras/configs.py

@@ -35,6 +35,10 @@ class Cv2Rotation(Enum):
 
 @dataclass
 class CameraConfig(draccus.ChoiceRegistry, abc.ABC):
+    fps: int | None = None
+    width: int | None = None
+    height: int | None = None
+
     @property
     def type(self) -> str:
         return self.get_choice_name(self.__class__)

lerobot/common/cameras/intel/camera_realsense.py

@@ -115,6 +115,9 @@ class RealSenseCamera(Camera):
         Args:
             config: The configuration settings for the camera.
         """
+
+        super().__init__(config)
+
         self.config = config
 
         if config.name is not None:  # TODO(Steven): Do we want to continue supporting this?
@@ -124,11 +127,6 @@ class RealSenseCamera(Camera):
         else:
             raise ValueError("RealSenseCameraConfig must provide either 'serial_number' or 'name'.")
 
-        self.capture_width: int | None = config.width
-        self.capture_height: int | None = config.height
-        self.width: int | None = None
-        self.height: int | None = None
-
         self.fps: int | None = config.fps
         self.channels: int = config.channels
         self.color_mode: ColorMode = config.color_mode
@@ -145,6 +143,14 @@ class RealSenseCamera(Camera):
 
         self.rotation: int | None = get_cv2_rotation(config.rotation)
 
+        # NOTE(Steven): What happens if rotation is specified but we leave width and height to None?
+        # NOTE(Steven): Should we enforce these parameters if rotation is set?
+        if self.height and self.width:
+            if self.rotation in [cv2.ROTATE_90_CLOCKWISE, cv2.ROTATE_90_COUNTERCLOCKWISE]:
+                self.prerotated_width, self.prerotated_height = self.height, self.width
+            else:
+                self.prerotated_width, self.prerotated_height = self.width, self.height
+
     def __str__(self) -> str:
         """Returns a string representation of the camera instance."""
         return f"{self.__class__.__name__}({self.serial_number})"
@@ -246,24 +252,24 @@ class RealSenseCamera(Camera):
         rs_config = rs.config()
         rs.config.enable_device(rs_config, self.serial_number)
 
-        if self.capture_width and self.capture_height and self.fps:
+        if self.width and self.height and self.fps:
             logger.debug(
-                f"Requesting Color Stream: {self.capture_width}x{self.capture_height} @ {self.fps} FPS, Format: {rs.format.rgb8}"
+                f"Requesting Color Stream: {self.prerotated_width}x{self.prerotated_height} @ {self.fps} FPS, Format: {rs.format.rgb8}"
             )
             rs_config.enable_stream(
-                rs.stream.color, self.capture_width, self.capture_height, rs.format.rgb8, self.fps
+                rs.stream.color, self.prerotated_width, self.prerotated_height, rs.format.rgb8, self.fps
             )
         else:
             logger.debug(f"Requesting Color Stream: Default settings, Format: {rs.stream.color}")
             rs_config.enable_stream(rs.stream.color)
 
         if self.use_depth:
-            if self.capture_width and self.capture_height and self.fps:
+            if self.width and self.height and self.fps:
                 logger.debug(
-                    f"Requesting Depth Stream: {self.capture_width}x{self.capture_height} @ {self.fps} FPS, Format: {rs.format.z16}"
+                    f"Requesting Depth Stream: {self.prerotated_width}x{self.prerotated_height} @ {self.fps} FPS, Format: {rs.format.z16}"
                 )
                 rs_config.enable_stream(
-                    rs.stream.depth, self.capture_width, self.capture_height, rs.format.z16, self.fps
+                    rs.stream.depth, self.prerotated_width, self.prerotated_height, rs.format.z16, self.fps
                 )
             else:
                 logger.debug(f"Requesting Depth Stream: Default settings, Format: {rs.stream.depth}")
@@ -289,8 +295,7 @@ class RealSenseCamera(Camera):
             raise DeviceNotConnectedError(f"Cannot validate settings for {self} as it is not connected.")
 
         self._validate_fps()
-        self._validate_capture_width()
+        self._validate_width_and_height()
-        self._validate_capture_height()
 
         if self.use_depth:
             try:
@@ -319,13 +324,6 @@ class RealSenseCamera(Camera):
             except Exception as e:
                 logger.error(f"Failed to get or validate active depth stream profile on {self}: {e}")
 
-        # Set final width/height considering rotation
-        if self.rotation in [cv2.ROTATE_90_CLOCKWISE, cv2.ROTATE_90_COUNTERCLOCKWISE]:
-            self.width, self.height = self.capture_height, self.capture_width
-        else:
-            self.width, self.height = self.capture_width, self.capture_height
-        logger.debug(f"Final image dimensions set to: {self.width}x{self.height} (after rotation if any)")
-
     # NOTE(Steven): Add a wamr-up period time config
     def connect(self):
         """
@@ -385,43 +383,39 @@ class RealSenseCamera(Camera):
             )
         logger.debug(f"FPS set to {actual_fps} for {self}.")
 
-    def _validate_capture_width(self) -> None:
+    def _validate_width_and_height(self) -> None:
-        """Validates and sets the internal capture width based on actual stream width."""
+        """Validates and sets the internal capture width and height based on actual stream width."""
 
         color_stream = self.rs_profile.get_stream(rs.stream.color).as_video_stream_profile()
         actual_width = int(round(color_stream.width()))
+        actual_height = int(round(color_stream.height()))
 
-        if self.capture_width is None:
+        if self.width is None or self.height is None:
-            self.capture_width = actual_width
+            if self.rotation in [cv2.ROTATE_90_CLOCKWISE, cv2.ROTATE_90_COUNTERCLOCKWISE]:
-            logger.info(f"Capture width not specified, using camera default: {self.capture_width} pixels.")
+                self.width, self.height = actual_height, actual_width
+                self.prerotated_width, self.prerotated_height = actual_width, actual_height
+            else:
+                self.width, self.height = actual_width, actual_height
+                self.prerotated_width, self.prerotated_height = actual_width, actual_height
+            logger.info(f"Capture width set to camera default: {self.width}.")
+            logger.info(f"Capture height set to camera default: {self.height}.")
             return
 
-        if self.capture_width != actual_width:
+        if self.prerotated_width != actual_width:
             logger.warning(
-                f"Requested capture width {self.capture_width} for {self}, but camera reported {actual_width}."
+                f"Requested capture width {self.prerotated_width} for {self}, but camera reported {actual_width}."
             )
             raise RuntimeError(
-                f"Failed to set requested capture width {self.capture_width} for {self}. Actual value: {actual_width}."
+                f"Failed to set requested capture width {self.prerotated_width} for {self}. Actual value: {actual_width}."
             )
         logger.debug(f"Capture width set to {actual_width} for {self}.")
 
-    def _validate_capture_height(self) -> None:
+        if self.prerotated_height != actual_height:
-        """Validates and sets the internal capture height based on actual stream height."""
-
-        color_stream = self.rs_profile.get_stream(rs.stream.color).as_video_stream_profile()
-        actual_height = int(round(color_stream.height()))
-
-        if self.capture_height is None:
-            self.capture_height = actual_height
-            logger.info(f"Capture height not specified, using camera default: {self.capture_height} pixels.")
-            return
-
-        if self.capture_height != actual_height:
             logger.warning(
-                f"Requested capture height {self.capture_height} for {self}, but camera reported {actual_height}."
+                f"Requested capture height {self.prerotated_height} for {self}, but camera reported {actual_height}."
             )
             raise RuntimeError(
-                f"Failed to set requested capture height {self.capture_height} for {self}. Actual value: {actual_height}."
+                f"Failed to set requested capture height {self.prerotated_height} for {self}. Actual value: {actual_height}."
             )
         logger.debug(f"Capture height set to {actual_height} for {self}.")
 
@@ -478,9 +472,9 @@ class RealSenseCamera(Camera):
             # NOTE(Steven): Simplified version of _postprocess_image() for depth image
             h, w = depth_map.shape
 
-            if h != self.capture_height or w != self.capture_width:
+            if h != self.prerotated_height or w != self.prerotated_width:
                 raise RuntimeError(
-                    f"Captured frame dimensions ({h}x{w}) do not match configured capture dimensions ({self.capture_height}x{self.capture_width}) for {self}."
+                    f"Captured frame dimensions ({h}x{w}) do not match configured capture dimensions ({self.prerotated_height}x{self.prerotated_width}) for {self}."
                 )
 
             if self.rotation in [cv2.ROTATE_90_CLOCKWISE, cv2.ROTATE_90_COUNTERCLOCKWISE]:
@@ -514,7 +508,7 @@ class RealSenseCamera(Camera):
         Raises:
             ValueError: If the requested `color_mode` is invalid.
             RuntimeError: If the raw frame dimensions do not match the configured
-                          `capture_width` and `capture_height`.
+                          `width` and `height`.
         """
         requested_color_mode = self.color_mode if color_mode is None else color_mode
 
@@ -525,9 +519,9 @@ class RealSenseCamera(Camera):
 
         h, w, c = image.shape
 
-        if h != self.capture_height or w != self.capture_width:
+        if h != self.prerotated_height or w != self.prerotated_width:
             raise RuntimeError(
-                f"Captured frame dimensions ({h}x{w}) do not match configured capture dimensions ({self.capture_height}x{self.capture_width}) for {self}."
+                f"Captured frame dimensions ({h}x{w}) do not match configured capture dimensions ({self.prerotated_height}x{self.prerotated_width}) for {self}."
             )
         if c != self.channels:
             logger.warning(

lerobot/common/cameras/intel/configuration_realsense.py

@@ -35,16 +35,11 @@ class RealSenseCameraConfig(CameraConfig):
 
     name: str | None = None
     serial_number: int | None = None
-    fps: int | None = None
-    width: int | None = None  # NOTE(Steven): Make this not None allowed!
-    height: int | None = None
     color_mode: ColorMode = ColorMode.RGB
     channels: int | None = 3
     use_depth: bool = False
-    force_hardware_reset: bool = True
+    # NOTE(Steven): Check how draccus would deal with this str -> enum
-    rotation: Cv2Rotation = (
+    rotation: Cv2Rotation = Cv2Rotation.NO_ROTATION
-        Cv2Rotation.NO_ROTATION
-    )  # NOTE(Steven): Check how draccus would deal with this str -> enum
 
     def __post_init__(self):
         if self.color_mode not in (ColorMode.RGB, ColorMode.BGR):

lerobot/common/cameras/opencv/camera_opencv.py

@@ -111,14 +111,11 @@ class OpenCVCamera(Camera):
         Args:
             config: The configuration settings for the camera.
         """
+        super().__init__(config)
+
         self.config = config
         self.index_or_path: IndexOrPath = config.index_or_path
 
-        self.capture_width: int | None = config.width
-        self.capture_height: int | None = config.height
-        self.width: int | None = None
-        self.height: int | None = None
-
         self.fps: int | None = config.fps
         self.channels: int = config.channels
         self.color_mode: ColorMode = config.color_mode
@@ -134,6 +131,14 @@ class OpenCVCamera(Camera):
         self.rotation: int | None = get_cv2_rotation(config.rotation)
         self.backend: int = get_cv2_backend()  # NOTE(Steven): If I specify backend the opencv open fails
 
+        # NOTE(Steven): What happens if rotation is specified but we leave width and height to None?
+        # NOTE(Steven): Should we enforce these parameters if rotation is set?
+        if self.height and self.width:
+            if self.rotation in [cv2.ROTATE_90_CLOCKWISE, cv2.ROTATE_90_COUNTERCLOCKWISE]:
+                self.prerotated_width, self.prerotated_height = self.height, self.width
+            else:
+                self.prerotated_width, self.prerotated_height = self.width, self.height
+
     def __str__(self) -> str:
         """Returns a string representation of the camera instance."""
         return f"{self.__class__.__name__}({self.index_or_path})"
@@ -165,14 +170,7 @@ class OpenCVCamera(Camera):
             raise DeviceNotConnectedError(f"Cannot configure settings for {self} as it is not connected.")
 
         self._validate_fps()
-        self._validate_capture_width()
+        self._validate_width_and_height()
-        self._validate_capture_height()
-
-        if self.rotation in [cv2.ROTATE_90_CLOCKWISE, cv2.ROTATE_90_COUNTERCLOCKWISE]:
-            self.width, self.height = self.capture_height, self.capture_width
-        else:
-            self.width, self.height = self.capture_width, self.capture_height
-        logger.debug(f"Final image dimensions set to: {self.width}x{self.height} (after rotation if any)")
 
     def connect(self):
         """
@@ -230,43 +228,41 @@ class OpenCVCamera(Camera):
             )
         logger.debug(f"FPS set to {actual_fps} for {self}.")
 
-    def _validate_capture_width(self) -> None:
+    def _validate_width_and_height(self) -> None:
-        """Validates and sets the camera's frame capture width."""
+        """Validates and sets the camera's frame capture width and height."""
 
         actual_width = int(round(self.videocapture_camera.get(cv2.CAP_PROP_FRAME_WIDTH)))
+        actual_height = int(round(self.videocapture_camera.get(cv2.CAP_PROP_FRAME_HEIGHT)))
 
-        if self.capture_width is None:
+        # NOTE(Steven): When do we constraint the possibility of only setting one?
-            self.capture_width = actual_width
+        if self.width is None or self.height is None:
-            logger.info(f"Capture width set to camera default: {self.capture_width}.")
+            if self.rotation in [cv2.ROTATE_90_CLOCKWISE, cv2.ROTATE_90_COUNTERCLOCKWISE]:
+                self.width, self.height = actual_height, actual_width
+                self.prerotated_width, self.prerotated_height = actual_width, actual_height
+            else:
+                self.width, self.height = actual_width, actual_height
+                self.prerotated_width, self.prerotated_height = actual_width, actual_height
+            logger.info(f"Capture width set to camera default: {self.width}.")
+            logger.info(f"Capture height set to camera default: {self.height}.")
             return
 
-        success = self.videocapture_camera.set(cv2.CAP_PROP_FRAME_WIDTH, float(self.capture_width))
+        success = self.videocapture_camera.set(cv2.CAP_PROP_FRAME_WIDTH, float(self.prerotated_width))
-        if not success or self.capture_width != actual_width:
+        if not success or self.prerotated_width != actual_width:
             logger.warning(
-                f"Requested capture width {self.capture_width} for {self}, but camera reported {actual_width} (set success: {success})."
+                f"Requested capture width {self.prerotated_width} for {self}, but camera reported {actual_width} (set success: {success})."
             )
             raise RuntimeError(
-                f"Failed to set requested capture width {self.capture_width} for {self}. Actual value: {actual_width}."
+                f"Failed to set requested capture width {self.prerotated_width} for {self}. Actual value: {actual_width}."
             )
         logger.debug(f"Capture width set to {actual_width} for {self}.")
 
-    def _validate_capture_height(self) -> None:
+        success = self.videocapture_camera.set(cv2.CAP_PROP_FRAME_HEIGHT, float(self.prerotated_height))
-        """Validates and sets the camera's frame capture height."""
+        if not success or self.prerotated_height != actual_height:
-
-        actual_height = int(round(self.videocapture_camera.get(cv2.CAP_PROP_FRAME_HEIGHT)))
-
-        if self.capture_height is None:
-            self.capture_height = actual_height
-            logger.info(f"Capture height set to camera default: {self.capture_height}.")
-            return
-
-        success = self.videocapture_camera.set(cv2.CAP_PROP_FRAME_HEIGHT, float(self.capture_height))
-        if not success or self.capture_height != actual_height:
             logger.warning(
-                f"Requested capture height {self.capture_height} for {self}, but camera reported {actual_height} (set success: {success})."
+                f"Requested capture height {self.prerotated_height} for {self}, but camera reported {actual_height} (set success: {success})."
             )
             raise RuntimeError(
-                f"Failed to set requested capture height {self.capture_height} for {self}. Actual value: {actual_height}."
+                f"Failed to set requested capture height {self.prerotated_height} for {self}. Actual value: {actual_height}."
             )
         logger.debug(f"Capture height set to {actual_height} for {self}.")
 
@@ -394,7 +390,7 @@ class OpenCVCamera(Camera):
         Raises:
             ValueError: If the requested `color_mode` is invalid.
             RuntimeError: If the raw frame dimensions do not match the configured
-                          `capture_width` and `capture_height`.
+                          `width` and `height`.
         """
         requested_color_mode = self.color_mode if color_mode is None else color_mode
 
@@ -405,9 +401,9 @@ class OpenCVCamera(Camera):
 
         h, w, c = image.shape
 
-        if h != self.capture_height or w != self.capture_width:
+        if h != self.prerotated_height or w != self.prerotated_width:
             raise RuntimeError(
-                f"Captured frame dimensions ({h}x{w}) do not match configured capture dimensions ({self.capture_height}x{self.capture_width}) for {self}."
+                f"Captured frame dimensions ({h}x{w}) do not match configured capture dimensions ({self.prerotated_height}x{self.prerotated_width}) for {self}."
             )
         if c != self.channels:
             logger.warning(

lerobot/common/cameras/opencv/configuration_opencv.py

@@ -1,4 +1,4 @@
-from dataclasses import dataclass
+from dataclasses import dataclass, field
 from pathlib import Path
 
 from ..configs import CameraConfig, ColorMode, Cv2Rotation
@@ -9,7 +9,7 @@ from ..configs import CameraConfig, ColorMode, Cv2Rotation
 class OpenCVCameraConfig(CameraConfig):
     """
     Example of tested options for Intel Real Sense D405:
-
+    #NOTE(Steven): update this doc
     ```python
     OpenCVCameraConfig(0, 30, 640, 480)
     OpenCVCameraConfig(0, 60, 640, 480)
@@ -18,10 +18,9 @@ class OpenCVCameraConfig(CameraConfig):
     ```
     """
 
-    index_or_path: int | Path
+    index_or_path: int | Path = field(
-    fps: int | None = None
+        default=...,
-    width: int | None = None
+    )
-    height: int | None = None
     color_mode: ColorMode = ColorMode.RGB
     channels: int = 3  # NOTE(Steven): Why is this a config?
     rotation: Cv2Rotation = Cv2Rotation.NO_ROTATION