Name the four cameras of the Trossen station - #692
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 34cdd25d3e
ℹ️ 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".
34cdd25 to
00680b5
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 00680b5b72
ℹ️ 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".
4ab923f to
5717a45
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5717a45b96
ℹ️ 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".
64b5ac9 to
b9d6e07
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b9d6e07532
ℹ️ 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".
`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.
b9d6e07 to
6d637ca
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6d637cadff
ℹ️ 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".
`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.
6d637ca to
c1e6c8a
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c1e6c8a424
ℹ️ 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".
`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.
A D405 enumerates as a plain UVC device, so the driver every other camera on a Linux host uses reads it too. Measured on the station: each camera alone holds 30.1 Hz at 640x480, and all four together hold 30.0 Hz over three minutes on one USB 3 hub, which is 590 Mbit/s of the 5 Gbit/s the hub carries. The serial in each link is the USB one. It is not the serial the RealSense SDK reports for the same camera, and the colour stream is the `-video-index4` node of the six a D405 exposes.
`_D405` reads as a camera or a configuration; what it holds is the head of the four device paths.
`linuxpy.Device` takes the path as a `str` and `LinuxVideo` hands it over untouched, so a `Path` here would be converted back at the only boundary that reads it; the arducam configurations above spell theirs the same way.
The waiver also named how the arducam configurations above are spelled, which can change without changing what `linuxpy.Device` takes.
c1e6c8a to
1eed372
Compare
1 participant
The four RealSense D405 of the Trossen station, named for the driver every other camera on a Linux host
uses. A D405 enumerates as a plain UVC device, so
LinuxVideoreads it: no vendor SDK, no extra.Two things about the links that are easy to get wrong, and are written beside them:
by-idlink is the USB serial. It is not the serial the RealSense SDK reports forthe same camera.
-video-index4.Measured on the station
Each camera alone holds 30.1 Hz at 640x480, with 35 ms between frames at the worst. All four together
hold 30.0 Hz over three minutes — 590 Mbit/s of the 5 Gbit/s their USB 3 hub carries — and recording all
four at once writes 29.9 Hz each, which
load_datasetreads back.This is worth stating because the opposite was expected: four D405 streaming colour and depth through
the RealSense SDK do not fit. Colour over UVC does.
Recording a camera on this driver needs #689, which gives the driver's frame the shape the recorder reads.