Merge branch 'master' into next

Signed-off-by: Naushir Patuck <[email protected]>

Author: naushir

include/libcamera/framebuffer.h

@@ -26,6 +26,7 @@ struct FrameMetadata {
 		FrameSuccess,
 		FrameError,
 		FrameCancelled,
+		FrameStartup,
 	};
 
 	struct Plane {

include/libcamera/internal/process.h

@@ -11,6 +11,7 @@
 #include <string>
 #include <vector>
 
+#include <libcamera/base/class.h>
 #include <libcamera/base/signal.h>
 #include <libcamera/base/unique_fd.h>
 
@@ -42,6 +43,8 @@ class Process final
 	Signal<enum ExitStatus, int> finished;
 
 private:
+	LIBCAMERA_DISABLE_COPY_AND_MOVE(Process)
+
 	void closeAllFdsExcept(const std::vector<int> &fds);
 	int isolate();
 	void died(int wstatus);

include/libcamera/ipa/raspberrypi.mojom

@@ -52,7 +52,8 @@ struct ConfigResult {
 
 struct StartResult {
 	libcamera.ControlList controls;
-	int32 dropFrameCount;
+	int32 startupFrameCount;
+	int32 invalidFrameCount;
 };
 
 struct PrepareParams {

src/android/camera_device.cpp

@@ -1079,7 +1079,7 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
 		buffer.internalBuffer = frameBuffer;
 
 		descriptor->request_->addBuffer(sourceStream->stream(),
-						frameBuffer, nullptr);
+						frameBuffer);
 
 		requestedStreams.insert(sourceStream);
 	}

src/apps/common/image.cpp

@@ -98,12 +98,12 @@ unsigned int Image::numPlanes() const
 
 Span<uint8_t> Image::data(unsigned int plane)
 {
-	assert(plane <= planes_.size());
+	assert(plane < planes_.size());
 	return planes_[plane];
 }
 
 Span<const uint8_t> Image::data(unsigned int plane) const
 {
-	assert(plane <= planes_.size());
+	assert(plane < planes_.size());
 	return planes_[plane];
 }

src/gstreamer/gstlibcamera-controls.cpp.in

@@ -68,7 +68,7 @@ static const GEnumValue {{ ctrl.name|snake_case }}_types[] = {
 		"{{ enum.gst_name }}"
 	},
 {%- endfor %}
-	{0, NULL, NULL}
+	{0, nullptr, nullptr}
 };
 
 #define TYPE_{{ ctrl.name|snake_case|upper }} \

src/gstreamer/gstlibcamerapad.cpp

@@ -102,7 +102,7 @@ gst_libcamera_stream_role_get_type()
 			"libcamera::Viewfinder",
 			"view-finder",
 		},
-		{ 0, NULL, NULL }
+		{ 0, nullptr, nullptr }
 	};
 
 	if (!type)

src/gstreamer/gstlibcamerasrc.cpp

@@ -29,6 +29,8 @@
 
 #include <atomic>
 #include <queue>
+#include <tuple>
+#include <utility>
 #include <vector>
 
 #include <libcamera/camera.h>
@@ -285,10 +287,19 @@ gst_libcamera_extrapolate_info(GstVideoInfo *info, guint32 stride)
 }
 
 static GstFlowReturn
