From 48772eb41f064c39ff4e37ee498f0afa04ecd350 Mon Sep 17 00:00:00 2001 From: Mike Langmayr <1809691+mikelangmayr@users.noreply.github.com> Date: Tue, 29 Sep 2026 17:14:46 -0700 Subject: [PATCH 1/2] Fetch the image and its RAW region under one buffer lock --- camerad/archon_controller.cpp | 182 ++++++++++++++++++++++++++++++---- camerad/archon_controller.h | 3 + camerad/archon_interface.cpp | 6 +- docs/commands/controller.md | 8 ++ 4 files changed, 178 insertions(+), 21 deletions(-) diff --git a/camerad/archon_controller.cpp b/camerad/archon_controller.cpp index 27e836f..3da04ab 100644 --- a/camerad/archon_controller.cpp +++ b/camerad/archon_controller.cpp @@ -2138,10 +2138,52 @@ namespace Camera { long ArchonController::read_frame(frametype_t type, char* &imagebufferptr) { - const std::string function("Camera::ArchonController::read_frame"); + if (this->lock_newest_buffer() != NO_ERROR) return ERROR; + + long error = this->fetch_region(type, imagebufferptr); + + if (error == NO_ERROR) error = this->unlock_buffer(); + + return error; + } + /***** Camera::ArchonController::read_frame *********************************/ + + + /***** Camera::ArchonController::lock_newest_buffer *************************/ + /** + * @brief lock the buffer holding the newest frame, for reading + */ + long ArchonController::lock_newest_buffer() { + const std::string function("Camera::ArchonController::lock_newest_buffer"); + char message[256]; + + const int bufready = this->frameinfo.index.load() + 1; + + if (bufready < 1 || bufready > this->activebufs) { + SNPRINTF(message, "invalid Archon buffer %d requested. Expected {1:%d}", bufready, this->activebufs); + logwrite(function, std::string(message)); + return ERROR; + } + + if (this->lock_buffer(bufready) == ERROR) { + logwrite(function, "ERROR locking frame buffer"); + return ERROR; + } + return NO_ERROR; + } + /***** Camera::ArchonController::lock_newest_buffer *************************/ + + + /***** Camera::ArchonController::fetch_region *******************************/ + /** + * @brief fetch one region of the locked buffer into caller memory + * @details Assumes the buffer is already locked, so several regions of the + * same frame can be fetched under one lock. + */ + long ArchonController::fetch_region(frametype_t type, char* &imagebufferptr) { + const std::string function("Camera::ArchonController::fetch_region"); char message[256]; int retval; - int bufready; char check[5], header[5]; int bytesread, totalbytesread, toread; uint64_t bufaddr; @@ -2154,20 +2196,6 @@ namespace Camera { this->frametype = type; - // Archon buffer number of the last frame read into memory - // - bufready = index + 1; - - if (bufready < 1 || bufready > this->activebufs) { - SNPRINTF(message, "invalid Archon buffer %d requested. Expected {1:%d}", bufready, this->activebufs); - logwrite(function, std::string(message)); - return ERROR; - } - - // Lock the frame buffer before reading it - // - if ( this->lock_buffer(bufready) == ERROR) { logwrite(function, "ERROR locking frame buffer"); return ERROR; } - // Send the FETCH command to read the memory buffer from the Archon backplane. // Archon replies with one binary response per requested block. Each response // has a message header. @@ -2308,12 +2336,9 @@ namespace Camera { this->print_frame_status(); } - // Unlock the frame buffer - // - if (error == NO_ERROR) error = this->unlock_buffer(); - return error; } + /***** Camera::ArchonController::fetch_region *******************************/ /***** Camera::ArchonController::is_raw_config_key *************************/ @@ -2557,6 +2582,123 @@ namespace Camera { /***** Camera::ArchonController::read_raw *******************************/ + /***** Camera::ArchonController::read_image_and_raw *************************/ + /** + * @brief fetch the image and the RAW region of one buffer, under one lock + * @details Fetching them separately unlocks in between, which lets a new + * frame land in the buffer, so the two halves need not describe + * the same exposure. Holding one lock across both guarantees a + * matched pair. Dispatches the image first, then the RAW capture. + * @param[out] retstring a summary of both payloads + */ + long ArchonController::read_image_and_raw(std::string &retstring) { + const std::string function("Camera::ArchonController::read_image_and_raw"); + + if (this->rawinfo.enable == 0) { + logwrite(function, "ERROR RAW capture is disabled"); + retstring = "RAW capture is disabled; set it with \"raw set RAWENABLE 1\""; + return ERROR; + } + + if (this->get_frame_status() != NO_ERROR) { + logwrite(function, "ERROR getting frame status"); + retstring = "frame status query failed"; + return ERROR; + } + + const int num_detect = this->modemap[this->selectedmode].geometry.num_detect; + if (num_detect != 1) { + logwrite(function, "ERROR pair fetch supports one detector, not "+std::to_string(num_detect)); + retstring = "pair fetch supports a single detector"; + return ERROR; + } + + const auto index = this->frameinfo.index.load(); + const uint32_t width = static_cast(this->frameinfo.bufwidth[index]); + const uint32_t height = static_cast(this->frameinfo.bufheight[index]); + const uint32_t bytes_per_pixel = (this->frameinfo.bufsample[index] == 1) ? 4 : 2; + if (width == 0 || height == 0) { + logwrite(function, "ERROR buffer reports no image; has a frame completed?"); + retstring = "buffer holds no image"; + return ERROR; + } + + const raw_geometry_t geom = this->raw_geometry(); + if (geom.samples == 0 || geom.lines == 0) { + logwrite(function, "ERROR RAW geometry is empty; check RAW config"); + retstring = "invalid RAW geometry"; + return ERROR; + } + if (geom.from_config) { + logwrite(function, "WARNING controller reports no raw data in this buffer; " + "using configured geometry"); + } + + const size_t raw_fetch_bytes = + static_cast(geom.blocks_per_line) * geom.lines * BLOCK_LEN; + std::shared_ptr raw_buffer(new char[raw_fetch_bytes]); + + if (this->lock_newest_buffer() != NO_ERROR) { + retstring = "could not lock the frame buffer"; + return ERROR; + } + + // Both fetches advance their pointer, so keep the originals for dispatch + char* image_start = this->framebuf; + char* image_cursor = image_start; + long error = this->fetch_region(FRAME_IMAGE, image_cursor); + image_start = this->framebuf; // a grow inside fetch_region moves it + + char* raw_cursor = raw_buffer.get(); + if (error == NO_ERROR) error = this->fetch_region(FRAME_RAW, raw_cursor); + + const long unlock_error = this->unlock_buffer(); + if (error != NO_ERROR || unlock_error != NO_ERROR) { + logwrite(function, "ERROR fetching the image and RAW pair"); + retstring = "pair fetch failed"; + return ERROR; + } + + // The Archon pads each raw line out to whole blocks, so copy only the valid + // samples into a contiguous lines x samples array + const size_t line_stride = static_cast(geom.blocks_per_line) * BLOCK_LEN; + const size_t payload_samples = static_cast(geom.lines) * geom.samples; + std::vector samples(payload_samples); + for (uint32_t line = 0; line < geom.lines; ++line) { + std::memcpy(samples.data() + static_cast(line) * geom.samples, + raw_buffer.get() + line * line_stride, + static_cast(geom.samples) * sizeof(uint16_t)); + } + + Camera::FrameMetadata meta; + meta.frame_number = this->frameinfo.bufframen[index]; + meta.timestamp = this->frameinfo.buftimestamp[index]; + meta.width = width; + meta.height = height; + meta.bytes_per_pixel = bytes_per_pixel; + this->interface->dispatch_frame(image_start, + static_cast(width) * height * bytes_per_pixel, + meta); + + meta.width = geom.samples; + meta.height = geom.lines; + meta.bytes_per_pixel = sizeof(uint16_t); + meta.stream = RAW_STREAM; + meta.frame_keys = this->raw_frame_keys(); + this->interface->dispatch_frame(reinterpret_cast(samples.data()), + payload_samples * sizeof(uint16_t), meta); + + std::ostringstream oss; + oss << "frame=" << meta.frame_number + << " image=" << width << "x" << height + << " raw=" << geom.samples << "x" << geom.lines; + retstring = oss.str(); + logwrite(function, retstring); + return NO_ERROR; + } + /***** Camera::ArchonController::read_image_and_raw *************************/ + + /***** Camera::ArchonController::wait_for_readout ***************************/ /** * @brief creates a wait until the next completed frame buffer is ready diff --git a/camerad/archon_controller.h b/camerad/archon_controller.h index b777ef7..550385d 100644 --- a/camerad/archon_controller.h +++ b/camerad/archon_controller.h @@ -376,6 +376,8 @@ namespace Camera { long allocate_framebuf(uint32_t reqsz); long read_frame(frametype_t type, char* &imagebufferptr); + long lock_newest_buffer(); + long fetch_region(frametype_t type, char* &imagebufferptr); long write_config_key(const char* key, const char* newvalue, bool &changed); long write_config_key(const char* key, int newvalue, bool &changed); @@ -395,6 +397,7 @@ namespace Camera { long get_raw_config(std::string &retstring); std::shared_ptr raw_frame_keys() const; long read_raw(std::string &retstring); + long read_image_and_raw(std::string &retstring); std::map modemap; diff --git a/camerad/archon_interface.cpp b/camerad/archon_interface.cpp index e08fa80..62f70b5 100644 --- a/camerad/archon_interface.cpp +++ b/camerad/archon_interface.cpp @@ -997,10 +997,11 @@ namespace Camera { if (args=="?" || args=="help") { retstring = CAMERAD_RAW; - retstring.append( " [ config | set [...] | read ]\n" ); + retstring.append( " [ config | set [...] | read | pair ]\n" ); retstring.append( " config report the RAW config keywords\n" ); retstring.append( " set .. set RAW keyword(s) then apply\n" ); retstring.append( " read retrieve RAW data in-band as 16-bit samples\n" ); + retstring.append( " pair retrieve the image and its RAW data under one lock\n" ); retstring.append( " Keys: RAWENABLE RAWSEL RAWSTARTLINE RAWENDLINE RAWSTARTPIXEL RAWSAMPLES\n" ); retstring.append( " RAWENABLE must be set before the exposure the raw data comes from\n" ); return HELP; @@ -1019,6 +1020,9 @@ namespace Camera { if (subcmd=="read") { return this->controller->read_raw(retstring); } + if (subcmd=="pair") { + return this->controller->read_image_and_raw(retstring); + } logwrite(function, "ERROR unrecognized subcommand: "+subcmd); retstring = "unrecognized subcommand: "+subcmd; diff --git a/docs/commands/controller.md b/docs/commands/controller.md index 6425d9a..dd4910f 100644 --- a/docs/commands/controller.md +++ b/docs/commands/controller.md @@ -60,6 +60,7 @@ raw [ config | set [...] | read ] | `raw config` | Report the six RAW keywords | | `raw set ...` | Write the keyword(s) to configuration memory, then apply | | `raw read` | Fetch the raw region of the newest buffer and dispatch it as a frame | +| `raw pair` | Fetch the image and the raw region of the newest buffer under one lock | The keywords are `RAWENABLE`, `RAWSEL`, `RAWSTARTLINE`, `RAWENDLINE`, `RAWSTARTPIXEL` and `RAWSAMPLES`. `RAWSAMPLES` is rounded up to a whole 1024-byte block per line. @@ -81,6 +82,13 @@ The result is dispatched on its own stream, named `raw`, so it never collides wi FITS writer gives it a separate file and the shared-memory writer a separate segment. See [frame output keys](../configuration/frame-outputs.md). +`raw pair` fetches the image and the raw region together. Fetching them with separate `read` +commands unlocks the buffer in between, which lets a new frame land there, so the two halves need +not describe the same exposure. `pair` holds one lock across both and dispatches them with the same +frame number, which is what makes them comparable. It reads whatever buffer is newest rather than +triggering an exposure, so it also works on a frame the controller already holds. It supports a +single detector and refuses otherwise, rather than dispatching part of a frame. + ### Interpreting the samples An AD channel and an ADM channel both arrive as an identical block of `uint16`, but they are not From 5b427a663ede04418236fefdac85f129c424ef35 Mon Sep 17 00:00:00 2001 From: Mike Langmayr <1809691+mikelangmayr@users.noreply.github.com> Date: Wed, 30 Sep 2026 15:52:21 -0700 Subject: [PATCH 2/2] Unlock only after a successful fetch, matching read_frame --- camerad/archon_controller.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/camerad/archon_controller.cpp b/camerad/archon_controller.cpp index 3da04ab..98adb23 100644 --- a/camerad/archon_controller.cpp +++ b/camerad/archon_controller.cpp @@ -2652,8 +2652,11 @@ namespace Camera { char* raw_cursor = raw_buffer.get(); if (error == NO_ERROR) error = this->fetch_region(FRAME_RAW, raw_cursor); - const long unlock_error = this->unlock_buffer(); - if (error != NO_ERROR || unlock_error != NO_ERROR) { + // A failed fetch can leave block data unread, which UNLOCK would consume as + // its own reply, and the next LOCKn replaces this lock anyway + if (error == NO_ERROR) error = this->unlock_buffer(); + + if (error != NO_ERROR) { logwrite(function, "ERROR fetching the image and RAW pair"); retstring = "pair fetch failed"; return ERROR;