Memory optimize. #19
Memory optimize. #19
Conversation
…ean tested for rand. Signed-off-by: Sandro Cantarella <sandro@Sandros-Mac-mini.fritz.box>
Signed-off-by: Sandro Cantarella <sandro@Sandros-Mac-mini.fritz.box>
Signed-off-by: Sandro Cantarella <sandro@Sandros-Mac-mini.fritz.box>
…unction with C based from mcvrender. Signed-off-by: Sandro Cantarella <sandro@509dc541-f930-4e46-87da-4482e9d9a7bb.fritz.box>
Signed-off-by: Sandro Cantarella <sandro@13de464c-e6f5-4665-b8d7-d6acf4207b24.fritz.box>
Signed-off-by: Sandro Cantarella <sandro@13de464c-e6f5-4665-b8d7-d6acf4207b24.fritz.box>
Signed-off-by: Sandro Cantarella <sandro@Sandros-Mac-mini.fritz.box>
Signed-off-by: Sandro Cantarella <sandro@Sandros-Mac-mini.fritz.box>
Signed-off-by: Sandro Cantarella <sandro@03829bc9-740a-4ae1-9848-e7d8baa7c8ba.fritz.box>
WalkthroughRemoves optimized/batch drawing helpers and simplifies alpha handling in drawable. Adds docstrings in the rand256 parser. Introduces pil_size_rotation and adjusts size computation, plus memory management improvements and persistent working buffers in handlers. Expands a type union, reduces logging, pins mvcrender to 0.0.6, and applies minor test refactors. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant C as Caller
participant RH as Rand256Handler
participant U as Utils
participant D as Drawable
participant BL as img_base_layer (np)
participant WL as img_work_layer (np)
participant PIL as PIL.Image
C->>RH: get_image_from_rrm(m_json, destinations?)
RH->>RH: if BL exists and outdated -> delete BL
RH->>RH: ensure BL from JSON (allocate/copy)
Note right of RH: Old intermediates deleted to free memory
RH->>RH: if WL absent/mismatch -> allocate WL (shape/dtype of BL)
RH->>WL: copy BL into WL
RH->>D: draw elements onto WL (async/iterative)
D-->>RH: WL with drawings
RH->>U: convert WL to PIL (as needed)
U-->>RH: PIL image
RH-->>C: PIL
Note over WL: WL persists across frames (reused)
sequenceDiagram
autonumber
participant CS as CameraShared
participant U as Utils
participant P as PIL.Image
CS->>CS: to_dict()
alt new_image present
CS->>U: pil_size_rotation(image_rotate, new_image)
U-->>CS: (width, height)
CS-->>CS: size = returned tuple
else
CS-->>CS: size = (10, 10)
end
CS-->>CS: build dict with size respecting rotation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
SCR/valetudo_map_parser/config/rand256_parser.py (1)
277-279: Use block_header_length (int16) for block advancement, not 8-bit header fieldIncrement currently adds _get_int8(header, 2). Since block_header_length was already parsed as int16, advance by block_header_length for consistency and to avoid mis-parsing.
Apply this diff:
- block_start_position = ( - block_start_position + block_data_length + self._get_int8(header, 2) - ) + block_start_position = ( + block_start_position + block_header_length + block_data_length + )SCR/valetudo_map_parser/config/utils.py (1)
188-198: Fix return shape on exception path (must return tuple)async_get_image is annotated to return Tuple[PilPNG | bytes, dict], but the exception path returns a single value. Return (image_or_bytes, {}) to avoid runtime/type issues.
Apply this diff:
- return ( - self.shared.last_image if hasattr(self.shared, "last_image") else None - ) + img = self.shared.last_image if hasattr(self.shared, "last_image") else None + return img, {}SCR/valetudo_map_parser/rand256_handler.py (1)
396-405: Correct return annotation/signature for get_rooms_attributesThe method returns a single RoomsProperties (or None), not a tuple. Fixing the annotation avoids confusion.
Apply this diff:
- async def get_rooms_attributes( - self, destinations: JsonType = None - ) -> tuple[RoomsProperties, Any, Any]: + async def get_rooms_attributes( + self, destinations: Destinations | None = None + ) -> RoomsProperties | None: """Return the rooms attributes."""
🧹 Nitpick comments (6)
SCR/valetudo_map_parser/rooms_handler.py (1)
164-168: Good memory cleanup; consider boolean masks for further reductionExplicitly deleting intermediates is helpful. You can cut mask memory further by:
- Using dtype=bool for local_mask/struct_elem and keeping morph results as bool until hull extraction.
- Cast to uint8 only if a downstream consumer truly needs it.
Example:
- local_mask = np.zeros((local_height, local_width), dtype=np.uint8) + local_mask = np.zeros((local_height, local_width), dtype=bool) ... - struct_elem = np.ones((3, 3), dtype=np.uint8) + struct_elem = np.ones((3, 3), dtype=bool) - eroded = binary_erosion(local_mask, structure=struct_elem, iterations=1) - mask = binary_dilation(eroded, structure=struct_elem, iterations=1).astype(np.uint8) + eroded = binary_erosion(local_mask, structure=struct_elem, iterations=1) + mask = binary_dilation(eroded, structure=struct_elem, iterations=1)convex_hull_outline works with a boolean mask (via np.where(mask)). Convert to uint8 only if needed later.
Also applies to: 172-174
SCR/valetudo_map_parser/config/drawable.py (1)
56-56: Simplified alpha path looks good; minor quality tweak optionalThe center-sample-and-fill approach is efficient. If you want slightly better edge fidelity for large pixel_size, consider sampling multiple points (e.g., average of 4 corners) before filling; keeps complexity low while improving gradients.
Otherwise LGTM.
Also applies to: 68-80
tests/refactored.py (2)
575-581: Incorrect type hint for centerscenters is iterated as a sequence of points and docs say (N, 2) array. Update hint to reflect multiple centers.
- def draw_filled_circle(image: np.ndarray, centers: Tuple[int, int], ... + from typing import Iterable, Sequence, Tuple + def draw_filled_circle(image: np.ndarray, centers: Sequence[Tuple[int, int]] | np.ndarray, ...
610-645: Avoid executor overhead for small, CPU-light preprocessingBuilding the centers array is trivial; calling run_in_executor adds unnecessary overhead. Inline the comprehension:
- centers = await asyncio.get_running_loop().run_in_executor( - None, extract_centers, obstacle_info_list - ) - Drawable.draw_filled_circle(image, centers, 6, color) + centers = np.array([[obs["points"]["x"], obs["points"]["y"]] for obs in obstacle_info_list], dtype=np.int32) + Drawable.draw_filled_circle(image, centers, 6, color)SCR/valetudo_map_parser/config/utils.py (1)
698-707: Add type hints to pil_size_rotation for claritySmall polish: annotate parameters and return type.
Apply this diff:
-def pil_size_rotation(image_rotate, pil_img): - """Return the size of the image.""" +def pil_size_rotation(image_rotate: int, pil_img: PilPNG | None) -> tuple[int, int]: + """Return (width, height) adjusted for 90/270 rotation; (0, 0) if pil_img is None.""" if not pil_img: return 0, 0 if image_rotate in [0, 180]: width, height = pil_img.size else: height, width = pil_img.size return width, heightSCR/valetudo_map_parser/rand256_handler.py (1)
389-394: Clarify log message when no resize is neededThis branch executes when resizing isn’t required; “Invalid image dimensions” is misleading. Consider logging a low-level debug or nothing.
- LOGGER.warning( - "%s: Invalid image dimensions. Returning original image.", - self.file_name, - ) + # No aspect ratio change requested; return original image + # LOGGER.debug("%s: No resizing applied.", self.file_name)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
SCR/valetudo_map_parser/config/drawable.py(1 hunks)SCR/valetudo_map_parser/config/rand256_parser.py(6 hunks)SCR/valetudo_map_parser/config/shared.py(2 hunks)SCR/valetudo_map_parser/config/types.py(1 hunks)SCR/valetudo_map_parser/config/utils.py(3 hunks)SCR/valetudo_map_parser/hypfer_draw.py(0 hunks)SCR/valetudo_map_parser/hypfer_handler.py(2 hunks)SCR/valetudo_map_parser/rand256_handler.py(8 hunks)SCR/valetudo_map_parser/rooms_handler.py(1 hunks)pyproject.toml(1 hunks)tests/refactored.py(5 hunks)
💤 Files with no reviewable changes (1)
- SCR/valetudo_map_parser/hypfer_draw.py
🧰 Additional context used
🧬 Code graph analysis (5)
SCR/valetudo_map_parser/hypfer_handler.py (1)
SCR/valetudo_map_parser/config/utils.py (1)
async_copy_array(477-479)
SCR/valetudo_map_parser/rand256_handler.py (4)
SCR/valetudo_map_parser/config/types.py (1)
Destinations(37-41)SCR/valetudo_map_parser/config/async_utils.py (2)
AsyncPIL(38-66)async_fromarray(42-44)SCR/valetudo_map_parser/hypfer_draw.py (1)
async_get_robot_in_room(379-542)SCR/valetudo_map_parser/config/utils.py (1)
async_copy_array(477-479)
SCR/valetudo_map_parser/config/shared.py (1)
SCR/valetudo_map_parser/config/utils.py (1)
pil_size_rotation(698-706)
SCR/valetudo_map_parser/config/drawable.py (1)
SCR/valetudo_map_parser/config/colors.py (1)
sample_and_blend_color(457-519)
tests/refactored.py (2)
SCR/valetudo_map_parser/hypfer_draw.py (1)
ImageDraw(19-573)SCR/valetudo_map_parser/config/drawable.py (1)
async_draw_obstacles(548-604)
🔇 Additional comments (11)
SCR/valetudo_map_parser/config/types.py (1)
41-41: LGTM: broader Destinations.updated typeAllowing int in addition to float is compatible and practical.
pyproject.toml (1)
21-21: Pinning to non-existent mvcrender==0.0.6 will break installs and demands Python ≥3.13PyPI’s latest release is 0.0.5 (which itself requires Python ≥3.13); 0.0.6 isn’t published. Either pin to
mvcrender ==0.0.5(if you target ≥3.13) or use a range like>=0.0.5,<0.1.0. Manually confirm that v0.0.5 actually exports the needed symbols inmvcrender.blend(get_blended_color, sample_and_blend_color) andmvcrender.draw(circle_u8, line_u8).SCR/valetudo_map_parser/config/shared.py (1)
219-220: Rotated size computation looks good; verify zero-size semanticsUsing pil_size_rotation is correct and rotation-safe. It returns (0, 0) when pil_image is None; confirm consumers tolerate 0,0 (if they previously assumed non-zero fallback).
SCR/valetudo_map_parser/hypfer_handler.py (2)
257-263: Proactive base-layer cleanup reduces peak memoryDeleting the previous base layer before assigning the new copy is a good move to keep memory bounded.
271-280: Persistent working buffer reuse is correctShape/dtype-guarded reallocation + np.copyto avoids per-frame allocations. This should lower memory churn.
SCR/valetudo_map_parser/config/utils.py (2)
115-121: Good: releasing previous last_image before rolloverClosing the old last_image before reassigning helps cap memory. Safe and aligned with the fallback logic.
240-250: prepare_resize_params: rotation-aware sizing is correctUsing pil_size_rotation here makes downstream resizing consistent with rotation.
SCR/valetudo_map_parser/rand256_handler.py (4)
62-62: Persistent working buffer field is a good additionKeeping img_work_layer across frames avoids repeated large allocations.
163-177: Buffer reuse block looks correctShape/dtype checks + np.copyto into np.empty_like(self.img_base_layer) are appropriate to reduce allocs.
246-265: Base/source array cleanup before/after copy is appropriateDeleting the previous base and the transient source array will reduce peak memory. Order is safe.
22-25: Verify mvcrender version pin
The project pinsmvcrender==0.0.6in pyproject.toml but only0.0.5is published on PyPI. Confirm that0.0.6exists before release—or adjust the pin to a published version.
Remove possible leak to keep the images in a baseline of around 250mb per frame.
Summary by CodeRabbit
New Features
Refactor
Bug Fixes
Documentation
Chores