-gst_libcamera_video_frame_copy(GstBuffer *src, GstBuffer *dest, const GstVideoInfo *dest_info, guint32 stride)
+gst_libcamera_video_frame_copy(GstBuffer *src, GstBuffer *dest,
+			       const GstVideoInfo *dest_info, guint32 stride)
 {
-	GstVideoInfo src_info = *dest_info;
+	/*
+	 * When dropping support for versions earlier than v1.22.0, use
+	 *
+	 * g_auto (GstVideoFrame) src_frame = GST_VIDEO_FRAME_INIT;
+	 * g_auto (GstVideoFrame) dest_frame = GST_VIDEO_FRAME_INIT;
+	 *
+	 * and drop the gst_video_frame_unmap() calls.
+	 */
 	GstVideoFrame src_frame, dest_frame;
+	GstVideoInfo src_info = *dest_info;
 
 	gst_libcamera_extrapolate_info(&src_info, stride);
 	src_info.size = gst_buffer_get_size(src);
@@ -298,7 +309,12 @@ gst_libcamera_video_frame_copy(GstBuffer *src, GstBuffer *dest, const GstVideoIn
 		return GST_FLOW_ERROR;
 	}
 
-	if (!gst_video_frame_map(&dest_frame, const_cast<GstVideoInfo *>(dest_info), dest, GST_MAP_WRITE)) {
+	/*
+	 * When dropping support for versions earlier than 1.20.0, drop the
+	 * const_cast<>().
+	 */
+	if (!gst_video_frame_map(&dest_frame, const_cast<GstVideoInfo *>(dest_info),
+				 dest, GST_MAP_WRITE)) {
 		GST_ERROR("Could not map dest buffer");
 		gst_video_frame_unmap(&src_frame);
 		return GST_FLOW_ERROR;
@@ -352,10 +368,10 @@ int GstLibcameraSrcState::processRequest()
 
 		if (video_pool) {
 			/* Only set video pool when a copy is needed. */
-			GstBuffer *copy = NULL;
+			GstBuffer *copy = nullptr;
 			const GstVideoInfo info = gst_libcamera_pad_get_video_info(srcpad);
 
-			ret = gst_buffer_pool_acquire_buffer(video_pool, &copy, NULL);
+			ret = gst_buffer_pool_acquire_buffer(video_pool, &copy, nullptr);
 			if (ret != GST_FLOW_OK) {
 				gst_buffer_unref(buffer);
 				GST_ELEMENT_ERROR(src_, RESOURCE, SETTINGS,
@@ -507,6 +523,73 @@ gst_libcamera_src_open(GstLibcameraSrc *self)
 	return true;
 }
 
+/**
+ * \brief Create a video pool for a pad
+ * \param[in] self The libcamerasrc instance
+ * \param[in] srcpad The pad
+ * \param[in] caps The pad caps
+ * \param[in] info The video info for the pad
+ *
+ * This function creates and returns a video buffer pool for the given pad if
+ * needed to accommodate stride mismatch. If the peer element supports stride
+ * negotiation through the meta API, no pool is needed and the function will
+ * return a null pool.
+ *
+ * \return A tuple containing the video buffers pool pointer and an error code
+ */
+static std::tuple<GstBufferPool *, int>
+gst_libcamera_create_video_pool(GstLibcameraSrc *self, GstPad *srcpad,
+				GstCaps *caps, const GstVideoInfo *info)
+{
+	g_autoptr(GstQuery) query = nullptr;
+	g_autoptr(GstBufferPool) pool = nullptr;
+	const gboolean need_pool = true;
+
+	/*
+	 * Get the peer allocation hints to check if it supports the meta API.
+	 * If so, the stride will be negotiated, and there's no need to create a
+	 * video pool.
+	 */
+	query = gst_query_new_allocation(caps, need_pool);
+
+	if (!gst_pad_peer_query(srcpad, query))
+		GST_DEBUG_OBJECT(self, "Didn't get downstream ALLOCATION hints");
+	else if (gst_query_find_allocation_meta(query, GST_VIDEO_META_API_TYPE, nullptr))
+		return { nullptr, 0 };
+
+	GST_WARNING_OBJECT(self, "Downstream doesn't support video meta, need to copy frame.");
+
+	/*
+	 * If the allocation query has pools, use the first one. Otherwise,
+	 * create a new pool.
+	 */
+	if (gst_query_get_n_allocation_pools(query) > 0)
+		gst_query_parse_nth_allocation_pool(query, 0, &pool, nullptr,
+						    nullptr, nullptr);
+
+	if (!pool) {
+		GstStructure *config;
+		guint min_buffers = 3;
+
+		pool = gst_video_buffer_pool_new();
+		config = gst_buffer_pool_get_config(pool);
+		gst_buffer_pool_config_set_params(config, caps, info->size, min_buffers, 0);
+
+		GST_DEBUG_OBJECT(self, "Own pool config is %" GST_PTR_FORMAT, config);
+
+		gst_buffer_pool_set_config(GST_BUFFER_POOL_CAST(pool), config);
+	}
+
+	if (!gst_buffer_pool_set_active(pool, true)) {
+		GST_ELEMENT_ERROR(self, RESOURCE, SETTINGS,
+				  ("Failed to active buffer pool"),
+				  ("gst_libcamera_src_negotiate() failed."));
+		return { nullptr, -EINVAL };
+	}
+
+	return { std::exchange(pool, nullptr), 0 };
+}
+
 /* Must be called with stream_lock held. */
 static bool
 gst_libcamera_src_negotiate(GstLibcameraSrc *self)
@@ -578,7 +661,7 @@ gst_libcamera_src_negotiate(GstLibcameraSrc *self)
 	for (gsize i = 0; i < state->srcpads_.size(); i++) {
 		GstPad *srcpad = state->srcpads_[i];
 		const StreamConfiguration &stream_cfg = state->config_->at(i);
-		GstBufferPool *video_pool = NULL;
+		GstBufferPool *video_pool = nullptr;
 		GstVideoInfo info;
 
 		g_autoptr(GstCaps) caps = gst_libcamera_stream_configuration_to_caps(stream_cfg, transfer[i]);
@@ -589,50 +672,13 @@ gst_libcamera_src_negotiate(GstLibcameraSrc *self)
 		/* Stride mismatch between camera stride and that calculated by video-info. */
 		if (static_cast<unsigned int>(info.stride[0]) != stream_cfg.stride &&
 		    GST_VIDEO_INFO_FORMAT(&info) != GST_VIDEO_FORMAT_ENCODED) {
-			GstQuery *query = NULL;
-			const gboolean need_pool = true;
-			gboolean has_video_meta = false;
-
 			gst_libcamera_extrapolate_info(&info, stream_cfg.stride);
 
-			query = gst_query_new_allocation(caps, need_pool);
-			if (!gst_pad_peer_query(srcpad, query))
-				GST_DEBUG_OBJECT(self, "Didn't get downstream ALLOCATION hints");
-			else
-				has_video_meta = gst_query_find_allocation_meta(query, GST_VIDEO_META_API_TYPE, NULL);
-
-			if (!has_video_meta) {
-				GstBufferPool *pool = NULL;
-
-				if (gst_query_get_n_allocation_pools(query) > 0)
-					gst_query_parse_nth_allocation_pool(query, 0, &pool, NULL, NULL, NULL);
-
-				if (pool)
-					video_pool = pool;
-				else {
-					GstStructure *config;
-					guint min_buffers = 3;
-					video_pool = gst_video_buffer_pool_new();
-
-					config = gst_buffer_pool_get_config(video_pool);
-					gst_buffer_pool_config_set_params(config, caps, info.size, min_buffers, 0);
-
-					GST_DEBUG_OBJECT(self, "Own pool config is %" GST_PTR_FORMAT, config);
-
-					gst_buffer_pool_set_config(GST_BUFFER_POOL_CAST(video_pool), config);
-				}
-
-				GST_WARNING_OBJECT(self, "Downstream doesn't support video meta, need to copy frame.");
-
-				if (!gst_buffer_pool_set_active(video_pool, true)) {
-					gst_caps_unref(caps);
-					GST_ELEMENT_ERROR(self, RESOURCE, SETTINGS,
-							  ("Failed to active buffer pool"),
-							  ("gst_libcamera_src_negotiate() failed."));
-					return false;
-				}
-			}
-			gst_query_unref(query);
+			std::tie(video_pool, ret) =
+				gst_libcamera_create_video_pool(self, srcpad,
+								caps, &info);
+			if (ret)
+				return false;
 		}
 
 		GstLibcameraPool *pool = gst_libcamera_pool_new(self->allocator,
@@ -1020,7 +1066,7 @@ gst_libcamera_src_request_new_pad(GstElement *element, GstPadTemplate *templ,
 				  const gchar *name, [[maybe_unused]] const GstCaps *caps)
 {
 	GstLibcameraSrc *self = GST_LIBCAMERA_SRC(element);
-	g_autoptr(GstPad) pad = NULL;
+	g_autoptr(GstPad) pad = nullptr;
 
 	GST_DEBUG_OBJECT(self, "new request pad created");
 
@@ -1034,7 +1080,7 @@ gst_libcamera_src_request_new_pad(GstElement *element, GstPadTemplate *templ,
 		GST_ELEMENT_ERROR(element, STREAM, FAILED,
 				  ("Internal data stream error."),
 				  ("Could not add pad to element"));
-		return NULL;
+		return nullptr;
 	}
 
 	gst_child_proxy_child_added(GST_CHILD_PROXY(self), G_OBJECT(pad), GST_OBJECT_NAME(pad));

src/ipa/rpi/common/ipa_base.cpp

@@ -60,23 +60,24 @@ const ControlInfoMap::Map ipaControls{
 	/* \todo Move this to the Camera class */
 	{ &controls::AeEnable, ControlInfo(false, true, true) },
 	{ &controls::ExposureTimeMode,
-	  ControlInfo(static_cast<int32_t>(controls::ExposureTimeModeAuto),
-		      static_cast<int32_t>(controls::ExposureTimeModeManual),
-		      static_cast<int32_t>(controls::ExposureTimeModeAuto)) },
+	  ControlInfo({ { ControlValue(controls::ExposureTimeModeAuto),
+			  ControlValue(controls::ExposureTimeModeManual) } },
+		      ControlValue(controls::ExposureTimeModeAuto)) },
 	{ &controls::ExposureTime,
 	  ControlInfo(1, 66666, static_cast<int32_t>(defaultExposureTime.get<std::micro>())) },
 	{ &controls::AnalogueGainMode,
-	  ControlInfo(static_cast<int32_t>(controls::AnalogueGainModeAuto),
-		      static_cast<int32_t>(controls::AnalogueGainModeManual),
-		      static_cast<int32_t>(controls::AnalogueGainModeAuto)) },
+	  ControlInfo({ { ControlValue(controls::AnalogueGainModeAuto),
+			  ControlValue(controls::AnalogueGainModeManual) } },
+		      ControlValue(controls::AnalogueGainModeAuto)) },
 	{ &controls::AnalogueGain, ControlInfo(1.0f, 16.0f, 1.0f) },
 	{ &controls::AeMeteringMode, ControlInfo(controls::AeMeteringModeValues) },
 	{ &controls::AeConstraintMode, ControlInfo(controls::AeConstraintModeValues) },
 	{ &controls::AeExposureMode, ControlInfo(controls::AeExposureModeValues) },
 	{ &controls::ExposureValue, ControlInfo(-8.0f, 8.0f, 0.0f) },
-	{ &controls::AeFlickerMode, ControlInfo(static_cast<int>(controls::FlickerOff),
-						static_cast<int>(controls::FlickerManual),
-						static_cast<int>(controls::FlickerOff)) },
+	{ &controls::AeFlickerMode,
+	  ControlInfo({ { ControlValue(controls::FlickerOff),
+			  ControlValue(controls::FlickerManual) } },
+		      ControlValue(controls::FlickerOff)) },
 	{ &controls::AeFlickerPeriod, ControlInfo(100, 1000000) },
 	{ &controls::Brightness, ControlInfo(-1.0f, 1.0f, 0.0f) },
 	{ &controls::Contrast, ControlInfo(0.0f, 32.0f, 1.0f) },
@@ -329,9 +330,10 @@ void IpaBase::start(const ControlList &controls, StartResult *result)
 	 * "mistrusted", which depends on whether this is a startup from cold,
 	 * or merely a mode switch in a running system.
 	 */
+	unsigned int agcConvergenceFrames = 0, awbConvergenceFrames = 0;
 	frameCount_ = 0;
 	if (firstStart_) {
-		dropFrameCount_ = helper_->hideFramesStartup();
+		invalidCount_ = helper_->hideFramesStartup();
 		mistrustCount_ = helper_->mistrustFramesStartup();
 
 		/*
@@ -341,7 +343,6 @@ void IpaBase::start(const ControlList &controls, StartResult *result)
 		 * (mistrustCount_) that they won't see. But if zero (i.e.
 		 * no convergence necessary), no frames need to be dropped.
 		 */
-		unsigned int agcConvergenceFrames = 0;
 		RPiController::AgcAlgorithm *agc = dynamic_cast<RPiController::AgcAlgorithm *>(
 			controller_.getAlgorithm("agc"));
 		if (agc) {
@@ -350,23 +351,25 @@ void IpaBase::start(const ControlList &controls, StartResult *result)
 				agcConvergenceFrames += mistrustCount_;
 		}
 
-		unsigned int awbConvergenceFrames = 0;
 		RPiController::AwbAlgorithm *awb = dynamic_cast<RPiController::AwbAlgorithm *>(
 			controller_.getAlgorithm("awb"));
 		if (awb) {
 			awbConvergenceFrames = awb->getConvergenceFrames();
 			if (awbConvergenceFrames)
 				awbConvergenceFrames += mistrustCount_;
 		}
-
-		dropFrameCount_ = std::max({ dropFrameCount_, agcConvergenceFrames, awbConvergenceFrames });
-		LOG(IPARPI, Debug) << "Drop " << dropFrameCount_ << " frames on startup";
 	} else {
-		dropFrameCount_ = helper_->hideFramesModeSwitch();
+		invalidCount_ = helper_->hideFramesModeSwitch();
 		mistrustCount_ = helper_->mistrustFramesModeSwitch();
 	}
 
-	result->dropFrameCount = dropFrameCount_;
+	result->startupFrameCount = std::max({ agcConvergenceFrames, awbConvergenceFrames });
+	result->invalidFrameCount = invalidCount_;
+
+	invalidCount_ = std::max({ invalidCount_, agcConvergenceFrames, awbConvergenceFrames });
+
+	LOG(IPARPI, Debug) << "Startup frames: " << result->startupFrameCount
+			   << " Invalid frames: " << result->invalidFrameCount;
 
 	firstStart_ = false;
 	lastRunTimestamp_ = 0;
@@ -447,7 +450,7 @@ void IpaBase::prepareIsp(const PrepareParams &params)
 
 	/* Allow a 10% margin on the comparison below. */
 	Duration delta = (frameTimestamp - lastRunTimestamp_) * 1.0ns;
-	if (lastRunTimestamp_ && frameCount_ > dropFrameCount_ &&
+	if (lastRunTimestamp_ && frameCount_ > invalidCount_ &&
 	    delta < controllerMinFrameDuration * 0.9 && !hdrChange) {
 		/*
 		 * Ensure we merge the previous frame's metadata with the current

src/ipa/rpi/common/ipa_base.h

@@ -117,8 +117,8 @@ class IpaBase : public IPARPiInterface
 	/* How many frames we should avoid running control algos on. */
 	unsigned int mistrustCount_;
 
-	/* Number of frames that need to be dropped on startup. */
-	unsigned int dropFrameCount_;
+	/* Number of frames that need to be marked as dropped on startup. */
+	unsigned int invalidCount_;
 
 	/* Frame timestamp for the last run of the controller. */
 	uint64_t lastRunTimestamp_;