Skip to content

Give a Linux video frame the shape the other cameras give - #689

Open
DarksaCY wants to merge 12 commits into
Positronic-Robotics:mainfrom
DarksaCY:linux-video-adapter
Open

Give a Linux video frame the shape the other cameras give#689
DarksaCY wants to merge 12 commits into
Positronic-Robotics:mainfrom
DarksaCY:linux-video-adapter

Conversation

@DarksaCY

@DarksaCY DarksaCY commented Aug 28, 2026

Copy link
Copy Markdown

LinuxVideo emits a dict of arrays, {'image': ndarray}. Every other camera driver emits a
NumpySMAdapter, and that is what the recorder reads: Serializers.camera_images returns data.array.
So a station that records a camera on this driver raises AttributeError and writes no episode. The frame
the headset shows takes the same field.

This holds on main for the configurations that use the driver today — main_cfg names arducam_left and
arducam_right.

The branch that decodes several frames out of one packet wrote them into a dict that was still None, so
it raised TypeError. Frames now leave the driver one at a time, each on the port, which is what a
consumer of a single image expects.

Testing

positronic/drivers/camera/tests/test_linux_video.py drives the loop over a device of its own: a YUYV
buffer arrives as an RGB image on the port, three buffers are three frames, the device is set to the
format and rate the driver was asked for, and it is closed when the frames run out. A conftest stands in
for linuxpy, which ships only in the hardware extra, so the tests need neither that extra nor a camera.

On hardware, against a RealSense D405 over UVC (the camera enumerates as a plain UVC device, and its colour
stream is the -video-index4 node): 640x480 at 27.8 Hz, and a recorded episode of 91 frames that
load_dataset opens and decodes.

Also here

linuxpy logs every ioctl, which is several lines per frame per camera: two cameras at 30 Hz bury
everything else a debug run has to say. It joins the libraries whose level pimm.logging pins.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a0b8f8ae06

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread positronic/drivers/camera/linux_video.py Outdated
Comment thread positronic/drivers/camera/tests/test_linux_video.py Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d4f88db5a9

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread positronic/drivers/camera/linux_video.py Outdated
Comment thread positronic/drivers/camera/linux_video.py Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a43220d321

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread positronic/drivers/camera/linux_video.py Outdated
Comment thread positronic/drivers/camera/linux_video.py
Comment thread positronic/drivers/camera/tests/conftest.py Outdated
Comment thread pimm/logging.py

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ccef848d36

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread positronic/drivers/camera/tests/conftest.py Outdated
Comment thread positronic/drivers/camera/linux_video.py Outdated
DarksaCY added a commit to DarksaCY/positronic that referenced this pull request Sep 7, 2026
`_abandon`, `_recorded`, `_readied`, `_parked` and `_drove` all acted and
answered with a bare `bool`, so what each verdict meant was invisible at the
call site. They are commands now, and `run` sets `recording` and `in_error`
where a reader can see it.

`_readied` and `_parked` did nothing but give up the recording, stop the
tracking and call `_ready` or `_park`; the pairs are one method each, and the
recording is given up at the caller. `_drove` split into the state the arm
reads, `_in_error`, and the emission the caller makes with it. `_recorded` is
`_record`, an instruction like its neighbours.

The Trossen driver takes what Positronic-Robotics#683 settled: a move the link drops under is
failed by the new session instead of running out its 15 s deadline. The Linux
video driver takes what Positronic-Robotics#689 settled: a buffer that decodes to several images
hands over the newest, because the frame port holds one. The D405 paths carry
the waiver that says why they are strings.
`LinuxVideo` emitted a dict of arrays. Every other camera driver emits a
`NumpySMAdapter`, and that is what `Serializers.camera_images` reads, so a
station recording a camera on this driver raised `AttributeError` and wrote no
episode. The frame the headset shows reads the same field.

The branch that decodes several frames out of one packet wrote them into a
dict that was still `None`, and raised `TypeError`. Frames now leave one at a
time, each on the port, which is what a consumer of a single image expects.

