Skip to content

fix(transform): preserve declared empty camera channels - #654

Closed
harshitethic wants to merge 3 commits into
Hebbian-Robotics:mainfrom
harshitethic:fix/preserve-empty-camera-channel-528
Closed

harshitethic wants to merge 3 commits into
Hebbian-Robotics:mainfrom
harshitethic:fix/preserve-empty-camera-channel-528

Conversation

@harshitethic

Copy link
Copy Markdown
Contributor

Summary

  • keep declared camera channels in the canonical MCAP even when they contain zero messages
  • register those channels with the canonical CompressedVideo schema without invoking ffmpeg
  • add a regression proving the empty camera survives and changes canonical bytes
  • update converter docs to match the preserved-channel behavior

This keeps camera handling consistent with the existing rule that a declared channel is present even when it is empty, and prevents distinct source declarations from collapsing to the same canonical representation.

Fixes #528.

Testing

Focused regression coverage is included in tests/test_transform.py. Tests were not executed through this GitHub connector session.

@github-actions

github-actions Bot commented Oct 1, 2026

Copy link
Copy Markdown

👋 Hi @harshitethic — thanks for the contribution! To keep starter issues available
for other contributors and give every pull request a real review, we accept
1 open pull request per contributor at a time.

You already have #651 open, so this one is being closed automatically.
Once your open pull request is merged or closed, feel free to reopen this one —
no work is lost.

@github-actions github-actions Bot closed this Oct 1, 2026
@greptile-apps

greptile-apps Bot commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 4/5

[Medium risk] Changes how empty camera channels are handled in data transformation.

The PR is not safe to merge until the media step skips empty cameras.

Findings

  1. P1 Empty camera breaks media step ▶
Summary

The transform now keeps declared compressed-image camera channels even when they have no messages, so their presence affects canonical output identity.

  • Empty cameras are registered with the canonical protobuf video schema without image decoding or transcoding.
  • The regression test checks that an empty camera survives and changes the output bytes; the converter guide now describes this behavior.
Diagram
%%{init: {'theme': 'neutral'}}%%
flowchart LR
  A[Empty camera declaration] --> B[Canonical video channel]
  B --> C[Episode.cameras]
  C --> D[Contact-sheet step]
  D --> E[Frame-rate error]
Loading

Reviews (1) · Last reviewed commit: "docs(transform): document empty camera p..."

Comment thread src/hflow/transform.py
Comment on lines 897 to 899
for source_channel_id in sorted(infos, key=lambda cid: (infos[cid].topic, cid)):
info = infos[source_channel_id]
if source_channel_id in camera_payloads:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Empty camera breaks media step

When a source declares an empty camera, this change adds it to Episode.cameras. The built-in contact-sheet step then calls Episode.frames() for it. Episode.video() raises because there are no timestamps to estimate a frame rate, so the media step fails. Keep the declaration, but skip cameras with no frames when making contact sheets.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: transform: declared-but-empty camera channel is dropped, merging distinct sources and erasing provenance

1 participant