[libcamera-devel] [PATCH v2 1/3] gstreamer: Add sensor mode selection
Jacopo Mondi
jacopo.mondi at ideasonboard.com
Mon Mar 27 11:21:55 CEST 2023
Hi Nicolas,
On Fri, Mar 24, 2023 at 02:12:45PM -0400, Nicolas Dufresne via libcamera-devel wrote:
> From: Nicolas Dufresne <nicolas.dufresne at collabora.com>
>
> This add support for selecting the sensor mode. A new read-only
> property called sensor-modes is filled when the element reaches
> READY state. It contains the list of all available sensor modes,
> including the supported framerate range. This is exposed as GstCaps
> in the form of sensor/mode,width=X,height=Y,format=Y,framerate=[...].
> The format string matches the libcamera format string representation.
>
> The application can then select a mode using the read/write sensor-mode
> control. The selected mode is also a caps, it will be intersected with
> the supported mode and the "best" match will be picked. This allows
> application to use simple filter when they want to pick a mode for lets
> say a specific framerate (e.g. sensor/mode,framerate=60/1).
>
> Signed-off-by: Nicolas Dufresne <nicolas.dufresne at collabora.com>
> ---
> src/gstreamer/gstlibcamera-utils.cpp | 4 +
> src/gstreamer/gstlibcamerasrc.cpp | 110 ++++++++++++++++++++++++++-
> 2 files changed, 112 insertions(+), 2 deletions(-)
>
> diff --git a/src/gstreamer/gstlibcamera-utils.cpp b/src/gstreamer/gstlibcamera-utils.cpp
> index 750ec351..c8a8df09 100644
> --- a/src/gstreamer/gstlibcamera-utils.cpp
> +++ b/src/gstreamer/gstlibcamera-utils.cpp
> @@ -416,6 +416,10 @@ gst_libcamera_configure_stream_from_caps(StreamConfiguration &stream_cfg,
> stream_cfg.pixelFormat = gst_format_to_pixel_format(gst_format);
> } else if (gst_structure_has_name(s, "image/jpeg")) {
> stream_cfg.pixelFormat = formats::MJPEG;
> + } else if (gst_structure_has_name(s, "sensor/mode")) {
> + gst_structure_fixate_field(s, "format");
> + const gchar *format = gst_structure_get_string(s, "format");
> + stream_cfg.pixelFormat = PixelFormat::fromString(format);
> } else {
> g_critical("Unsupported media type: %s", gst_structure_get_name(s));
> }
> diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp
> index a10cbd4f..2f05a03f 100644
> --- a/src/gstreamer/gstlibcamerasrc.cpp
> +++ b/src/gstreamer/gstlibcamerasrc.cpp
> @@ -147,6 +147,9 @@ struct _GstLibcameraSrc {
>
> gchar *camera_name;
>
> + GstCaps *sensor_modes;
> + GstCaps *sensor_mode;
> +
> GstLibcameraSrcState *state;
> GstLibcameraAllocator *allocator;
> GstFlowCombiner *flow_combiner;
> @@ -154,7 +157,10 @@ struct _GstLibcameraSrc {
>
> enum {
> PROP_0,
> - PROP_CAMERA_NAME
> + PROP_CAMERA_NAME,
> + PROP_SENSOR_MODES,
> + PROP_SENSOR_MODE,
> + PROP_LAST
> };
>
> G_DEFINE_TYPE_WITH_CODE(GstLibcameraSrc, gst_libcamera_src, GST_TYPE_ELEMENT,
> @@ -318,6 +324,61 @@ int GstLibcameraSrcState::processRequest()
> return err;
> }
>
> +static GstCaps *
> +gst_libcamera_src_enumerate_sensor_modes(GstLibcameraSrc *self)
> +{
> + GstCaps *modes = gst_caps_new_empty();
> + GstLibcameraSrcState *state = self->state;
> + auto config = state->cam_->generateConfiguration({ libcamera::StreamRole::Raw,
> + libcamera::StreamRole::VideoRecording });
This clearly shows our API falls short, as the presence of the RAW
stream, and the ability to capture it alongside other streams, is
conditional on the platform's capabilities. So this can't be
considered a sufficiently stable way of retrieving information about
the sensor's configuration. Not your fault, we currently don't have
anything better to offer.
We currently have a way to report all the possible configurations for
a stream using the StreamFormat class. StreamFormat is currently
nothing but
std::map<PixelFormat, std::vector<SizeRange>> formats_;
which gets populated by the pipeline handler while creating
the StreamConfiguration items which will be part of the generated
CameraConfiguration.
That's for the "Camera-provided information to applications" side.
On the "application to configure the Camera" side, StreamConfiguration
is used to tell the library how a Stream should be configured.
These are the fields a StreamConfiguration describe:
PixelFormat pixelFormat;
Size size;
unsigned int stride;
unsigned int frameSize;
unsigned int bufferCount;
std::optional<ColorSpace> colorSpace;
With pixelformat, size, bufferCount and (optionally) colorSpace which
are expected to be set by the application.
We have been discussing about adding to StreamConfiguration a
frameDuration since a long time. It is needed for some parts of the
Android HAL as well, and in general, as your patch shows, it is a
desirable feature. A similar reasoning can be made for the sensor's
configuration, even if this has received several push backs in the
past when RPi wanted something similar. Furthermore, I would be
tempted to consider the possible FrameDuration a property of the
sensor's configuration, but I presume that advanced use cases for
StillCapture chain to the canonical frame processing more stages, so
I presume it's better to have the FrameDurations separate from the
sensor configuration to allow describe additional delays there.
Summing it all up, we have StreamFormats to convey back to application
what a Stream can do, and StreamConfiguration to set the Stream
characteristics.
Addressing the needs you have here would require:
- Pipeline handlers to populate StreamFormats not only with
<PixelFormat, vector<SizeRange>> but also with the possible sensor's
configuration that can generated that mode and the possible frame
durations
- Application to populate a StreamConfiguration with the desired
FrameDuration and sensor mode
- Pipeline handler to validate that the desired sensor mode and
durations are compatible with the stream characteristics.
(as an example, if your pipeline can't upscale, you can't have a
sensor mode with a size smaller than the desired output).
This puts more complexity on the pipeline side, in both the generation
of configurations and in their validation, but would such an API
address your needs in your opinion ? Have you had any thoughts on how
you would like to have this handled ?
That's really an open question for everyone, as I guess this part is
becoming critical as this patch shows and the workaround we pushed RPi
to is really not generic enough.
One more point here below
Thanks
j
-------------------------------------------------------------------------------
Slightly unrelated point, but while you're here I would like to pick
your brain on the issue: the way StreamFormat are currently created is
slightly under-specified. The documentation goes in great length in
describing how the sizes associated with PixelFormat can be inspected
[1] but it only superficially describe how a pipeline handler should
populate the formats associated with a stream configuration [2]. I'll
make two examples:
- if the StreamConfiguration is for a 1080p Viewfinder stream should
the associated StreamFormats list resolutions > 1080p ?
- if the StreamConfiguration is for a !Raw role, should RAW
pixelformats be listed in the StreamFormats ?
There are arguments for both cases, but I guess the big picture
question is about how an application should expect to retrieve the
full camera capabilities:
- Does an application expect to receive only the available formats for
the role it has asked a configuration for ?
- Does an application expect to receive all the possible formats a
camera can produce regardless of the size/pixelformat of the
StreamConfiguration that has been generated ?
As a concrete example, currently the RkISP1 pipeline handler list a
RAW StreamFormat for all configurations, regardless of the role. I had
a patch to "fix" this but I've been pointed to the fact the API was
not very well specified.
What would your expectations be in this case ?
[1] https://libcamera.org/api-html/classlibcamera_1_1StreamFormats.html
[2] https://libcamera.org/api-html/structlibcamera_1_1StreamConfiguration.html#a37725e6b9e179ca80be0e57ed97857d3
-------------------------------------------------------------------------------
> + if (config == nullptr) {
> + GST_DEBUG_OBJECT(self, "Sensor mode selection is not supported, skipping enumeration.");
> + return modes;
> + }
> +
> + const libcamera::StreamFormats &formats = config->at(0).formats();
> +
> + for (const auto &pixfmt : formats.pixelformats()) {
> + for (const auto &size : formats.sizes(pixfmt)) {
> + config->at(0).size = size;
> + config->at(0).pixelFormat = pixfmt;
> +
> + if (config->validate() == CameraConfiguration::Invalid)
> + continue;
> +
> + if (state->cam_->configure(config.get()))
> + continue;
> +
> + auto fd_ctrl = state->cam_->controls().find(&controls::FrameDurationLimits);
> + if (fd_ctrl == state->cam_->controls().end())
> + continue;
> +
> + int minrate_num, minrate_denom;
> + int maxrate_num, maxrate_denom;
> + double min_framerate = gst_util_guint64_to_gdouble(1.0e6) /
> + gst_util_guint64_to_gdouble(fd_ctrl->second.max().get<int64_t>());
> + double max_framerate = gst_util_guint64_to_gdouble(1.0e6) /
> + gst_util_guint64_to_gdouble(fd_ctrl->second.min().get<int64_t>());
> + gst_util_double_to_fraction(min_framerate, &minrate_num, &minrate_denom);
> + gst_util_double_to_fraction(max_framerate, &maxrate_num, &maxrate_denom);
> +
> + GstStructure *s = gst_structure_new("sensor/mode",
> + "format", G_TYPE_STRING, pixfmt.toString().c_str(),
> + "width", G_TYPE_INT, size.width,
> + "height", G_TYPE_INT, size.height,
> + "framerate", GST_TYPE_FRACTION_RANGE,
> + minrate_num, minrate_denom,
> + maxrate_num, maxrate_denom,
> + nullptr);
> + gst_caps_append_structure(modes, s);
> + }
> + }
> +
> + GST_DEBUG_OBJECT(self, "Camera sensor modes: %" GST_PTR_FORMAT, modes);
> +
> + return modes;
> +}
> +
> static bool
> gst_libcamera_src_open(GstLibcameraSrc *self)
> {
> @@ -375,6 +436,7 @@ gst_libcamera_src_open(GstLibcameraSrc *self)
> /* No need to lock here, we didn't start our threads yet. */
> self->state->cm_ = cm;
> self->state->cam_ = cam;
> + self->sensor_modes = gst_libcamera_src_enumerate_sensor_modes(self);
>
> return true;
> }
> @@ -462,6 +524,7 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread,
> GstLibcameraSrcState *state = self->state;
> GstFlowReturn flow_ret = GST_FLOW_OK;
> gint ret;
> + g_autoptr(GstCaps) sensor_mode = nullptr;
>
> g_autoptr(GstStructure) element_caps = gst_structure_new_empty("caps");
>
> @@ -481,6 +544,16 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread,
> roles.push_back(gst_libcamera_pad_get_role(srcpad));
> }
>
> + if (!gst_caps_is_any(self->sensor_mode)) {
> + sensor_mode = gst_caps_intersect(self->sensor_mode, self->sensor_modes);
> + if (!gst_caps_is_empty(sensor_mode)) {
> + roles.push_back(libcamera::StreamRole::Raw);
> + } else {
> + GST_WARNING_OBJECT(self, "No sensor mode matching the selection, ignoring.");
> + gst_clear_caps(&sensor_mode);
> + }
> + }
> +
> /* Generate the stream configurations, there should be one per pad. */
> state->config_ = state->cam_->generateConfiguration(roles);
> if (state->config_ == nullptr) {
> @@ -490,7 +563,7 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread,
> gst_task_stop(task);
> return;
> }
> - g_assert(state->config_->size() == state->srcpads_.size());
> + g_assert(state->config_->size() == state->srcpads_.size() + (!!sensor_mode));
>
> for (gsize i = 0; i < state->srcpads_.size(); i++) {
> GstPad *srcpad = state->srcpads_[i];
> @@ -510,6 +583,12 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread,
> gst_libcamera_get_framerate_from_caps(caps, element_caps);
> }
>
> + if (sensor_mode) {
> + StreamConfiguration &stream_cfg = state->config_->at(state->srcpads_.size());
> + g_assert(gst_caps_is_writable(sensor_mode));
> + gst_libcamera_configure_stream_from_caps(stream_cfg, sensor_mode);
> + }
> +
> if (flow_ret != GST_FLOW_OK)
> goto done;
>
> @@ -624,6 +703,7 @@ gst_libcamera_src_task_leave([[maybe_unused]] GstTask *task,
> g_clear_object(&self->allocator);
> g_clear_pointer(&self->flow_combiner,
> (GDestroyNotify)gst_flow_combiner_free);
> + gst_clear_caps(&self->sensor_modes);
> }
>
> static void
> @@ -659,6 +739,10 @@ gst_libcamera_src_set_property(GObject *object, guint prop_id,
> g_free(self->camera_name);
> self->camera_name = g_value_dup_string(value);
> break;
> + case PROP_SENSOR_MODE:
> + gst_clear_caps(&self->sensor_mode);
> + self->sensor_mode = GST_CAPS(g_value_dup_boxed(value));
> + break;
> default:
> G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec);
> break;
> @@ -676,6 +760,12 @@ gst_libcamera_src_get_property(GObject *object, guint prop_id, GValue *value,
> case PROP_CAMERA_NAME:
> g_value_set_string(value, self->camera_name);
> break;
> + case PROP_SENSOR_MODES:
> + g_value_set_boxed(value, self->sensor_modes);
> + break;
> + case PROP_SENSOR_MODE:
> + g_value_set_boxed(value, self->sensor_mode);
> + break;
> default:
> G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec);
> break;
> @@ -763,6 +853,7 @@ gst_libcamera_src_init(GstLibcameraSrc *self)
> /* C-style friend. */
> state->src_ = self;
> self->state = state;
> + self->sensor_mode = gst_caps_new_any();
> }
>
> static GstPad *
> @@ -844,4 +935,19 @@ gst_libcamera_src_class_init(GstLibcameraSrcClass *klass)
> | G_PARAM_READWRITE
> | G_PARAM_STATIC_STRINGS));
> g_object_class_install_property(object_class, PROP_CAMERA_NAME, spec);
> +
> + spec = g_param_spec_boxed("sensor-modes", "Sensor Modes",
> + "GstCaps representing available sensor modes.",
> + GST_TYPE_CAPS,
> + (GParamFlags)(G_PARAM_READABLE
> + | G_PARAM_STATIC_STRINGS));
> + g_object_class_install_property(object_class, PROP_SENSOR_MODES, spec);
> +
> + spec = g_param_spec_boxed("sensor-mode", "Sensor Mode",
> + "GstCaps representing selected sensor mode.",
> + GST_TYPE_CAPS,
> + (GParamFlags)(GST_PARAM_MUTABLE_READY
> + | G_PARAM_READWRITE
> + | G_PARAM_STATIC_STRINGS));
> + g_object_class_install_property(object_class, PROP_SENSOR_MODE, spec);
> }
> --
> 2.39.2
>
More information about the libcamera-devel
mailing list