diff --git a/DeviceAdapters/Aravis/AravisCamera.cpp b/DeviceAdapters/Aravis/AravisCamera.cpp index 7aad61a2b..2a01dca5a 100644 --- a/DeviceAdapters/Aravis/AravisCamera.cpp +++ b/DeviceAdapters/Aravis/AravisCamera.cpp @@ -117,6 +117,7 @@ AravisCamera::AravisCamera(const char *name) : capturing(false), counter(0), exposure_time(0.0), + has_binning(false), img_buffer_bit_depth(0), img_buffer_bytes_per_pixel(0), img_buffer_height(0), @@ -124,21 +125,26 @@ AravisCamera::AravisCamera(const char *name) : img_buffer_number_pixels(0), img_buffer_size(0), img_buffer_width(0), - initialized(false), + initialized(false), arv_buffer(nullptr), arv_cam(nullptr), - arv_cam_name(nullptr), + arv_cam_name(name ? name : ""), + arv_device(nullptr), arv_stream(nullptr), img_buffer(nullptr), pixel_type(nullptr) { - arv_cam_name = (char *)malloc(sizeof(char) * strlen(name)); - CDeviceUtils::CopyLimitedString(arv_cam_name, name); + // The name was previously copied into malloc(strlen(name)) with + // CDeviceUtils::CopyLimitedString(), which writes strlen(name) + 1 bytes. + // That put the terminating NUL one byte past the end of the allocation on + // every camera. A std::string removes the arithmetic entirely. } AravisCamera::~AravisCamera() { + // Shutdown() is idempotent, and Micro-Manager does not guarantee it ran. + Shutdown(); g_clear_object(&arv_cam); } @@ -162,32 +168,53 @@ void AravisCamera::AcquisitionCallback(ArvStreamCallbackType type, ArvBuffer *cb arv_make_thread_high_priority(-10); break; case ARV_STREAM_CALLBACK_TYPE_BUFFER_DONE: + { + // Pop the completed buffer. This used to sit inside g_assert(), which + // makes the stream depend on an assertion: built with G_DISABLE_ASSERT the + // pop would vanish and the stream would starve once its buffers ran out, + // and a genuine mismatch would abort Micro-Manager rather than be handled. + ArvBuffer *popped_arv_buffer = arv_stream_pop_buffer(arv_stream); + + if (popped_arv_buffer == NULL){ + LogMessage("Aravis Error, stream returned a NULL buffer", false); + break; + } + if (popped_arv_buffer != cb_arv_buffer){ + // Not expected: the callback reports the buffer the stream just + // completed, which is the one at the head of the output queue. Trust the + // popped buffer, since that is the one we now own and must push back. + LogMessage("Aravis Error, popped buffer is not the completed buffer", false); + } + + { + // ArvBufferUpdate() may reallocate img_buffer, and InsertImage() reads + // it, so both are held under the lock that GetImageBuffer() also takes. + std::lock_guard lock(img_buffer_mutex); + + ArvBufferUpdate(popped_arv_buffer); + + // Image metadata. + md.AddTag(MM::g_Keyword_Metadata_CameraLabel, ""); + md.AddTag(MM::g_Keyword_Metadata_ROI_X, CDeviceUtils::ConvertToString((long)img_buffer_width)); + md.AddTag(MM::g_Keyword_Metadata_ROI_Y, CDeviceUtils::ConvertToString((long)img_buffer_height)); + md.AddTag(MM::g_Keyword_Metadata_ImageNumber, CDeviceUtils::ConvertToString(counter)); + md.AddTag(MM::g_Keyword_Metadata_Exposure, exposure_time); + md.AddTag(MM::g_Keyword_PixelType, pixel_type); + + // Pass data to MM. + GetCoreCallback()->InsertImage(this, + img_buffer, + img_buffer_width, + img_buffer_height, + img_buffer_bytes_per_pixel, + 1, + md.Serialize()); + } - // Copy buffer data. - g_assert(cb_arv_buffer == arv_stream_pop_buffer(arv_stream)); - g_assert(cb_arv_buffer != NULL); - ArvBufferUpdate(cb_arv_buffer); - - // Image metadata. - md.AddTag(MM::g_Keyword_Metadata_CameraLabel, ""); - md.AddTag(MM::g_Keyword_Metadata_ROI_X, CDeviceUtils::ConvertToString((long)img_buffer_width)); - md.AddTag(MM::g_Keyword_Metadata_ROI_Y, CDeviceUtils::ConvertToString((long)img_buffer_height)); - md.AddTag(MM::g_Keyword_Metadata_ImageNumber, CDeviceUtils::ConvertToString(counter)); - md.AddTag(MM::g_Keyword_Metadata_Exposure, exposure_time); - md.AddTag(MM::g_Keyword_PixelType, pixel_type); - - // Pass data to MM. - int ret = GetCoreCallback()->InsertImage(this, - img_buffer, - img_buffer_width, - img_buffer_height, - img_buffer_bytes_per_pixel, - 1, - md.Serialize()); - - arv_stream_push_buffer(arv_stream, cb_arv_buffer); + arv_stream_push_buffer(arv_stream, popped_arv_buffer); counter += 1; break; + } } } @@ -249,6 +276,17 @@ void AravisCamera::ArvBufferUpdate(ArvBuffer *aBuffer) arvPixelFormat = arv_buffer_get_image_pixel_format(aBuffer); ArvPixelFormatUpdate(arvPixelFormat); + // A format with no case in ArvPixelFormatUpdate() leaves these at zero. + // Zero components is not one, so the copy below would take the RGB path and + // write four bytes per pixel into a buffer sized for zero. Refuse instead. + if ((img_buffer_bytes_per_pixel < 1) || (img_buffer_number_components < 1)){ + std::stringstream msg; + msg << "Aravis Error, cannot copy an image in unsupported pixel format " + << arvPixelFormat; + LogMessage(msg.str(), false); + return; + } + // Image size updates. img_buffer_width = (int)arv_buffer_get_image_width(aBuffer); img_buffer_height = (int)arv_buffer_get_image_height(aBuffer); @@ -258,6 +296,21 @@ void AravisCamera::ArvBufferUpdate(ArvBuffer *aBuffer) arvBufferData = (unsigned char *)arv_buffer_get_data(aBuffer, &arvSize); size = img_buffer_width * img_buffer_height * img_buffer_bytes_per_pixel; + // The source must hold everything the destination is about to be filled + // with. For the packed RGB formats the camera sends three bytes per pixel + // and we expand to four, so compare against what will actually be read. + size_t arvNeeded = (img_buffer_number_components == 1) + ? size + : img_buffer_number_pixels * 3; + if (arvSize < arvNeeded){ + std::stringstream msg; + msg << "Aravis Error, buffer holds " << arvSize << " bytes but " + << arvNeeded << " are needed for a " << img_buffer_width << "x" + << img_buffer_height << " image"; + LogMessage(msg.str(), false); + return; + } + if (img_buffer_size != size){ if (img_buffer != nullptr){ free(img_buffer); @@ -270,17 +323,25 @@ void AravisCamera::ArvBufferUpdate(ArvBuffer *aBuffer) } else{ rgb_to_rgba(img_buffer, arvBufferData, img_buffer_number_pixels); - } + } } -int AravisCamera::ArvCheckError(GError *gerror) const +// Log and clear an Aravis error, if there is one. +// +// The GError is taken by address, not by value. Taken by value, g_clear_error() +// freed the error but cleared only this function's own copy of the pointer, +// leaving the caller holding a dangling non-NULL pointer. The caller would then +// pass that pointer to its next Aravis call and check it again, reading freed +// memory. Any camera that refused arv_camera_set_region() crashed Micro-Manager +// during Initialize() this way. +int AravisCamera::ArvCheckError(GError **gerror) const { - if (gerror != NULL) { + if ((gerror != NULL) && (*gerror != NULL)) { std::stringstream msg; - msg << "Aravis Error: " << gerror->message; + msg << "Aravis Error: " << (*gerror)->message; LogMessage(msg.str(), false); - g_clear_error(&gerror); + g_clear_error(gerror); return 1; } return 0; @@ -294,7 +355,7 @@ void AravisCamera::ArvGetExposure() GError *gerror = nullptr; expTimeUs = arv_camera_get_exposure_time(arv_cam, &gerror); - if(!ArvCheckError(gerror)){ + if(!ArvCheckError(&gerror)){ exposure_time = expTimeUs * 1.0e-3; } } @@ -374,7 +435,20 @@ void AravisCamera::ArvPixelFormatUpdate(guint32 arvPixelFormat) break; default: - printf ("Aravis Error: Pixel Format %d is not implemented\n", (int)arvPixelFormat); + // Leave the image description in a state callers can recognise as + // unusable, rather than keeping the previous format's values and + // describing the new data with them. printf() went to a console that + // Micro-Manager users do not have; this belongs in the log. + { + std::stringstream msg; + msg << "Aravis Error, pixel format " << (int)arvPixelFormat + << " is not implemented"; + LogMessage(msg.str(), false); + } + img_buffer_bit_depth = 0; + img_buffer_bytes_per_pixel = 0; + img_buffer_number_components = 0; + pixel_type = "Unknown"; break; } } @@ -389,9 +463,9 @@ int AravisCamera::ArvStartSequenceAcquisition() counter = 0; arv_camera_set_acquisition_mode(arv_cam, ARV_ACQUISITION_MODE_CONTINUOUS, &gerror); - if (!ArvCheckError(gerror)){ + if (!ArvCheckError(&gerror)){ arv_stream = arv_camera_create_stream(arv_cam, stream_callback, this, &gerror); - if (ArvCheckError(gerror)){ + if (ArvCheckError(&gerror)){ return 1; } } @@ -401,12 +475,12 @@ int AravisCamera::ArvStartSequenceAcquisition() if (ARV_IS_STREAM(arv_stream)){ payload = arv_camera_get_payload(arv_cam, &gerror); - if (!ArvCheckError(gerror)){ + if (!ArvCheckError(&gerror)){ for (i = 0; i < 20; i++) arv_stream_push_buffer(arv_stream, arv_buffer_new(payload, NULL)); } arv_camera_start_acquisition(arv_cam, &gerror); - if (ArvCheckError(gerror)){ + if (ArvCheckError(&gerror)){ return 1; } } @@ -424,16 +498,16 @@ int AravisCamera::ClearROI() GError *gerror = nullptr; arv_camera_set_region(arv_cam, 0, 0, 64, 64, &gerror); - ArvCheckError(gerror); + ArvCheckError(&gerror); arv_camera_get_height_bounds(arv_cam, &tmp, &h, &gerror); - ArvCheckError(gerror); + ArvCheckError(&gerror); arv_camera_get_width_bounds(arv_cam, &tmp, &w, &gerror); - ArvCheckError(gerror); + ArvCheckError(&gerror); arv_camera_set_region(arv_cam, 0, 0, w, h, &gerror); - ArvCheckError(gerror); + ArvCheckError(&gerror); return DEVICE_OK; } @@ -445,10 +519,19 @@ int AravisCamera::GetBinning() const gint dy; GError *gerror = nullptr; + // Micro-Manager calls this whenever it needs the binning factor, which + // includes once per image while tagging metadata. On a camera without + // binning the Aravis call fails every time, so a camera that simply does not + // bin filled the log with "[BinningHorizontal] Not found" during live + // acquisition. It bins by one, and that needs no camera to answer. + if (!has_binning){ + return 1; + } + arv_camera_get_binning(arv_cam, &dx, &dy, &gerror); - ArvCheckError(gerror); - - // dx is always dy for MM? Add check? + ArvCheckError(&gerror); + + // dx is always dy for MM? Add check? return (int)dx; } @@ -467,13 +550,11 @@ double AravisCamera::GetExposure() const const unsigned char* AravisCamera::GetImageBuffer() { - int status; - size_t arv_size, size; - gboolean chunks; - unsigned char *arv_buffer_data; - if (ARV_IS_BUFFER (arv_buffer)) { - ArvBufferUpdate(arv_buffer); + { + std::lock_guard lock(img_buffer_mutex); + ArvBufferUpdate(arv_buffer); + } g_clear_object(&arv_buffer); SetProperty(MM::g_Keyword_PixelType, pixel_type); return img_buffer; @@ -508,7 +589,7 @@ unsigned AravisCamera::GetImageHeight() const void AravisCamera::GetName(char *name) const { - CDeviceUtils::CopyLimitedString(name, arv_cam_name); + CDeviceUtils::CopyLimitedString(name, arv_cam_name.c_str()); } @@ -524,7 +605,7 @@ int AravisCamera::GetROI(unsigned& x, unsigned& y, unsigned& xSize, unsigned& yS GError *gerror = nullptr; arv_camera_get_region(arv_cam, &gx, &gy, &gwidth, &gheight, &gerror); - ArvCheckError(gerror); + ArvCheckError(&gerror); x = (unsigned)gx; y = (unsigned)gx; @@ -545,8 +626,8 @@ int AravisCamera::Initialize() return DEVICE_OK; } - arv_cam = arv_camera_new(arv_cam_name, &gerror); - if (ArvCheckError(gerror)) return ARV_ERROR; + arv_cam = arv_camera_new(arv_cam_name.c_str(), &gerror); + if (ArvCheckError(&gerror)) return ARV_ERROR; arv_device = arv_camera_get_device(arv_cam); @@ -556,10 +637,10 @@ int AravisCamera::Initialize() // Get starting image size. gint h,w; arv_camera_get_height_bounds(arv_cam, &tmp, &h, &gerror); - ArvCheckError(gerror); + ArvCheckError(&gerror); arv_camera_get_width_bounds(arv_cam, &tmp, &w, &gerror); - ArvCheckError(gerror); + ArvCheckError(&gerror); img_buffer_height = (int)h; img_buffer_width = (int)w; @@ -567,14 +648,14 @@ int AravisCamera::Initialize() // Set image properties based on current pixel type. guint32 arvPixelFormat; arvPixelFormat = arv_camera_get_pixel_format(arv_cam, &gerror); - ArvCheckError(gerror); + ArvCheckError(&gerror); ArvPixelFormatUpdate(arvPixelFormat); // Turn off auto exposure (if available). if(arv_camera_is_exposure_auto_available(arv_cam, &gerror)){ - ArvCheckError(gerror); + ArvCheckError(&gerror); arv_camera_set_exposure_time_auto(arv_cam, ARV_AUTO_OFF, &gerror); - ArvCheckError(gerror); + ArvCheckError(&gerror); } // Get current exposure time. @@ -584,7 +665,7 @@ int AravisCamera::Initialize() // FIXME: Camera might start with a format that is not supported. const char *pixel_format; pixel_format = arv_camera_get_pixel_format_as_string (arv_cam, &gerror); - ArvCheckError(gerror); + ArvCheckError(&gerror); CPropertyAction* pAct = new CPropertyAction(this, &AravisCamera::OnPixelType); ret = CreateProperty(MM::g_Keyword_PixelType, pixel_format, MM::String, false, pAct); @@ -595,7 +676,7 @@ int AravisCamera::Initialize() const char **pixelFormats; pixelFormats = arv_camera_dup_available_pixel_formats_as_strings(arv_cam, &nPixelFormats, &gerror); - ArvCheckError(gerror); + ArvCheckError(&gerror); for(i=0;i 1){ CPropertyAction* pAct = new CPropertyAction(this, &AravisCamera::OnTriggerMode); @@ -744,7 +826,7 @@ int AravisCamera::Initialize() guint nTriggerSelectors = 0; const char **triggerSelectors; triggerSelectors = arv_camera_dup_available_triggers(arv_cam, &nTriggerSelectors, &gerror); - ArvCheckError(gerror); + ArvCheckError(&gerror); if (nTriggerSelectors > 1){ CPropertyAction* pAct = new CPropertyAction(this, &AravisCamera::OnTriggerSelector); @@ -763,7 +845,7 @@ int AravisCamera::Initialize() guint nTriggerSources = 0; const char **triggerSources; triggerSources = arv_camera_dup_available_trigger_sources(arv_cam, &nTriggerSources, &gerror); - ArvCheckError(gerror); + ArvCheckError(&gerror); if (nTriggerSources > 1){ CPropertyAction* pAct = new CPropertyAction(this, &AravisCamera::OnTriggerSource); @@ -820,13 +902,13 @@ int AravisCamera::OnAutoBlackLevel(MM::PropertyBase* pProp, MM::ActionType eAct) else{ printf("Unrecognized auto black level mode %s", autoBlackLevelMode.c_str()); } - ArvCheckError(gerror); + ArvCheckError(&gerror); } } else if (eAct == MM::BeforeGet) { int mode; mode = arv_camera_get_black_level_auto(arv_cam, &gerror); - ArvCheckError(gerror); + ArvCheckError(&gerror); if (mode == ARV_AUTO_OFF){ pProp->Set("AUTO_OFF"); @@ -864,13 +946,13 @@ int AravisCamera::OnAutoGain(MM::PropertyBase* pProp, MM::ActionType eAct) else{ printf("Unrecognized auto gain mode %s", autoGainMode.c_str()); } - ArvCheckError(gerror); + ArvCheckError(&gerror); } } else if (eAct == MM::BeforeGet) { int mode; mode = arv_camera_get_gain_auto(arv_cam, &gerror); - ArvCheckError(gerror); + ArvCheckError(&gerror); if (mode == ARV_AUTO_OFF){ pProp->Set("AUTO_OFF"); @@ -893,13 +975,24 @@ int AravisCamera::OnBinning(MM::PropertyBase* pProp, MM::ActionType eAct) std::string binning; GError *gerror = nullptr; + // Micro-Manager refreshes properties on a timer, so a camera without binning + // logged an Aravis failure here once per refresh, forever. The property is + // still offered, fixed at 1, because Micro-Manager expects cameras to have + // one; it just no longer costs a failed register read to report it. + if (!has_binning){ + if (eAct == MM::BeforeGet){ + pProp->Set(1L); + } + return DEVICE_OK; + } + if (eAct == MM::AfterSet){ if (!capturing){ pProp->Get(binning); bx = std::stoi(binning); arv_camera_set_binning(arv_cam, bx, bx, &gerror); - ArvCheckError(gerror); + ArvCheckError(&gerror); // This restores the image size when we decrease the binning. ClearROI(); @@ -907,7 +1000,7 @@ int AravisCamera::OnBinning(MM::PropertyBase* pProp, MM::ActionType eAct) } else if (eAct == MM::BeforeGet) { arv_camera_get_binning(arv_cam, &bx, &by, &gerror); - ArvCheckError(gerror); + ArvCheckError(&gerror); std::string bxs = std::to_string(bx); pProp->Set(bxs.c_str()); @@ -925,17 +1018,17 @@ int AravisCamera::OnBlackLevel(MM::PropertyBase* pProp, MM::ActionType eAct) if (eAct == MM::AfterSet){ int mode; mode = arv_camera_get_black_level_auto(arv_cam, &gerror); - ArvCheckError(gerror); + ArvCheckError(&gerror); if (mode == ARV_AUTO_OFF){ pProp->Get(blackLevel); arv_camera_set_black_level(arv_cam, blackLevel, &gerror); - ArvCheckError(gerror); + ArvCheckError(&gerror); } } else if (eAct == MM::BeforeGet){ blackLevel = arv_camera_get_black_level(arv_cam, &gerror); - ArvCheckError(gerror); + ArvCheckError(&gerror); pProp->Set(blackLevel); } @@ -951,17 +1044,17 @@ int AravisCamera::OnGain(MM::PropertyBase* pProp, MM::ActionType eAct) if (eAct == MM::AfterSet){ int mode; mode = arv_camera_get_gain_auto(arv_cam, &gerror); - ArvCheckError(gerror); + ArvCheckError(&gerror); if (mode == ARV_AUTO_OFF){ pProp->Get(gain); arv_camera_set_gain(arv_cam, gain, &gerror); - ArvCheckError(gerror); + ArvCheckError(&gerror); } } else if (eAct == MM::BeforeGet) { gain = arv_camera_get_gain(arv_cam, &gerror); - ArvCheckError(gerror); + ArvCheckError(&gerror); pProp->Set(gain); } @@ -977,11 +1070,11 @@ int AravisCamera::OnGamma(MM::PropertyBase* pProp, MM::ActionType eAct) if (eAct == MM::AfterSet){ pProp->Get(gamma); arv_device_set_float_feature_value(arv_device, "Gamma", gamma, &gerror); - ArvCheckError(gerror); + ArvCheckError(&gerror); } else if (eAct == MM::BeforeGet){ gamma = arv_device_get_float_feature_value(arv_device, "Gamma", &gerror); - ArvCheckError(gerror); + ArvCheckError(&gerror); pProp->Set(gamma); } return DEVICE_OK; @@ -998,11 +1091,11 @@ int AravisCamera::OnGammaEnable(MM::PropertyBase* pProp, MM::ActionType eAct) pProp->Get(gammaEnable); ge = std::stoi(gammaEnable); arv_device_set_boolean_feature_value(arv_device, "GammaEnable", ge, &gerror); - ArvCheckError(gerror); + ArvCheckError(&gerror); } else if (eAct == MM::BeforeGet){ ge = arv_device_get_boolean_feature_value(arv_device, "GammaEnable", &gerror); - ArvCheckError(gerror); + ArvCheckError(&gerror); gammaEnable = std::to_string(ge); pProp->Set(gammaEnable.c_str()); } @@ -1021,17 +1114,17 @@ int AravisCamera::OnPixelType(MM::PropertyBase* pProp, MM::ActionType eAct) pProp->Get(pixelType); arv_camera_set_pixel_format_from_string(arv_cam, pixelType.c_str(), &gerror); - ArvCheckError(gerror); + ArvCheckError(&gerror); arvPixelFormat = arv_camera_get_pixel_format(arv_cam, &gerror); - ArvCheckError(gerror); + ArvCheckError(&gerror); ArvPixelFormatUpdate(arvPixelFormat); } } else if (eAct == MM::BeforeGet) { const char *pixelFormat; pixelFormat = arv_camera_get_pixel_format_as_string(arv_cam, &gerror); - ArvCheckError(gerror); + ArvCheckError(&gerror); pProp->Set(pixelFormat); } @@ -1050,13 +1143,13 @@ int AravisCamera::OnTriggerMode(MM::PropertyBase* pProp, MM::ActionType eAct) pProp->Get(mode); arv_device_set_string_feature_value(arv_device, "TriggerMode", mode.c_str(), &gerror); - ArvCheckError(gerror); + ArvCheckError(&gerror); } } else if (eAct == MM::BeforeGet) { const char *mode; mode = arv_device_get_string_feature_value(arv_device, "TriggerMode", &gerror); - ArvCheckError(gerror); + ArvCheckError(&gerror); pProp->Set(mode); } @@ -1076,13 +1169,13 @@ int AravisCamera::OnTriggerSelector(MM::PropertyBase* pProp, MM::ActionType eAct arv_device_set_string_feature_value(arv_device, "TriggerSelector", trigger.c_str(), &gerror); //arv_camera_set_trigger(arv_cam, trigger.c_str(), &gerror); - ArvCheckError(gerror); + ArvCheckError(&gerror); } } else if (eAct == MM::BeforeGet) { const char *trigger; trigger = arv_device_get_string_feature_value(arv_device, "TriggerSelector", &gerror); - ArvCheckError(gerror); + ArvCheckError(&gerror); pProp->Set(trigger); } @@ -1101,13 +1194,13 @@ int AravisCamera::OnTriggerSource(MM::PropertyBase* pProp, MM::ActionType eAct) pProp->Get(triggerSource); arv_camera_set_trigger_source(arv_cam, triggerSource.c_str(), &gerror); - ArvCheckError(gerror); + ArvCheckError(&gerror); } } else if (eAct == MM::BeforeGet) { const char *triggerSource; triggerSource = arv_camera_get_trigger_source(arv_cam, &gerror); - ArvCheckError(gerror); + ArvCheckError(&gerror); pProp->Set(triggerSource); } @@ -1120,8 +1213,14 @@ int AravisCamera::SetBinning(int binSize) { GError *gerror = nullptr; + // The Binning property offers only "1" on a camera that cannot bin, so the + // only value that reaches here is the one it already has. + if (!has_binning){ + return (binSize == 1) ? DEVICE_OK : DEVICE_UNSUPPORTED_COMMAND; + } + arv_camera_set_binning(arv_cam, (gint)binSize, (gint)binSize, &gerror); - ArvCheckError(gerror); + ArvCheckError(&gerror); return DEVICE_OK; } @@ -1134,23 +1233,23 @@ void AravisCamera::SetExposure(double expMs) GError *gerror = nullptr; arv_camera_get_exposure_time_bounds(arv_cam, &min, &max, &gerror); - ArvCheckError(gerror); + ArvCheckError(&gerror); if (expUs < min){ expUs = min; } if (expUs > max){ expUs = max; } arv_camera_set_exposure_time(arv_cam, expUs, &gerror); - ArvCheckError(gerror); + ArvCheckError(&gerror); // This always returns the same bounds, independent of exposure time.. arv_camera_get_frame_rate_bounds(arv_cam, &min, &max, &gerror); - ArvCheckError(gerror); + ArvCheckError(&gerror); // arv_camera_set_frame_rate(arv_cam, max, &gerror); // This is supposed to disable the frame rate, which will then // presumably be set by the exposure time. arv_camera_set_frame_rate(arv_cam, -1.0, &gerror); - ArvCheckError(gerror); + ArvCheckError(&gerror); ArvGetExposure(); } @@ -1162,30 +1261,56 @@ int AravisCamera::SetROI(unsigned x, unsigned y, unsigned xSize, unsigned ySize) GError *gerror = nullptr; inc = arv_camera_get_x_offset_increment(arv_cam, &gerror); - ArvCheckError(gerror); + ArvCheckError(&gerror); ix = ((gint)x/inc)*inc; inc = arv_camera_get_y_offset_increment(arv_cam, &gerror); - ArvCheckError(gerror); + ArvCheckError(&gerror); iy = ((gint)y/inc)*inc; inc = arv_camera_get_width_increment(arv_cam, &gerror); - ArvCheckError(gerror); + ArvCheckError(&gerror); ixs = ((gint)xSize/inc)*inc; inc = arv_camera_get_height_increment(arv_cam, &gerror); - ArvCheckError(gerror); + ArvCheckError(&gerror); iys = ((gint)ySize/inc)*inc; arv_camera_set_region(arv_cam, ix, iy, ixs, iys, &gerror); - ArvCheckError(gerror); + ArvCheckError(&gerror); return DEVICE_OK; } +// Release everything acquired since Initialize(). Idempotent: Micro-Manager +// may call this more than once, and the destructor calls it again. +// +// This used to do nothing at all, which left a running acquisition streaming +// into a callback whose target was about to be freed, and leaked the stream, +// the snap buffer and the image buffer on every load/unload of a configuration. int AravisCamera::Shutdown() { + StopSequenceAcquisition(); + + g_clear_object(&arv_stream); + g_clear_object(&arv_buffer); + + { + std::lock_guard lock(img_buffer_mutex); + if (img_buffer != nullptr){ + free(img_buffer); + img_buffer = nullptr; + } + img_buffer_size = 0; + img_buffer_number_pixels = 0; + } + + // arv_device is owned by arv_cam, which the destructor clears; it must not + // be unreffed here. + arv_device = nullptr; + initialized = false; + return DEVICE_OK; } @@ -1195,8 +1320,27 @@ int AravisCamera::SnapImage() { GError *gerror = nullptr; - arv_buffer = arv_camera_acquisition(arv_cam, 0, &gerror); - if (ArvCheckError(gerror)) return ARV_ERROR; + // A zero timeout means "no timeout" to Aravis, which pops the buffer with a + // blocking call. A camera left in hardware trigger mode, or one frame lost + // to a dropped packet, would then block this thread forever and hang the + // application with no way back. Wait generously but finitely: several times + // the exposure, and never less than ARV_SNAP_MIN_TIMEOUT_US. + guint64 timeout_us = (guint64)(exposure_time * 1000.0 * ARV_SNAP_EXPOSURE_FACTOR); + if (timeout_us < ARV_SNAP_MIN_TIMEOUT_US){ + timeout_us = ARV_SNAP_MIN_TIMEOUT_US; + } + + arv_buffer = arv_camera_acquisition(arv_cam, timeout_us, &gerror); + if (ArvCheckError(&gerror)) return ARV_ERROR; + + if (arv_buffer == nullptr){ + std::stringstream msg; + msg << "Aravis Error, no image after " << (timeout_us / 1000) << "ms. " + << "If this camera is waiting on a hardware trigger, that trigger did " + << "not arrive."; + LogMessage(msg.str(), false); + return ARV_ERROR; + } return DEVICE_OK; } @@ -1233,10 +1377,17 @@ int AravisCamera::StopSequenceAcquisition() if (capturing){ capturing = false; - arv_camera_stop_acquisition(arv_cam, &gerror); - ArvCheckError(gerror); + + if (arv_cam != nullptr){ + arv_camera_stop_acquisition(arv_cam, &gerror); + ArvCheckError(&gerror); + } + + // Unreffing the stream stops and joins its thread, so no callback can be + // in flight once this returns. Shutdown() relies on that before it frees + // the image buffer the callback writes into. g_clear_object(&arv_stream); - + GetCoreCallback()->AcqFinished(this, 0); } return DEVICE_OK; diff --git a/DeviceAdapters/Aravis/AravisCamera.h b/DeviceAdapters/Aravis/AravisCamera.h index fe6d0675b..c8fb35276 100644 --- a/DeviceAdapters/Aravis/AravisCamera.h +++ b/DeviceAdapters/Aravis/AravisCamera.h @@ -39,9 +39,20 @@ #include "arv.h" #include "glib.h" +#include +#include +#include + #define ARV_ERROR 3141 // Should this be something specific? +// SnapImage() waits this multiple of the exposure time for a frame, and never +// less than the floor. Generous, because the alternative to a wrong guess is a +// spurious timeout on a slow link -- but finite, because Aravis treats a zero +// timeout as "block forever", which hangs the application. +#define ARV_SNAP_EXPOSURE_FACTOR 5.0 +#define ARV_SNAP_MIN_TIMEOUT_US 5000000 // 5 seconds + class AravisAcquisitionThread; @@ -98,16 +109,25 @@ class AravisCamera : public CCameraBase // Internal. void AcquisitionCallback(ArvStreamCallbackType, ArvBuffer *); void ArvBufferUpdate(ArvBuffer *aBuffer); - int ArvCheckError(GError *gerror) const; + int ArvCheckError(GError **gerror) const; void ArvGetExposure(); void ArvPixelFormatUpdate(guint32 arvPixelFormat); int ArvStartSequenceAcquisition(); private: - bool capturing; + // Written by the Micro-Manager thread and read by the Aravis stream + // callback thread, so plain bool is a data race. + std::atomic capturing; long counter; double exposure_time; + + // Whether this camera has binning at all, answered once by Initialize(). + // Asking Aravis for a feature the camera does not have fails every time, and + // Micro-Manager asks for binning on a timer, so the answer has to be + // remembered rather than rediscovered. + bool has_binning; + unsigned img_buffer_bit_depth; int img_buffer_bytes_per_pixel; int img_buffer_height; @@ -117,14 +137,18 @@ class AravisCamera : public CCameraBase int img_buffer_width; bool initialized; + // Guards img_buffer and the size/format fields describing it. The stream + // callback may reallocate the buffer while the Micro-Manager thread is + // reading it. + mutable std::mutex img_buffer_mutex; + ArvBuffer *arv_buffer; ArvCamera *arv_cam; - char *arv_cam_name; + std::string arv_cam_name; ArvDevice *arv_device; ArvStream *arv_stream; unsigned char *img_buffer; const char *pixel_type; - const char *trigger; }; #endif // !_ARAVIS_CAMERA_H_