Verified against a RealSense D405 over UVC: 640x480 at 27.8 Hz, and an episode
of 91 frames that `load_dataset` opens.
A camera hands over a buffer with its tail missing when the bus is busy, and
the driver reshaped it and raised `ValueError`, which ends the run. Four D405
cameras on one USB 3 hub are enough to see it: the first minute of the first
run raised on a buffer of 312072 bytes where a 640x480 frame is 614400.

A short buffer now goes, and the driver says so once and counts the rest. Over
three minutes of four cameras at 30 Hz that is one buffer per camera; while
the same four are recorded, and the machine encodes their video, it is five to
eight per camera per minute.

Reading a buffer moves out of `run` into `_images`, which is what the whole
`match` was, and what put `run` over the complexity the linter allows.
`linuxpy` logs every ioctl, which is a handful of lines per frame per camera:
two cameras at 30 Hz bury everything else a debug run has to say. It joins the
libraries whose level is pinned.
Four things the review of this branch found.

A buffer that decodes to several images emitted them all through one adapter,
overwriting it between emissions and with nothing read in between, so every
message of that batch carried the last image. Measured on a four-frame H.264
stream: three emissions, all of them the third image. Such a buffer now gives
each image an adapter of its own; the one-image buffer every uncompressed
camera hands over still reuses the driver's.

`short` counted every buffer that yielded no image, but a parser and a decoder
both hold whole data back until they have a frame to give: the first 64 bytes
of an H.264 stream are healthy and decode to nothing. `_images` now returns
`None` for a buffer short of a frame and an empty list for a wait, and only
the former is counted or logged.

`_framed` said in its docstring which rig the short buffers were seen on and
assumed 30 fps; it now states the contract and stops there.

`_driven` in the tests worked only after the `device` fixture had patched the
driver, and said so nowhere -- a test that called it without the otherwise
unused fixture would have opened the real device. It takes the device now.
Three things the second review round of this branch found.

`short` claimed truncation, but `_framed` refuses every buffer whose size is
not one frame's -- an oversized or padded one as much as a truncated one. The
counter, both warnings and the two tests that read them now say the size is
wrong and stop there.

`_CODECS` sat at module scope with `LinuxVideo._images` its only reader; it
is a class attribute beside that method now.

`test_every_buffer_is_one_frame` claimed of every buffer what the compressed
tests in the same file disprove: it is about a whole YUYV buffer, and says so.

The fake `linuxpy` enum was bound as `pixel_format` and installed as
`PixelFormat`; it carries the one name now.
A V4L2 buffer short of a frame is what a busy bus hands over: measured on the
station's four D405, one per camera over three minutes of capture and five to
eight per camera per minute while all four are also being encoded. The waiver
records that, so the count is read as traffic rather than as a swallowed fault.
The port is a latest-value slot -- shared memory across a process boundary,
one message in place within one -- and nothing runs between two emissions of
the same tick. So every image but the last of a buffer that decodes to several
was written over before any reader could see it, and `fps_counter` counted
sends nobody received.

Such a buffer now hands over its newest image and counts the rest, logged once
at the end of the run beside the buffers of the wrong size. Only H.264 and the
other compressed formats can produce one; a YUYV or UYVY buffer is a single
image by construction.
…straint

`f'{VENDOR}.video'` is the import name the module object and `sys.modules` must
spell alike, so it joins `DEVICE_MODULE` as a constant.

The waiver on a buffer of the wrong size carried the rate one station measures
and a frame period this driver takes as a parameter. It states what holds for
every device instead: the stream goes on, the buffers after it read whole, and
the count says how many went.
The driver's own frame type ends three errors the baseline grandfathered.
@DarksaCY
DarksaCY force-pushed the linux-video-adapter branch from 5ac2de0 to eb9b422 Compare September 7, 2026 17:22

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: eb9b422f6c

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread positronic/drivers/camera/linux_video.py Outdated
`LinuxVideo.run` yields `pimm.Yield()` and declared `Iterator[pimm.Sleep]`; the
error the baseline held for that goes with the annotation.

`_driven` in the tests took `**kwargs` it could not pass: every constructor
argument is already named, so any override raised a duplicate keyword.
`LinuxVideo.run` says what it yields, so the entry that grandfathered the wrong
annotation has nothing left to hold.
`FakeDevice.opened` was set in the constructor, so it held the last device
built and the assertion that the driver opened one could not fail.
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.

1 participant