| Message ID | 20250516073044.426489-1-qi.hou@nxp.com |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
Hi, Le vendredi 16 mai 2025 à 16:30 +0900, Hou Qi a écrit : > GStreamer video-info calculated stride and offset may differ from > those used by the camera. > > For stride and offset mismatch, this patch adds video meta to buffer > if downstream supports VideoMeta through allocation query. Otherwise, > create a internal VideoPool using the caps, and copy video frame to > this system memory. > > Signed-off-by: Hou Qi <qi.hou@nxp.com> > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> I'm happy with this version. Tested-by: Nicolas Dufresne <nicolas.dufresne@collabora.com> Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com> > --- > src/gstreamer/gstlibcamera-utils.cpp | 33 ++++++ > src/gstreamer/gstlibcamera-utils.h | 5 + > src/gstreamer/gstlibcamerapad.cpp | 31 ++++++ > src/gstreamer/gstlibcamerapad.h | 8 ++ > src/gstreamer/gstlibcamerapool.cpp | 15 ++- > src/gstreamer/gstlibcamerapool.h | 3 +- > src/gstreamer/gstlibcamerasrc.cpp | 147 ++++++++++++++++++++++++++- > 7 files changed, 238 insertions(+), 4 deletions(-) > > diff --git a/src/gstreamer/gstlibcamera-utils.cpp b/src/gstreamer/gstlibcamera-utils.cpp > index 2edebba0..bf12cb07 100644 > --- a/src/gstreamer/gstlibcamera-utils.cpp > +++ b/src/gstreamer/gstlibcamera-utils.cpp > @@ -599,6 +599,39 @@ gst_task_resume(GstTask *task) > } > #endif > > +#if !GST_CHECK_VERSION(1, 22, 0) > +/* > + * Copyright (C) <1999> Erik Walthinsen <omega@cse.ogi.edu> > + * Library <2002> Ronald Bultje <rbultje@ronald.bitfreak.net> > + * Copyright (C) <2007> David A. Schleef <ds@schleef.org> > + */ > +/* This function has been imported directly from the gstreamer project to > + * support backwards compatibility and should be removed when the older > + * version is no longer supported. */ > +gint gst_video_format_info_extrapolate_stride(const GstVideoFormatInfo *finfo, gint plane, gint stride) > +{ > + gint estride; > + gint comp[GST_VIDEO_MAX_COMPONENTS]; > + gint i; > + > + /* there is nothing to extrapolate on first plane */ > + if (plane == 0) > + return stride; > + > + gst_video_format_info_component(finfo, plane, comp); > + > + /* For now, all planar formats have a single component on first plane, but > + * if there was a planar format with more, we'd have to make a ratio of the > + * number of component on the first plane against the number of component on > + * the current plane. */ > + estride = 0; > + for (i = 0; i < GST_VIDEO_MAX_COMPONENTS && comp[i] >= 0; i++) > + estride += GST_VIDEO_FORMAT_INFO_SCALE_WIDTH(finfo, comp[i], stride); > + > + return estride; > +} > +#endif > + > G_LOCK_DEFINE_STATIC(cm_singleton_lock); > static std::weak_ptr<CameraManager> cm_singleton_ptr; > > diff --git a/src/gstreamer/gstlibcamera-utils.h b/src/gstreamer/gstlibcamera-utils.h > index 4978987c..5f4e8a0f 100644 > --- a/src/gstreamer/gstlibcamera-utils.h > +++ b/src/gstreamer/gstlibcamera-utils.h > @@ -36,6 +36,11 @@ static inline void gst_clear_event(GstEvent **event_ptr) > #if !GST_CHECK_VERSION(1, 17, 1) > gboolean gst_task_resume(GstTask *task); > #endif > + > +#if !GST_CHECK_VERSION(1, 22, 0) > +gint gst_video_format_info_extrapolate_stride(const GstVideoFormatInfo *finfo, gint plane, gint stride); > +#endif > + > std::shared_ptr<libcamera::CameraManager> gst_libcamera_get_camera_manager(int &ret); > > /** > diff --git a/src/gstreamer/gstlibcamerapad.cpp b/src/gstreamer/gstlibcamerapad.cpp > index 7b22aebe..3bc2bc87 100644 > --- a/src/gstreamer/gstlibcamerapad.cpp > +++ b/src/gstreamer/gstlibcamerapad.cpp > @@ -18,6 +18,8 @@ struct _GstLibcameraPad { > GstPad parent; > StreamRole role; > GstLibcameraPool *pool; > + GstBufferPool *video_pool; > + GstVideoInfo info; > GstClockTime latency; > }; > > @@ -153,6 +155,35 @@ gst_libcamera_pad_set_pool(GstPad *pad, GstLibcameraPool *pool) > self->pool = pool; > } > > +GstBufferPool * > +gst_libcamera_pad_get_video_pool(GstPad *pad) > +{ > + auto *self = GST_LIBCAMERA_PAD(pad); > + return self->video_pool; > +} > + > +void gst_libcamera_pad_set_video_pool(GstPad *pad, GstBufferPool *video_pool) > +{ > + auto *self = GST_LIBCAMERA_PAD(pad); > + > + if (self->video_pool) > + g_object_unref(self->video_pool); > + self->video_pool = video_pool; > +} > + > +GstVideoInfo gst_libcamera_pad_get_video_info(GstPad *pad) > +{ > + auto *self = GST_LIBCAMERA_PAD(pad); > + return self->info; > +} > + > +void gst_libcamera_pad_set_video_info(GstPad *pad, const GstVideoInfo *info) > +{ > + auto *self = GST_LIBCAMERA_PAD(pad); > + > + self->info = *info; > +} > + > Stream * > gst_libcamera_pad_get_stream(GstPad *pad) > { > diff --git a/src/gstreamer/gstlibcamerapad.h b/src/gstreamer/gstlibcamerapad.h > index 630c168a..f98b8a7f 100644 > --- a/src/gstreamer/gstlibcamerapad.h > +++ b/src/gstreamer/gstlibcamerapad.h > @@ -23,6 +23,14 @@ GstLibcameraPool *gst_libcamera_pad_get_pool(GstPad *pad); > > void gst_libcamera_pad_set_pool(GstPad *pad, GstLibcameraPool *pool); > > +GstBufferPool *gst_libcamera_pad_get_video_pool(GstPad *pad); > + > +void gst_libcamera_pad_set_video_pool(GstPad *pad, GstBufferPool *video_pool); > + > +GstVideoInfo gst_libcamera_pad_get_video_info(GstPad *pad); > + > +void gst_libcamera_pad_set_video_info(GstPad *pad, const GstVideoInfo *info); > + > libcamera::Stream *gst_libcamera_pad_get_stream(GstPad *pad); > > void gst_libcamera_pad_set_latency(GstPad *pad, GstClockTime latency); > diff --git a/src/gstreamer/gstlibcamerapool.cpp b/src/gstreamer/gstlibcamerapool.cpp > index 9cd7eccb..8278144f 100644 > --- a/src/gstreamer/gstlibcamerapool.cpp > +++ b/src/gstreamer/gstlibcamerapool.cpp > @@ -134,8 +134,20 @@ gst_libcamera_pool_class_init(GstLibcameraPoolClass *klass) > G_TYPE_NONE, 0); > } > > +static void > +gst_libcamera_buffer_add_video_meta(GstBuffer *buffer, GstVideoInfo *info) > +{ > + GstVideoMeta *vmeta; > + vmeta = gst_buffer_add_video_meta_full(buffer, GST_VIDEO_FRAME_FLAG_NONE, > + GST_VIDEO_INFO_FORMAT(info), GST_VIDEO_INFO_WIDTH(info), > + GST_VIDEO_INFO_HEIGHT(info), GST_VIDEO_INFO_N_PLANES(info), > + info->offset, info->stride); > + GST_META_FLAGS(vmeta) = (GstMetaFlags)(GST_META_FLAGS(vmeta) | GST_META_FLAG_POOLED); > +} > + > GstLibcameraPool * > -gst_libcamera_pool_new(GstLibcameraAllocator *allocator, Stream *stream) > +gst_libcamera_pool_new(GstLibcameraAllocator *allocator, Stream *stream, > + GstVideoInfo *info) > { > auto *pool = GST_LIBCAMERA_POOL(g_object_new(GST_TYPE_LIBCAMERA_POOL, nullptr)); > > @@ -145,6 +157,7 @@ gst_libcamera_pool_new(GstLibcameraAllocator *allocator, Stream *stream) > gsize pool_size = gst_libcamera_allocator_get_pool_size(allocator, stream); > for (gsize i = 0; i < pool_size; i++) { > GstBuffer *buffer = gst_buffer_new(); > + gst_libcamera_buffer_add_video_meta(buffer, info); > pool->queue->push_back(buffer); > } > > diff --git a/src/gstreamer/gstlibcamerapool.h b/src/gstreamer/gstlibcamerapool.h > index 2a7a9c77..02ee4dd4 100644 > --- a/src/gstreamer/gstlibcamerapool.h > +++ b/src/gstreamer/gstlibcamerapool.h > @@ -14,6 +14,7 @@ > #include "gstlibcameraallocator.h" > > #include <gst/gst.h> > +#include <gst/video/video.h> > > #include <libcamera/stream.h> > > @@ -21,7 +22,7 @@ > G_DECLARE_FINAL_TYPE(GstLibcameraPool, gst_libcamera_pool, GST_LIBCAMERA, POOL, GstBufferPool) > > GstLibcameraPool *gst_libcamera_pool_new(GstLibcameraAllocator *allocator, > - libcamera::Stream *stream); > + libcamera::Stream *stream, GstVideoInfo *info); > > libcamera::Stream *gst_libcamera_pool_get_stream(GstLibcameraPool *self); > > diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp > index 5e9e843d..4b81c94e 100644 > --- a/src/gstreamer/gstlibcamerasrc.cpp > +++ b/src/gstreamer/gstlibcamerasrc.cpp > @@ -268,6 +268,58 @@ GstLibcameraSrcState::requestCompleted(Request *request) > gst_task_resume(src_->task); > } > > +static void > +gst_libcamera_extrapolate_info(GstVideoInfo *info, guint32 stride) > +{ > + guint i, estride; > + gsize offset = 0; > + > + /* this should be updated if tiled formats get added in the future. */ > + for (i = 0; i < GST_VIDEO_INFO_N_PLANES(info); i++) { > + estride = gst_video_format_info_extrapolate_stride(info->finfo, i, stride); > + info->stride[i] = estride; > + info->offset[i] = offset; > + offset += estride * GST_VIDEO_FORMAT_INFO_SCALE_HEIGHT(info->finfo, i, > + GST_VIDEO_INFO_HEIGHT(info)); > + } > +} > + > +static GstFlowReturn > +gst_libcamera_video_frame_copy(GstBuffer *src, GstBuffer *dest, const GstVideoInfo *dest_info, guint32 stride) > +{ > + GstVideoInfo src_info = *dest_info; > + GstVideoFrame src_frame, dest_frame; > + > + gst_libcamera_extrapolate_info(&src_info, stride); > + src_info.size = gst_buffer_get_size(src); > + > + if (!gst_video_frame_map(&src_frame, &src_info, src, GST_MAP_READ)) { > + GST_ERROR("Could not map buffer"); > + goto error; > + } > + > + if (!gst_video_frame_map(&dest_frame, dest_info, dest, GST_MAP_WRITE)) { > + GST_ERROR("Could not map buffer"); > + gst_video_frame_unmap(&src_frame); > + goto error; > + } > + > + if (!gst_video_frame_copy(&dest_frame, &src_frame)) { > + GST_ERROR("Could not copy frame"); > + gst_video_frame_unmap(&src_frame); > + gst_video_frame_unmap(&dest_frame); > + goto error; > + } > + > + gst_video_frame_unmap(&src_frame); > + gst_video_frame_unmap(&dest_frame); > + > + return GST_FLOW_OK; > + > +error: > + return GST_FLOW_ERROR; > +} > + > /* Must be called with stream_lock held. */ > int GstLibcameraSrcState::processRequest() > { > @@ -292,11 +344,41 @@ int GstLibcameraSrcState::processRequest() > GstFlowReturn ret = GST_FLOW_OK; > gst_flow_combiner_reset(src_->flow_combiner); > > - for (GstPad *srcpad : srcpads_) { > + for (gsize i = 0; i < srcpads_.size(); i++) { > + GstPad *srcpad = srcpads_[i]; > Stream *stream = gst_libcamera_pad_get_stream(srcpad); > GstBuffer *buffer = wrap->detachBuffer(stream); > > FrameBuffer *fb = gst_libcamera_buffer_get_frame_buffer(buffer); > + const StreamConfiguration &stream_cfg = config_->at(i); > + GstBufferPool *video_pool = gst_libcamera_pad_get_video_pool(srcpad); > + > + if (video_pool) { > + /* Only set video pool when a copy is needed */ > + GstBuffer *copy = NULL; > + const GstVideoInfo info = gst_libcamera_pad_get_video_info(srcpad); > + > + ret = gst_buffer_pool_acquire_buffer(video_pool, ©, NULL); > + if (ret != GST_FLOW_OK) { > + gst_buffer_unref(buffer); > + GST_ELEMENT_ERROR(src_, RESOURCE, SETTINGS, > + ("Failed to acquire buffer"), > + ("GstLibcameraSrcState::processRequest() failed: %s", g_strerror(- > ret))); > + return -EPIPE; > + } > + > + ret = gst_libcamera_video_frame_copy(buffer, copy, &info, stream_cfg.stride); > + gst_buffer_unref(buffer); > + if (ret != GST_FLOW_OK) { > + gst_buffer_unref(copy); > + GST_ELEMENT_ERROR(src_, RESOURCE, SETTINGS, > + ("Failed to copy buffer"), > + ("GstLibcameraSrcState::processRequest() failed: %s", g_strerror(- > ret))); > + return -EPIPE; > + } > + > + buffer = copy; > + } > > if (GST_CLOCK_TIME_IS_VALID(wrap->pts_)) { > GST_BUFFER_PTS(buffer) = wrap->pts_; > @@ -499,13 +581,68 @@ 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; > + GstVideoInfo info; > + > + g_autoptr(GstCaps) caps = gst_libcamera_stream_configuration_to_caps(stream_cfg, transfer[i]); > + > + gst_video_info_from_caps(&info, caps); > + gst_libcamera_pad_set_video_info(srcpad, &info); > + > + /* 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); > + } > + > + 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); > + } > > GstLibcameraPool *pool = gst_libcamera_pool_new(self->allocator, > - stream_cfg.stream()); > + stream_cfg.stream(), &info); > g_signal_connect_swapped(pool, "buffer-notify", > G_CALLBACK(gst_task_resume), self->task); > > gst_libcamera_pad_set_pool(srcpad, pool); > + gst_libcamera_pad_set_video_pool(srcpad, video_pool); > > /* Clear all reconfigure flags. */ > gst_pad_check_reconfigure(srcpad); > @@ -922,6 +1059,12 @@ gst_libcamera_src_release_pad(GstElement *element, GstPad *pad) > auto end_iterator = pads.end(); > auto pad_iterator = std::find(begin_iterator, end_iterator, pad); > > + GstBufferPool *video_pool = gst_libcamera_pad_get_video_pool(pad); > + if (video_pool) { > + gst_buffer_pool_set_active(video_pool, false); > + gst_object_unref(video_pool); > + } > + > if (pad_iterator != end_iterator) { > g_object_unref(*pad_iterator); > pads.erase(pad_iterator);
Quoting Nicolas Dufresne (2025-05-16 11:16:50) > Hi, > > Le vendredi 16 mai 2025 à 16:30 +0900, Hou Qi a écrit : > > GStreamer video-info calculated stride and offset may differ from > > those used by the camera. > > > > For stride and offset mismatch, this patch adds video meta to buffer > > if downstream supports VideoMeta through allocation query. Otherwise, > > create a internal VideoPool using the caps, and copy video frame to > > this system memory. > > > > Signed-off-by: Hou Qi <qi.hou@nxp.com> > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > I'm happy with this version. > > Tested-by: Nicolas Dufresne <nicolas.dufresne@collabora.com> > Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com> > We're close to merging - but the CI build is failing. Could you take a look please? https://gitlab.freedesktop.org/camera/libcamera/-/pipelines/1425366 -- Kieran > > --- > > src/gstreamer/gstlibcamera-utils.cpp | 33 ++++++ > > src/gstreamer/gstlibcamera-utils.h | 5 + > > src/gstreamer/gstlibcamerapad.cpp | 31 ++++++ > > src/gstreamer/gstlibcamerapad.h | 8 ++ > > src/gstreamer/gstlibcamerapool.cpp | 15 ++- > > src/gstreamer/gstlibcamerapool.h | 3 +- > > src/gstreamer/gstlibcamerasrc.cpp | 147 ++++++++++++++++++++++++++- > > 7 files changed, 238 insertions(+), 4 deletions(-) > > > > diff --git a/src/gstreamer/gstlibcamera-utils.cpp b/src/gstreamer/gstlibcamera-utils.cpp > > index 2edebba0..bf12cb07 100644 > > --- a/src/gstreamer/gstlibcamera-utils.cpp > > +++ b/src/gstreamer/gstlibcamera-utils.cpp > > @@ -599,6 +599,39 @@ gst_task_resume(GstTask *task) > > } > > #endif > > > > +#if !GST_CHECK_VERSION(1, 22, 0) > > +/* > > + * Copyright (C) <1999> Erik Walthinsen <omega@cse.ogi.edu> > > + * Library <2002> Ronald Bultje <rbultje@ronald.bitfreak.net> > > + * Copyright (C) <2007> David A. Schleef <ds@schleef.org> > > + */ > > +/* This function has been imported directly from the gstreamer project to > > + * support backwards compatibility and should be removed when the older > > + * version is no longer supported. */ > > +gint gst_video_format_info_extrapolate_stride(const GstVideoFormatInfo *finfo, gint plane, gint stride) > > +{ > > + gint estride; > > + gint comp[GST_VIDEO_MAX_COMPONENTS]; > > + gint i; > > + > > + /* there is nothing to extrapolate on first plane */ > > + if (plane == 0) > > + return stride; > > + > > + gst_video_format_info_component(finfo, plane, comp); > > + > > + /* For now, all planar formats have a single component on first plane, but > > + * if there was a planar format with more, we'd have to make a ratio of the > > + * number of component on the first plane against the number of component on > > + * the current plane. */ > > + estride = 0; > > + for (i = 0; i < GST_VIDEO_MAX_COMPONENTS && comp[i] >= 0; i++) > > + estride += GST_VIDEO_FORMAT_INFO_SCALE_WIDTH(finfo, comp[i], stride); > > + > > + return estride; > > +} > > +#endif > > + > > G_LOCK_DEFINE_STATIC(cm_singleton_lock); > > static std::weak_ptr<CameraManager> cm_singleton_ptr; > > > > diff --git a/src/gstreamer/gstlibcamera-utils.h b/src/gstreamer/gstlibcamera-utils.h > > index 4978987c..5f4e8a0f 100644 > > --- a/src/gstreamer/gstlibcamera-utils.h > > +++ b/src/gstreamer/gstlibcamera-utils.h > > @@ -36,6 +36,11 @@ static inline void gst_clear_event(GstEvent **event_ptr) > > #if !GST_CHECK_VERSION(1, 17, 1) > > gboolean gst_task_resume(GstTask *task); > > #endif > > + > > +#if !GST_CHECK_VERSION(1, 22, 0) > > +gint gst_video_format_info_extrapolate_stride(const GstVideoFormatInfo *finfo, gint plane, gint stride); > > +#endif > > + > > std::shared_ptr<libcamera::CameraManager> gst_libcamera_get_camera_manager(int &ret); > > > > /** > > diff --git a/src/gstreamer/gstlibcamerapad.cpp b/src/gstreamer/gstlibcamerapad.cpp > > index 7b22aebe..3bc2bc87 100644 > > --- a/src/gstreamer/gstlibcamerapad.cpp > > +++ b/src/gstreamer/gstlibcamerapad.cpp > > @@ -18,6 +18,8 @@ struct _GstLibcameraPad { > > GstPad parent; > > StreamRole role; > > GstLibcameraPool *pool; > > + GstBufferPool *video_pool; > > + GstVideoInfo info; > > GstClockTime latency; > > }; > > > > @@ -153,6 +155,35 @@ gst_libcamera_pad_set_pool(GstPad *pad, GstLibcameraPool *pool) > > self->pool = pool; > > } > > > > +GstBufferPool * > > +gst_libcamera_pad_get_video_pool(GstPad *pad) > > +{ > > + auto *self = GST_LIBCAMERA_PAD(pad); > > + return self->video_pool; > > +} > > + > > +void gst_libcamera_pad_set_video_pool(GstPad *pad, GstBufferPool *video_pool) > > +{ > > + auto *self = GST_LIBCAMERA_PAD(pad); > > + > > + if (self->video_pool) > > + g_object_unref(self->video_pool); > > + self->video_pool = video_pool; > > +} > > + > > +GstVideoInfo gst_libcamera_pad_get_video_info(GstPad *pad) > > +{ > > + auto *self = GST_LIBCAMERA_PAD(pad); > > + return self->info; > > +} > > + > > +void gst_libcamera_pad_set_video_info(GstPad *pad, const GstVideoInfo *info) > > +{ > > + auto *self = GST_LIBCAMERA_PAD(pad); > > + > > + self->info = *info; > > +} > > + > > Stream * > > gst_libcamera_pad_get_stream(GstPad *pad) > > { > > diff --git a/src/gstreamer/gstlibcamerapad.h b/src/gstreamer/gstlibcamerapad.h > > index 630c168a..f98b8a7f 100644 > > --- a/src/gstreamer/gstlibcamerapad.h > > +++ b/src/gstreamer/gstlibcamerapad.h > > @@ -23,6 +23,14 @@ GstLibcameraPool *gst_libcamera_pad_get_pool(GstPad *pad); > > > > void gst_libcamera_pad_set_pool(GstPad *pad, GstLibcameraPool *pool); > > > > +GstBufferPool *gst_libcamera_pad_get_video_pool(GstPad *pad); > > + > > +void gst_libcamera_pad_set_video_pool(GstPad *pad, GstBufferPool *video_pool); > > + > > +GstVideoInfo gst_libcamera_pad_get_video_info(GstPad *pad); > > + > > +void gst_libcamera_pad_set_video_info(GstPad *pad, const GstVideoInfo *info); > > + > > libcamera::Stream *gst_libcamera_pad_get_stream(GstPad *pad); > > > > void gst_libcamera_pad_set_latency(GstPad *pad, GstClockTime latency); > > diff --git a/src/gstreamer/gstlibcamerapool.cpp b/src/gstreamer/gstlibcamerapool.cpp > > index 9cd7eccb..8278144f 100644 > > --- a/src/gstreamer/gstlibcamerapool.cpp > > +++ b/src/gstreamer/gstlibcamerapool.cpp > > @@ -134,8 +134,20 @@ gst_libcamera_pool_class_init(GstLibcameraPoolClass *klass) > > G_TYPE_NONE, 0); > > } > > > > +static void > > +gst_libcamera_buffer_add_video_meta(GstBuffer *buffer, GstVideoInfo *info) > > +{ > > + GstVideoMeta *vmeta; > > + vmeta = gst_buffer_add_video_meta_full(buffer, GST_VIDEO_FRAME_FLAG_NONE, > > + GST_VIDEO_INFO_FORMAT(info), GST_VIDEO_INFO_WIDTH(info), > > + GST_VIDEO_INFO_HEIGHT(info), GST_VIDEO_INFO_N_PLANES(info), > > + info->offset, info->stride); > > + GST_META_FLAGS(vmeta) = (GstMetaFlags)(GST_META_FLAGS(vmeta) | GST_META_FLAG_POOLED); > > +} > > + > > GstLibcameraPool * > > -gst_libcamera_pool_new(GstLibcameraAllocator *allocator, Stream *stream) > > +gst_libcamera_pool_new(GstLibcameraAllocator *allocator, Stream *stream, > > + GstVideoInfo *info) > > { > > auto *pool = GST_LIBCAMERA_POOL(g_object_new(GST_TYPE_LIBCAMERA_POOL, nullptr)); > > > > @@ -145,6 +157,7 @@ gst_libcamera_pool_new(GstLibcameraAllocator *allocator, Stream *stream) > > gsize pool_size = gst_libcamera_allocator_get_pool_size(allocator, stream); > > for (gsize i = 0; i < pool_size; i++) { > > GstBuffer *buffer = gst_buffer_new(); > > + gst_libcamera_buffer_add_video_meta(buffer, info); > > pool->queue->push_back(buffer); > > } > > > > diff --git a/src/gstreamer/gstlibcamerapool.h b/src/gstreamer/gstlibcamerapool.h > > index 2a7a9c77..02ee4dd4 100644 > > --- a/src/gstreamer/gstlibcamerapool.h > > +++ b/src/gstreamer/gstlibcamerapool.h > > @@ -14,6 +14,7 @@ > > #include "gstlibcameraallocator.h" > > > > #include <gst/gst.h> > > +#include <gst/video/video.h> > > > > #include <libcamera/stream.h> > > > > @@ -21,7 +22,7 @@ > > G_DECLARE_FINAL_TYPE(GstLibcameraPool, gst_libcamera_pool, GST_LIBCAMERA, POOL, GstBufferPool) > > > > GstLibcameraPool *gst_libcamera_pool_new(GstLibcameraAllocator *allocator, > > - libcamera::Stream *stream); > > + libcamera::Stream *stream, GstVideoInfo *info); > > > > libcamera::Stream *gst_libcamera_pool_get_stream(GstLibcameraPool *self); > > > > diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp > > index 5e9e843d..4b81c94e 100644 > > --- a/src/gstreamer/gstlibcamerasrc.cpp > > +++ b/src/gstreamer/gstlibcamerasrc.cpp > > @@ -268,6 +268,58 @@ GstLibcameraSrcState::requestCompleted(Request *request) > > gst_task_resume(src_->task); > > } > > > > +static void > > +gst_libcamera_extrapolate_info(GstVideoInfo *info, guint32 stride) > > +{ > > + guint i, estride; > > + gsize offset = 0; > > + > > + /* this should be updated if tiled formats get added in the future. */ > > + for (i = 0; i < GST_VIDEO_INFO_N_PLANES(info); i++) { > > + estride = gst_video_format_info_extrapolate_stride(info->finfo, i, stride); > > + info->stride[i] = estride; > > + info->offset[i] = offset; > > + offset += estride * GST_VIDEO_FORMAT_INFO_SCALE_HEIGHT(info->finfo, i, > > + GST_VIDEO_INFO_HEIGHT(info)); > > + } > > +} > > + > > +static GstFlowReturn > > +gst_libcamera_video_frame_copy(GstBuffer *src, GstBuffer *dest, const GstVideoInfo *dest_info, guint32 stride) > > +{ > > + GstVideoInfo src_info = *dest_info; > > + GstVideoFrame src_frame, dest_frame; > > + > > + gst_libcamera_extrapolate_info(&src_info, stride); > > + src_info.size = gst_buffer_get_size(src); > > + > > + if (!gst_video_frame_map(&src_frame, &src_info, src, GST_MAP_READ)) { > > + GST_ERROR("Could not map buffer"); > > + goto error; > > + } > > + > > + if (!gst_video_frame_map(&dest_frame, dest_info, dest, GST_MAP_WRITE)) { > > + GST_ERROR("Could not map buffer"); > > + gst_video_frame_unmap(&src_frame); > > + goto error; > > + } > > + > > + if (!gst_video_frame_copy(&dest_frame, &src_frame)) { > > + GST_ERROR("Could not copy frame"); > > + gst_video_frame_unmap(&src_frame); > > + gst_video_frame_unmap(&dest_frame); > > + goto error; > > + } > > + > > + gst_video_frame_unmap(&src_frame); > > + gst_video_frame_unmap(&dest_frame); > > + > > + return GST_FLOW_OK; > > + > > +error: > > + return GST_FLOW_ERROR; > > +} > > + > > /* Must be called with stream_lock held. */ > > int GstLibcameraSrcState::processRequest() > > { > > @@ -292,11 +344,41 @@ int GstLibcameraSrcState::processRequest() > > GstFlowReturn ret = GST_FLOW_OK; > > gst_flow_combiner_reset(src_->flow_combiner); > > > > - for (GstPad *srcpad : srcpads_) { > > + for (gsize i = 0; i < srcpads_.size(); i++) { > > + GstPad *srcpad = srcpads_[i]; > > Stream *stream = gst_libcamera_pad_get_stream(srcpad); > > GstBuffer *buffer = wrap->detachBuffer(stream); > > > > FrameBuffer *fb = gst_libcamera_buffer_get_frame_buffer(buffer); > > + const StreamConfiguration &stream_cfg = config_->at(i); > > + GstBufferPool *video_pool = gst_libcamera_pad_get_video_pool(srcpad); > > + > > + if (video_pool) { > > + /* Only set video pool when a copy is needed */ > > + GstBuffer *copy = NULL; > > + const GstVideoInfo info = gst_libcamera_pad_get_video_info(srcpad); > > + > > + ret = gst_buffer_pool_acquire_buffer(video_pool, ©, NULL); > > + if (ret != GST_FLOW_OK) { > > + gst_buffer_unref(buffer); > > + GST_ELEMENT_ERROR(src_, RESOURCE, SETTINGS, > > + ("Failed to acquire buffer"), > > + ("GstLibcameraSrcState::processRequest() failed: %s", g_strerror(- > > ret))); > > + return -EPIPE; > > + } > > + > > + ret = gst_libcamera_video_frame_copy(buffer, copy, &info, stream_cfg.stride); > > + gst_buffer_unref(buffer); > > + if (ret != GST_FLOW_OK) { > > + gst_buffer_unref(copy); > > + GST_ELEMENT_ERROR(src_, RESOURCE, SETTINGS, > > + ("Failed to copy buffer"), > > + ("GstLibcameraSrcState::processRequest() failed: %s", g_strerror(- > > ret))); > > + return -EPIPE; > > + } > > + > > + buffer = copy; > > + } > > > > if (GST_CLOCK_TIME_IS_VALID(wrap->pts_)) { > > GST_BUFFER_PTS(buffer) = wrap->pts_; > > @@ -499,13 +581,68 @@ 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; > > + GstVideoInfo info; > > + > > + g_autoptr(GstCaps) caps = gst_libcamera_stream_configuration_to_caps(stream_cfg, transfer[i]); > > + > > + gst_video_info_from_caps(&info, caps); > > + gst_libcamera_pad_set_video_info(srcpad, &info); > > + > > + /* 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); > > + } > > + > > + 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); > > + } > > > > GstLibcameraPool *pool = gst_libcamera_pool_new(self->allocator, > > - stream_cfg.stream()); > > + stream_cfg.stream(), &info); > > g_signal_connect_swapped(pool, "buffer-notify", > > G_CALLBACK(gst_task_resume), self->task); > > > > gst_libcamera_pad_set_pool(srcpad, pool); > > + gst_libcamera_pad_set_video_pool(srcpad, video_pool); > > > > /* Clear all reconfigure flags. */ > > gst_pad_check_reconfigure(srcpad); > > @@ -922,6 +1059,12 @@ gst_libcamera_src_release_pad(GstElement *element, GstPad *pad) > > auto end_iterator = pads.end(); > > auto pad_iterator = std::find(begin_iterator, end_iterator, pad); > > > > + GstBufferPool *video_pool = gst_libcamera_pad_get_video_pool(pad); > > + if (video_pool) { > > + gst_buffer_pool_set_active(video_pool, false); > > + gst_object_unref(video_pool); > > + } > > + > > if (pad_iterator != end_iterator) { > > g_object_unref(*pad_iterator); > > pads.erase(pad_iterator);
On Fri, May 16, 2025 at 12:16:50PM +0200, Nicolas Dufresne wrote: > Hi, > > Le vendredi 16 mai 2025 à 16:30 +0900, Hou Qi a écrit : > > GStreamer video-info calculated stride and offset may differ from > > those used by the camera. > > > > For stride and offset mismatch, this patch adds video meta to buffer > > if downstream supports VideoMeta through allocation query. Otherwise, > > create a internal VideoPool using the caps, and copy video frame to > > this system memory. > > > > Signed-off-by: Hou Qi <qi.hou@nxp.com> > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > I'm happy with this version. Unfortunately CI isn't as happy as you :-( https://gitlab.freedesktop.org/camera/libcamera/-/jobs/76508291 ../src/gstreamer/gstlibcamerasrc.cpp: In function ‘GstFlowReturn gst_libcamera_video_frame_copy(GstBuffer*, GstBuffer*, const GstVideoInfo*, guint32)’: ../src/gstreamer/gstlibcamerasrc.cpp:301:40: error: invalid conversion from ‘const GstVideoInfo*’ {aka ‘const _GstVideoInfo*’} to ‘GstVideoInfo*’ {aka ‘_GstVideoInfo*’} [-fpermissive] 301 | if (!gst_video_frame_map(&dest_frame, dest_info, dest, GST_MAP_WRITE)) { | ^~~~~~~~~ | | | const GstVideoInfo* {aka const _GstVideoInfo*} The issue was fixed in GStreamer 1.19.3, in commit abb026ec6ab1617b44de69e9a251317592eee755 Author: Marijn Suijten <marijns95@gmail.com> Date: Mon Jan 4 23:25:10 2021 +0100 gl,video: Make ptrs to VideoInfo and (GL)AllocationParams immutable These parameters are incorrectly regarded as mutable in G-IR making them "incompatible" with languages that are explicit about mutability like Rust. In order to clean up the code and expected API there, update the signatures here, right at the source (instead of overriding them in Gir.toml and hoping for the best). Part-of: <https://gitlab.freedesktop.org/gstreamer/gst-plugins-base/-/merge_requests/1005> that changed the function prototype as follows: GST_VIDEO_API -gboolean gst_video_frame_map (GstVideoFrame *frame, GstVideoInfo *info, +gboolean gst_video_frame_map (GstVideoFrame *frame, const GstVideoInfo *info, GstBuffer *buffer, GstMapFlags flags); libcamera currently requires GStreamer 1.14.0 or newer. We could upgrade to a newer version, but I'd like to support Debian Bullseye until it's end of life, which is scheduled for August 2026, and that comes with GStreamer 1.18. I think we can just const_cast<> the dest_info argument to gst_video_frame_map(), with a comment that explains what's going on: /* * Before v1.19.3, the gst_video_frame_map() function took a non-const * info argument, even if it didn't modify the pointer structure. To * avoid breaking compilation, cast dest_info to a non-const pointer. * This should be removed when dropping support for older GStreamer * versions. */ if (!gst_video_frame_map(&dest_frame, const_cast<GstVideoInfo>(dest_info), dest, GST_MAP_WRITE)) { I could make that change when applying, but I can't easily test it at the moment. Hou, would you mind fixing and testing this, and send a new version ? As a new version is needed, I also have a few cosmetic comments, please see below. There's also one question for Nicolas. > Tested-by: Nicolas Dufresne <nicolas.dufresne@collabora.com> > Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com> > > > --- > > src/gstreamer/gstlibcamera-utils.cpp | 33 ++++++ > > src/gstreamer/gstlibcamera-utils.h | 5 + > > src/gstreamer/gstlibcamerapad.cpp | 31 ++++++ > > src/gstreamer/gstlibcamerapad.h | 8 ++ > > src/gstreamer/gstlibcamerapool.cpp | 15 ++- > > src/gstreamer/gstlibcamerapool.h | 3 +- > > src/gstreamer/gstlibcamerasrc.cpp | 147 ++++++++++++++++++++++++++- > > 7 files changed, 238 insertions(+), 4 deletions(-) > > > > diff --git a/src/gstreamer/gstlibcamera-utils.cpp b/src/gstreamer/gstlibcamera-utils.cpp > > index 2edebba0..bf12cb07 100644 > > --- a/src/gstreamer/gstlibcamera-utils.cpp > > +++ b/src/gstreamer/gstlibcamera-utils.cpp > > @@ -599,6 +599,39 @@ gst_task_resume(GstTask *task) > > } > > #endif > > > > +#if !GST_CHECK_VERSION(1, 22, 0) > > +/* > > + * Copyright (C) <1999> Erik Walthinsen <omega@cse.ogi.edu> > > + * Library <2002> Ronald Bultje <rbultje@ronald.bitfreak.net> > > + * Copyright (C) <2007> David A. Schleef <ds@schleef.org> > > + */ > > +/* This function has been imported directly from the gstreamer project to > > + * support backwards compatibility and should be removed when the older > > + * version is no longer supported. */ Small style issue: /* * This function has been imported directly from the gstreamer project to * support backwards compatibility and should be removed when the older version * is no longer supported. */ Same below where applicable. > > +gint gst_video_format_info_extrapolate_stride(const GstVideoFormatInfo *finfo, gint plane, gint stride) > > +{ > > + gint estride; > > + gint comp[GST_VIDEO_MAX_COMPONENTS]; > > + gint i; > > + > > + /* there is nothing to extrapolate on first plane */ > > + if (plane == 0) > > + return stride; > > + > > + gst_video_format_info_component(finfo, plane, comp); > > + > > + /* For now, all planar formats have a single component on first plane, but > > + * if there was a planar format with more, we'd have to make a ratio of the > > + * number of component on the first plane against the number of component on > > + * the current plane. */ > > + estride = 0; > > + for (i = 0; i < GST_VIDEO_MAX_COMPONENTS && comp[i] >= 0; i++) > > + estride += GST_VIDEO_FORMAT_INFO_SCALE_WIDTH(finfo, comp[i], stride); > > + > > + return estride; > > +} > > +#endif > > + > > G_LOCK_DEFINE_STATIC(cm_singleton_lock); > > static std::weak_ptr<CameraManager> cm_singleton_ptr; > > > > diff --git a/src/gstreamer/gstlibcamera-utils.h b/src/gstreamer/gstlibcamera-utils.h > > index 4978987c..5f4e8a0f 100644 > > --- a/src/gstreamer/gstlibcamera-utils.h > > +++ b/src/gstreamer/gstlibcamera-utils.h > > @@ -36,6 +36,11 @@ static inline void gst_clear_event(GstEvent **event_ptr) > > #if !GST_CHECK_VERSION(1, 17, 1) > > gboolean gst_task_resume(GstTask *task); > > #endif > > + > > +#if !GST_CHECK_VERSION(1, 22, 0) > > +gint gst_video_format_info_extrapolate_stride(const GstVideoFormatInfo *finfo, gint plane, gint stride); > > +#endif > > + > > std::shared_ptr<libcamera::CameraManager> gst_libcamera_get_camera_manager(int &ret); > > > > /** > > diff --git a/src/gstreamer/gstlibcamerapad.cpp b/src/gstreamer/gstlibcamerapad.cpp > > index 7b22aebe..3bc2bc87 100644 > > --- a/src/gstreamer/gstlibcamerapad.cpp > > +++ b/src/gstreamer/gstlibcamerapad.cpp > > @@ -18,6 +18,8 @@ struct _GstLibcameraPad { > > GstPad parent; > > StreamRole role; > > GstLibcameraPool *pool; > > + GstBufferPool *video_pool; > > + GstVideoInfo info; > > GstClockTime latency; > > }; > > > > @@ -153,6 +155,35 @@ gst_libcamera_pad_set_pool(GstPad *pad, GstLibcameraPool *pool) > > self->pool = pool; > > } > > > > +GstBufferPool * > > +gst_libcamera_pad_get_video_pool(GstPad *pad) > > +{ > > + auto *self = GST_LIBCAMERA_PAD(pad); > > + return self->video_pool; > > +} > > + > > +void gst_libcamera_pad_set_video_pool(GstPad *pad, GstBufferPool *video_pool) > > +{ > > + auto *self = GST_LIBCAMERA_PAD(pad); > > + > > + if (self->video_pool) > > + g_object_unref(self->video_pool); > > + self->video_pool = video_pool; > > +} > > + > > +GstVideoInfo gst_libcamera_pad_get_video_info(GstPad *pad) > > +{ > > + auto *self = GST_LIBCAMERA_PAD(pad); > > + return self->info; > > +} > > + > > +void gst_libcamera_pad_set_video_info(GstPad *pad, const GstVideoInfo *info) > > +{ > > + auto *self = GST_LIBCAMERA_PAD(pad); > > + > > + self->info = *info; > > +} > > + > > Stream * > > gst_libcamera_pad_get_stream(GstPad *pad) > > { > > diff --git a/src/gstreamer/gstlibcamerapad.h b/src/gstreamer/gstlibcamerapad.h > > index 630c168a..f98b8a7f 100644 > > --- a/src/gstreamer/gstlibcamerapad.h > > +++ b/src/gstreamer/gstlibcamerapad.h > > @@ -23,6 +23,14 @@ GstLibcameraPool *gst_libcamera_pad_get_pool(GstPad *pad); > > > > void gst_libcamera_pad_set_pool(GstPad *pad, GstLibcameraPool *pool); > > > > +GstBufferPool *gst_libcamera_pad_get_video_pool(GstPad *pad); > > + > > +void gst_libcamera_pad_set_video_pool(GstPad *pad, GstBufferPool *video_pool); > > + > > +GstVideoInfo gst_libcamera_pad_get_video_info(GstPad *pad); > > + > > +void gst_libcamera_pad_set_video_info(GstPad *pad, const GstVideoInfo *info); > > + > > libcamera::Stream *gst_libcamera_pad_get_stream(GstPad *pad); > > > > void gst_libcamera_pad_set_latency(GstPad *pad, GstClockTime latency); > > diff --git a/src/gstreamer/gstlibcamerapool.cpp b/src/gstreamer/gstlibcamerapool.cpp > > index 9cd7eccb..8278144f 100644 > > --- a/src/gstreamer/gstlibcamerapool.cpp > > +++ b/src/gstreamer/gstlibcamerapool.cpp > > @@ -134,8 +134,20 @@ gst_libcamera_pool_class_init(GstLibcameraPoolClass *klass) > > G_TYPE_NONE, 0); > > } > > > > +static void > > +gst_libcamera_buffer_add_video_meta(GstBuffer *buffer, GstVideoInfo *info) > > +{ > > + GstVideoMeta *vmeta; > > + vmeta = gst_buffer_add_video_meta_full(buffer, GST_VIDEO_FRAME_FLAG_NONE, > > + GST_VIDEO_INFO_FORMAT(info), GST_VIDEO_INFO_WIDTH(info), > > + GST_VIDEO_INFO_HEIGHT(info), GST_VIDEO_INFO_N_PLANES(info), > > + info->offset, info->stride); > > + GST_META_FLAGS(vmeta) = (GstMetaFlags)(GST_META_FLAGS(vmeta) | GST_META_FLAG_POOLED); > > +} > > + > > GstLibcameraPool * > > -gst_libcamera_pool_new(GstLibcameraAllocator *allocator, Stream *stream) > > +gst_libcamera_pool_new(GstLibcameraAllocator *allocator, Stream *stream, > > + GstVideoInfo *info) > > { > > auto *pool = GST_LIBCAMERA_POOL(g_object_new(GST_TYPE_LIBCAMERA_POOL, nullptr)); > > > > @@ -145,6 +157,7 @@ gst_libcamera_pool_new(GstLibcameraAllocator *allocator, Stream *stream) > > gsize pool_size = gst_libcamera_allocator_get_pool_size(allocator, stream); > > for (gsize i = 0; i < pool_size; i++) { > > GstBuffer *buffer = gst_buffer_new(); > > + gst_libcamera_buffer_add_video_meta(buffer, info); > > pool->queue->push_back(buffer); > > } > > > > diff --git a/src/gstreamer/gstlibcamerapool.h b/src/gstreamer/gstlibcamerapool.h > > index 2a7a9c77..02ee4dd4 100644 > > --- a/src/gstreamer/gstlibcamerapool.h > > +++ b/src/gstreamer/gstlibcamerapool.h > > @@ -14,6 +14,7 @@ > > #include "gstlibcameraallocator.h" > > > > #include <gst/gst.h> > > +#include <gst/video/video.h> > > > > #include <libcamera/stream.h> > > > > @@ -21,7 +22,7 @@ > > G_DECLARE_FINAL_TYPE(GstLibcameraPool, gst_libcamera_pool, GST_LIBCAMERA, POOL, GstBufferPool) > > > > GstLibcameraPool *gst_libcamera_pool_new(GstLibcameraAllocator *allocator, > > - libcamera::Stream *stream); > > + libcamera::Stream *stream, GstVideoInfo *info); > > > > libcamera::Stream *gst_libcamera_pool_get_stream(GstLibcameraPool *self); > > > > diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp > > index 5e9e843d..4b81c94e 100644 > > --- a/src/gstreamer/gstlibcamerasrc.cpp > > +++ b/src/gstreamer/gstlibcamerasrc.cpp > > @@ -268,6 +268,58 @@ GstLibcameraSrcState::requestCompleted(Request *request) > > gst_task_resume(src_->task); > > } > > > > +static void > > +gst_libcamera_extrapolate_info(GstVideoInfo *info, guint32 stride) > > +{ > > + guint i, estride; > > + gsize offset = 0; > > + > > + /* this should be updated if tiled formats get added in the future. */ Sentences should start with a capital letter, and end with a period: /* This should be updated if tiled formats get added in the future. */ > > + for (i = 0; i < GST_VIDEO_INFO_N_PLANES(info); i++) { > > + estride = gst_video_format_info_extrapolate_stride(info->finfo, i, stride); > > + info->stride[i] = estride; > > + info->offset[i] = offset; > > + offset += estride * GST_VIDEO_FORMAT_INFO_SCALE_HEIGHT(info->finfo, i, > > + GST_VIDEO_INFO_HEIGHT(info)); > > + } > > +} > > + > > +static GstFlowReturn > > +gst_libcamera_video_frame_copy(GstBuffer *src, GstBuffer *dest, const GstVideoInfo *dest_info, guint32 stride) > > +{ > > + GstVideoInfo src_info = *dest_info; > > + GstVideoFrame src_frame, dest_frame; > > + > > + gst_libcamera_extrapolate_info(&src_info, stride); > > + src_info.size = gst_buffer_get_size(src); > > + > > + if (!gst_video_frame_map(&src_frame, &src_info, src, GST_MAP_READ)) { > > + GST_ERROR("Could not map buffer"); > > + goto error; You can just return GST_FLOW_ERROR; here. Same below, and drop the "error:" label. > > + } > > + > > + if (!gst_video_frame_map(&dest_frame, dest_info, dest, GST_MAP_WRITE)) { > > + GST_ERROR("Could not map buffer"); > > + gst_video_frame_unmap(&src_frame); > > + goto error; > > + } > > + > > + if (!gst_video_frame_copy(&dest_frame, &src_frame)) { > > + GST_ERROR("Could not copy frame"); > > + gst_video_frame_unmap(&src_frame); > > + gst_video_frame_unmap(&dest_frame); > > + goto error; > > + } > > + > > + gst_video_frame_unmap(&src_frame); > > + gst_video_frame_unmap(&dest_frame); > > + > > + return GST_FLOW_OK; > > + > > +error: > > + return GST_FLOW_ERROR; > > +} > > + > > /* Must be called with stream_lock held. */ > > int GstLibcameraSrcState::processRequest() > > { > > @@ -292,11 +344,41 @@ int GstLibcameraSrcState::processRequest() > > GstFlowReturn ret = GST_FLOW_OK; > > gst_flow_combiner_reset(src_->flow_combiner); > > > > - for (GstPad *srcpad : srcpads_) { > > + for (gsize i = 0; i < srcpads_.size(); i++) { > > + GstPad *srcpad = srcpads_[i]; > > Stream *stream = gst_libcamera_pad_get_stream(srcpad); > > GstBuffer *buffer = wrap->detachBuffer(stream); > > > > FrameBuffer *fb = gst_libcamera_buffer_get_frame_buffer(buffer); > > + const StreamConfiguration &stream_cfg = config_->at(i); > > + GstBufferPool *video_pool = gst_libcamera_pad_get_video_pool(srcpad); > > + > > + if (video_pool) { > > + /* Only set video pool when a copy is needed */ > > + GstBuffer *copy = NULL; > > + const GstVideoInfo info = gst_libcamera_pad_get_video_info(srcpad); > > + > > + ret = gst_buffer_pool_acquire_buffer(video_pool, ©, NULL); > > + if (ret != GST_FLOW_OK) { > > + gst_buffer_unref(buffer); > > + GST_ELEMENT_ERROR(src_, RESOURCE, SETTINGS, > > + ("Failed to acquire buffer"), > > + ("GstLibcameraSrcState::processRequest() failed: %s", g_strerror(- > > ret))); > > + return -EPIPE; > > + } > > + > > + ret = gst_libcamera_video_frame_copy(buffer, copy, &info, stream_cfg.stride); > > + gst_buffer_unref(buffer); > > + if (ret != GST_FLOW_OK) { > > + gst_buffer_unref(copy); > > + GST_ELEMENT_ERROR(src_, RESOURCE, SETTINGS, > > + ("Failed to copy buffer"), > > + ("GstLibcameraSrcState::processRequest() failed: %s", g_strerror(- > > ret))); > > + return -EPIPE; > > + } > > + > > + buffer = copy; > > + } > > > > if (GST_CLOCK_TIME_IS_VALID(wrap->pts_)) { > > GST_BUFFER_PTS(buffer) = wrap->pts_; > > @@ -499,13 +581,68 @@ 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; > > + GstVideoInfo info; > > + > > + g_autoptr(GstCaps) caps = gst_libcamera_stream_configuration_to_caps(stream_cfg, transfer[i]); > > + > > + gst_video_info_from_caps(&info, caps); > > + gst_libcamera_pad_set_video_info(srcpad, &info); > > + > > + /* 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) { Nicolas, as copying frames is very expensive, should we log a message somewhere to warn the user ? I'm concerned we'll get bug reports for bad performance, and I'd like a way to know that copies are happening. > > + 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); > > + } > > + > > + 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); > > + } > > > > GstLibcameraPool *pool = gst_libcamera_pool_new(self->allocator, > > - stream_cfg.stream()); > > + stream_cfg.stream(), &info); > > g_signal_connect_swapped(pool, "buffer-notify", > > G_CALLBACK(gst_task_resume), self->task); > > > > gst_libcamera_pad_set_pool(srcpad, pool); > > + gst_libcamera_pad_set_video_pool(srcpad, video_pool); > > > > /* Clear all reconfigure flags. */ > > gst_pad_check_reconfigure(srcpad); > > @@ -922,6 +1059,12 @@ gst_libcamera_src_release_pad(GstElement *element, GstPad *pad) > > auto end_iterator = pads.end(); > > auto pad_iterator = std::find(begin_iterator, end_iterator, pad); > > > > + GstBufferPool *video_pool = gst_libcamera_pad_get_video_pool(pad); > > + if (video_pool) { > > + gst_buffer_pool_set_active(video_pool, false); > > + gst_object_unref(video_pool); > > + } > > + > > if (pad_iterator != end_iterator) { > > g_object_unref(*pad_iterator); > > pads.erase(pad_iterator);
On Fri, May 16, 2025 at 02:54:27PM +0200, Laurent Pinchart wrote: > On Fri, May 16, 2025 at 12:16:50PM +0200, Nicolas Dufresne wrote: > > Hi, > > > > Le vendredi 16 mai 2025 à 16:30 +0900, Hou Qi a écrit : > > > GStreamer video-info calculated stride and offset may differ from > > > those used by the camera. > > > > > > For stride and offset mismatch, this patch adds video meta to buffer > > > if downstream supports VideoMeta through allocation query. Otherwise, > > > create a internal VideoPool using the caps, and copy video frame to > > > this system memory. > > > > > > Signed-off-by: Hou Qi <qi.hou@nxp.com> > > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > > I'm happy with this version. > > Unfortunately CI isn't as happy as you :-( > > https://gitlab.freedesktop.org/camera/libcamera/-/jobs/76508291 > > ../src/gstreamer/gstlibcamerasrc.cpp: In function ‘GstFlowReturn gst_libcamera_video_frame_copy(GstBuffer*, GstBuffer*, const GstVideoInfo*, guint32)’: > ../src/gstreamer/gstlibcamerasrc.cpp:301:40: error: invalid conversion from ‘const GstVideoInfo*’ {aka ‘const _GstVideoInfo*’} to ‘GstVideoInfo*’ {aka ‘_GstVideoInfo*’} [-fpermissive] > 301 | if (!gst_video_frame_map(&dest_frame, dest_info, dest, GST_MAP_WRITE)) { > | ^~~~~~~~~ > | | > | const GstVideoInfo* {aka const _GstVideoInfo*} > > The issue was fixed in GStreamer 1.19.3, in > > commit abb026ec6ab1617b44de69e9a251317592eee755 > Author: Marijn Suijten <marijns95@gmail.com> > Date: Mon Jan 4 23:25:10 2021 +0100 > > gl,video: Make ptrs to VideoInfo and (GL)AllocationParams immutable > > These parameters are incorrectly regarded as mutable in G-IR making them > "incompatible" with languages that are explicit about mutability like > Rust. In order to clean up the code and expected API there, update the > signatures here, right at the source (instead of overriding them in > Gir.toml and hoping for the best). > > Part-of: <https://gitlab.freedesktop.org/gstreamer/gst-plugins-base/-/merge_requests/1005> > > that changed the function prototype as follows: > > GST_VIDEO_API > -gboolean gst_video_frame_map (GstVideoFrame *frame, GstVideoInfo *info, > +gboolean gst_video_frame_map (GstVideoFrame *frame, const GstVideoInfo *info, > GstBuffer *buffer, GstMapFlags flags); > > libcamera currently requires GStreamer 1.14.0 or newer. We could upgrade > to a newer version, but I'd like to support Debian Bullseye until it's > end of life, which is scheduled for August 2026, and that comes with > GStreamer 1.18. > > I think we can just const_cast<> the dest_info argument to > gst_video_frame_map(), with a comment that explains what's going on: > > /* > * Before v1.19.3, the gst_video_frame_map() function took a non-const > * info argument, even if it didn't modify the pointer structure. To > * avoid breaking compilation, cast dest_info to a non-const pointer. > * This should be removed when dropping support for older GStreamer > * versions. > */ > if (!gst_video_frame_map(&dest_frame, const_cast<GstVideoInfo>(dest_info), > dest, GST_MAP_WRITE)) { > > I could make that change when applying, but I can't easily test it at > the moment. Hou, would you mind fixing and testing this, and send a new > version ? > > As a new version is needed, I also have a few cosmetic comments, please > see below. There's also one question for Nicolas. Also, please look at the CI linter warnings from https://gitlab.freedesktop.org/camera/libcamera/-/jobs/76508306 and fix issues if they're not false positives. > > Tested-by: Nicolas Dufresne <nicolas.dufresne@collabora.com> > > Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com> > > > > > --- > > > src/gstreamer/gstlibcamera-utils.cpp | 33 ++++++ > > > src/gstreamer/gstlibcamera-utils.h | 5 + > > > src/gstreamer/gstlibcamerapad.cpp | 31 ++++++ > > > src/gstreamer/gstlibcamerapad.h | 8 ++ > > > src/gstreamer/gstlibcamerapool.cpp | 15 ++- > > > src/gstreamer/gstlibcamerapool.h | 3 +- > > > src/gstreamer/gstlibcamerasrc.cpp | 147 ++++++++++++++++++++++++++- > > > 7 files changed, 238 insertions(+), 4 deletions(-) > > > > > > diff --git a/src/gstreamer/gstlibcamera-utils.cpp b/src/gstreamer/gstlibcamera-utils.cpp > > > index 2edebba0..bf12cb07 100644 > > > --- a/src/gstreamer/gstlibcamera-utils.cpp > > > +++ b/src/gstreamer/gstlibcamera-utils.cpp > > > @@ -599,6 +599,39 @@ gst_task_resume(GstTask *task) > > > } > > > #endif > > > > > > +#if !GST_CHECK_VERSION(1, 22, 0) > > > +/* > > > + * Copyright (C) <1999> Erik Walthinsen <omega@cse.ogi.edu> > > > + * Library <2002> Ronald Bultje <rbultje@ronald.bitfreak.net> > > > + * Copyright (C) <2007> David A. Schleef <ds@schleef.org> > > > + */ > > > +/* This function has been imported directly from the gstreamer project to > > > + * support backwards compatibility and should be removed when the older > > > + * version is no longer supported. */ > > Small style issue: > > /* > * This function has been imported directly from the gstreamer project to > * support backwards compatibility and should be removed when the older version > * is no longer supported. > */ > > Same below where applicable. > > > > +gint gst_video_format_info_extrapolate_stride(const GstVideoFormatInfo *finfo, gint plane, gint stride) > > > +{ > > > + gint estride; > > > + gint comp[GST_VIDEO_MAX_COMPONENTS]; > > > + gint i; > > > + > > > + /* there is nothing to extrapolate on first plane */ > > > + if (plane == 0) > > > + return stride; > > > + > > > + gst_video_format_info_component(finfo, plane, comp); > > > + > > > + /* For now, all planar formats have a single component on first plane, but > > > + * if there was a planar format with more, we'd have to make a ratio of the > > > + * number of component on the first plane against the number of component on > > > + * the current plane. */ > > > + estride = 0; > > > + for (i = 0; i < GST_VIDEO_MAX_COMPONENTS && comp[i] >= 0; i++) > > > + estride += GST_VIDEO_FORMAT_INFO_SCALE_WIDTH(finfo, comp[i], stride); > > > + > > > + return estride; > > > +} > > > +#endif > > > + > > > G_LOCK_DEFINE_STATIC(cm_singleton_lock); > > > static std::weak_ptr<CameraManager> cm_singleton_ptr; > > > > > > diff --git a/src/gstreamer/gstlibcamera-utils.h b/src/gstreamer/gstlibcamera-utils.h > > > index 4978987c..5f4e8a0f 100644 > > > --- a/src/gstreamer/gstlibcamera-utils.h > > > +++ b/src/gstreamer/gstlibcamera-utils.h > > > @@ -36,6 +36,11 @@ static inline void gst_clear_event(GstEvent **event_ptr) > > > #if !GST_CHECK_VERSION(1, 17, 1) > > > gboolean gst_task_resume(GstTask *task); > > > #endif > > > + > > > +#if !GST_CHECK_VERSION(1, 22, 0) > > > +gint gst_video_format_info_extrapolate_stride(const GstVideoFormatInfo *finfo, gint plane, gint stride); > > > +#endif > > > + > > > std::shared_ptr<libcamera::CameraManager> gst_libcamera_get_camera_manager(int &ret); > > > > > > /** > > > diff --git a/src/gstreamer/gstlibcamerapad.cpp b/src/gstreamer/gstlibcamerapad.cpp > > > index 7b22aebe..3bc2bc87 100644 > > > --- a/src/gstreamer/gstlibcamerapad.cpp > > > +++ b/src/gstreamer/gstlibcamerapad.cpp > > > @@ -18,6 +18,8 @@ struct _GstLibcameraPad { > > > GstPad parent; > > > StreamRole role; > > > GstLibcameraPool *pool; > > > + GstBufferPool *video_pool; > > > + GstVideoInfo info; > > > GstClockTime latency; > > > }; > > > > > > @@ -153,6 +155,35 @@ gst_libcamera_pad_set_pool(GstPad *pad, GstLibcameraPool *pool) > > > self->pool = pool; > > > } > > > > > > +GstBufferPool * > > > +gst_libcamera_pad_get_video_pool(GstPad *pad) > > > +{ > > > + auto *self = GST_LIBCAMERA_PAD(pad); > > > + return self->video_pool; > > > +} > > > + > > > +void gst_libcamera_pad_set_video_pool(GstPad *pad, GstBufferPool *video_pool) > > > +{ > > > + auto *self = GST_LIBCAMERA_PAD(pad); > > > + > > > + if (self->video_pool) > > > + g_object_unref(self->video_pool); > > > + self->video_pool = video_pool; > > > +} > > > + > > > +GstVideoInfo gst_libcamera_pad_get_video_info(GstPad *pad) > > > +{ > > > + auto *self = GST_LIBCAMERA_PAD(pad); > > > + return self->info; > > > +} > > > + > > > +void gst_libcamera_pad_set_video_info(GstPad *pad, const GstVideoInfo *info) > > > +{ > > > + auto *self = GST_LIBCAMERA_PAD(pad); > > > + > > > + self->info = *info; > > > +} > > > + > > > Stream * > > > gst_libcamera_pad_get_stream(GstPad *pad) > > > { > > > diff --git a/src/gstreamer/gstlibcamerapad.h b/src/gstreamer/gstlibcamerapad.h > > > index 630c168a..f98b8a7f 100644 > > > --- a/src/gstreamer/gstlibcamerapad.h > > > +++ b/src/gstreamer/gstlibcamerapad.h > > > @@ -23,6 +23,14 @@ GstLibcameraPool *gst_libcamera_pad_get_pool(GstPad *pad); > > > > > > void gst_libcamera_pad_set_pool(GstPad *pad, GstLibcameraPool *pool); > > > > > > +GstBufferPool *gst_libcamera_pad_get_video_pool(GstPad *pad); > > > + > > > +void gst_libcamera_pad_set_video_pool(GstPad *pad, GstBufferPool *video_pool); > > > + > > > +GstVideoInfo gst_libcamera_pad_get_video_info(GstPad *pad); > > > + > > > +void gst_libcamera_pad_set_video_info(GstPad *pad, const GstVideoInfo *info); > > > + > > > libcamera::Stream *gst_libcamera_pad_get_stream(GstPad *pad); > > > > > > void gst_libcamera_pad_set_latency(GstPad *pad, GstClockTime latency); > > > diff --git a/src/gstreamer/gstlibcamerapool.cpp b/src/gstreamer/gstlibcamerapool.cpp > > > index 9cd7eccb..8278144f 100644 > > > --- a/src/gstreamer/gstlibcamerapool.cpp > > > +++ b/src/gstreamer/gstlibcamerapool.cpp > > > @@ -134,8 +134,20 @@ gst_libcamera_pool_class_init(GstLibcameraPoolClass *klass) > > > G_TYPE_NONE, 0); > > > } > > > > > > +static void > > > +gst_libcamera_buffer_add_video_meta(GstBuffer *buffer, GstVideoInfo *info) > > > +{ > > > + GstVideoMeta *vmeta; > > > + vmeta = gst_buffer_add_video_meta_full(buffer, GST_VIDEO_FRAME_FLAG_NONE, > > > + GST_VIDEO_INFO_FORMAT(info), GST_VIDEO_INFO_WIDTH(info), > > > + GST_VIDEO_INFO_HEIGHT(info), GST_VIDEO_INFO_N_PLANES(info), > > > + info->offset, info->stride); > > > + GST_META_FLAGS(vmeta) = (GstMetaFlags)(GST_META_FLAGS(vmeta) | GST_META_FLAG_POOLED); > > > +} > > > + > > > GstLibcameraPool * > > > -gst_libcamera_pool_new(GstLibcameraAllocator *allocator, Stream *stream) > > > +gst_libcamera_pool_new(GstLibcameraAllocator *allocator, Stream *stream, > > > + GstVideoInfo *info) > > > { > > > auto *pool = GST_LIBCAMERA_POOL(g_object_new(GST_TYPE_LIBCAMERA_POOL, nullptr)); > > > > > > @@ -145,6 +157,7 @@ gst_libcamera_pool_new(GstLibcameraAllocator *allocator, Stream *stream) > > > gsize pool_size = gst_libcamera_allocator_get_pool_size(allocator, stream); > > > for (gsize i = 0; i < pool_size; i++) { > > > GstBuffer *buffer = gst_buffer_new(); > > > + gst_libcamera_buffer_add_video_meta(buffer, info); > > > pool->queue->push_back(buffer); > > > } > > > > > > diff --git a/src/gstreamer/gstlibcamerapool.h b/src/gstreamer/gstlibcamerapool.h > > > index 2a7a9c77..02ee4dd4 100644 > > > --- a/src/gstreamer/gstlibcamerapool.h > > > +++ b/src/gstreamer/gstlibcamerapool.h > > > @@ -14,6 +14,7 @@ > > > #include "gstlibcameraallocator.h" > > > > > > #include <gst/gst.h> > > > +#include <gst/video/video.h> > > > > > > #include <libcamera/stream.h> > > > > > > @@ -21,7 +22,7 @@ > > > G_DECLARE_FINAL_TYPE(GstLibcameraPool, gst_libcamera_pool, GST_LIBCAMERA, POOL, GstBufferPool) > > > > > > GstLibcameraPool *gst_libcamera_pool_new(GstLibcameraAllocator *allocator, > > > - libcamera::Stream *stream); > > > + libcamera::Stream *stream, GstVideoInfo *info); > > > > > > libcamera::Stream *gst_libcamera_pool_get_stream(GstLibcameraPool *self); > > > > > > diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp > > > index 5e9e843d..4b81c94e 100644 > > > --- a/src/gstreamer/gstlibcamerasrc.cpp > > > +++ b/src/gstreamer/gstlibcamerasrc.cpp > > > @@ -268,6 +268,58 @@ GstLibcameraSrcState::requestCompleted(Request *request) > > > gst_task_resume(src_->task); > > > } > > > > > > +static void > > > +gst_libcamera_extrapolate_info(GstVideoInfo *info, guint32 stride) > > > +{ > > > + guint i, estride; > > > + gsize offset = 0; > > > + > > > + /* this should be updated if tiled formats get added in the future. */ > > Sentences should start with a capital letter, and end with a period: > > /* This should be updated if tiled formats get added in the future. */ > > > > + for (i = 0; i < GST_VIDEO_INFO_N_PLANES(info); i++) { > > > + estride = gst_video_format_info_extrapolate_stride(info->finfo, i, stride); > > > + info->stride[i] = estride; > > > + info->offset[i] = offset; > > > + offset += estride * GST_VIDEO_FORMAT_INFO_SCALE_HEIGHT(info->finfo, i, > > > + GST_VIDEO_INFO_HEIGHT(info)); > > > + } > > > +} > > > + > > > +static GstFlowReturn > > > +gst_libcamera_video_frame_copy(GstBuffer *src, GstBuffer *dest, const GstVideoInfo *dest_info, guint32 stride) > > > +{ > > > + GstVideoInfo src_info = *dest_info; > > > + GstVideoFrame src_frame, dest_frame; > > > + > > > + gst_libcamera_extrapolate_info(&src_info, stride); > > > + src_info.size = gst_buffer_get_size(src); > > > + > > > + if (!gst_video_frame_map(&src_frame, &src_info, src, GST_MAP_READ)) { > > > + GST_ERROR("Could not map buffer"); > > > + goto error; > > You can just > > return GST_FLOW_ERROR; > > here. Same below, and drop the "error:" label. > > > > + } > > > + > > > + if (!gst_video_frame_map(&dest_frame, dest_info, dest, GST_MAP_WRITE)) { > > > + GST_ERROR("Could not map buffer"); > > > + gst_video_frame_unmap(&src_frame); > > > + goto error; > > > + } > > > + > > > + if (!gst_video_frame_copy(&dest_frame, &src_frame)) { > > > + GST_ERROR("Could not copy frame"); > > > + gst_video_frame_unmap(&src_frame); > > > + gst_video_frame_unmap(&dest_frame); > > > + goto error; > > > + } > > > + > > > + gst_video_frame_unmap(&src_frame); > > > + gst_video_frame_unmap(&dest_frame); > > > + > > > + return GST_FLOW_OK; > > > + > > > +error: > > > + return GST_FLOW_ERROR; > > > +} > > > + > > > /* Must be called with stream_lock held. */ > > > int GstLibcameraSrcState::processRequest() > > > { > > > @@ -292,11 +344,41 @@ int GstLibcameraSrcState::processRequest() > > > GstFlowReturn ret = GST_FLOW_OK; > > > gst_flow_combiner_reset(src_->flow_combiner); > > > > > > - for (GstPad *srcpad : srcpads_) { > > > + for (gsize i = 0; i < srcpads_.size(); i++) { > > > + GstPad *srcpad = srcpads_[i]; > > > Stream *stream = gst_libcamera_pad_get_stream(srcpad); > > > GstBuffer *buffer = wrap->detachBuffer(stream); > > > > > > FrameBuffer *fb = gst_libcamera_buffer_get_frame_buffer(buffer); > > > + const StreamConfiguration &stream_cfg = config_->at(i); > > > + GstBufferPool *video_pool = gst_libcamera_pad_get_video_pool(srcpad); > > > + > > > + if (video_pool) { > > > + /* Only set video pool when a copy is needed */ > > > + GstBuffer *copy = NULL; > > > + const GstVideoInfo info = gst_libcamera_pad_get_video_info(srcpad); > > > + > > > + ret = gst_buffer_pool_acquire_buffer(video_pool, ©, NULL); > > > + if (ret != GST_FLOW_OK) { > > > + gst_buffer_unref(buffer); > > > + GST_ELEMENT_ERROR(src_, RESOURCE, SETTINGS, > > > + ("Failed to acquire buffer"), > > > + ("GstLibcameraSrcState::processRequest() failed: %s", g_strerror(- > > > ret))); > > > + return -EPIPE; > > > + } > > > + > > > + ret = gst_libcamera_video_frame_copy(buffer, copy, &info, stream_cfg.stride); > > > + gst_buffer_unref(buffer); > > > + if (ret != GST_FLOW_OK) { > > > + gst_buffer_unref(copy); > > > + GST_ELEMENT_ERROR(src_, RESOURCE, SETTINGS, > > > + ("Failed to copy buffer"), > > > + ("GstLibcameraSrcState::processRequest() failed: %s", g_strerror(- > > > ret))); > > > + return -EPIPE; > > > + } > > > + > > > + buffer = copy; > > > + } > > > > > > if (GST_CLOCK_TIME_IS_VALID(wrap->pts_)) { > > > GST_BUFFER_PTS(buffer) = wrap->pts_; > > > @@ -499,13 +581,68 @@ 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; > > > + GstVideoInfo info; > > > + > > > + g_autoptr(GstCaps) caps = gst_libcamera_stream_configuration_to_caps(stream_cfg, transfer[i]); > > > + > > > + gst_video_info_from_caps(&info, caps); > > > + gst_libcamera_pad_set_video_info(srcpad, &info); > > > + > > > + /* 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) { > > Nicolas, as copying frames is very expensive, should we log a message > somewhere to warn the user ? I'm concerned we'll get bug reports for bad > performance, and I'd like a way to know that copies are happening. > > > > + 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); > > > + } > > > + > > > + 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); > > > + } > > > > > > GstLibcameraPool *pool = gst_libcamera_pool_new(self->allocator, > > > - stream_cfg.stream()); > > > + stream_cfg.stream(), &info); > > > g_signal_connect_swapped(pool, "buffer-notify", > > > G_CALLBACK(gst_task_resume), self->task); > > > > > > gst_libcamera_pad_set_pool(srcpad, pool); > > > + gst_libcamera_pad_set_video_pool(srcpad, video_pool); > > > > > > /* Clear all reconfigure flags. */ > > > gst_pad_check_reconfigure(srcpad); > > > @@ -922,6 +1059,12 @@ gst_libcamera_src_release_pad(GstElement *element, GstPad *pad) > > > auto end_iterator = pads.end(); > > > auto pad_iterator = std::find(begin_iterator, end_iterator, pad); > > > > > > + GstBufferPool *video_pool = gst_libcamera_pad_get_video_pool(pad); > > > + if (video_pool) { > > > + gst_buffer_pool_set_active(video_pool, false); > > > + gst_object_unref(video_pool); > > > + } > > > + > > > if (pad_iterator != end_iterator) { > > > g_object_unref(*pad_iterator); > > > pads.erase(pad_iterator);
diff --git a/src/gstreamer/gstlibcamera-utils.cpp b/src/gstreamer/gstlibcamera-utils.cpp index 2edebba0..bf12cb07 100644 --- a/src/gstreamer/gstlibcamera-utils.cpp +++ b/src/gstreamer/gstlibcamera-utils.cpp @@ -599,6 +599,39 @@ gst_task_resume(GstTask *task) } #endif +#if !GST_CHECK_VERSION(1, 22, 0) +/* + * Copyright (C) <1999> Erik Walthinsen <omega@cse.ogi.edu> + * Library <2002> Ronald Bultje <rbultje@ronald.bitfreak.net> + * Copyright (C) <2007> David A. Schleef <ds@schleef.org> + */ +/* This function has been imported directly from the gstreamer project to + * support backwards compatibility and should be removed when the older + * version is no longer supported. */ +gint gst_video_format_info_extrapolate_stride(const GstVideoFormatInfo *finfo, gint plane, gint stride) +{ + gint estride; + gint comp[GST_VIDEO_MAX_COMPONENTS]; + gint i; + + /* there is nothing to extrapolate on first plane */ + if (plane == 0) + return stride; + + gst_video_format_info_component(finfo, plane, comp); + + /* For now, all planar formats have a single component on first plane, but + * if there was a planar format with more, we'd have to make a ratio of the + * number of component on the first plane against the number of component on + * the current plane. */ + estride = 0; + for (i = 0; i < GST_VIDEO_MAX_COMPONENTS && comp[i] >= 0; i++) + estride += GST_VIDEO_FORMAT_INFO_SCALE_WIDTH(finfo, comp[i], stride); + + return estride; +} +#endif + G_LOCK_DEFINE_STATIC(cm_singleton_lock); static std::weak_ptr<CameraManager> cm_singleton_ptr; diff --git a/src/gstreamer/gstlibcamera-utils.h b/src/gstreamer/gstlibcamera-utils.h index 4978987c..5f4e8a0f 100644 --- a/src/gstreamer/gstlibcamera-utils.h +++ b/src/gstreamer/gstlibcamera-utils.h @@ -36,6 +36,11 @@ static inline void gst_clear_event(GstEvent **event_ptr) #if !GST_CHECK_VERSION(1, 17, 1) gboolean gst_task_resume(GstTask *task); #endif + +#if !GST_CHECK_VERSION(1, 22, 0) +gint gst_video_format_info_extrapolate_stride(const GstVideoFormatInfo *finfo, gint plane, gint stride); +#endif + std::shared_ptr<libcamera::CameraManager> gst_libcamera_get_camera_manager(int &ret); /** diff --git a/src/gstreamer/gstlibcamerapad.cpp b/src/gstreamer/gstlibcamerapad.cpp index 7b22aebe..3bc2bc87 100644 --- a/src/gstreamer/gstlibcamerapad.cpp +++ b/src/gstreamer/gstlibcamerapad.cpp @@ -18,6 +18,8 @@ struct _GstLibcameraPad { GstPad parent; StreamRole role; GstLibcameraPool *pool; + GstBufferPool *video_pool; + GstVideoInfo info; GstClockTime latency; }; @@ -153,6 +155,35 @@ gst_libcamera_pad_set_pool(GstPad *pad, GstLibcameraPool *pool) self->pool = pool; } +GstBufferPool * +gst_libcamera_pad_get_video_pool(GstPad *pad) +{ + auto *self = GST_LIBCAMERA_PAD(pad); + return self->video_pool; +} + +void gst_libcamera_pad_set_video_pool(GstPad *pad, GstBufferPool *video_pool) +{ + auto *self = GST_LIBCAMERA_PAD(pad); + + if (self->video_pool) + g_object_unref(self->video_pool); + self->video_pool = video_pool; +} + +GstVideoInfo gst_libcamera_pad_get_video_info(GstPad *pad) +{ + auto *self = GST_LIBCAMERA_PAD(pad); + return self->info; +} + +void gst_libcamera_pad_set_video_info(GstPad *pad, const GstVideoInfo *info) +{ + auto *self = GST_LIBCAMERA_PAD(pad); + + self->info = *info; +} + Stream * gst_libcamera_pad_get_stream(GstPad *pad) { diff --git a/src/gstreamer/gstlibcamerapad.h b/src/gstreamer/gstlibcamerapad.h index 630c168a..f98b8a7f 100644 --- a/src/gstreamer/gstlibcamerapad.h +++ b/src/gstreamer/gstlibcamerapad.h @@ -23,6 +23,14 @@ GstLibcameraPool *gst_libcamera_pad_get_pool(GstPad *pad); void gst_libcamera_pad_set_pool(GstPad *pad, GstLibcameraPool *pool); +GstBufferPool *gst_libcamera_pad_get_video_pool(GstPad *pad); + +void gst_libcamera_pad_set_video_pool(GstPad *pad, GstBufferPool *video_pool); + +GstVideoInfo gst_libcamera_pad_get_video_info(GstPad *pad); + +void gst_libcamera_pad_set_video_info(GstPad *pad, const GstVideoInfo *info); + libcamera::Stream *gst_libcamera_pad_get_stream(GstPad *pad); void gst_libcamera_pad_set_latency(GstPad *pad, GstClockTime latency); diff --git a/src/gstreamer/gstlibcamerapool.cpp b/src/gstreamer/gstlibcamerapool.cpp index 9cd7eccb..8278144f 100644 --- a/src/gstreamer/gstlibcamerapool.cpp +++ b/src/gstreamer/gstlibcamerapool.cpp @@ -134,8 +134,20 @@ gst_libcamera_pool_class_init(GstLibcameraPoolClass *klass) G_TYPE_NONE, 0); } +static void +gst_libcamera_buffer_add_video_meta(GstBuffer *buffer, GstVideoInfo *info) +{ + GstVideoMeta *vmeta; + vmeta = gst_buffer_add_video_meta_full(buffer, GST_VIDEO_FRAME_FLAG_NONE, + GST_VIDEO_INFO_FORMAT(info), GST_VIDEO_INFO_WIDTH(info), + GST_VIDEO_INFO_HEIGHT(info), GST_VIDEO_INFO_N_PLANES(info), + info->offset, info->stride); + GST_META_FLAGS(vmeta) = (GstMetaFlags)(GST_META_FLAGS(vmeta) | GST_META_FLAG_POOLED); +} + GstLibcameraPool * -gst_libcamera_pool_new(GstLibcameraAllocator *allocator, Stream *stream) +gst_libcamera_pool_new(GstLibcameraAllocator *allocator, Stream *stream, + GstVideoInfo *info) { auto *pool = GST_LIBCAMERA_POOL(g_object_new(GST_TYPE_LIBCAMERA_POOL, nullptr)); @@ -145,6 +157,7 @@ gst_libcamera_pool_new(GstLibcameraAllocator *allocator, Stream *stream) gsize pool_size = gst_libcamera_allocator_get_pool_size(allocator, stream); for (gsize i = 0; i < pool_size; i++) { GstBuffer *buffer = gst_buffer_new(); + gst_libcamera_buffer_add_video_meta(buffer, info); pool->queue->push_back(buffer); } diff --git a/src/gstreamer/gstlibcamerapool.h b/src/gstreamer/gstlibcamerapool.h index 2a7a9c77..02ee4dd4 100644 --- a/src/gstreamer/gstlibcamerapool.h +++ b/src/gstreamer/gstlibcamerapool.h @@ -14,6 +14,7 @@ #include "gstlibcameraallocator.h" #include <gst/gst.h> +#include <gst/video/video.h> #include <libcamera/stream.h> @@ -21,7 +22,7 @@ G_DECLARE_FINAL_TYPE(GstLibcameraPool, gst_libcamera_pool, GST_LIBCAMERA, POOL, GstBufferPool) GstLibcameraPool *gst_libcamera_pool_new(GstLibcameraAllocator *allocator, - libcamera::Stream *stream); + libcamera::Stream *stream, GstVideoInfo *info); libcamera::Stream *gst_libcamera_pool_get_stream(GstLibcameraPool *self); diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp index 5e9e843d..4b81c94e 100644 --- a/src/gstreamer/gstlibcamerasrc.cpp +++ b/src/gstreamer/gstlibcamerasrc.cpp @@ -268,6 +268,58 @@ GstLibcameraSrcState::requestCompleted(Request *request) gst_task_resume(src_->task); } +static void +gst_libcamera_extrapolate_info(GstVideoInfo *info, guint32 stride) +{ + guint i, estride; + gsize offset = 0; + + /* this should be updated if tiled formats get added in the future. */ + for (i = 0; i < GST_VIDEO_INFO_N_PLANES(info); i++) { + estride = gst_video_format_info_extrapolate_stride(info->finfo, i, stride); + info->stride[i] = estride; + info->offset[i] = offset; + offset += estride * GST_VIDEO_FORMAT_INFO_SCALE_HEIGHT(info->finfo, i, + GST_VIDEO_INFO_HEIGHT(info)); + } +} + +static GstFlowReturn +gst_libcamera_video_frame_copy(GstBuffer *src, GstBuffer *dest, const GstVideoInfo *dest_info, guint32 stride) +{ + GstVideoInfo src_info = *dest_info; + GstVideoFrame src_frame, dest_frame; + + gst_libcamera_extrapolate_info(&src_info, stride); + src_info.size = gst_buffer_get_size(src); + + if (!gst_video_frame_map(&src_frame, &src_info, src, GST_MAP_READ)) { + GST_ERROR("Could not map buffer"); + goto error; + } + + if (!gst_video_frame_map(&dest_frame, dest_info, dest, GST_MAP_WRITE)) { + GST_ERROR("Could not map buffer"); + gst_video_frame_unmap(&src_frame); + goto error; + } + + if (!gst_video_frame_copy(&dest_frame, &src_frame)) { + GST_ERROR("Could not copy frame"); + gst_video_frame_unmap(&src_frame); + gst_video_frame_unmap(&dest_frame); + goto error; + } + + gst_video_frame_unmap(&src_frame); + gst_video_frame_unmap(&dest_frame); + + return GST_FLOW_OK; + +error: + return GST_FLOW_ERROR; +} + /* Must be called with stream_lock held. */ int GstLibcameraSrcState::processRequest() { @@ -292,11 +344,41 @@ int GstLibcameraSrcState::processRequest() GstFlowReturn ret = GST_FLOW_OK; gst_flow_combiner_reset(src_->flow_combiner); - for (GstPad *srcpad : srcpads_) { + for (gsize i = 0; i < srcpads_.size(); i++) { + GstPad *srcpad = srcpads_[i]; Stream *stream = gst_libcamera_pad_get_stream(srcpad); GstBuffer *buffer = wrap->detachBuffer(stream); FrameBuffer *fb = gst_libcamera_buffer_get_frame_buffer(buffer); + const StreamConfiguration &stream_cfg = config_->at(i); + GstBufferPool *video_pool = gst_libcamera_pad_get_video_pool(srcpad); + + if (video_pool) { + /* Only set video pool when a copy is needed */ + GstBuffer *copy = NULL; + const GstVideoInfo info = gst_libcamera_pad_get_video_info(srcpad); + + ret = gst_buffer_pool_acquire_buffer(video_pool, ©, NULL); + if (ret != GST_FLOW_OK) { + gst_buffer_unref(buffer); + GST_ELEMENT_ERROR(src_, RESOURCE, SETTINGS, + ("Failed to acquire buffer"), + ("GstLibcameraSrcState::processRequest() failed: %s", g_strerror(-ret))); + return -EPIPE; + } + + ret = gst_libcamera_video_frame_copy(buffer, copy, &info, stream_cfg.stride); + gst_buffer_unref(buffer); + if (ret != GST_FLOW_OK) { + gst_buffer_unref(copy); + GST_ELEMENT_ERROR(src_, RESOURCE, SETTINGS, + ("Failed to copy buffer"), + ("GstLibcameraSrcState::processRequest() failed: %s", g_strerror(-ret))); + return -EPIPE; + } + + buffer = copy; + } if (GST_CLOCK_TIME_IS_VALID(wrap->pts_)) { GST_BUFFER_PTS(buffer) = wrap->pts_; @@ -499,13 +581,68 @@ 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; + GstVideoInfo info; + + g_autoptr(GstCaps) caps = gst_libcamera_stream_configuration_to_caps(stream_cfg, transfer[i]); + + gst_video_info_from_caps(&info, caps); + gst_libcamera_pad_set_video_info(srcpad, &info); + + /* 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); + } + + 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); + } GstLibcameraPool *pool = gst_libcamera_pool_new(self->allocator, - stream_cfg.stream()); + stream_cfg.stream(), &info); g_signal_connect_swapped(pool, "buffer-notify", G_CALLBACK(gst_task_resume), self->task); gst_libcamera_pad_set_pool(srcpad, pool); + gst_libcamera_pad_set_video_pool(srcpad, video_pool); /* Clear all reconfigure flags. */ gst_pad_check_reconfigure(srcpad); @@ -922,6 +1059,12 @@ gst_libcamera_src_release_pad(GstElement *element, GstPad *pad) auto end_iterator = pads.end(); auto pad_iterator = std::find(begin_iterator, end_iterator, pad); + GstBufferPool *video_pool = gst_libcamera_pad_get_video_pool(pad); + if (video_pool) { + gst_buffer_pool_set_active(video_pool, false); + gst_object_unref(video_pool); + } + if (pad_iterator != end_iterator) { g_object_unref(*pad_iterator); pads.erase(pad_iterator);