[{"id":32560,"web_url":"https://patchwork.libcamera.org/comment/32560/","msgid":"<173344538436.1906024.6544755337982667066@ping.linuxembedded.co.uk>","date":"2024-12-06T00:36:24","subject":"Re: [PATCH v6] gstreamer: Add GstVideoMeta support","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Hou Qi (2024-12-04 08:55:11)\n> GStreamer video-info calculated stride and offset may differ from\n> those used by the camera.\n> \n> For stride and offset mismatch, this patch adds video meta to buffer\n> if downstream supports VideoMeta through allocation query. Otherwise,\n> create a internal VideoPool using the caps, and copy video frame to\n> this system memory.\n\nHow do we test this patch ?\n(How do you use it ?)\n\nI'm not sure if the existing unit tests are going to be confirming this\n? Is there anything we need to do to make sure this doesn't break\nsomething else?\n\n\n> \n> Signed-off-by: Hou Qi <qi.hou@nxp.com>\n> ---\n>  src/gstreamer/gstlibcamera-utils.cpp |  33 +++++++\n>  src/gstreamer/gstlibcamera-utils.h   |   4 +\n>  src/gstreamer/gstlibcamerapad.cpp    |  31 +++++++\n>  src/gstreamer/gstlibcamerapad.h      |   8 ++\n>  src/gstreamer/gstlibcamerapool.cpp   |  18 +++-\n>  src/gstreamer/gstlibcamerapool.h     |   3 +-\n>  src/gstreamer/gstlibcamerasrc.cpp    | 133 ++++++++++++++++++++++++++-\n>  7 files changed, 226 insertions(+), 4 deletions(-)\n> \n> diff --git a/src/gstreamer/gstlibcamera-utils.cpp b/src/gstreamer/gstlibcamera-utils.cpp\n> index 732987ef..09af9204 100644\n> --- a/src/gstreamer/gstlibcamera-utils.cpp\n> +++ b/src/gstreamer/gstlibcamera-utils.cpp\n> @@ -591,6 +591,39 @@ gst_task_resume(GstTask *task)\n>  }\n>  #endif\n>  \n> +#if !GST_CHECK_VERSION(1, 22, 0)\n> +/*\n> + * Copyright (C) <1999> Erik Walthinsen <omega@cse.ogi.edu>\n> + * Library       <2002> Ronald Bultje <rbultje@ronald.bitfreak.net>\n> + * Copyright (C) <2007> David A. Schleef <ds@schleef.org>\n> + */\n> +/* This function has been imported directly from the gstreamer project to\n> + * support backwards compatibility and should be removed when the older\n> + * version is no longer supported. */\n\n /*\n  * Thanks, this is a better way to import the single function for now.\n  * I would keep the banner comment to our usual style though\n  */\n\n> +gint gst_video_format_info_extrapolate_stride(const GstVideoFormatInfo *finfo, gint plane, gint stride)\n> +{\n> +       gint estride;\n> +       gint comp[GST_VIDEO_MAX_COMPONENTS];\n> +       gint i;\n> +\n> +       /* there is nothing to extrapolate on first plane */\n> +       if (plane == 0)\n> +               return stride;\n> +\n> +       gst_video_format_info_component(finfo, plane, comp);\n> +\n> +       /* For now, all planar formats have a single component on first plane, but\n> +       * if there was a planar format with more, we'd have to make a ratio of the\n> +       * number of component on the first plane against the number of component on\n> +       * the current plane. */\n\nHere the indentation has changed too even from the original sources.\n\nBut as we'll aim to remove this code when we can it's not something I'll\ndwell on.\n\n\n\n> +       estride = 0;\n> +       for (i = 0; i < GST_VIDEO_MAX_COMPONENTS && comp[i] >= 0; i++)\n> +               estride += GST_VIDEO_FORMAT_INFO_SCALE_WIDTH(finfo, comp[i], stride);\n> +\n> +       return estride;\n> +}\n> +#endif\n> +\n>  G_LOCK_DEFINE_STATIC(cm_singleton_lock);\n>  static std::weak_ptr<CameraManager> cm_singleton_ptr;\n>  \n> diff --git a/src/gstreamer/gstlibcamera-utils.h b/src/gstreamer/gstlibcamera-utils.h\n> index cab1c814..81149280 100644\n> --- a/src/gstreamer/gstlibcamera-utils.h\n> +++ b/src/gstreamer/gstlibcamera-utils.h\n> @@ -35,6 +35,10 @@ static inline void gst_clear_event(GstEvent **event_ptr)\n>  #if !GST_CHECK_VERSION(1, 17, 1)\n>  gboolean gst_task_resume(GstTask *task);\n>  #endif\n> +\n> +#if !GST_CHECK_VERSION(1, 22, 0)\n> +gint gst_video_format_info_extrapolate_stride(const GstVideoFormatInfo *finfo, gint plane, gint stride);\n> +#endif\n>  std::shared_ptr<libcamera::CameraManager> gst_libcamera_get_camera_manager(int &ret);\n>  \n>  /**\n> diff --git a/src/gstreamer/gstlibcamerapad.cpp b/src/gstreamer/gstlibcamerapad.cpp\n> index 7b22aebe..7fd11a8f 100644\n> --- a/src/gstreamer/gstlibcamerapad.cpp\n> +++ b/src/gstreamer/gstlibcamerapad.cpp\n> @@ -18,6 +18,8 @@ struct _GstLibcameraPad {\n>         GstPad parent;\n>         StreamRole role;\n>         GstLibcameraPool *pool;\n> +       GstBufferPool *video_pool;\n> +       GstVideoInfo info;\n>         GstClockTime latency;\n>  };\n>  \n> @@ -153,6 +155,35 @@ gst_libcamera_pad_set_pool(GstPad *pad, GstLibcameraPool *pool)\n>         self->pool = pool;\n>  }\n>  \n> +GstBufferPool *\n> +gst_libcamera_pad_get_video_pool(GstPad *pad)\n> +{\n> +       auto *self = GST_LIBCAMERA_PAD(pad);\n> +       return self->video_pool;\n> +}\n> +\n> +void gst_libcamera_pad_set_video_pool(GstPad *pad, GstBufferPool *video_pool)\n> +{\n> +       auto *self = GST_LIBCAMERA_PAD(pad);\n> +\n> +       if (self->video_pool)\n> +               g_object_unref(self->video_pool);\n> +       self->video_pool = video_pool;\n> +}\n> +\n> +GstVideoInfo gst_libcamera_pad_get_video_info(GstPad *pad)\n> +{\n> +       auto *self = GST_LIBCAMERA_PAD(pad);\n> +       return self->info;\n> +}\n> +\n> +void gst_libcamera_pad_set_video_info(GstPad *pad, GstVideoInfo info)\n> +{\n> +       auto *self = GST_LIBCAMERA_PAD(pad);\n> +\n> +       self->info = info;\n> +}\n> +\n>  Stream *\n>  gst_libcamera_pad_get_stream(GstPad *pad)\n>  {\n> diff --git a/src/gstreamer/gstlibcamerapad.h b/src/gstreamer/gstlibcamerapad.h\n> index 630c168a..ed0beb1f 100644\n> --- a/src/gstreamer/gstlibcamerapad.h\n> +++ b/src/gstreamer/gstlibcamerapad.h\n> @@ -23,6 +23,14 @@ GstLibcameraPool *gst_libcamera_pad_get_pool(GstPad *pad);\n>  \n>  void gst_libcamera_pad_set_pool(GstPad *pad, GstLibcameraPool *pool);\n>  \n> +GstBufferPool *gst_libcamera_pad_get_video_pool(GstPad *pad);\n> +\n> +void gst_libcamera_pad_set_video_pool(GstPad *pad, GstBufferPool *video_pool);\n> +\n> +GstVideoInfo gst_libcamera_pad_get_video_info(GstPad *pad);\n> +\n> +void gst_libcamera_pad_set_video_info(GstPad *pad, GstVideoInfo info);\n> +\n>  libcamera::Stream *gst_libcamera_pad_get_stream(GstPad *pad);\n>  \n>  void gst_libcamera_pad_set_latency(GstPad *pad, GstClockTime latency);\n> diff --git a/src/gstreamer/gstlibcamerapool.cpp b/src/gstreamer/gstlibcamerapool.cpp\n> index 9cd7eccb..471f71da 100644\n> --- a/src/gstreamer/gstlibcamerapool.cpp\n> +++ b/src/gstreamer/gstlibcamerapool.cpp\n> @@ -14,6 +14,9 @@\n>  \n>  #include \"gstlibcamera-utils.h\"\n>  \n> +#include <gst/gstmeta.h>\n> +\n> +\n>  using namespace libcamera;\n>  \n>  enum {\n> @@ -134,8 +137,20 @@ gst_libcamera_pool_class_init(GstLibcameraPoolClass *klass)\n>                                                      G_TYPE_NONE, 0);\n>  }\n>  \n> +static void\n> +gst_libcamera_buffer_add_video_meta(GstBuffer *buffer, GstVideoInfo *info)\n> +{\n> +       GstVideoMeta *vmeta;\n> +       vmeta = gst_buffer_add_video_meta_full(buffer, GST_VIDEO_FRAME_FLAG_NONE,\n> +                                              GST_VIDEO_INFO_FORMAT(info), GST_VIDEO_INFO_WIDTH(info),\n> +                                              GST_VIDEO_INFO_HEIGHT(info), GST_VIDEO_INFO_N_PLANES(info),\n> +                                              info->offset, info->stride);\n> +       GST_META_FLAGS(vmeta) = (GstMetaFlags)(GST_META_FLAGS(vmeta) | GST_META_FLAG_POOLED);\n> +}\n> +\n>  GstLibcameraPool *\n> -gst_libcamera_pool_new(GstLibcameraAllocator *allocator, Stream *stream)\n> +gst_libcamera_pool_new(GstLibcameraAllocator *allocator, Stream *stream,\n> +                      GstVideoInfo *info)\n>  {\n>         auto *pool = GST_LIBCAMERA_POOL(g_object_new(GST_TYPE_LIBCAMERA_POOL, nullptr));\n>  \n> @@ -145,6 +160,7 @@ gst_libcamera_pool_new(GstLibcameraAllocator *allocator, Stream *stream)\n>         gsize pool_size = gst_libcamera_allocator_get_pool_size(allocator, stream);\n>         for (gsize i = 0; i < pool_size; i++) {\n>                 GstBuffer *buffer = gst_buffer_new();\n> +               gst_libcamera_buffer_add_video_meta(buffer, info);\n>                 pool->queue->push_back(buffer);\n>         }\n>  \n> diff --git a/src/gstreamer/gstlibcamerapool.h b/src/gstreamer/gstlibcamerapool.h\n> index 2a7a9c77..02ee4dd4 100644\n> --- a/src/gstreamer/gstlibcamerapool.h\n> +++ b/src/gstreamer/gstlibcamerapool.h\n> @@ -14,6 +14,7 @@\n>  #include \"gstlibcameraallocator.h\"\n>  \n>  #include <gst/gst.h>\n> +#include <gst/video/video.h>\n>  \n>  #include <libcamera/stream.h>\n>  \n> @@ -21,7 +22,7 @@\n>  G_DECLARE_FINAL_TYPE(GstLibcameraPool, gst_libcamera_pool, GST_LIBCAMERA, POOL, GstBufferPool)\n>  \n>  GstLibcameraPool *gst_libcamera_pool_new(GstLibcameraAllocator *allocator,\n> -                                        libcamera::Stream *stream);\n> +                                        libcamera::Stream *stream, GstVideoInfo *info);\n>  \n>  libcamera::Stream *gst_libcamera_pool_get_stream(GstLibcameraPool *self);\n>  \n> diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp\n> index 8efa25f4..82b06eb4 100644\n> --- a/src/gstreamer/gstlibcamerasrc.cpp\n> +++ b/src/gstreamer/gstlibcamerasrc.cpp\n> @@ -268,6 +268,52 @@ GstLibcameraSrcState::requestCompleted(Request *request)\n>         gst_task_resume(src_->task);\n>  }\n>  \n> +static void\n> +gst_libcamera_extrapolate_info(GstVideoInfo *info, guint32 stride)\n> +{\n> +       guint i, estride;\n> +       gsize offset = 0;\n> +\n> +       /* this should be updated if tiled formats get added in the future. */\n> +       for (i = 0; i < GST_VIDEO_INFO_N_PLANES(info); i++) {\n> +               estride = gst_video_format_info_extrapolate_stride(info->finfo, i, stride);\n> +               info->stride[i] = estride;\n> +               info->offset[i] = offset;\n> +               offset += estride * GST_VIDEO_FORMAT_INFO_SCALE_HEIGHT(info->finfo, i,\n> +                                                                      GST_VIDEO_INFO_HEIGHT(info));\n> +       }\n> +}\n> +\n> +static GstFlowReturn\n> +gst_libcamera_video_frame_copy(GstBuffer *src, GstBuffer *dest, GstVideoInfo dest_info, guint32 stride)\n> +{\n> +       GstVideoInfo src_info = dest_info;\n> +       GstVideoFrame src_frame, dest_frame;\n> +\n> +       gst_libcamera_extrapolate_info(&src_info, stride);\n> +       src_info.size = gst_buffer_get_size(src);\n> +\n> +       if (!gst_video_frame_map(&src_frame, &src_info, src, GST_MAP_READ))\n> +               goto invalid_buffer;\n> +\n> +       if (!gst_video_frame_map(&dest_frame, &dest_info, dest, GST_MAP_WRITE)) {\n> +               gst_video_frame_unmap(&src_frame);\n> +               goto invalid_buffer;\n> +       }\n> +\n> +       gst_video_frame_copy(&dest_frame, &src_frame);\n> +\n> +       gst_video_frame_unmap(&src_frame);\n> +       gst_video_frame_unmap(&dest_frame);\n> +\n> +       return GST_FLOW_OK;\n> +\n> +invalid_buffer : {\n> +       GST_ERROR(\"Could not map buffer\");\n> +       return GST_FLOW_ERROR;\n> +}\n> +}\n> +\n>  /* Must be called with stream_lock held. */\n>  int GstLibcameraSrcState::processRequest()\n>  {\n> @@ -292,11 +338,36 @@ int GstLibcameraSrcState::processRequest()\n>         GstFlowReturn ret = GST_FLOW_OK;\n>         gst_flow_combiner_reset(src_->flow_combiner);\n>  \n> -       for (GstPad *srcpad : srcpads_) {\n> +       for (gsize i = 0; i < src_->state->srcpads_.size(); i++) {\n> +               GstPad *srcpad = src_->state->srcpads_[i];\n>                 Stream *stream = gst_libcamera_pad_get_stream(srcpad);\n>                 GstBuffer *buffer = wrap->detachBuffer(stream);\n>  \n>                 FrameBuffer *fb = gst_libcamera_buffer_get_frame_buffer(buffer);\n> +               const StreamConfiguration &stream_cfg = src_->state->config_->at(i);\n> +               GstBufferPool *video_pool = gst_libcamera_pad_get_video_pool(srcpad);\n> +\n> +               if (video_pool) {\n> +                       GstBuffer *copy = NULL;\n> +                       GstVideoInfo info = gst_libcamera_pad_get_video_info(srcpad);\n> +\n> +                       ret = gst_buffer_pool_acquire_buffer(video_pool, &copy, NULL);\n> +                       if (ret != GST_FLOW_OK) {\n> +                               GST_ERROR(\"Failed to acquire buffer\");\n> +                               gst_buffer_unref(buffer);\n> +                               return -EPIPE;\n> +                       }\n> +\n> +                       ret = gst_libcamera_video_frame_copy(buffer, copy, info, stream_cfg.stride);\n> +                       gst_buffer_unref(buffer);\n> +                       if (ret != GST_FLOW_OK) {\n> +                               GST_ERROR(\"Failed to copy buffer\");\n> +                               gst_buffer_unref(copy);\n> +                               return -EPIPE;\n> +                       }\n> +\n> +                       buffer = copy;\n> +               }\n>  \n>                 if (GST_CLOCK_TIME_IS_VALID(wrap->pts_)) {\n>                         GST_BUFFER_PTS(buffer) = wrap->pts_;\n> @@ -497,13 +568,65 @@ gst_libcamera_src_negotiate(GstLibcameraSrc *self)\n>         for (gsize i = 0; i < state->srcpads_.size(); i++) {\n>                 GstPad *srcpad = state->srcpads_[i];\n>                 const StreamConfiguration &stream_cfg = state->config_->at(i);\n> +               GstBufferPool *video_pool = NULL;\n> +               GstVideoInfo info;\n> +\n> +               g_autoptr(GstCaps) caps = gst_libcamera_stream_configuration_to_caps(stream_cfg);\n> +\n> +               gst_video_info_init(&info);\n> +               gst_video_info_from_caps(&info, caps);\n> +               gst_libcamera_pad_set_video_info(srcpad, info);\n> +\n> +               /* stride mismatch between camera stride and that calculated by video-info */\n> +               if (static_cast<unsigned int>(info.stride[0]) != stream_cfg.stride &&\n> +                   GST_VIDEO_INFO_FORMAT(&info) != GST_VIDEO_FORMAT_ENCODED) {\n> +                       GstQuery *query = NULL;\n> +                       gboolean need_pool = true;\n> +                       gboolean has_video_meta = false;\n> +\n> +                       gst_libcamera_extrapolate_info(&info, stream_cfg.stride);\n> +\n> +                       query = gst_query_new_allocation(caps, need_pool);\n> +                       if (!gst_pad_peer_query(srcpad, query))\n> +                               GST_DEBUG_OBJECT(self, \"Didn't get downstream ALLOCATION hints\");\n> +                       else\n> +                               has_video_meta = gst_query_find_allocation_meta(query, GST_VIDEO_META_API_TYPE, NULL);\n> +\n> +                       if (!has_video_meta) {\n> +                               GstBufferPool *pool = NULL;\n> +\n> +                               if (gst_query_get_n_allocation_pools(query) > 0)\n> +                                       gst_query_parse_nth_allocation_pool(query, 0, &pool, NULL, NULL, NULL);\n> +\n> +                               if (pool)\n> +                                       video_pool = pool;\n> +                               else {\n> +                                       GstStructure *config;\n> +                                       guint min_buffers = 3;\n> +                                       video_pool = gst_video_buffer_pool_new();\n> +\n> +                                       config = gst_buffer_pool_get_config(video_pool);\n> +                                       gst_buffer_pool_config_set_params(config, caps, info.size, min_buffers, 0);\n> +                                       gst_buffer_pool_set_config(GST_BUFFER_POOL_CAST(video_pool), config);\n> +                                       GST_DEBUG_OBJECT(self, \"Own pool config is %\" GST_PTR_FORMAT, config);\n> +                               }\n> +\n> +                               if (!gst_buffer_pool_set_active(video_pool, true)) {\n> +                                       GST_ERROR_OBJECT(self, \"Failed to active buffer pool\");\n> +                                       gst_caps_unref(caps);\n> +                                       return false;\n> +                               }\n> +                       }\n> +                       gst_query_unref(query);\n> +               }\n>  \n>                 GstLibcameraPool *pool = gst_libcamera_pool_new(self->allocator,\n> -                                                               stream_cfg.stream());\n> +                                                               stream_cfg.stream(), &info);\n>                 g_signal_connect_swapped(pool, \"buffer-notify\",\n>                                          G_CALLBACK(gst_task_resume), self->task);\n>  \n>                 gst_libcamera_pad_set_pool(srcpad, pool);\n> +               gst_libcamera_pad_set_video_pool(srcpad, video_pool);\n>  \n>                 /* Clear all reconfigure flags. */\n>                 gst_pad_check_reconfigure(srcpad);\n> @@ -920,6 +1043,12 @@ gst_libcamera_src_release_pad(GstElement *element, GstPad *pad)\n>                 auto end_iterator = pads.end();\n>                 auto pad_iterator = std::find(begin_iterator, end_iterator, pad);\n>  \n> +               GstBufferPool *video_pool = gst_libcamera_pad_get_video_pool(pad);\n> +               if (video_pool) {\n> +                       gst_buffer_pool_set_active(video_pool, false);\n> +                       gst_object_unref(video_pool);\n> +               }\n> +\n>                 if (pad_iterator != end_iterator) {\n>                         g_object_unref(*pad_iterator);\n>                         pads.erase(pad_iterator);\n> -- \n> 2.34.1\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 21A37BF415\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri,  6 Dec 2024 00:36:30 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2AD5C66122;\n\tFri,  6 Dec 2024 01:36:29 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7F0706611C\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri,  6 Dec 2024 01:36:27 +0100 (CET)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 6030D16A;\n\tFri,  6 Dec 2024 01:35:58 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"fbK39yuc\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1733445358;\n\tbh=XiLJamBDrbfqlRQfkfMPWUeR4n57jdORMQ1j49qObbw=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=fbK39yucxIgOtEnnAD/O5zTmDbTAfZUsAaDzlYnFwpMsOYhSFrbgWaeB/1sAeX500\n\tClCgnA5H7UjGP6KrIiHXPUMYUlvLkjY12QWonTQFU+h4JpzaE8yFvC9WZFdpg5KGYu\n\typUE7lTRa5e9frx0C3pBoCVw3D1SrmayPHXVPqPs=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20241204085511.1733434-1-qi.hou@nxp.com>","References":"<20241204085511.1733434-1-qi.hou@nxp.com>","Subject":"Re: [PATCH v6] gstreamer: Add GstVideoMeta support","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"jared.hu@nxp.com, qi.hou@nxp.com, julien.vuillaumier@nxp.com","To":"Hou Qi <qi.hou@nxp.com>, libcamera-devel@lists.libcamera.org","Date":"Fri, 06 Dec 2024 00:36:24 +0000","Message-ID":"<173344538436.1906024.6544755337982667066@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":33928,"web_url":"https://patchwork.libcamera.org/comment/33928/","msgid":"<PAXPR04MB828522C4E042B2714683A34797B52@PAXPR04MB8285.eurprd04.prod.outlook.com>","date":"2025-04-08T01:25:49","subject":"RE: [PATCH v6] gstreamer: Add GstVideoMeta support","submitter":{"id":195,"url":"https://patchwork.libcamera.org/api/people/195/","name":"Qi Hou","email":"qi.hou@nxp.com"},"content":"Hi Nicolas,\n\nCan you help continue to review v6? I have made the code change according to your comments of v5. Look forward to your reply.\n\nRegards,\nQi Hou\n\n-----Original Message-----\nFrom: Qi Hou \nSent: 2024年12月4日 16:56\nTo: libcamera-devel@lists.libcamera.org\nCc: Jared Hu <jared.hu@nxp.com>; Qi Hou <qi.hou@nxp.com>; Julien Vuillaumier <julien.vuillaumier@nxp.com>\nSubject: [PATCH v6] gstreamer: Add GstVideoMeta support\n\nGStreamer video-info calculated stride and offset may differ from those used by the camera.\n\nFor 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.\n\nSigned-off-by: Hou Qi <qi.hou@nxp.com>\n---\n src/gstreamer/gstlibcamera-utils.cpp |  33 +++++++\n src/gstreamer/gstlibcamera-utils.h   |   4 +\n src/gstreamer/gstlibcamerapad.cpp    |  31 +++++++\n src/gstreamer/gstlibcamerapad.h      |   8 ++\n src/gstreamer/gstlibcamerapool.cpp   |  18 +++-\n src/gstreamer/gstlibcamerapool.h     |   3 +-\n src/gstreamer/gstlibcamerasrc.cpp    | 133 ++++++++++++++++++++++++++-\n 7 files changed, 226 insertions(+), 4 deletions(-)\n\ndiff --git a/src/gstreamer/gstlibcamera-utils.cpp b/src/gstreamer/gstlibcamera-utils.cpp\nindex 732987ef..09af9204 100644\n--- a/src/gstreamer/gstlibcamera-utils.cpp\n+++ b/src/gstreamer/gstlibcamera-utils.cpp\n@@ -591,6 +591,39 @@ gst_task_resume(GstTask *task)  }  #endif\n \n+#if !GST_CHECK_VERSION(1, 22, 0)\n+/*\n+ * Copyright (C) <1999> Erik Walthinsen <omega@cse.ogi.edu>\n+ * Library       <2002> Ronald Bultje <rbultje@ronald.bitfreak.net>\n+ * Copyright (C) <2007> David A. Schleef <ds@schleef.org>  */\n+/* This function has been imported directly from the gstreamer project \n+to\n+ * support backwards compatibility and should be removed when the older\n+ * version is no longer supported. */\n+gint gst_video_format_info_extrapolate_stride(const GstVideoFormatInfo \n+*finfo, gint plane, gint stride) {\n+\tgint estride;\n+\tgint comp[GST_VIDEO_MAX_COMPONENTS];\n+\tgint i;\n+\n+\t/* there is nothing to extrapolate on first plane */\n+\tif (plane == 0)\n+\t\treturn stride;\n+\n+\tgst_video_format_info_component(finfo, plane, comp);\n+\n+\t/* For now, all planar formats have a single component on first plane, but\n+\t* if there was a planar format with more, we'd have to make a ratio of the\n+\t* number of component on the first plane against the number of component on\n+\t* the current plane. */\n+\testride = 0;\n+\tfor (i = 0; i < GST_VIDEO_MAX_COMPONENTS && comp[i] >= 0; i++)\n+\t\testride += GST_VIDEO_FORMAT_INFO_SCALE_WIDTH(finfo, comp[i], stride);\n+\n+\treturn estride;\n+}\n+#endif\n+\n G_LOCK_DEFINE_STATIC(cm_singleton_lock);\n static std::weak_ptr<CameraManager> cm_singleton_ptr;\n \ndiff --git a/src/gstreamer/gstlibcamera-utils.h b/src/gstreamer/gstlibcamera-utils.h\nindex cab1c814..81149280 100644\n--- a/src/gstreamer/gstlibcamera-utils.h\n+++ b/src/gstreamer/gstlibcamera-utils.h\n@@ -35,6 +35,10 @@ static inline void gst_clear_event(GstEvent **event_ptr)  #if !GST_CHECK_VERSION(1, 17, 1)  gboolean gst_task_resume(GstTask *task);  #endif\n+\n+#if !GST_CHECK_VERSION(1, 22, 0)\n+gint gst_video_format_info_extrapolate_stride(const GstVideoFormatInfo \n+*finfo, gint plane, gint stride); #endif\n std::shared_ptr<libcamera::CameraManager> gst_libcamera_get_camera_manager(int &ret);\n \n /**\ndiff --git a/src/gstreamer/gstlibcamerapad.cpp b/src/gstreamer/gstlibcamerapad.cpp\nindex 7b22aebe..7fd11a8f 100644\n--- a/src/gstreamer/gstlibcamerapad.cpp\n+++ b/src/gstreamer/gstlibcamerapad.cpp\n@@ -18,6 +18,8 @@ struct _GstLibcameraPad {\n \tGstPad parent;\n \tStreamRole role;\n \tGstLibcameraPool *pool;\n+\tGstBufferPool *video_pool;\n+\tGstVideoInfo info;\n \tGstClockTime latency;\n };\n \n@@ -153,6 +155,35 @@ gst_libcamera_pad_set_pool(GstPad *pad, GstLibcameraPool *pool)\n \tself->pool = pool;\n }\n \n+GstBufferPool *\n+gst_libcamera_pad_get_video_pool(GstPad *pad) {\n+\tauto *self = GST_LIBCAMERA_PAD(pad);\n+\treturn self->video_pool;\n+}\n+\n+void gst_libcamera_pad_set_video_pool(GstPad *pad, GstBufferPool \n+*video_pool) {\n+\tauto *self = GST_LIBCAMERA_PAD(pad);\n+\n+\tif (self->video_pool)\n+\t\tg_object_unref(self->video_pool);\n+\tself->video_pool = video_pool;\n+}\n+\n+GstVideoInfo gst_libcamera_pad_get_video_info(GstPad *pad) {\n+\tauto *self = GST_LIBCAMERA_PAD(pad);\n+\treturn self->info;\n+}\n+\n+void gst_libcamera_pad_set_video_info(GstPad *pad, GstVideoInfo info) {\n+\tauto *self = GST_LIBCAMERA_PAD(pad);\n+\n+\tself->info = info;\n+}\n+\n Stream *\n gst_libcamera_pad_get_stream(GstPad *pad)  { diff --git a/src/gstreamer/gstlibcamerapad.h b/src/gstreamer/gstlibcamerapad.h index 630c168a..ed0beb1f 100644\n--- a/src/gstreamer/gstlibcamerapad.h\n+++ b/src/gstreamer/gstlibcamerapad.h\n@@ -23,6 +23,14 @@ GstLibcameraPool *gst_libcamera_pad_get_pool(GstPad *pad);\n \n void gst_libcamera_pad_set_pool(GstPad *pad, GstLibcameraPool *pool);\n \n+GstBufferPool *gst_libcamera_pad_get_video_pool(GstPad *pad);\n+\n+void gst_libcamera_pad_set_video_pool(GstPad *pad, GstBufferPool \n+*video_pool);\n+\n+GstVideoInfo gst_libcamera_pad_get_video_info(GstPad *pad);\n+\n+void gst_libcamera_pad_set_video_info(GstPad *pad, GstVideoInfo info);\n+\n libcamera::Stream *gst_libcamera_pad_get_stream(GstPad *pad);\n \n void gst_libcamera_pad_set_latency(GstPad *pad, GstClockTime latency); diff --git a/src/gstreamer/gstlibcamerapool.cpp b/src/gstreamer/gstlibcamerapool.cpp\nindex 9cd7eccb..471f71da 100644\n--- a/src/gstreamer/gstlibcamerapool.cpp\n+++ b/src/gstreamer/gstlibcamerapool.cpp\n@@ -14,6 +14,9 @@\n \n #include \"gstlibcamera-utils.h\"\n \n+#include <gst/gstmeta.h>\n+\n+\n using namespace libcamera;\n \n enum {\n@@ -134,8 +137,20 @@ gst_libcamera_pool_class_init(GstLibcameraPoolClass *klass)\n \t\t\t\t\t\t     G_TYPE_NONE, 0);\n }\n \n+static void\n+gst_libcamera_buffer_add_video_meta(GstBuffer *buffer, GstVideoInfo \n+*info) {\n+\tGstVideoMeta *vmeta;\n+\tvmeta = gst_buffer_add_video_meta_full(buffer, GST_VIDEO_FRAME_FLAG_NONE,\n+\t\t\t\t\t       GST_VIDEO_INFO_FORMAT(info), GST_VIDEO_INFO_WIDTH(info),\n+\t\t\t\t\t       GST_VIDEO_INFO_HEIGHT(info), GST_VIDEO_INFO_N_PLANES(info),\n+\t\t\t\t\t       info->offset, info->stride);\n+\tGST_META_FLAGS(vmeta) = (GstMetaFlags)(GST_META_FLAGS(vmeta) | \n+GST_META_FLAG_POOLED); }\n+\n GstLibcameraPool *\n-gst_libcamera_pool_new(GstLibcameraAllocator *allocator, Stream *stream)\n+gst_libcamera_pool_new(GstLibcameraAllocator *allocator, Stream *stream,\n+\t\t       GstVideoInfo *info)\n {\n \tauto *pool = GST_LIBCAMERA_POOL(g_object_new(GST_TYPE_LIBCAMERA_POOL, nullptr));\n \n@@ -145,6 +160,7 @@ gst_libcamera_pool_new(GstLibcameraAllocator *allocator, Stream *stream)\n \tgsize pool_size = gst_libcamera_allocator_get_pool_size(allocator, stream);\n \tfor (gsize i = 0; i < pool_size; i++) {\n \t\tGstBuffer *buffer = gst_buffer_new();\n+\t\tgst_libcamera_buffer_add_video_meta(buffer, info);\n \t\tpool->queue->push_back(buffer);\n \t}\n \ndiff --git a/src/gstreamer/gstlibcamerapool.h b/src/gstreamer/gstlibcamerapool.h\nindex 2a7a9c77..02ee4dd4 100644\n--- a/src/gstreamer/gstlibcamerapool.h\n+++ b/src/gstreamer/gstlibcamerapool.h\n@@ -14,6 +14,7 @@\n #include \"gstlibcameraallocator.h\"\n \n #include <gst/gst.h>\n+#include <gst/video/video.h>\n \n #include <libcamera/stream.h>\n \n@@ -21,7 +22,7 @@\n G_DECLARE_FINAL_TYPE(GstLibcameraPool, gst_libcamera_pool, GST_LIBCAMERA, POOL, GstBufferPool)\n \n GstLibcameraPool *gst_libcamera_pool_new(GstLibcameraAllocator *allocator,\n-\t\t\t\t\t libcamera::Stream *stream);\n+\t\t\t\t\t libcamera::Stream *stream, GstVideoInfo *info);\n \n libcamera::Stream *gst_libcamera_pool_get_stream(GstLibcameraPool *self);\n \ndiff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp\nindex 8efa25f4..82b06eb4 100644\n--- a/src/gstreamer/gstlibcamerasrc.cpp\n+++ b/src/gstreamer/gstlibcamerasrc.cpp\n@@ -268,6 +268,52 @@ GstLibcameraSrcState::requestCompleted(Request *request)\n \tgst_task_resume(src_->task);\n }\n \n+static void\n+gst_libcamera_extrapolate_info(GstVideoInfo *info, guint32 stride) {\n+\tguint i, estride;\n+\tgsize offset = 0;\n+\n+\t/* this should be updated if tiled formats get added in the future. */\n+\tfor (i = 0; i < GST_VIDEO_INFO_N_PLANES(info); i++) {\n+\t\testride = gst_video_format_info_extrapolate_stride(info->finfo, i, stride);\n+\t\tinfo->stride[i] = estride;\n+\t\tinfo->offset[i] = offset;\n+\t\toffset += estride * GST_VIDEO_FORMAT_INFO_SCALE_HEIGHT(info->finfo, i,\n+\t\t\t\t\t\t\t\t       GST_VIDEO_INFO_HEIGHT(info));\n+\t}\n+}\n+\n+static GstFlowReturn\n+gst_libcamera_video_frame_copy(GstBuffer *src, GstBuffer *dest, \n+GstVideoInfo dest_info, guint32 stride) {\n+\tGstVideoInfo src_info = dest_info;\n+\tGstVideoFrame src_frame, dest_frame;\n+\n+\tgst_libcamera_extrapolate_info(&src_info, stride);\n+\tsrc_info.size = gst_buffer_get_size(src);\n+\n+\tif (!gst_video_frame_map(&src_frame, &src_info, src, GST_MAP_READ))\n+\t\tgoto invalid_buffer;\n+\n+\tif (!gst_video_frame_map(&dest_frame, &dest_info, dest, GST_MAP_WRITE)) {\n+\t\tgst_video_frame_unmap(&src_frame);\n+\t\tgoto invalid_buffer;\n+\t}\n+\n+\tgst_video_frame_copy(&dest_frame, &src_frame);\n+\n+\tgst_video_frame_unmap(&src_frame);\n+\tgst_video_frame_unmap(&dest_frame);\n+\n+\treturn GST_FLOW_OK;\n+\n+invalid_buffer : {\n+\tGST_ERROR(\"Could not map buffer\");\n+\treturn GST_FLOW_ERROR;\n+}\n+}\n+\n /* Must be called with stream_lock held. */  int GstLibcameraSrcState::processRequest()\n {\n@@ -292,11 +338,36 @@ int GstLibcameraSrcState::processRequest()\n \tGstFlowReturn ret = GST_FLOW_OK;\n \tgst_flow_combiner_reset(src_->flow_combiner);\n \n-\tfor (GstPad *srcpad : srcpads_) {\n+\tfor (gsize i = 0; i < src_->state->srcpads_.size(); i++) {\n+\t\tGstPad *srcpad = src_->state->srcpads_[i];\n \t\tStream *stream = gst_libcamera_pad_get_stream(srcpad);\n \t\tGstBuffer *buffer = wrap->detachBuffer(stream);\n \n \t\tFrameBuffer *fb = gst_libcamera_buffer_get_frame_buffer(buffer);\n+\t\tconst StreamConfiguration &stream_cfg = src_->state->config_->at(i);\n+\t\tGstBufferPool *video_pool = gst_libcamera_pad_get_video_pool(srcpad);\n+\n+\t\tif (video_pool) {\n+\t\t\tGstBuffer *copy = NULL;\n+\t\t\tGstVideoInfo info = gst_libcamera_pad_get_video_info(srcpad);\n+\n+\t\t\tret = gst_buffer_pool_acquire_buffer(video_pool, &copy, NULL);\n+\t\t\tif (ret != GST_FLOW_OK) {\n+\t\t\t\tGST_ERROR(\"Failed to acquire buffer\");\n+\t\t\t\tgst_buffer_unref(buffer);\n+\t\t\t\treturn -EPIPE;\n+\t\t\t}\n+\n+\t\t\tret = gst_libcamera_video_frame_copy(buffer, copy, info, stream_cfg.stride);\n+\t\t\tgst_buffer_unref(buffer);\n+\t\t\tif (ret != GST_FLOW_OK) {\n+\t\t\t\tGST_ERROR(\"Failed to copy buffer\");\n+\t\t\t\tgst_buffer_unref(copy);\n+\t\t\t\treturn -EPIPE;\n+\t\t\t}\n+\n+\t\t\tbuffer = copy;\n+\t\t}\n \n \t\tif (GST_CLOCK_TIME_IS_VALID(wrap->pts_)) {\n \t\t\tGST_BUFFER_PTS(buffer) = wrap->pts_; @@ -497,13 +568,65 @@ gst_libcamera_src_negotiate(GstLibcameraSrc *self)\n \tfor (gsize i = 0; i < state->srcpads_.size(); i++) {\n \t\tGstPad *srcpad = state->srcpads_[i];\n \t\tconst StreamConfiguration &stream_cfg = state->config_->at(i);\n+\t\tGstBufferPool *video_pool = NULL;\n+\t\tGstVideoInfo info;\n+\n+\t\tg_autoptr(GstCaps) caps = \n+gst_libcamera_stream_configuration_to_caps(stream_cfg);\n+\n+\t\tgst_video_info_init(&info);\n+\t\tgst_video_info_from_caps(&info, caps);\n+\t\tgst_libcamera_pad_set_video_info(srcpad, info);\n+\n+\t\t/* stride mismatch between camera stride and that calculated by video-info */\n+\t\tif (static_cast<unsigned int>(info.stride[0]) != stream_cfg.stride &&\n+\t\t    GST_VIDEO_INFO_FORMAT(&info) != GST_VIDEO_FORMAT_ENCODED) {\n+\t\t\tGstQuery *query = NULL;\n+\t\t\tgboolean need_pool = true;\n+\t\t\tgboolean has_video_meta = false;\n+\n+\t\t\tgst_libcamera_extrapolate_info(&info, stream_cfg.stride);\n+\n+\t\t\tquery = gst_query_new_allocation(caps, need_pool);\n+\t\t\tif (!gst_pad_peer_query(srcpad, query))\n+\t\t\t\tGST_DEBUG_OBJECT(self, \"Didn't get downstream ALLOCATION hints\");\n+\t\t\telse\n+\t\t\t\thas_video_meta = gst_query_find_allocation_meta(query, \n+GST_VIDEO_META_API_TYPE, NULL);\n+\n+\t\t\tif (!has_video_meta) {\n+\t\t\t\tGstBufferPool *pool = NULL;\n+\n+\t\t\t\tif (gst_query_get_n_allocation_pools(query) > 0)\n+\t\t\t\t\tgst_query_parse_nth_allocation_pool(query, 0, &pool, NULL, NULL, \n+NULL);\n+\n+\t\t\t\tif (pool)\n+\t\t\t\t\tvideo_pool = pool;\n+\t\t\t\telse {\n+\t\t\t\t\tGstStructure *config;\n+\t\t\t\t\tguint min_buffers = 3;\n+\t\t\t\t\tvideo_pool = gst_video_buffer_pool_new();\n+\n+\t\t\t\t\tconfig = gst_buffer_pool_get_config(video_pool);\n+\t\t\t\t\tgst_buffer_pool_config_set_params(config, caps, info.size, min_buffers, 0);\n+\t\t\t\t\tgst_buffer_pool_set_config(GST_BUFFER_POOL_CAST(video_pool), config);\n+\t\t\t\t\tGST_DEBUG_OBJECT(self, \"Own pool config is %\" GST_PTR_FORMAT, config);\n+\t\t\t\t}\n+\n+\t\t\t\tif (!gst_buffer_pool_set_active(video_pool, true)) {\n+\t\t\t\t\tGST_ERROR_OBJECT(self, \"Failed to active buffer pool\");\n+\t\t\t\t\tgst_caps_unref(caps);\n+\t\t\t\t\treturn false;\n+\t\t\t\t}\n+\t\t\t}\n+\t\t\tgst_query_unref(query);\n+\t\t}\n \n \t\tGstLibcameraPool *pool = gst_libcamera_pool_new(self->allocator,\n-\t\t\t\t\t\t\t\tstream_cfg.stream());\n+\t\t\t\t\t\t\t\tstream_cfg.stream(), &info);\n \t\tg_signal_connect_swapped(pool, \"buffer-notify\",\n \t\t\t\t\t G_CALLBACK(gst_task_resume), self->task);\n \n \t\tgst_libcamera_pad_set_pool(srcpad, pool);\n+\t\tgst_libcamera_pad_set_video_pool(srcpad, video_pool);\n \n \t\t/* Clear all reconfigure flags. */\n \t\tgst_pad_check_reconfigure(srcpad);\n@@ -920,6 +1043,12 @@ gst_libcamera_src_release_pad(GstElement *element, GstPad *pad)\n \t\tauto end_iterator = pads.end();\n \t\tauto pad_iterator = std::find(begin_iterator, end_iterator, pad);\n \n+\t\tGstBufferPool *video_pool = gst_libcamera_pad_get_video_pool(pad);\n+\t\tif (video_pool) {\n+\t\t\tgst_buffer_pool_set_active(video_pool, false);\n+\t\t\tgst_object_unref(video_pool);\n+\t\t}\n+\n \t\tif (pad_iterator != end_iterator) {\n \t\t\tg_object_unref(*pad_iterator);\n \t\t\tpads.erase(pad_iterator);\n--\n2.34.1","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 4C65EC3213\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue,  8 Apr 2025 01:25:54 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7DDE4689A0;\n\tTue,  8 Apr 2025 03:25:53 +0200 (CEST)","from AM0PR83CU005.outbound.protection.outlook.com\n\t(mail-westeuropeazlp170100001.outbound.protection.outlook.com\n\t[IPv6:2a01:111:f403:c201::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1ED3A627F4\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue,  8 Apr 2025 03:25:51 +0200 (CEST)","from PAXPR04MB8285.eurprd04.prod.outlook.com\n\t(2603:10a6:102:1ca::15)\n\tby PAXPR04MB8491.eurprd04.prod.outlook.com (2603:10a6:102:1df::8)\n\twith Microsoft SMTP Server (version=TLS1_2,\n\tcipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.8606.31;\n\tTue, 8 Apr 2025 01:25:49 +0000","from PAXPR04MB8285.eurprd04.prod.outlook.com\n\t([fe80::e003:8fb:64ea:acfd]) by\n\tPAXPR04MB8285.eurprd04.prod.outlook.com\n\t([fe80::e003:8fb:64ea:acfd%6]) with mapi id 15.20.8606.033;\n\tTue, 8 Apr 2025 01:25:49 +0000"],"Authentication-Results":["lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=nxp.com header.i=@nxp.com header.b=\"QgGWLX+q\";\n\tdkim-atps=neutral","dkim=none (message not signed)\n\theader.d=none;dmarc=none action=none header.from=nxp.com;"],"ARC-Seal":"i=1; a=rsa-sha256; s=arcselector10001; d=microsoft.com; cv=none;\n\tb=BvYP+mpRW6io/i2SUc79Egjs4VBE4J0IbnKHyA4ESXOTJREmbB2Jj57U6udVBc4RMd1Sz6VE8zMwRdpntLoKea4w2x/uDbas9C1ZwqOO4N9jVDDyoCvw0tHfwl0Ggv6atQgMHXYLsH5jRXwpsUhoZ+uFikFbFiy0/4U9ibs8Sm3YIDzzj+iDH0S+6Ium5KqlwWhGDuR6S/kwIxcz3RfWuxmRlg8uP8QeGoWPhtPnLB8suo9mKdia7XA6diFmx21YQd2Xc9mFIfOgJdnghT9gtNYophWwIObkVThaKM6hnuqy4gc42+y3M/WKAG9xqgzdLdh8nZLef0+p5P/4oUJt6w==","ARC-Message-Signature":"i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com;\n\ts=arcselector10001;\n\th=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1;\n\tbh=1Vaq6Nykc+TWVlDLImrt+1cFQ0L9cjJEXFyeqPsltXk=;\n\tb=HGIFsGFRuaOmX0B7UVlqBUYpA4zMWLNH41umHbVKjD+MVt5AxaTTY/SeAP2EhvI4LSgp8IefUNmmmBBCaQPE0pWLwDux4BqQxD8dFWhqSJ4NIU56Rx60ApcTwyts73Mtf/TH/InVuMNc9iKLtqKhj8QJCzIMmqbGGLCzfGXYlWXEX2/rSfsvgX8NvSxHhYUltUP1G7VjmE6C+/B/rU+qmfFHsibj8S5qi9JsflcX/DFztm/uVMqJYFmVQHWb0zX7V7/w6bIUrLlXt4er5E6aYRy3PBoP7jHFXRrmDOMtVY9ethZlq2sq7bEoMmPki75sExeAehessa8FDJNH1awG4A==","ARC-Authentication-Results":"i=1; mx.microsoft.com 1; spf=pass\n\tsmtp.mailfrom=nxp.com; dmarc=pass action=none header.from=nxp.com;\n\tdkim=pass header.d=nxp.com; arc=none","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=nxp.com; s=selector1;\n\th=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck;\n\tbh=1Vaq6Nykc+TWVlDLImrt+1cFQ0L9cjJEXFyeqPsltXk=;\n\tb=QgGWLX+q1Go8OFKy2+99X3cgYD3nCN0K8P/IDWe4XId8Km2j4JSRDAuCNu7J4HjBSOH58AvtQT4C+TQ25f96MOE2RSD+7ydJBBlGlK6khKUBjXPuruhnTulQqrzqQj/Q1puLIFt+b++3AEJFjLGoAfQz6W3hbrbjVfTnuTwNLn7whPdbRrY5Qs8r4Purp/qI/MstLYN+56UUPTViW7WxpBA/W9gBL3SZUfPx8QeoIzAyRlIBIlnbBLQ8AAhH3zOpbDKtNX+9b1n7CUZUZVXUdTvqxgAAbJCWekYFmp7NaPWelANWTBM76gDxZE5oFOhQm6NEbZs+JDuoDzvlJ9A29A==","From":"Qi Hou <qi.hou@nxp.com>","To":"\"libcamera-devel@lists.libcamera.org\"\n\t<libcamera-devel@lists.libcamera.org>","CC":"Jared Hu <jared.hu@nxp.com>, Julien Vuillaumier\n\t<julien.vuillaumier@nxp.com>","Subject":"RE: [PATCH v6] gstreamer: Add GstVideoMeta support","Thread-Topic":"[PATCH v6] gstreamer: Add GstVideoMeta support","Thread-Index":"AQHbRipHNCd3F3VqYU6DyDG//BDktrOZvUtA","Date":"Tue, 8 Apr 2025 01:25:49 +0000","Message-ID":"<PAXPR04MB828522C4E042B2714683A34797B52@PAXPR04MB8285.eurprd04.prod.outlook.com>","References":"<20241204085511.1733434-1-qi.hou@nxp.com>","In-Reply-To":"<20241204085511.1733434-1-qi.hou@nxp.com>","Accept-Language":"zh-CN, en-US","Content-Language":"en-US","X-MS-Has-Attach":"","X-MS-TNEF-Correlator":"","authentication-results":["lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=nxp.com header.i=@nxp.com header.b=\"QgGWLX+q\";\n\tdkim-atps=neutral","dkim=none (message not signed)\n\theader.d=none;dmarc=none action=none header.from=nxp.com;"],"x-ms-publictraffictype":"Email","x-ms-traffictypediagnostic":"PAXPR04MB8285:EE_|PAXPR04MB8491:EE_","x-ms-office365-filtering-correlation-id":"a55c9683-f577-4433-a513-08dd763c4bd3","x-ms-exchange-senderadcheck":"1","x-ms-exchange-antispam-relay":"0","x-microsoft-antispam":"BCL:0;\n\tARA:13230040|366016|1800799024|376014|7053199007|38070700018; ","x-microsoft-antispam-message-info":"=?gb2312?b?djNvcjNVclI5N0ZnRkFaZFls?=\n\t=?gb2312?b?QXk0M21Tbk5LbE5YOGVmQ1VORGx6cWZFSVBqZldhaW43cmJ0UmFV?=\n\t=?gb2312?b?MUFnQ2tiaGZNZGhFVmNkU3BzWHF0OHgyL04wUGpLVGVLMXFsSy81?=\n\t=?gb2312?b?L0hzQmJFeFRVRFFDTGRTVmp6YlJPeU5VaFNxdWNWYk5vSjdNSk9P?=\n\t=?gb2312?b?VDVGQit1T0hpR1VMVHdjUjIrcmNtakpmOE0yT2VwcDZoSEc4RHEy?=\n\t=?gb2312?b?TldmUnR0QjQ2allaUVFTLzcyOU9FTHJtWTBKbjk5YUF1aFBReGVi?=\n\t=?gb2312?b?Y0k4NVhxcVk3VU5BTHpLaWZKK2FEazZ5OUpobEdYTzdxZjR0YXdU?=\n\t=?gb2312?b?K3IwaWRkSm1KU2tBa1FwMnhNTUZEOTZ6RldxSmxLOUx3N253Skc4?=\n\t=?gb2312?b?b0NhR3R1QUFmVEV5RHJTamxKTTNzbXNLZDNzNS9FTGRBejJYQkU5?=\n\t=?gb2312?b?NFVJQ2ZxRkdXWnFNOFFRVVZDQkpqV0F1SmxmVXRyT3ZKNTRZdnhw?=\n\t=?gb2312?b?NWllUHBnSG5rK1NaREw1b3cyWFRHWEIxRnQwS3R6ZTFQQklsV0x5?=\n\t=?gb2312?b?dWhKYURzZ1RjengyOXBXeHZzRzVhOFBPYWdiZytkci8yUFV6V0Ri?=\n\t=?gb2312?b?cW55WjlYVDJESGg5alhJU0c3RzBYdGs2ZVVlc0dPd3p6OW9MWDFZ?=\n\t=?gb2312?b?Q0FhK25nanRkbmRmaERsekU3WDI2Z0Z5UDNja1lpeVpxNlZEWTNV?=\n\t=?gb2312?b?TUVVWHVjdlFUNDV3SVg1WER0NHlLb3dVS01FdkVyZlhOTGxsS0ZC?=\n\t=?gb2312?b?TUk2WjYvbXlMVDdWRUorcDduQ3hjdXNjYS9YM2VldThkN0tJWGl2?=\n\t=?gb2312?b?T2p6amVHUHo0WjI4RHpWRWQrY2RRNFV0clExMlF2NnhFUGduMTRP?=\n\t=?gb2312?b?dnV4dzc1OGI2bGF0Wng2UG5HbDF6bmQ4THNOOUtDVVRPb1NKSlE5?=\n\t=?gb2312?b?dVpBYlJOQjNKWjRyTXFYVVBoK1VIbVdTcDhWMDdXNWR0SnRGb0xk?=\n\t=?gb2312?b?TERFYkxpQ2FZVko4cTJWZjM0cnp0RnNSM3I0aHl0aDJScGRNVG1M?=\n\t=?gb2312?b?d3NPTWFSZjVUTWcrbEgvTkFoRkQ4OTc3ZldkRE10bEQ3RGhWMWtF?=\n\t=?gb2312?b?SWxFUUR3SEtGZXVmUTUrUllFVVlBaUFpV2kvQzE4MnlqenZ1dllT?=\n\t=?gb2312?b?QVE0SHhJTzVybHBGQWVwRkxqTlM2TFozcEUzckNUNERqamVNYkNT?=\n\t=?gb2312?b?QTNPQUhMZ0QvQ1ZqdGI3UTNFNmFQbGhxQTVjNlFFaUtUTDRMZHZC?=\n\t=?gb2312?b?OEhRdzY0UkpGS1JLUXd0eFhlcktyUlFsa2tjbTJDby9idU9rNVF3?=\n\t=?gb2312?b?ZExhSC9oWGhDdk1jR2RjVUoxWkdXblArMmVjbW5VYnJWdU5LM0Q0?=\n\t=?gb2312?b?QXFNTzBycnNtbUJSNXY3TkZ3T2RXWGVXaERjcDNlelVFdlZ5NmJs?=\n\t=?gb2312?b?Q25pOVM0NEVqeXNpU1NXQi9XeVlaTnhOVkZDcEs4MzdQYm9KY3Qr?=\n\t=?gb2312?b?N3VCeVU5Z3YvTm8vMVpSV29vZnMrVmtpaVBDaWE4T2xycUxQNDZG?=\n\t=?gb2312?b?NGxzZlp0bFpsQy8zVFRCU0FtMklZOXhsOW5UMFA3VnRIclZiUTgz?=\n\t=?gb2312?b?NHNVQmJqa05jb21VLzhBcDVOOTU2YzhNUUYyN2w4NGJtZ3BqcHFm?=\n\t=?gb2312?b?QWdFaGxnRlRxU0JUcVdBdDVEWkhJSlJGNU9sS0JjNnJWS3Zocmdz?=\n\t=?gb2312?b?N1VyV2ZIS0J4L3h5M3pKdTJWUmo2d3dFVk9FSFd2NW1tRUpVemhG?=\n\t=?gb2312?b?NVNLcUtjbis4VmRtQ2NubGMvS1JPaEhqSm0rWU4yNHBpZVd6WGZI?=\n\t=?gb2312?b?V2lDOVpGT2hYMVBCdVdBaW9NS0xvOWpHMWdJUklEaUV6MjJ6bm9O?=\n\t=?gb2312?b?MXRKcXlRQ1lhMnNjVGg4d0xOSnVYbU5pRG15SnI3ejRzMzd6UW9W?=\n\t=?gb2312?b?TWovSVhyaWlDTWJUVFI5ejhJZHlESXdVd2dEbHZxZ0t0dGVuUFJn?=\n\t=?gb2312?b?eDFWY0tNUXRLRHRqaFNNL3k4d3lFNm5BNEIzeGY1RVF3VlU4bFA3?=\n\t=?gb2312?b?c0l5c0JGVHpCOEw=?=","x-forefront-antispam-report":"CIP:255.255.255.255; CTRY:; LANG:zh-cn; SCL:1; \n\tSRV:; IPV:NLI; SFV:NSPM;\n\tH:PAXPR04MB8285.eurprd04.prod.outlook.com; PTR:; \n\tCAT:NONE;\n\tSFS:(13230040)(366016)(1800799024)(376014)(7053199007)(38070700018);\n\tDIR:OUT; SFP:1101; ","x-ms-exchange-antispam-messagedata-chunkcount":"1","x-ms-exchange-antispam-messagedata-0":"=?gb2312?b?L1BqMjFPM3J3QUlUUDNjVkNs?=\n\t=?gb2312?b?ZUJyODhMTnpOQStZMGdsZ0NhODBRWHNJbHlTbFUxakZ6Vk9pUitO?=\n\t=?gb2312?b?UkhSR0hLV0tlOFJoRGs1aWFPYTNrRjErbjdYbm1neUJuNGNIUnVn?=\n\t=?gb2312?b?NVl6TlVEaW5XN2l2WWxCYnNGV2d3N1pGbWd0Y0xocjlQWGIvcXIw?=\n\t=?gb2312?b?Tmg0R0k0TCtBdEh0SDBDaDlyWHMzSyt3cmM0YnljaUFkWUcvYUdQ?=\n\t=?gb2312?b?UURrd2hSM0tkQzJjYjZwRUZ4WWZreER0SFEzb25XWExBeUpBaU5S?=\n\t=?gb2312?b?YXMzL1RCL25BZjljWU9JcXlFREFPZFdqU24rRGlvdkNLV3hLTTVa?=\n\t=?gb2312?b?dVpkMnl6UXlOZ09pMGVsS1Vndll1Ti9WbU5JYk9TVlBFaDJkZTlm?=\n\t=?gb2312?b?YVNmWXNuSVUxN3JQKy8xVGZKNGJqdDNnVHcxempucjUxZUtOdi9n?=\n\t=?gb2312?b?czJFTmwrL2kxcUtQb2hxNFE3d3l3ajN2VmRocTlEc2dFcXFUZVJZ?=\n\t=?gb2312?b?MVNJcjVzQ2ZCVHF5dU5IdHZKQ3VrTDNkcmxxMzFBdi9OWU9Ud0xz?=\n\t=?gb2312?b?c1g1SWg0NXB1WUNMVWw5UzYvcFg5dGh3VEJiTnJKYTQyU3VadVRz?=\n\t=?gb2312?b?VnpJREF1WGdLMnVVOGlxQnRRZm9ocGx0Z1JDMVY0S244REZBRXRG?=\n\t=?gb2312?b?V0RBYS8vMnhNV3ltTVNQQktZSFRJVTVUaDlnRzdqTUxiOS9KaGJO?=\n\t=?gb2312?b?ZUl4Smxac050a3FNelpqQ0tNKzM2MlZnK0FWZGFjaExvaXJVdC9s?=\n\t=?gb2312?b?OGZsWWY1QVR5WTdYMXB0RkZUV3p2cXE4R3d0R2piMXRvSkc0eCtN?=\n\t=?gb2312?b?NmhmQTV2cUVMUytDU3hob0xGdjZQK29BMklJTFNmTEFiSEdoTERn?=\n\t=?gb2312?b?S0R6WDBRLzQzSUp0bXlWSGMxOVBZKzhDSG5ORENCaHBiTFdNYlhZ?=\n\t=?gb2312?b?QWRrUFZkK2pyV2JrcWplTXVIa1lFOXdLNFNGZG9ic25XQzhGaHQv?=\n\t=?gb2312?b?Q1lrSmo2WEU1ZEVFbXROU0NNNUZqb1UzcHpWcEdNWFVnaWlVQVVB?=\n\t=?gb2312?b?clRsMkVycnNJbEtnTXI5M2RMWmRLTzFXVXVFZURLWlZ1WW5Kd3ds?=\n\t=?gb2312?b?K3ZBZmd3WFFrNUNXUmlVcE0weUtadDRJbG01OWRLcWw1bmQvMWtE?=\n\t=?gb2312?b?WlpDT3hiM3o5U1cyYjNpOEtLMk9nbS91aVVyWmVucFVmb1d3UDlp?=\n\t=?gb2312?b?dDF6WWxvbnJnMUJaSnhvR2FyY2l3bTZsWm90M2tvcWV6ZmYxZTY1?=\n\t=?gb2312?b?cmwzUkFveWZkYklzazBJNjhUYXRFYVIrWXlKRjBzTHQvbmlUUEhs?=\n\t=?gb2312?b?R1VIYlA1dEN2b0daQTZuNFlLZUhlejRHdnlnNXFMOG9lZVkwbDVJ?=\n\t=?gb2312?b?OXhnUjl3SFBUWUE4Q2JMQlVveU1iVWpIRGQ2eWVlYnluVHNmTTdY?=\n\t=?gb2312?b?cWJZaUNOU1hodXJySDc0SFUrWXY3aTJhajllM3QwRmFkWmp5cDNm?=\n\t=?gb2312?b?UVc5ZUZOUGI0ak5sRFZhUTNxdmtTdnhUbUpFRkcydjhybXNtL3VT?=\n\t=?gb2312?b?U1RxTHkzQXJMY2xyUjhWVkdiK2lxWXBnNEZYS1RFd01WYjEzVndP?=\n\t=?gb2312?b?Z0FTbjFsdlFjTEZKNlV6enZFWVN4ODRVUzdNOERpMjZYbUJQZUM2?=\n\t=?gb2312?b?SUR4aXRKWmJFVkFyN0tHVFcrTFF0N2ZnRnhLY0F4M3BCaVMya2hW?=\n\t=?gb2312?b?TklLK2VKTnZmTDVGSytoUTdHbzF4RG1OcFNJWkhFOXhRMXFPVGF5?=\n\t=?gb2312?b?bkRzQ2NBTlZjWWhxVjI2Slp2aDlUUGQvcHozSWRBUGFIZzRWQ0RZ?=\n\t=?gb2312?b?Zk5wdCs0b3ViZXR3UmNNWHl2TGF0UUp3SDc4WHFwdjdrQ2Nhc0VF?=\n\t=?gb2312?b?Y0diaUZRVzZ3QnFqbm9ZNXRweXNpQ05aU09oeTlZTWNqK01xcHlX?=\n\t=?gb2312?b?WDgrTEp0TUh4QnA4UUxtZVE0VmZVQjQrcHdCNzFBazRoQ2l2SEh1?=\n\t=?gb2312?b?Y1ZFeHRyYU1WQy9MMklucnhJVEdjS1pMdFhzNkNHRDdoMEQvajU5?=\n\t=?gb2312?b?N24zMmxVZkVhU2s1WWYvNCtRYlBxdmwwVmtQV09mVzZzdHpic2hu?=\n\t=?gb2312?b?eGJRR1RUeld4UXdwQk96WStCK0NKZ0NETks3UkJ2bE9WV3pwNUJq?=\n\t=?gb2312?b?Wk1EWkNLSnMwOXp1QjB6dG5nUE9aa3JUYzRaMHQxUy9uYS9nODcy?=\n\t=?gb2312?b?Yz0=?=","Content-Type":"text/plain; charset=\"gb2312\"","Content-Transfer-Encoding":"base64","MIME-Version":"1.0","X-OriginatorOrg":"nxp.com","X-MS-Exchange-CrossTenant-AuthAs":"Internal","X-MS-Exchange-CrossTenant-AuthSource":"PAXPR04MB8285.eurprd04.prod.outlook.com","X-MS-Exchange-CrossTenant-Network-Message-Id":"a55c9683-f577-4433-a513-08dd763c4bd3","X-MS-Exchange-CrossTenant-originalarrivaltime":"08 Apr 2025 01:25:49.8608\n\t(UTC)","X-MS-Exchange-CrossTenant-fromentityheader":"Hosted","X-MS-Exchange-CrossTenant-id":"686ea1d3-bc2b-4c6f-a92c-d99c5c301635","X-MS-Exchange-CrossTenant-mailboxtype":"HOSTED","X-MS-Exchange-CrossTenant-userprincipalname":"ezPEBJQl2WaevWq4SNA/8DOYYKvK7aB7sqZpIs7IxNJFg75QgtVSx3YpDohOtlKd","X-MS-Exchange-Transport-CrossTenantHeadersStamped":"PAXPR04MB8491","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":34017,"web_url":"https://patchwork.libcamera.org/comment/34017/","msgid":"<174541334067.2882969.6331976171380887482@ping.linuxembedded.co.uk>","date":"2025-04-23T13:02:20","subject":"RE: [PATCH v6] gstreamer: Add GstVideoMeta support","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Qi,\n\nQuoting Qi Hou (2025-04-08 02:25:49)\n> \"/tmp/wn36eqf0\" may be a binary file.  See it anyway? \n\nHrm, my mail client doesn't like your reply. Fortunately patchwork has a\ncopy I can read!\n\nhttps://patchwork.libcamera.org/patch/22157/#33928:\n\n> Hi Nicolas,\n> \n> Can you help continue to review v6? I have made the code change\n> according to your comments of v5. Look forward to your reply.\n> \n> Regards, Qi Hou\n\nI'll add Nicolas on Cc, he may not have seen your question as you didn't\nsend the mail to him.\n\n\nQi, do you have any suggestion for testing or how we check on how this\nis used as I asked in https://patchwork.libcamera.org/patch/22157/#32560\n?\n> How do we test this patch ?\n> (How do you use it ?)\n> \n> I'm not sure if the existing unit tests are going to be confirming this\n> ? Is there anything we need to do to make sure this doesn't break\n> something else?\n\nIt's a huge chunk of changes, I'd like to know how we can validate/test\nthis?\n\n--\nKieran","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 8E873BE08B\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 23 Apr 2025 13:02:25 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A644868ACA;\n\tWed, 23 Apr 2025 15:02:24 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 53AC768AC7\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 23 Apr 2025 15:02:23 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id DEAB622A;\n\tWed, 23 Apr 2025 15:02:21 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"YRl4bm3C\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1745413341;\n\tbh=nx9m6KR2KaL7MFOe/gVF1Y4CsxSS907/7cw7AS+E8XI=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=YRl4bm3CfGSf12v+OOTdHT/2llWk+JqghGW05PhTOqd9cCHbGcgJ1FnNijSRO171w\n\tqB9R29ZRRz8GH+AbQD6UTEwRfDJqUtsJoHFJxwRhM07KjSxq0Joe8WLXoRbmx0bHq0\n\tpKnh8cJIl+T984JHNhTUmUwU882QaIDYan4lf/x4=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<PAXPR04MB828522C4E042B2714683A34797B52@PAXPR04MB8285.eurprd04.prod.outlook.com>","References":"<20241204085511.1733434-1-qi.hou@nxp.com>\n\t<PAXPR04MB828522C4E042B2714683A34797B52@PAXPR04MB8285.eurprd04.prod.outlook.com>","Subject":"RE: [PATCH v6] gstreamer: Add GstVideoMeta support","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"Jared Hu <jared.hu@nxp.com>,\n\tJulien Vuillaumier <julien.vuillaumier@nxp.com>","To":"qi.hou@nxp.com, Nicolas Dufresne <nicolas@ndufresne.ca>,\n\tlibcamera-devel@lists.libcamera.org, ","Date":"Wed, 23 Apr 2025 14:02:20 +0100","Message-ID":"<174541334067.2882969.6331976171380887482@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":34200,"web_url":"https://patchwork.libcamera.org/comment/34200/","msgid":"<174704953076.1952545.3171535624022805043@ping.linuxembedded.co.uk>","date":"2025-05-12T11:32:10","subject":"Re: [PATCH v6] gstreamer: Add GstVideoMeta support","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Hou Qi (2024-12-04 08:55:11)\n> GStreamer video-info calculated stride and offset may differ from\n> those used by the camera.\n> \n> For stride and offset mismatch, this patch adds video meta to buffer\n> if downstream supports VideoMeta through allocation query. Otherwise,\n> create a internal VideoPool using the caps, and copy video frame to\n> this system memory.\n\nThis has passed CI:\n\n- https://gitlab.freedesktop.org/camera/libcamera/-/pipelines/1323117\n\n\n> \n> Signed-off-by: Hou Qi <qi.hou@nxp.com>\n> ---\n>  src/gstreamer/gstlibcamera-utils.cpp |  33 +++++++\n>  src/gstreamer/gstlibcamera-utils.h   |   4 +\n>  src/gstreamer/gstlibcamerapad.cpp    |  31 +++++++\n>  src/gstreamer/gstlibcamerapad.h      |   8 ++\n>  src/gstreamer/gstlibcamerapool.cpp   |  18 +++-\n>  src/gstreamer/gstlibcamerapool.h     |   3 +-\n>  src/gstreamer/gstlibcamerasrc.cpp    | 133 ++++++++++++++++++++++++++-\n>  7 files changed, 226 insertions(+), 4 deletions(-)\n> \n> diff --git a/src/gstreamer/gstlibcamera-utils.cpp b/src/gstreamer/gstlibcamera-utils.cpp\n> index 732987ef..09af9204 100644\n> --- a/src/gstreamer/gstlibcamera-utils.cpp\n> +++ b/src/gstreamer/gstlibcamera-utils.cpp\n> @@ -591,6 +591,39 @@ gst_task_resume(GstTask *task)\n>  }\n>  #endif\n>  \n> +#if !GST_CHECK_VERSION(1, 22, 0)\n> +/*\n> + * Copyright (C) <1999> Erik Walthinsen <omega@cse.ogi.edu>\n> + * Library       <2002> Ronald Bultje <rbultje@ronald.bitfreak.net>\n> + * Copyright (C) <2007> David A. Schleef <ds@schleef.org>\n> + */\n> +/* This function has been imported directly from the gstreamer project to\n> + * support backwards compatibility and should be removed when the older\n> + * version is no longer supported. */\n> +gint gst_video_format_info_extrapolate_stride(const GstVideoFormatInfo *finfo, gint plane, gint stride)\n> +{\n> +       gint estride;\n> +       gint comp[GST_VIDEO_MAX_COMPONENTS];\n> +       gint i;\n> +\n> +       /* there is nothing to extrapolate on first plane */\n> +       if (plane == 0)\n> +               return stride;\n> +\n> +       gst_video_format_info_component(finfo, plane, comp);\n> +\n> +       /* For now, all planar formats have a single component on first plane, but\n> +       * if there was a planar format with more, we'd have to make a ratio of the\n> +       * number of component on the first plane against the number of component on\n> +       * the current plane. */\n> +       estride = 0;\n> +       for (i = 0; i < GST_VIDEO_MAX_COMPONENTS && comp[i] >= 0; i++)\n> +               estride += GST_VIDEO_FORMAT_INFO_SCALE_WIDTH(finfo, comp[i], stride);\n> +\n> +       return estride;\n> +}\n> +#endif\n> +\n\nAll looking good there...\n\n>  G_LOCK_DEFINE_STATIC(cm_singleton_lock);\n>  static std::weak_ptr<CameraManager> cm_singleton_ptr;\n>  \n> diff --git a/src/gstreamer/gstlibcamera-utils.h b/src/gstreamer/gstlibcamera-utils.h\n> index cab1c814..81149280 100644\n> --- a/src/gstreamer/gstlibcamera-utils.h\n> +++ b/src/gstreamer/gstlibcamera-utils.h\n> @@ -35,6 +35,10 @@ static inline void gst_clear_event(GstEvent **event_ptr)\n>  #if !GST_CHECK_VERSION(1, 17, 1)\n>  gboolean gst_task_resume(GstTask *task);\n>  #endif\n> +\n> +#if !GST_CHECK_VERSION(1, 22, 0)\n> +gint gst_video_format_info_extrapolate_stride(const GstVideoFormatInfo *finfo, gint plane, gint stride);\n> +#endif\n>  std::shared_ptr<libcamera::CameraManager> gst_libcamera_get_camera_manager(int &ret);\n>  \n>  /**\n> diff --git a/src/gstreamer/gstlibcamerapad.cpp b/src/gstreamer/gstlibcamerapad.cpp\n> index 7b22aebe..7fd11a8f 100644\n> --- a/src/gstreamer/gstlibcamerapad.cpp\n> +++ b/src/gstreamer/gstlibcamerapad.cpp\n> @@ -18,6 +18,8 @@ struct _GstLibcameraPad {\n>         GstPad parent;\n>         StreamRole role;\n>         GstLibcameraPool *pool;\n> +       GstBufferPool *video_pool;\n> +       GstVideoInfo info;\n\nThis looks like we are now 'per pad' as requested.\n\n>         GstClockTime latency;\n>  };\n>  \n> @@ -153,6 +155,35 @@ gst_libcamera_pad_set_pool(GstPad *pad, GstLibcameraPool *pool)\n>         self->pool = pool;\n>  }\n>  \n> +GstBufferPool *\n> +gst_libcamera_pad_get_video_pool(GstPad *pad)\n> +{\n> +       auto *self = GST_LIBCAMERA_PAD(pad);\n> +       return self->video_pool;\n> +}\n> +\n> +void gst_libcamera_pad_set_video_pool(GstPad *pad, GstBufferPool *video_pool)\n> +{\n> +       auto *self = GST_LIBCAMERA_PAD(pad);\n> +\n> +       if (self->video_pool)\n> +               g_object_unref(self->video_pool);\n> +       self->video_pool = video_pool;\n> +}\n> +\n> +GstVideoInfo gst_libcamera_pad_get_video_info(GstPad *pad)\n> +{\n> +       auto *self = GST_LIBCAMERA_PAD(pad);\n> +       return self->info;\n> +}\n> +\n> +void gst_libcamera_pad_set_video_info(GstPad *pad, GstVideoInfo info)\n> +{\n> +       auto *self = GST_LIBCAMERA_PAD(pad);\n> +\n> +       self->info = info;\n> +}\n> +\n>  Stream *\n>  gst_libcamera_pad_get_stream(GstPad *pad)\n>  {\n> diff --git a/src/gstreamer/gstlibcamerapad.h b/src/gstreamer/gstlibcamerapad.h\n> index 630c168a..ed0beb1f 100644\n> --- a/src/gstreamer/gstlibcamerapad.h\n> +++ b/src/gstreamer/gstlibcamerapad.h\n> @@ -23,6 +23,14 @@ GstLibcameraPool *gst_libcamera_pad_get_pool(GstPad *pad);\n>  \n>  void gst_libcamera_pad_set_pool(GstPad *pad, GstLibcameraPool *pool);\n>  \n> +GstBufferPool *gst_libcamera_pad_get_video_pool(GstPad *pad);\n> +\n> +void gst_libcamera_pad_set_video_pool(GstPad *pad, GstBufferPool *video_pool);\n> +\n> +GstVideoInfo gst_libcamera_pad_get_video_info(GstPad *pad);\n> +\n> +void gst_libcamera_pad_set_video_info(GstPad *pad, GstVideoInfo info);\n> +\n>  libcamera::Stream *gst_libcamera_pad_get_stream(GstPad *pad);\n>  \n>  void gst_libcamera_pad_set_latency(GstPad *pad, GstClockTime latency);\n> diff --git a/src/gstreamer/gstlibcamerapool.cpp b/src/gstreamer/gstlibcamerapool.cpp\n> index 9cd7eccb..471f71da 100644\n> --- a/src/gstreamer/gstlibcamerapool.cpp\n> +++ b/src/gstreamer/gstlibcamerapool.cpp\n> @@ -14,6 +14,9 @@\n>  \n>  #include \"gstlibcamera-utils.h\"\n>  \n> +#include <gst/gstmeta.h>\n> +\n> +\n>  using namespace libcamera;\n>  \n>  enum {\n> @@ -134,8 +137,20 @@ gst_libcamera_pool_class_init(GstLibcameraPoolClass *klass)\n>                                                      G_TYPE_NONE, 0);\n>  }\n>  \n> +static void\n> +gst_libcamera_buffer_add_video_meta(GstBuffer *buffer, GstVideoInfo *info)\n> +{\n> +       GstVideoMeta *vmeta;\n> +       vmeta = gst_buffer_add_video_meta_full(buffer, GST_VIDEO_FRAME_FLAG_NONE,\n> +                                              GST_VIDEO_INFO_FORMAT(info), GST_VIDEO_INFO_WIDTH(info),\n> +                                              GST_VIDEO_INFO_HEIGHT(info), GST_VIDEO_INFO_N_PLANES(info),\n> +                                              info->offset, info->stride);\n> +       GST_META_FLAGS(vmeta) = (GstMetaFlags)(GST_META_FLAGS(vmeta) | GST_META_FLAG_POOLED);\n> +}\n\nThis looks like it satisfies the review comment in v5.\n\n> +\n>  GstLibcameraPool *\n> -gst_libcamera_pool_new(GstLibcameraAllocator *allocator, Stream *stream)\n> +gst_libcamera_pool_new(GstLibcameraAllocator *allocator, Stream *stream,\n> +                      GstVideoInfo *info)\n>  {\n\nI can see this was simplfied as requested.\n\n\n>         auto *pool = GST_LIBCAMERA_POOL(g_object_new(GST_TYPE_LIBCAMERA_POOL, nullptr));\n>  \n> @@ -145,6 +160,7 @@ gst_libcamera_pool_new(GstLibcameraAllocator *allocator, Stream *stream)\n>         gsize pool_size = gst_libcamera_allocator_get_pool_size(allocator, stream);\n>         for (gsize i = 0; i < pool_size; i++) {\n>                 GstBuffer *buffer = gst_buffer_new();\n> +               gst_libcamera_buffer_add_video_meta(buffer, info);\n>                 pool->queue->push_back(buffer);\n>         }\n>  \n> diff --git a/src/gstreamer/gstlibcamerapool.h b/src/gstreamer/gstlibcamerapool.h\n> index 2a7a9c77..02ee4dd4 100644\n> --- a/src/gstreamer/gstlibcamerapool.h\n> +++ b/src/gstreamer/gstlibcamerapool.h\n> @@ -14,6 +14,7 @@\n>  #include \"gstlibcameraallocator.h\"\n>  \n>  #include <gst/gst.h>\n> +#include <gst/video/video.h>\n>  \n>  #include <libcamera/stream.h>\n>  \n> @@ -21,7 +22,7 @@\n>  G_DECLARE_FINAL_TYPE(GstLibcameraPool, gst_libcamera_pool, GST_LIBCAMERA, POOL, GstBufferPool)\n>  \n>  GstLibcameraPool *gst_libcamera_pool_new(GstLibcameraAllocator *allocator,\n> -                                        libcamera::Stream *stream);\n> +                                        libcamera::Stream *stream, GstVideoInfo *info);\n>  \n>  libcamera::Stream *gst_libcamera_pool_get_stream(GstLibcameraPool *self);\n>  \n> diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp\n> index 8efa25f4..82b06eb4 100644\n> --- a/src/gstreamer/gstlibcamerasrc.cpp\n> +++ b/src/gstreamer/gstlibcamerasrc.cpp\n> @@ -268,6 +268,52 @@ GstLibcameraSrcState::requestCompleted(Request *request)\n>         gst_task_resume(src_->task);\n>  }\n>  \n> +static void\n> +gst_libcamera_extrapolate_info(GstVideoInfo *info, guint32 stride)\n> +{\n> +       guint i, estride;\n> +       gsize offset = 0;\n> +\n> +       /* this should be updated if tiled formats get added in the future. */\n> +       for (i = 0; i < GST_VIDEO_INFO_N_PLANES(info); i++) {\n> +               estride = gst_video_format_info_extrapolate_stride(info->finfo, i, stride);\n> +               info->stride[i] = estride;\n> +               info->offset[i] = offset;\n> +               offset += estride * GST_VIDEO_FORMAT_INFO_SCALE_HEIGHT(info->finfo, i,\n> +                                                                      GST_VIDEO_INFO_HEIGHT(info));\n> +       }\n> +}\n> +\n> +static GstFlowReturn\n> +gst_libcamera_video_frame_copy(GstBuffer *src, GstBuffer *dest, GstVideoInfo dest_info, guint32 stride)\n> +{\n> +       GstVideoInfo src_info = dest_info;\n> +       GstVideoFrame src_frame, dest_frame;\n> +\n> +       gst_libcamera_extrapolate_info(&src_info, stride);\n> +       src_info.size = gst_buffer_get_size(src);\n> +\n> +       if (!gst_video_frame_map(&src_frame, &src_info, src, GST_MAP_READ))\n> +               goto invalid_buffer;\n> +\n> +       if (!gst_video_frame_map(&dest_frame, &dest_info, dest, GST_MAP_WRITE)) {\n> +               gst_video_frame_unmap(&src_frame);\n> +               goto invalid_buffer;\n> +       }\n> +\n> +       gst_video_frame_copy(&dest_frame, &src_frame);\n> +\n> +       gst_video_frame_unmap(&src_frame);\n> +       gst_video_frame_unmap(&dest_frame);\n> +\n> +       return GST_FLOW_OK;\n> +\n> +invalid_buffer : {\n> +       GST_ERROR(\"Could not map buffer\");\n> +       return GST_FLOW_ERROR;\n> +}\n\nWhy do we have these extra { } here ? Can't they be removed?\n\n> +}\n> +\n>  /* Must be called with stream_lock held. */\n>  int GstLibcameraSrcState::processRequest()\n>  {\n> @@ -292,11 +338,36 @@ int GstLibcameraSrcState::processRequest()\n>         GstFlowReturn ret = GST_FLOW_OK;\n>         gst_flow_combiner_reset(src_->flow_combiner);\n>  \n> -       for (GstPad *srcpad : srcpads_) {\n> +       for (gsize i = 0; i < src_->state->srcpads_.size(); i++) {\n> +               GstPad *srcpad = src_->state->srcpads_[i];\n>                 Stream *stream = gst_libcamera_pad_get_stream(srcpad);\n>                 GstBuffer *buffer = wrap->detachBuffer(stream);\n>  \n>                 FrameBuffer *fb = gst_libcamera_buffer_get_frame_buffer(buffer);\n> +               const StreamConfiguration &stream_cfg = src_->state->config_->at(i);\n> +               GstBufferPool *video_pool = gst_libcamera_pad_get_video_pool(srcpad);\n> +\n> +               if (video_pool) {\n> +                       GstBuffer *copy = NULL;\n\nI can see tmp is now copy which looks fine to me.\n\n\nAs far as I can tell - previous review comments were handled.\n\nI think I saw a request to reduce the indentations here - which might\nhelp. I'm not sure what to suggest specifically ... so I'll put this\nhere as I think it would be good to see this progress:\n\n\nReviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n\n\nNicolas - is there anything else here you want to see fixed ?\n\n\n\n> +                       GstVideoInfo info = gst_libcamera_pad_get_video_info(srcpad);\n> +\n> +                       ret = gst_buffer_pool_acquire_buffer(video_pool, &copy, NULL);\n> +                       if (ret != GST_FLOW_OK) {\n> +                               GST_ERROR(\"Failed to acquire buffer\");\n> +                               gst_buffer_unref(buffer);\n> +                               return -EPIPE;\n> +                       }\n> +\n> +                       ret = gst_libcamera_video_frame_copy(buffer, copy, info, stream_cfg.stride);\n> +                       gst_buffer_unref(buffer);\n> +                       if (ret != GST_FLOW_OK) {\n> +                               GST_ERROR(\"Failed to copy buffer\");\n> +                               gst_buffer_unref(copy);\n> +                               return -EPIPE;\n> +                       }\n> +\n> +                       buffer = copy;\n> +               }\n>  \n>                 if (GST_CLOCK_TIME_IS_VALID(wrap->pts_)) {\n>                         GST_BUFFER_PTS(buffer) = wrap->pts_;\n> @@ -497,13 +568,65 @@ gst_libcamera_src_negotiate(GstLibcameraSrc *self)\n>         for (gsize i = 0; i < state->srcpads_.size(); i++) {\n>                 GstPad *srcpad = state->srcpads_[i];\n>                 const StreamConfiguration &stream_cfg = state->config_->at(i);\n> +               GstBufferPool *video_pool = NULL;\n> +               GstVideoInfo info;\n> +\n> +               g_autoptr(GstCaps) caps = gst_libcamera_stream_configuration_to_caps(stream_cfg);\n> +\n> +               gst_video_info_init(&info);\n> +               gst_video_info_from_caps(&info, caps);\n> +               gst_libcamera_pad_set_video_info(srcpad, info);\n> +\n> +               /* stride mismatch between camera stride and that calculated by video-info */\n> +               if (static_cast<unsigned int>(info.stride[0]) != stream_cfg.stride &&\n> +                   GST_VIDEO_INFO_FORMAT(&info) != GST_VIDEO_FORMAT_ENCODED) {\n> +                       GstQuery *query = NULL;\n> +                       gboolean need_pool = true;\n> +                       gboolean has_video_meta = false;\n> +\n> +                       gst_libcamera_extrapolate_info(&info, stream_cfg.stride);\n> +\n> +                       query = gst_query_new_allocation(caps, need_pool);\n> +                       if (!gst_pad_peer_query(srcpad, query))\n> +                               GST_DEBUG_OBJECT(self, \"Didn't get downstream ALLOCATION hints\");\n> +                       else\n> +                               has_video_meta = gst_query_find_allocation_meta(query, GST_VIDEO_META_API_TYPE, NULL);\n> +\n> +                       if (!has_video_meta) {\n> +                               GstBufferPool *pool = NULL;\n> +\n> +                               if (gst_query_get_n_allocation_pools(query) > 0)\n> +                                       gst_query_parse_nth_allocation_pool(query, 0, &pool, NULL, NULL, NULL);\n> +\n> +                               if (pool)\n> +                                       video_pool = pool;\n> +                               else {\n> +                                       GstStructure *config;\n> +                                       guint min_buffers = 3;\n> +                                       video_pool = gst_video_buffer_pool_new();\n> +\n> +                                       config = gst_buffer_pool_get_config(video_pool);\n> +                                       gst_buffer_pool_config_set_params(config, caps, info.size, min_buffers, 0);\n> +                                       gst_buffer_pool_set_config(GST_BUFFER_POOL_CAST(video_pool), config);\n> +                                       GST_DEBUG_OBJECT(self, \"Own pool config is %\" GST_PTR_FORMAT, config);\n> +                               }\n> +\n> +                               if (!gst_buffer_pool_set_active(video_pool, true)) {\n> +                                       GST_ERROR_OBJECT(self, \"Failed to active buffer pool\");\n> +                                       gst_caps_unref(caps);\n> +                                       return false;\n> +                               }\n> +                       }\n> +                       gst_query_unref(query);\n> +               }\n>  \n>                 GstLibcameraPool *pool = gst_libcamera_pool_new(self->allocator,\n> -                                                               stream_cfg.stream());\n> +                                                               stream_cfg.stream(), &info);\n>                 g_signal_connect_swapped(pool, \"buffer-notify\",\n>                                          G_CALLBACK(gst_task_resume), self->task);\n>  \n>                 gst_libcamera_pad_set_pool(srcpad, pool);\n> +               gst_libcamera_pad_set_video_pool(srcpad, video_pool);\n>  \n>                 /* Clear all reconfigure flags. */\n>                 gst_pad_check_reconfigure(srcpad);\n> @@ -920,6 +1043,12 @@ gst_libcamera_src_release_pad(GstElement *element, GstPad *pad)\n>                 auto end_iterator = pads.end();\n>                 auto pad_iterator = std::find(begin_iterator, end_iterator, pad);\n>  \n> +               GstBufferPool *video_pool = gst_libcamera_pad_get_video_pool(pad);\n> +               if (video_pool) {\n> +                       gst_buffer_pool_set_active(video_pool, false);\n> +                       gst_object_unref(video_pool);\n> +               }\n> +\n>                 if (pad_iterator != end_iterator) {\n>                         g_object_unref(*pad_iterator);\n>                         pads.erase(pad_iterator);\n> -- \n> 2.34.1\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id DB645C3200\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 12 May 2025 11:32:16 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1C9D46175C;\n\tMon, 12 May 2025 13:32:16 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3C200614E3\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 12 May 2025 13:32:14 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 2F56D6AF;\n\tMon, 12 May 2025 13:31:59 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"UWa+MQB9\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1747049519;\n\tbh=l8b3Y+XwWlaZFnFDY/L/kmXFxGJPCyIiwJlQFUvwl7M=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=UWa+MQB9qroQakSyh9z8vNqkTcjrVCxqLP+VWnwddQfLFuKifkarLZGZ6t7C/B/xY\n\t9KGSesb0bOaTcJAaSLBUC0oEQ9N8uK9AQyGtwyqlKhKC6Zl7ITDMfj97vXAg7QOwN/\n\tZ5Z/Ka06Kv+HbZfwvkLhAkxHfysqdJZquqxxMZl0=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20241204085511.1733434-1-qi.hou@nxp.com>","References":"<20241204085511.1733434-1-qi.hou@nxp.com>","Subject":"Re: [PATCH v6] gstreamer: Add GstVideoMeta support","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"jared.hu@nxp.com, qi.hou@nxp.com, julien.vuillaumier@nxp.com","To":"Hou Qi <qi.hou@nxp.com>, libcamera-devel@lists.libcamera.org","Date":"Mon, 12 May 2025 12:32:10 +0100","Message-ID":"<174704953076.1952545.3171535624022805043@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":34207,"web_url":"https://patchwork.libcamera.org/comment/34207/","msgid":"<f93c135d178d3869c8bfc842309ee63ae2907f99.camel@ndufresne.ca>","date":"2025-05-12T21:05:36","subject":"Re: [PATCH v6] gstreamer: Add GstVideoMeta support","submitter":{"id":30,"url":"https://patchwork.libcamera.org/api/people/30/","name":"Nicolas Dufresne","email":"nicolas@ndufresne.ca"},"content":"Hi,\n\nLe mercredi 04 décembre 2024 à 17:55 +0900, Hou Qi a écrit :\n> GStreamer video-info calculated stride and offset may differ from\n> those used by the camera.\n> \n> For stride and offset mismatch, this patch adds video meta to buffer\n> if downstream supports VideoMeta through allocation query. Otherwise,\n> create a internal VideoPool using the caps, and copy video frame to\n> this system memory.\n> \n> Signed-off-by: Hou Qi <qi.hou@nxp.com>\n> ---\n>  src/gstreamer/gstlibcamera-utils.cpp |  33 +++++++\n>  src/gstreamer/gstlibcamera-utils.h   |   4 +\n>  src/gstreamer/gstlibcamerapad.cpp    |  31 +++++++\n>  src/gstreamer/gstlibcamerapad.h      |   8 ++\n>  src/gstreamer/gstlibcamerapool.cpp   |  18 +++-\n>  src/gstreamer/gstlibcamerapool.h     |   3 +-\n>  src/gstreamer/gstlibcamerasrc.cpp    | 133 ++++++++++++++++++++++++++-\n>  7 files changed, 226 insertions(+), 4 deletions(-)\n> \n> diff --git a/src/gstreamer/gstlibcamera-utils.cpp b/src/gstreamer/gstlibcamera-utils.cpp\n> index 732987ef..09af9204 100644\n> --- a/src/gstreamer/gstlibcamera-utils.cpp\n> +++ b/src/gstreamer/gstlibcamera-utils.cpp\n> @@ -591,6 +591,39 @@ gst_task_resume(GstTask *task)\n>  }\n>  #endif\n>  \n> +#if !GST_CHECK_VERSION(1, 22, 0)\n> +/*\n> + * Copyright (C) <1999> Erik Walthinsen <omega@cse.ogi.edu>\n> + * Library       <2002> Ronald Bultje <rbultje@ronald.bitfreak.net>\n> + * Copyright (C) <2007> David A. Schleef <ds@schleef.org>\n> + */\n> +/* This function has been imported directly from the gstreamer project to\n> + * support backwards compatibility and should be removed when the older\n> + * version is no longer supported. */\n> +gint gst_video_format_info_extrapolate_stride(const GstVideoFormatInfo *finfo, gint plane, gint stride)\n> +{\n> +\tgint estride;\n> +\tgint comp[GST_VIDEO_MAX_COMPONENTS];\n> +\tgint i;\n> +\n> +\t/* there is nothing to extrapolate on first plane */\n> +\tif (plane == 0)\n> +\t\treturn stride;\n> +\n> +\tgst_video_format_info_component(finfo, plane, comp);\n> +\n> +\t/* For now, all planar formats have a single component on first plane, but\n> +\t* if there was a planar format with more, we'd have to make a ratio of the\n> +\t* number of component on the first plane against the number of component on\n> +\t* the current plane. */\n> +\testride = 0;\n> +\tfor (i = 0; i < GST_VIDEO_MAX_COMPONENTS && comp[i] >= 0; i++)\n> +\t\testride += GST_VIDEO_FORMAT_INFO_SCALE_WIDTH(finfo, comp[i], stride);\n> +\n> +\treturn estride;\n> +}\n> +#endif\n> +\n>  G_LOCK_DEFINE_STATIC(cm_singleton_lock);\n>  static std::weak_ptr<CameraManager> cm_singleton_ptr;\n>  \n> diff --git a/src/gstreamer/gstlibcamera-utils.h b/src/gstreamer/gstlibcamera-utils.h\n> index cab1c814..81149280 100644\n> --- a/src/gstreamer/gstlibcamera-utils.h\n> +++ b/src/gstreamer/gstlibcamera-utils.h\n> @@ -35,6 +35,10 @@ static inline void gst_clear_event(GstEvent **event_ptr)\n>  #if !GST_CHECK_VERSION(1, 17, 1)\n>  gboolean gst_task_resume(GstTask *task);\n>  #endif\n> +\n> +#if !GST_CHECK_VERSION(1, 22, 0)\n> +gint gst_video_format_info_extrapolate_stride(const GstVideoFormatInfo *finfo, gint plane, gint stride);\n> +#endif\n>  std::shared_ptr<libcamera::CameraManager> gst_libcamera_get_camera_manager(int &ret);\n\nnit: I'd like a blank line after endif\n\n>  \n>  /**\n> diff --git a/src/gstreamer/gstlibcamerapad.cpp b/src/gstreamer/gstlibcamerapad.cpp\n> index 7b22aebe..7fd11a8f 100644\n> --- a/src/gstreamer/gstlibcamerapad.cpp\n> +++ b/src/gstreamer/gstlibcamerapad.cpp\n> @@ -18,6 +18,8 @@ struct _GstLibcameraPad {\n>  \tGstPad parent;\n>  \tStreamRole role;\n>  \tGstLibcameraPool *pool;\n> +\tGstBufferPool *video_pool;\n> +\tGstVideoInfo info;\n>  \tGstClockTime latency;\n>  };\n>  \n> @@ -153,6 +155,35 @@ gst_libcamera_pad_set_pool(GstPad *pad, GstLibcameraPool *pool)\n>  \tself->pool = pool;\n>  }\n>  \n> +GstBufferPool *\n> +gst_libcamera_pad_get_video_pool(GstPad *pad)\n> +{\n> +\tauto *self = GST_LIBCAMERA_PAD(pad);\n> +\treturn self->video_pool;\n> +}\n> +\n> +void gst_libcamera_pad_set_video_pool(GstPad *pad, GstBufferPool *video_pool)\n> +{\n> +\tauto *self = GST_LIBCAMERA_PAD(pad);\n> +\n> +\tif (self->video_pool)\n> +\t\tg_object_unref(self->video_pool);\n> +\tself->video_pool = video_pool;\n> +}\n> +\n> +GstVideoInfo gst_libcamera_pad_get_video_info(GstPad *pad)\n> +{\n> +\tauto *self = GST_LIBCAMERA_PAD(pad);\n> +\treturn self->info;\n> +}\n> +\n> +void gst_libcamera_pad_set_video_info(GstPad *pad, GstVideoInfo info)\n\nVideoInfo, a large structure, is being copied twice, pass a const\nreference so you only copy it once.\n\n> +{\n> +\tauto *self = GST_LIBCAMERA_PAD(pad);\n> +\n> +\tself->info = info;\n> +}\n> +\n>  Stream *\n>  gst_libcamera_pad_get_stream(GstPad *pad)\n>  {\n> diff --git a/src/gstreamer/gstlibcamerapad.h b/src/gstreamer/gstlibcamerapad.h\n> index 630c168a..ed0beb1f 100644\n> --- a/src/gstreamer/gstlibcamerapad.h\n> +++ b/src/gstreamer/gstlibcamerapad.h\n> @@ -23,6 +23,14 @@ GstLibcameraPool *gst_libcamera_pad_get_pool(GstPad *pad);\n>  \n>  void gst_libcamera_pad_set_pool(GstPad *pad, GstLibcameraPool *pool);\n>  \n> +GstBufferPool *gst_libcamera_pad_get_video_pool(GstPad *pad);\n> +\n> +void gst_libcamera_pad_set_video_pool(GstPad *pad, GstBufferPool *video_pool);\n> +\n> +GstVideoInfo gst_libcamera_pad_get_video_info(GstPad *pad);\n> +\n> +void gst_libcamera_pad_set_video_info(GstPad *pad, GstVideoInfo info);\n> +\n>  libcamera::Stream *gst_libcamera_pad_get_stream(GstPad *pad);\n>  \n>  void gst_libcamera_pad_set_latency(GstPad *pad, GstClockTime latency);\n> diff --git a/src/gstreamer/gstlibcamerapool.cpp b/src/gstreamer/gstlibcamerapool.cpp\n> index 9cd7eccb..471f71da 100644\n> --- a/src/gstreamer/gstlibcamerapool.cpp\n> +++ b/src/gstreamer/gstlibcamerapool.cpp\n> @@ -14,6 +14,9 @@\n>  \n>  #include \"gstlibcamera-utils.h\"\n>  \n> +#include <gst/gstmeta.h>\n\nnit: gst/gst.h is sufficient and already included. n GStreamer we usually do\none header per library. Unlike C++ it has marginal effect on compilation time.\n\n> +\n> +\n>  using namespace libcamera;\n>  \n>  enum {\n> @@ -134,8 +137,20 @@ gst_libcamera_pool_class_init(GstLibcameraPoolClass *klass)\n>  \t\t\t\t\t\t     G_TYPE_NONE, 0);\n>  }\n>  \n> +static void\n> +gst_libcamera_buffer_add_video_meta(GstBuffer *buffer, GstVideoInfo *info)\n> +{\n> +\tGstVideoMeta *vmeta;\n> +\tvmeta = gst_buffer_add_video_meta_full(buffer, GST_VIDEO_FRAME_FLAG_NONE,\n> +\t\t\t\t\t       GST_VIDEO_INFO_FORMAT(info), GST_VIDEO_INFO_WIDTH(info),\n> +\t\t\t\t\t       GST_VIDEO_INFO_HEIGHT(info), GST_VIDEO_INFO_N_PLANES(info),\n> +\t\t\t\t\t       info->offset, info->stride);\n> +\tGST_META_FLAGS(vmeta) = (GstMetaFlags)(GST_META_FLAGS(vmeta) | GST_META_FLAG_POOLED);\n> +}\n> +\n>  GstLibcameraPool *\n> -gst_libcamera_pool_new(GstLibcameraAllocator *allocator, Stream *stream)\n> +gst_libcamera_pool_new(GstLibcameraAllocator *allocator, Stream *stream,\n> +\t\t       GstVideoInfo *info)\n>  {\n>  \tauto *pool = GST_LIBCAMERA_POOL(g_object_new(GST_TYPE_LIBCAMERA_POOL, nullptr));\n>  \n> @@ -145,6 +160,7 @@ gst_libcamera_pool_new(GstLibcameraAllocator *allocator, Stream *stream)\n>  \tgsize pool_size = gst_libcamera_allocator_get_pool_size(allocator, stream);\n>  \tfor (gsize i = 0; i < pool_size; i++) {\n>  \t\tGstBuffer *buffer = gst_buffer_new();\n> +\t\tgst_libcamera_buffer_add_video_meta(buffer, info);\n>  \t\tpool->queue->push_back(buffer);\n>  \t}\n>  \n> diff --git a/src/gstreamer/gstlibcamerapool.h b/src/gstreamer/gstlibcamerapool.h\n> index 2a7a9c77..02ee4dd4 100644\n> --- a/src/gstreamer/gstlibcamerapool.h\n> +++ b/src/gstreamer/gstlibcamerapool.h\n> @@ -14,6 +14,7 @@\n>  #include \"gstlibcameraallocator.h\"\n>  \n>  #include <gst/gst.h>\n> +#include <gst/video/video.h>\n>  \n>  #include <libcamera/stream.h>\n>  \n> @@ -21,7 +22,7 @@\n>  G_DECLARE_FINAL_TYPE(GstLibcameraPool, gst_libcamera_pool, GST_LIBCAMERA, POOL, GstBufferPool)\n>  \n>  GstLibcameraPool *gst_libcamera_pool_new(GstLibcameraAllocator *allocator,\n> -\t\t\t\t\t libcamera::Stream *stream);\n> +\t\t\t\t\t libcamera::Stream *stream, GstVideoInfo *info);\n>  \n>  libcamera::Stream *gst_libcamera_pool_get_stream(GstLibcameraPool *self);\n>  \n> diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp\n> index 8efa25f4..82b06eb4 100644\n> --- a/src/gstreamer/gstlibcamerasrc.cpp\n> +++ b/src/gstreamer/gstlibcamerasrc.cpp\n> @@ -268,6 +268,52 @@ GstLibcameraSrcState::requestCompleted(Request *request)\n>  \tgst_task_resume(src_->task);\n>  }\n>  \n> +static void\n> +gst_libcamera_extrapolate_info(GstVideoInfo *info, guint32 stride)\n> +{\n> +\tguint i, estride;\n> +\tgsize offset = 0;\n> +\n> +\t/* this should be updated if tiled formats get added in the future. */\n> +\tfor (i = 0; i < GST_VIDEO_INFO_N_PLANES(info); i++) {\n> +\t\testride = gst_video_format_info_extrapolate_stride(info->finfo, i, stride);\n> +\t\tinfo->stride[i] = estride;\n> +\t\tinfo->offset[i] = offset;\n> +\t\toffset += estride * GST_VIDEO_FORMAT_INFO_SCALE_HEIGHT(info->finfo, i,\n> +\t\t\t\t\t\t\t\t       GST_VIDEO_INFO_HEIGHT(info));\n> +\t}\n> +}\n> +\n> +static GstFlowReturn\n> +gst_libcamera_video_frame_copy(GstBuffer *src, GstBuffer *dest, GstVideoInfo dest_info, guint32 stride)\n\nWe usually avoid copying larger structure, even though valid. So instead we pass a const\npointer to VideoInfo.\n\n> +{\n> +\tGstVideoInfo src_info = dest_info;\n> +\tGstVideoFrame src_frame, dest_frame;\n> +\n> +\tgst_libcamera_extrapolate_info(&src_info, stride);\n> +\tsrc_info.size = gst_buffer_get_size(src);\n> +\n> +\tif (!gst_video_frame_map(&src_frame, &src_info, src, GST_MAP_READ))\n> +\t\tgoto invalid_buffer;\n> +\n> +\tif (!gst_video_frame_map(&dest_frame, &dest_info, dest, GST_MAP_WRITE)) {\n> +\t\tgst_video_frame_unmap(&src_frame);\n> +\t\tgoto invalid_buffer;\n> +\t}\n> +\n> +\tgst_video_frame_copy(&dest_frame, &src_frame);\n\nThis can fail, usually a programming error, but its nice to report it.\n\n> +\n> +\tgst_video_frame_unmap(&src_frame);\n> +\tgst_video_frame_unmap(&dest_frame);\n> +\n> +\treturn GST_FLOW_OK;\n> +\n> +invalid_buffer : {\n> +\tGST_ERROR(\"Could not map buffer\");\n> +\treturn GST_FLOW_ERROR;\n> +}\n> +}\n> +\n>  /* Must be called with stream_lock held. */\n>  int GstLibcameraSrcState::processRequest()\n>  {\n> @@ -292,11 +338,36 @@ int GstLibcameraSrcState::processRequest()\n>  \tGstFlowReturn ret = GST_FLOW_OK;\n>  \tgst_flow_combiner_reset(src_->flow_combiner);\n>  \n> -\tfor (GstPad *srcpad : srcpads_) {\n> +\tfor (gsize i = 0; i < src_->state->srcpads_.size(); i++) {\nWhy not just                  srcpads_.size() ?\n\nCause I think if \"this\" is not the same as src_->state, we have a problem.\n\n> +\t\tGstPad *srcpad = src_->state->srcpads_[i];\n\nidem.\n\n>  \t\tStream *stream = gst_libcamera_pad_get_stream(srcpad);\n>  \t\tGstBuffer *buffer = wrap->detachBuffer(stream);\n>  \n>  \t\tFrameBuffer *fb = gst_libcamera_buffer_get_frame_buffer(buffer);\n> +\t\tconst StreamConfiguration &stream_cfg = src_->state->config_->at(i);\n> +\t\tGstBufferPool *video_pool = gst_libcamera_pad_get_video_pool(srcpad);\n> +\n> +\t\tif (video_pool) {\n\nPerhaps a comment explaining that we only set a pool when a copy is needed ?\n\n> +\t\t\tGstBuffer *copy = NULL;\n> +\t\t\tGstVideoInfo info = gst_libcamera_pad_get_video_info(srcpad);\n> +\n> +\t\t\tret = gst_buffer_pool_acquire_buffer(video_pool, &copy, NULL);\n> +\t\t\tif (ret != GST_FLOW_OK) {\n> +\t\t\t\tGST_ERROR(\"Failed to acquire buffer\");\n> +\t\t\t\tgst_buffer_unref(buffer);\n> +\t\t\t\treturn -EPIPE;\n\nEPIPE paused the thread, this will lead to stall. If you want this to be fatal\nyou must post an error message on the bus (see GST_ELEMENT_ERROR() macro.\n\n> +\t\t\t}\n> +\n> +\t\t\tret = gst_libcamera_video_frame_copy(buffer, copy, info, stream_cfg.stride);\n> +\t\t\tgst_buffer_unref(buffer);\n> +\t\t\tif (ret != GST_FLOW_OK) {\n> +\t\t\t\tGST_ERROR(\"Failed to copy buffer\");\n> +\t\t\t\tgst_buffer_unref(copy);\n> +\t\t\t\treturn -EPIPE;\n\nSame.\n\n> +\t\t\t}\n> +\n> +\t\t\tbuffer = copy;\n> +\t\t}\n>  \n>  \t\tif (GST_CLOCK_TIME_IS_VALID(wrap->pts_)) {\n>  \t\t\tGST_BUFFER_PTS(buffer) = wrap->pts_;\n> @@ -497,13 +568,65 @@ gst_libcamera_src_negotiate(GstLibcameraSrc *self)\n>  \tfor (gsize i = 0; i < state->srcpads_.size(); i++) {\n>  \t\tGstPad *srcpad = state->srcpads_[i];\n>  \t\tconst StreamConfiguration &stream_cfg = state->config_->at(i);\n> +\t\tGstBufferPool *video_pool = NULL;\n> +\t\tGstVideoInfo info;\n> +\n> +\t\tg_autoptr(GstCaps) caps = gst_libcamera_stream_configuration_to_caps(stream_cfg);\n> +\n> +\t\tgst_video_info_init(&info);\n\nNot needed if you use from_caps() below ...\n\n> +\t\tgst_video_info_from_caps(&info, caps);\n> +\t\tgst_libcamera_pad_set_video_info(srcpad, info);\n> +\n> +\t\t/* stride mismatch between camera stride and that calculated by video-info */\n> +\t\tif (static_cast<unsigned int>(info.stride[0]) != stream_cfg.stride &&\n> +\t\t    GST_VIDEO_INFO_FORMAT(&info) != GST_VIDEO_FORMAT_ENCODED) {\n> +\t\t\tGstQuery *query = NULL;\n> +\t\t\tgboolean need_pool = true;\n\nSince you just use it for readability, can be const.\n\n> +\t\t\tgboolean has_video_meta = false;\n> +\n> +\t\t\tgst_libcamera_extrapolate_info(&info, stream_cfg.stride);\n> +\n> +\t\t\tquery = gst_query_new_allocation(caps, need_pool);\n> +\t\t\tif (!gst_pad_peer_query(srcpad, query))\n> +\t\t\t\tGST_DEBUG_OBJECT(self, \"Didn't get downstream ALLOCATION hints\");\n> +\t\t\telse\n> +\t\t\t\thas_video_meta = gst_query_find_allocation_meta(query, GST_VIDEO_META_API_TYPE,\n> NULL);\n> +\n> +\t\t\tif (!has_video_meta) {\n> +\t\t\t\tGstBufferPool *pool = NULL;\n> +\n> +\t\t\t\tif (gst_query_get_n_allocation_pools(query) > 0)\n> +\t\t\t\t\tgst_query_parse_nth_allocation_pool(query, 0, &pool, NULL, NULL, NULL);\n> +\n> +\t\t\t\tif (pool)\n> +\t\t\t\t\tvideo_pool = pool;\n> +\t\t\t\telse {\n> +\t\t\t\t\tGstStructure *config;\n> +\t\t\t\t\tguint min_buffers = 3;\n> +\t\t\t\t\tvideo_pool = gst_video_buffer_pool_new();\n> +\n> +\t\t\t\t\tconfig = gst_buffer_pool_get_config(video_pool);\n> +\t\t\t\t\tgst_buffer_pool_config_set_params(config, caps, info.size, min_buffers, 0);\n> +\t\t\t\t\tgst_buffer_pool_set_config(GST_BUFFER_POOL_CAST(video_pool), config);\n> +\t\t\t\t\tGST_DEBUG_OBJECT(self, \"Own pool config is %\" GST_PTR_FORMAT, config);\n\nMove the trace before the set, since you don't own the pointer afterward,\nwhich is risky.\n\n> +\t\t\t\t}\n> +\n> +\t\t\t\tif (!gst_buffer_pool_set_active(video_pool, true)) {\n> +\t\t\t\t\tGST_ERROR_OBJECT(self, \"Failed to active buffer pool\");\n\nUse GST_ELEMENT_ERROR() here.\n\n> +\t\t\t\t\tgst_caps_unref(caps);\n> +\t\t\t\t\treturn false;\n> +\t\t\t\t}\n> +\t\t\t}\n> +\t\t\tgst_query_unref(query);\n> +\t\t}\n>  \n>  \t\tGstLibcameraPool *pool = gst_libcamera_pool_new(self->allocator,\n> -\t\t\t\t\t\t\t\tstream_cfg.stream());\n> +\t\t\t\t\t\t\t\tstream_cfg.stream(), &info);\n>  \t\tg_signal_connect_swapped(pool, \"buffer-notify\",\n>  \t\t\t\t\t G_CALLBACK(gst_task_resume), self->task);\n>  \n>  \t\tgst_libcamera_pad_set_pool(srcpad, pool);\n> +\t\tgst_libcamera_pad_set_video_pool(srcpad, video_pool);\n\nSee comment about passing a reference. Might be nicer to combine passing the pool\nand the video info ?\n\n>  \n>  \t\t/* Clear all reconfigure flags. */\n>  \t\tgst_pad_check_reconfigure(srcpad);\n> @@ -920,6 +1043,12 @@ gst_libcamera_src_release_pad(GstElement *element, GstPad *pad)\n>  \t\tauto end_iterator = pads.end();\n>  \t\tauto pad_iterator = std::find(begin_iterator, end_iterator, pad);\n>  \n> +\t\tGstBufferPool *video_pool = gst_libcamera_pad_get_video_pool(pad);\n> +\t\tif (video_pool) {\n> +\t\t\tgst_buffer_pool_set_active(video_pool, false);\n> +\t\t\tgst_object_unref(video_pool);\n> +\t\t}\n\nWhat about moving this in the pad, and simpy calling gst_libcamera_pad_set_pool(nullptr),\nor pad_unset_pool() ?\n\n> +\n>  \t\tif (pad_iterator != end_iterator) {\n>  \t\t\tg_object_unref(*pad_iterator);\n>  \t\t\tpads.erase(pad_iterator);","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 987E7C3200\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 12 May 2025 21:05:41 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C56F468B52;\n\tMon, 12 May 2025 23:05:40 +0200 (CEST)","from mail-wm1-x335.google.com (mail-wm1-x335.google.com\n\t[IPv6:2a00:1450:4864:20::335])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 49587614E3\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 12 May 2025 23:05:39 +0200 (CEST)","by mail-wm1-x335.google.com with SMTP id\n\t5b1f17b1804b1-43d0359b1fcso33257535e9.0\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 12 May 2025 14:05:39 -0700 (PDT)","from [10.68.117.232] ([146.0.28.179])\n\tby smtp.gmail.com with ESMTPSA id\n\t5b1f17b1804b1-442cd3aeb79sm182556445e9.27.2025.05.12.14.05.37\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tMon, 12 May 2025 14:05:38 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=ndufresne-ca.20230601.gappssmtp.com\n\theader.i=@ndufresne-ca.20230601.gappssmtp.com\n\theader.b=\"kSeoYs3Z\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ndufresne-ca.20230601.gappssmtp.com; s=20230601; t=1747083939;\n\tx=1747688739; darn=lists.libcamera.org; \n\th=mime-version:user-agent:content-transfer-encoding:references\n\t:in-reply-to:date:cc:to:from:subject:message-id:from:to:cc:subject\n\t:date:message-id:reply-to;\n\tbh=vXIyDMH+M5sl+4bSW5oNOICeeYVepC8MOYImDX6T4UM=;\n\tb=kSeoYs3Z57ziJ7te1D6OV1rntnnY2W8I9vWtCTf1hXmgJ8O4qBGt/SvgcZcqZr1cSS\n\t+8frw7uT2BHURTbine22mQaKAj7VWA1a5sGW9Lbgh29ojk4zXxzo7+MgkhZ94wm3200n\n\tCZTSohfRmDLCxaXbWwWHxvxr7HKLTMCdyqWYbcyQP713vdZZ9sfsvKvnd0ZwUWJKkhCQ\n\tUkK46u6524DSEWQSQdtCen7lhdurTHboTF0QsiBYDk4HmHr9BedqkKFGwzz3DlhAvNuH\n\t2RzztvK+ZkqzPdSQh5t2aKXNDW/Md2Rt42ESlJ5y/PdEizBu3IIMpiK02kKS/2HQLUm7\n\tfDQw==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1747083939; x=1747688739;\n\th=mime-version:user-agent:content-transfer-encoding:references\n\t:in-reply-to:date:cc:to:from:subject:message-id:x-gm-message-state\n\t:from:to:cc:subject:date:message-id:reply-to;\n\tbh=vXIyDMH+M5sl+4bSW5oNOICeeYVepC8MOYImDX6T4UM=;\n\tb=XywcygsF01tk27c5gYsQje2w0n7nZSK7QcBhY/aAqoe86Nj7CYW83/XtY3oVQhnHbl\n\tKfOHlk2iODnw8NmYnDzN1HXWZJKFggbxnLOVc1lt0S5Yexax93xx4vVj1yu3M+rFwxPw\n\tTLbjEMKbECmQreAAL6EPH2RBJnsi4q2dWBBVBJTmathJ09Bkpkpxppm8F+qFAUwIkcUp\n\taJnnn//o2XZFMADgXN+uZVazJsmFAQouMErb0vYDuCwLHfXbG+5ft8YfFCaGoU/2FzsO\n\tKabYYmJ/4rzjA/l2VYQ8K9H41utzAb/McCU38Us4XAaVk1WpPR8Fxd45FmmYmppC8GTb\n\t+RRQ==","X-Forwarded-Encrypted":"i=1;\n\tAJvYcCWgGaVJzFjTQoE+GhYjF+byPHAvhHl2+MDv19Rq6M7axGKYEX2f2ZJfWqi/WmEb3d4rPbBd2olrAw7ZlOA4yzc=@lists.libcamera.org","X-Gm-Message-State":"AOJu0YzyzdDnAqAM5+JGgKITSV92UBrJ82X2OVWQ09cCGlmtfWBYFiXc\n\t8hBnBbQT03vmuPDBgPjow0h+W/riyH2x0XV9lLjdkSA+aCm9qw8bKc+KT9mOCm4=","X-Gm-Gg":"ASbGncu3dkTOME5eGwfCEJqSFqpPD2prvTo0wV6tTJnetmNYWgCsgAxfbwoyPgWyCkC\n\tS++y26nJ7X5WpLvjuC7xRbWSbdRViL6/tsW3vutcNkrUoB1vuxqbKiuvQKTxs1DD09ISzBmEQ7d\n\txXLaiAMLLYETC0SWHdoOPUp7GpmtMgP63rC83GPvp0zq8XiFTQN29ETvoX7V5opO3aFLQBtjAhM\n\t9k54E/Em/9c7UgDHqMpZSJD5qhbSpeqR9GT8f2nUFZEb+ScFp4sGw2J6eHKu1fvV767K3D4n9DA\n\tgDrBCQBoGDUuj+xFlFnyjrYRHvbLsZOhJXaRtnkrHzS/Q4+MdtzvCMs=","X-Google-Smtp-Source":"AGHT+IFV4TU9Q9/aav/JFA5x6ME26ZMnxvOQEfEpnEFyXTQ54fGdc4s1iuvDVD3oDhEEFSy1DCe8sQ==","X-Received":"by 2002:a05:600c:1596:b0:43b:bfa7:c7d with SMTP id\n\t5b1f17b1804b1-442eac97cc6mr5344395e9.2.1747083938458; \n\tMon, 12 May 2025 14:05:38 -0700 (PDT)","Message-ID":"<f93c135d178d3869c8bfc842309ee63ae2907f99.camel@ndufresne.ca>","Subject":"Re: [PATCH v6] gstreamer: Add GstVideoMeta support","From":"Nicolas Dufresne <nicolas@ndufresne.ca>","To":"Hou Qi <qi.hou@nxp.com>, libcamera-devel@lists.libcamera.org","Cc":"jared.hu@nxp.com, julien.vuillaumier@nxp.com","Date":"Mon, 12 May 2025 23:05:36 +0200","In-Reply-To":"<20241204085511.1733434-1-qi.hou@nxp.com>","References":"<20241204085511.1733434-1-qi.hou@nxp.com>","Content-Type":"text/plain; charset=\"UTF-8\"","Content-Transfer-Encoding":"quoted-printable","User-Agent":"Evolution 3.56.1 (3.56.1-1.fc42) ","MIME-Version":"1.0","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":34217,"web_url":"https://patchwork.libcamera.org/comment/34217/","msgid":"<PAXPR04MB828502168977D41976E67C039796A@PAXPR04MB8285.eurprd04.prod.outlook.com>","date":"2025-05-13T09:28:20","subject":"RE: [EXT] Re: [PATCH v6] gstreamer: Add GstVideoMeta support","submitter":{"id":195,"url":"https://patchwork.libcamera.org/api/people/195/","name":"Qi Hou","email":"qi.hou@nxp.com"},"content":"Hi Kieran,\n\nThank you very much for your review. This change is to fix display distortion when preview with some specific size, such as using imx8-isi handler to generate 854x480 buffer and show it with waylandsink.\n\nPipeline may be like below. For NV12, imx8-isi calculated stride is 854 owing to stride alignment is 1, while video-info used in waylandsink calculated stride is 856 owing to stride alignment is 4. Such stride or offset mismatch causes display abnormal.\n1.      gst-launch-1.0 libcamerasrc ! video/x-raw,width=854,height=480,format=NV12,colorimetry=bt709,stream-role=3,interlace-mode=progressive ! videoconvert ! queue ! waylandsink\n\nThis change also makes the dumped data of such size show correctly.\n2.      gst-launch-1.0 libcamerasrc ! video/x-raw,width=854,height=480,format=NV12,colorimetry=bt709,stream-role=3,interlace-mode=progressive ! filesink location=out.yuv\n\nRegards,\nQi Hou\n\n-----Original Message-----\nFrom: Kieran Bingham <kieran.bingham@ideasonboard.com>\nSent: 2025年5月12日 19:32\nTo: Qi Hou <qi.hou@nxp.com>; libcamera-devel@lists.libcamera.org\nCc: Jared Hu <jared.hu@nxp.com>; Qi Hou <qi.hou@nxp.com>; Julien Vuillaumier <julien.vuillaumier@nxp.com>\nSubject: [EXT] Re: [PATCH v6] gstreamer: Add GstVideoMeta support\n\nCaution: This is an external email. Please take care when clicking links or opening attachments. When in doubt, report the message using the 'Report this email' button\n\n\nQuoting Hou Qi (2024-12-04 08:55:11)\n> GStreamer video-info calculated stride and offset may differ from\n> those used by the camera.\n>\n> For stride and offset mismatch, this patch adds video meta to buffer\n> if downstream supports VideoMeta through allocation query. Otherwise,\n> create a internal VideoPool using the caps, and copy video frame to\n> this system memory.\n\nThis has passed CI:\n\n- https://gitlab.freedesktop.org/camera/libcamera/-/pipelines/1323117\n\n\n>\n> Signed-off-by: Hou Qi <qi.hou@nxp.com>\n> ---\n>  src/gstreamer/gstlibcamera-utils.cpp |  33 +++++++\n>  src/gstreamer/gstlibcamera-utils.h   |   4 +\n>  src/gstreamer/gstlibcamerapad.cpp    |  31 +++++++\n>  src/gstreamer/gstlibcamerapad.h      |   8 ++\n>  src/gstreamer/gstlibcamerapool.cpp   |  18 +++-\n>  src/gstreamer/gstlibcamerapool.h     |   3 +-\n>  src/gstreamer/gstlibcamerasrc.cpp    | 133 ++++++++++++++++++++++++++-\n>  7 files changed, 226 insertions(+), 4 deletions(-)\n>\n> diff --git a/src/gstreamer/gstlibcamera-utils.cpp\n> b/src/gstreamer/gstlibcamera-utils.cpp\n> index 732987ef..09af9204 100644\n> --- a/src/gstreamer/gstlibcamera-utils.cpp\n> +++ b/src/gstreamer/gstlibcamera-utils.cpp\n> @@ -591,6 +591,39 @@ gst_task_resume(GstTask *task)  }  #endif\n>\n> +#if !GST_CHECK_VERSION(1, 22, 0)\n> +/*\n> + * Copyright (C) <1999> Erik Walthinsen <omega@cse.ogi.edu>\n> + * Library       <2002> Ronald Bultje <rbultje@ronald.bitfreak.net>\n> + * Copyright (C) <2007> David A. Schleef <ds@schleef.org>  */\n> +/* This function has been imported directly from the gstreamer\n> +project to\n> + * support backwards compatibility and should be removed when the\n> +older\n> + * version is no longer supported. */ gint\n> +gst_video_format_info_extrapolate_stride(const GstVideoFormatInfo\n> +*finfo, gint plane, gint stride) {\n> +       gint estride;\n> +       gint comp[GST_VIDEO_MAX_COMPONENTS];\n> +       gint i;\n> +\n> +       /* there is nothing to extrapolate on first plane */\n> +       if (plane == 0)\n> +               return stride;\n> +\n> +       gst_video_format_info_component(finfo, plane, comp);\n> +\n> +       /* For now, all planar formats have a single component on first plane, but\n> +       * if there was a planar format with more, we'd have to make a ratio of the\n> +       * number of component on the first plane against the number of component on\n> +       * the current plane. */\n> +       estride = 0;\n> +       for (i = 0; i < GST_VIDEO_MAX_COMPONENTS && comp[i] >= 0; i++)\n> +               estride += GST_VIDEO_FORMAT_INFO_SCALE_WIDTH(finfo,\n> + comp[i], stride);\n> +\n> +       return estride;\n> +}\n> +#endif\n> +\n\nAll looking good there...\n\n>  G_LOCK_DEFINE_STATIC(cm_singleton_lock);\n>  static std::weak_ptr<CameraManager> cm_singleton_ptr;\n>\n> diff --git a/src/gstreamer/gstlibcamera-utils.h\n> b/src/gstreamer/gstlibcamera-utils.h\n> index cab1c814..81149280 100644\n> --- a/src/gstreamer/gstlibcamera-utils.h\n> +++ b/src/gstreamer/gstlibcamera-utils.h\n> @@ -35,6 +35,10 @@ static inline void gst_clear_event(GstEvent\n> **event_ptr)  #if !GST_CHECK_VERSION(1, 17, 1)  gboolean\n> gst_task_resume(GstTask *task);  #endif\n> +\n> +#if !GST_CHECK_VERSION(1, 22, 0)\n> +gint gst_video_format_info_extrapolate_stride(const\n> +GstVideoFormatInfo *finfo, gint plane, gint stride); #endif\n>  std::shared_ptr<libcamera::CameraManager>\n> gst_libcamera_get_camera_manager(int &ret);\n>\n>  /**\n> diff --git a/src/gstreamer/gstlibcamerapad.cpp\n> b/src/gstreamer/gstlibcamerapad.cpp\n> index 7b22aebe..7fd11a8f 100644\n> --- a/src/gstreamer/gstlibcamerapad.cpp\n> +++ b/src/gstreamer/gstlibcamerapad.cpp\n> @@ -18,6 +18,8 @@ struct _GstLibcameraPad {\n>         GstPad parent;\n>         StreamRole role;\n>         GstLibcameraPool *pool;\n> +       GstBufferPool *video_pool;\n> +       GstVideoInfo info;\n\nThis looks like we are now 'per pad' as requested.\n\n>         GstClockTime latency;\n>  };\n>\n> @@ -153,6 +155,35 @@ gst_libcamera_pad_set_pool(GstPad *pad, GstLibcameraPool *pool)\n>         self->pool = pool;\n>  }\n>\n> +GstBufferPool *\n> +gst_libcamera_pad_get_video_pool(GstPad *pad) {\n> +       auto *self = GST_LIBCAMERA_PAD(pad);\n> +       return self->video_pool;\n> +}\n> +\n> +void gst_libcamera_pad_set_video_pool(GstPad *pad, GstBufferPool\n> +*video_pool) {\n> +       auto *self = GST_LIBCAMERA_PAD(pad);\n> +\n> +       if (self->video_pool)\n> +               g_object_unref(self->video_pool);\n> +       self->video_pool = video_pool; }\n> +\n> +GstVideoInfo gst_libcamera_pad_get_video_info(GstPad *pad) {\n> +       auto *self = GST_LIBCAMERA_PAD(pad);\n> +       return self->info;\n> +}\n> +\n> +void gst_libcamera_pad_set_video_info(GstPad *pad, GstVideoInfo info)\n> +{\n> +       auto *self = GST_LIBCAMERA_PAD(pad);\n> +\n> +       self->info = info;\n> +}\n> +\n>  Stream *\n>  gst_libcamera_pad_get_stream(GstPad *pad)  { diff --git\n> a/src/gstreamer/gstlibcamerapad.h b/src/gstreamer/gstlibcamerapad.h\n> index 630c168a..ed0beb1f 100644\n> --- a/src/gstreamer/gstlibcamerapad.h\n> +++ b/src/gstreamer/gstlibcamerapad.h\n> @@ -23,6 +23,14 @@ GstLibcameraPool *gst_libcamera_pad_get_pool(GstPad\n> *pad);\n>\n>  void gst_libcamera_pad_set_pool(GstPad *pad, GstLibcameraPool *pool);\n>\n> +GstBufferPool *gst_libcamera_pad_get_video_pool(GstPad *pad);\n> +\n> +void gst_libcamera_pad_set_video_pool(GstPad *pad, GstBufferPool\n> +*video_pool);\n> +\n> +GstVideoInfo gst_libcamera_pad_get_video_info(GstPad *pad);\n> +\n> +void gst_libcamera_pad_set_video_info(GstPad *pad, GstVideoInfo\n> +info);\n> +\n>  libcamera::Stream *gst_libcamera_pad_get_stream(GstPad *pad);\n>\n>  void gst_libcamera_pad_set_latency(GstPad *pad, GstClockTime\n> latency); diff --git a/src/gstreamer/gstlibcamerapool.cpp\n> b/src/gstreamer/gstlibcamerapool.cpp\n> index 9cd7eccb..471f71da 100644\n> --- a/src/gstreamer/gstlibcamerapool.cpp\n> +++ b/src/gstreamer/gstlibcamerapool.cpp\n> @@ -14,6 +14,9 @@\n>\n>  #include \"gstlibcamera-utils.h\"\n>\n> +#include <gst/gstmeta.h>\n> +\n> +\n>  using namespace libcamera;\n>\n>  enum {\n> @@ -134,8 +137,20 @@ gst_libcamera_pool_class_init(GstLibcameraPoolClass *klass)\n>                                                      G_TYPE_NONE, 0);\n> }\n>\n> +static void\n> +gst_libcamera_buffer_add_video_meta(GstBuffer *buffer, GstVideoInfo\n> +*info) {\n> +       GstVideoMeta *vmeta;\n> +       vmeta = gst_buffer_add_video_meta_full(buffer, GST_VIDEO_FRAME_FLAG_NONE,\n> +                                              GST_VIDEO_INFO_FORMAT(info), GST_VIDEO_INFO_WIDTH(info),\n> +                                              GST_VIDEO_INFO_HEIGHT(info), GST_VIDEO_INFO_N_PLANES(info),\n> +                                              info->offset, info->stride);\n> +       GST_META_FLAGS(vmeta) = (GstMetaFlags)(GST_META_FLAGS(vmeta) |\n> +GST_META_FLAG_POOLED); }\n\nThis looks like it satisfies the review comment in v5.\n\n> +\n>  GstLibcameraPool *\n> -gst_libcamera_pool_new(GstLibcameraAllocator *allocator, Stream\n> *stream)\n> +gst_libcamera_pool_new(GstLibcameraAllocator *allocator, Stream *stream,\n> +                      GstVideoInfo *info)\n>  {\n\nI can see this was simplfied as requested.\n\n\n>         auto *pool =\n> GST_LIBCAMERA_POOL(g_object_new(GST_TYPE_LIBCAMERA_POOL, nullptr));\n>\n> @@ -145,6 +160,7 @@ gst_libcamera_pool_new(GstLibcameraAllocator *allocator, Stream *stream)\n>         gsize pool_size = gst_libcamera_allocator_get_pool_size(allocator, stream);\n>         for (gsize i = 0; i < pool_size; i++) {\n>                 GstBuffer *buffer = gst_buffer_new();\n> +               gst_libcamera_buffer_add_video_meta(buffer, info);\n>                 pool->queue->push_back(buffer);\n>         }\n>\n> diff --git a/src/gstreamer/gstlibcamerapool.h\n> b/src/gstreamer/gstlibcamerapool.h\n> index 2a7a9c77..02ee4dd4 100644\n> --- a/src/gstreamer/gstlibcamerapool.h\n> +++ b/src/gstreamer/gstlibcamerapool.h\n> @@ -14,6 +14,7 @@\n>  #include \"gstlibcameraallocator.h\"\n>\n>  #include <gst/gst.h>\n> +#include <gst/video/video.h>\n>\n>  #include <libcamera/stream.h>\n>\n> @@ -21,7 +22,7 @@\n>  G_DECLARE_FINAL_TYPE(GstLibcameraPool, gst_libcamera_pool,\n> GST_LIBCAMERA, POOL, GstBufferPool)\n>\n>  GstLibcameraPool *gst_libcamera_pool_new(GstLibcameraAllocator *allocator,\n> -                                        libcamera::Stream *stream);\n> +                                        libcamera::Stream *stream,\n> + GstVideoInfo *info);\n>\n>  libcamera::Stream *gst_libcamera_pool_get_stream(GstLibcameraPool\n> *self);\n>\n> diff --git a/src/gstreamer/gstlibcamerasrc.cpp\n> b/src/gstreamer/gstlibcamerasrc.cpp\n> index 8efa25f4..82b06eb4 100644\n> --- a/src/gstreamer/gstlibcamerasrc.cpp\n> +++ b/src/gstreamer/gstlibcamerasrc.cpp\n> @@ -268,6 +268,52 @@ GstLibcameraSrcState::requestCompleted(Request *request)\n>         gst_task_resume(src_->task);\n>  }\n>\n> +static void\n> +gst_libcamera_extrapolate_info(GstVideoInfo *info, guint32 stride) {\n> +       guint i, estride;\n> +       gsize offset = 0;\n> +\n> +       /* this should be updated if tiled formats get added in the future. */\n> +       for (i = 0; i < GST_VIDEO_INFO_N_PLANES(info); i++) {\n> +               estride = gst_video_format_info_extrapolate_stride(info->finfo, i, stride);\n> +               info->stride[i] = estride;\n> +               info->offset[i] = offset;\n> +               offset += estride * GST_VIDEO_FORMAT_INFO_SCALE_HEIGHT(info->finfo, i,\n> +                                                                      GST_VIDEO_INFO_HEIGHT(info));\n> +       }\n> +}\n> +\n> +static GstFlowReturn\n> +gst_libcamera_video_frame_copy(GstBuffer *src, GstBuffer *dest,\n> +GstVideoInfo dest_info, guint32 stride) {\n> +       GstVideoInfo src_info = dest_info;\n> +       GstVideoFrame src_frame, dest_frame;\n> +\n> +       gst_libcamera_extrapolate_info(&src_info, stride);\n> +       src_info.size = gst_buffer_get_size(src);\n> +\n> +       if (!gst_video_frame_map(&src_frame, &src_info, src, GST_MAP_READ))\n> +               goto invalid_buffer;\n> +\n> +       if (!gst_video_frame_map(&dest_frame, &dest_info, dest, GST_MAP_WRITE)) {\n> +               gst_video_frame_unmap(&src_frame);\n> +               goto invalid_buffer;\n> +       }\n> +\n> +       gst_video_frame_copy(&dest_frame, &src_frame);\n> +\n> +       gst_video_frame_unmap(&src_frame);\n> +       gst_video_frame_unmap(&dest_frame);\n> +\n> +       return GST_FLOW_OK;\n> +\n> +invalid_buffer : {\n> +       GST_ERROR(\"Could not map buffer\");\n> +       return GST_FLOW_ERROR;\n> +}\n\nWhy do we have these extra { } here ? Can't they be removed?\n\n> +}\n> +\n>  /* Must be called with stream_lock held. */  int\n> GstLibcameraSrcState::processRequest()\n>  {\n> @@ -292,11 +338,36 @@ int GstLibcameraSrcState::processRequest()\n>         GstFlowReturn ret = GST_FLOW_OK;\n>         gst_flow_combiner_reset(src_->flow_combiner);\n>\n> -       for (GstPad *srcpad : srcpads_) {\n> +       for (gsize i = 0; i < src_->state->srcpads_.size(); i++) {\n> +               GstPad *srcpad = src_->state->srcpads_[i];\n>                 Stream *stream = gst_libcamera_pad_get_stream(srcpad);\n>                 GstBuffer *buffer = wrap->detachBuffer(stream);\n>\n>                 FrameBuffer *fb =\n> gst_libcamera_buffer_get_frame_buffer(buffer);\n> +               const StreamConfiguration &stream_cfg = src_->state->config_->at(i);\n> +               GstBufferPool *video_pool =\n> + gst_libcamera_pad_get_video_pool(srcpad);\n> +\n> +               if (video_pool) {\n> +                       GstBuffer *copy = NULL;\n\nI can see tmp is now copy which looks fine to me.\n\n\nAs far as I can tell - previous review comments were handled.\n\nI think I saw a request to reduce the indentations here - which might help. I'm not sure what to suggest specifically ... so I'll put this here as I think it would be good to see this progress:\n\n\nReviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n\n\nNicolas - is there anything else here you want to see fixed ?\n\n\n\n> +                       GstVideoInfo info =\n> + gst_libcamera_pad_get_video_info(srcpad);\n> +\n> +                       ret = gst_buffer_pool_acquire_buffer(video_pool, &copy, NULL);\n> +                       if (ret != GST_FLOW_OK) {\n> +                               GST_ERROR(\"Failed to acquire buffer\");\n> +                               gst_buffer_unref(buffer);\n> +                               return -EPIPE;\n> +                       }\n> +\n> +                       ret = gst_libcamera_video_frame_copy(buffer, copy, info, stream_cfg.stride);\n> +                       gst_buffer_unref(buffer);\n> +                       if (ret != GST_FLOW_OK) {\n> +                               GST_ERROR(\"Failed to copy buffer\");\n> +                               gst_buffer_unref(copy);\n> +                               return -EPIPE;\n> +                       }\n> +\n> +                       buffer = copy;\n> +               }\n>\n>                 if (GST_CLOCK_TIME_IS_VALID(wrap->pts_)) {\n>                         GST_BUFFER_PTS(buffer) = wrap->pts_; @@\n> -497,13 +568,65 @@ gst_libcamera_src_negotiate(GstLibcameraSrc *self)\n>         for (gsize i = 0; i < state->srcpads_.size(); i++) {\n>                 GstPad *srcpad = state->srcpads_[i];\n>                 const StreamConfiguration &stream_cfg =\n> state->config_->at(i);\n> +               GstBufferPool *video_pool = NULL;\n> +               GstVideoInfo info;\n> +\n> +               g_autoptr(GstCaps) caps =\n> + gst_libcamera_stream_configuration_to_caps(stream_cfg);\n> +\n> +               gst_video_info_init(&info);\n> +               gst_video_info_from_caps(&info, caps);\n> +               gst_libcamera_pad_set_video_info(srcpad, info);\n> +\n> +               /* stride mismatch between camera stride and that calculated by video-info */\n> +               if (static_cast<unsigned int>(info.stride[0]) != stream_cfg.stride &&\n> +                   GST_VIDEO_INFO_FORMAT(&info) != GST_VIDEO_FORMAT_ENCODED) {\n> +                       GstQuery *query = NULL;\n> +                       gboolean need_pool = true;\n> +                       gboolean has_video_meta = false;\n> +\n> +                       gst_libcamera_extrapolate_info(&info,\n> + stream_cfg.stride);\n> +\n> +                       query = gst_query_new_allocation(caps, need_pool);\n> +                       if (!gst_pad_peer_query(srcpad, query))\n> +                               GST_DEBUG_OBJECT(self, \"Didn't get downstream ALLOCATION hints\");\n> +                       else\n> +                               has_video_meta =\n> + gst_query_find_allocation_meta(query, GST_VIDEO_META_API_TYPE,\n> + NULL);\n> +\n> +                       if (!has_video_meta) {\n> +                               GstBufferPool *pool = NULL;\n> +\n> +                               if (gst_query_get_n_allocation_pools(query) > 0)\n> +\n> + gst_query_parse_nth_allocation_pool(query, 0, &pool, NULL, NULL,\n> + NULL);\n> +\n> +                               if (pool)\n> +                                       video_pool = pool;\n> +                               else {\n> +                                       GstStructure *config;\n> +                                       guint min_buffers = 3;\n> +                                       video_pool =\n> + gst_video_buffer_pool_new();\n> +\n> +                                       config = gst_buffer_pool_get_config(video_pool);\n> +                                       gst_buffer_pool_config_set_params(config, caps, info.size, min_buffers, 0);\n> +                                       gst_buffer_pool_set_config(GST_BUFFER_POOL_CAST(video_pool), config);\n> +                                       GST_DEBUG_OBJECT(self, \"Own pool config is %\" GST_PTR_FORMAT, config);\n> +                               }\n> +\n> +                               if (!gst_buffer_pool_set_active(video_pool, true)) {\n> +                                       GST_ERROR_OBJECT(self, \"Failed to active buffer pool\");\n> +                                       gst_caps_unref(caps);\n> +                                       return false;\n> +                               }\n> +                       }\n> +                       gst_query_unref(query);\n> +               }\n>\n>                 GstLibcameraPool *pool = gst_libcamera_pool_new(self->allocator,\n> -                                                               stream_cfg.stream());\n> +\n> + stream_cfg.stream(), &info);\n>                 g_signal_connect_swapped(pool, \"buffer-notify\",\n>                                          G_CALLBACK(gst_task_resume),\n> self->task);\n>\n>                 gst_libcamera_pad_set_pool(srcpad, pool);\n> +               gst_libcamera_pad_set_video_pool(srcpad, video_pool);\n>\n>                 /* Clear all reconfigure flags. */\n>                 gst_pad_check_reconfigure(srcpad);\n> @@ -920,6 +1043,12 @@ gst_libcamera_src_release_pad(GstElement *element, GstPad *pad)\n>                 auto end_iterator = pads.end();\n>                 auto pad_iterator = std::find(begin_iterator,\n> end_iterator, pad);\n>\n> +               GstBufferPool *video_pool = gst_libcamera_pad_get_video_pool(pad);\n> +               if (video_pool) {\n> +                       gst_buffer_pool_set_active(video_pool, false);\n> +                       gst_object_unref(video_pool);\n> +               }\n> +\n>                 if (pad_iterator != end_iterator) {\n>                         g_object_unref(*pad_iterator);\n>                         pads.erase(pad_iterator);\n> --\n> 2.34.1\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 4AA6DC3220\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 13 May 2025 09:28:25 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 835EA68B52;\n\tTue, 13 May 2025 11:28:24 +0200 (CEST)","from EUR05-DB8-obe.outbound.protection.outlook.com\n\t(mail-db8eur05on2062b.outbound.protection.outlook.com\n\t[IPv6:2a01:111:f403:2614::62b])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8F18E68B51\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 13 May 2025 11:28:22 +0200 (CEST)","from PAXPR04MB8285.eurprd04.prod.outlook.com\n\t(2603:10a6:102:1ca::15)\n\tby AS8PR04MB7750.eurprd04.prod.outlook.com (2603:10a6:20b:2aa::5)\n\twith Microsoft SMTP Server (version=TLS1_2,\n\tcipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.8722.29;\n\tTue, 13 May 2025 09:28:20 +0000","from PAXPR04MB8285.eurprd04.prod.outlook.com\n\t([fe80::e003:8fb:64ea:acfd]) by\n\tPAXPR04MB8285.eurprd04.prod.outlook.com\n\t([fe80::e003:8fb:64ea:acfd%3]) with mapi id 15.20.8722.027;\n\tTue, 13 May 2025 09:28:20 +0000"],"Authentication-Results":["lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=nxp.com header.i=@nxp.com header.b=\"EbPRTmzI\";\n\tdkim-atps=neutral","dkim=none (message not signed)\n\theader.d=none;dmarc=none action=none header.from=nxp.com;"],"ARC-Seal":"i=1; a=rsa-sha256; s=arcselector10001; d=microsoft.com; cv=none;\n\tb=Tj7vFXnielLLe4/d7l/w9aDIlvRzg8rsrVPkVqPtSDF0sS9xYO85REccFgXxAEjqcMRkfOPZOp/NBsqTLMZcGVGXEjYKaD38oibqzvOAx55Wf69RmxayzPp06A83gTUg4h/oQOpLdBXiHRDtmzXW6tFDsYuAM8tPWCTVGEl3ydz3hdrFYYQr0h466M7OuOcirDK5Ebebfo5b8NZywqehbtlyhHVscvhKItM2xqI8t+ktYu8mkYP3FIlZJNv8T/f35x5nINxEUKtl4Vf7kJlIPxPEMnLaqh/B70vy8AVm430aJ61/mjVn3kbCWFx9Th7pOmNLIAAkwiE3Z6Iu328gCQ==","ARC-Message-Signature":"i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com;\n\ts=arcselector10001;\n\th=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1;\n\tbh=3C34Z91RTPOvpQM6N8RkVroIN+PHRnbj1YdiN/3FeD0=;\n\tb=wJgI1cXadgZpCRqMQboqNJtsChfhPmn8omG/J7IWcSg6hV+3cSVTeB8BuwKxqIWd0kWgLa/KhLHGgL4g4+zaDvQp5K/JHW0y5DHi5rbYvChrBdkk71IZZ9kUEBA1X8tfqY2vq671FpwTSVY864Vw46VRyfPmaOVAoo81ddfc0acS1jOfgDR2nzfqkDixqfQpJqdz2nRTUjzVlReYUPO7+NkSSmt2qeznAaEI1t1IvokInwb2pufmDjsfswfOQ4Mh1Sz2JH1kWJCAZSDZuzGX97UWiF53TrNqidCLTIdUrb4P6Uk97DjYS87KzGy34LO5yfNvy3ARz2UqY7Wyer7p7w==","ARC-Authentication-Results":"i=1; mx.microsoft.com 1; spf=pass\n\tsmtp.mailfrom=nxp.com; dmarc=pass action=none header.from=nxp.com;\n\tdkim=pass header.d=nxp.com; arc=none","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=nxp.com; s=selector1;\n\th=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck;\n\tbh=3C34Z91RTPOvpQM6N8RkVroIN+PHRnbj1YdiN/3FeD0=;\n\tb=EbPRTmzIjeecAk7ZPZVgTaLuogAchp1BqrRG+HuTj/Oij1iji3iOD4mNB1nWDG1dMidpW13brad7U8lYOV3MuRVS9aHFj7j+BKv+nYFgiD+Wt50OwqivMnDadG3DDKUtU4HnoVyWdlsqiVxKh4vYm3Rt9Tfh0J+hgQgfJaSMCdpZjL6o+pLoZPGELP+FnZ7p/85i0lfJ6kOpak8VqTtDXYpZYFVWHgYzPdpgQ7x9VMUGCKkhomgC4N+6uhJPy9kQ+NY2qAcz6wIm+X292V5mIszW51kcf/pyNb7yrF8rK1GgMiz7bk1qRRwMD+6N4a4763hqcURSWoskS4RJ24l4Pg==","From":"Qi Hou <qi.hou@nxp.com>","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>,\n\t\"libcamera-devel@lists.libcamera.org\"\n\t<libcamera-devel@lists.libcamera.org>","CC":"Jared Hu <jared.hu@nxp.com>, Julien Vuillaumier\n\t<julien.vuillaumier@nxp.com>","Subject":"RE: [EXT] Re: [PATCH v6] gstreamer: Add GstVideoMeta support","Thread-Topic":"[EXT] Re: [PATCH v6] gstreamer: Add GstVideoMeta support","Thread-Index":"AQHbRipHNCd3F3VqYU6DyDG//BDktrPP1qoAgAFrE4A=","Date":"Tue, 13 May 2025 09:28:20 +0000","Message-ID":"<PAXPR04MB828502168977D41976E67C039796A@PAXPR04MB8285.eurprd04.prod.outlook.com>","References":"<20241204085511.1733434-1-qi.hou@nxp.com>\n\t<174704953076.1952545.3171535624022805043@ping.linuxembedded.co.uk>","In-Reply-To":"<174704953076.1952545.3171535624022805043@ping.linuxembedded.co.uk>","Accept-Language":"zh-CN, en-US","Content-Language":"en-US","X-MS-Has-Attach":"","X-MS-TNEF-Correlator":"","authentication-results":["lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=nxp.com header.i=@nxp.com header.b=\"EbPRTmzI\";\n\tdkim-atps=neutral","dkim=none (message not signed)\n\theader.d=none;dmarc=none action=none header.from=nxp.com;"],"x-ms-publictraffictype":"Email","x-ms-traffictypediagnostic":"PAXPR04MB8285:EE_|AS8PR04MB7750:EE_","x-ms-office365-filtering-correlation-id":"dc0c037a-acbb-4f6e-7b06-08dd92008044","x-ms-exchange-senderadcheck":"1","x-ms-exchange-antispam-relay":"0","x-microsoft-antispam":"BCL:0;\n\tARA:13230040|366016|1800799024|376014|38070700018; ","x-microsoft-antispam-message-info":"=?gb2312?b?Q0ovY0x4cGVuUmJHNXY4OEU1?=\n\t=?gb2312?b?V05aeHdjSkpUWXVjWElUM0xlZFpOZHVBeFRmVkFIcHp3UTVsOFdq?=\n\t=?gb2312?b?MUtFWC9NamlSL1pDZDdwWnFGa0tucUU0T0tzZEJvdlp6OGFjaHVX?=\n\t=?gb2312?b?SlpjRlU5Sk5jSDVDSmlMRGNPb0ovWjJPTTFJVjZUR1FjSUk4cnhn?=\n\t=?gb2312?b?Mkl3M05kQm5keWt3NUkzVjQ4WFdRd2RSTklqQkRtRWV6SEhCdVJO?=\n\t=?gb2312?b?Q3VFbFVMaUdrSXR4MDQ0V1VrR1hCU0xIYVJScElDM3FRYk9GTUtT?=\n\t=?gb2312?b?bHZNMzgyNVBDWE45bjFMa1VXdGxxYmpWeUdWY0VGSEVKT2VHTFdh?=\n\t=?gb2312?b?UGFxS2lLbVI5aVg3YXNmV1RTMExDbE91dSs5akt1Z25wMHpPM3JV?=\n\t=?gb2312?b?c3FScVdtbS9IaEdYQU5idlN0R3NhVGdkLzU0NEFXVWVTMnl2S3Rx?=\n\t=?gb2312?b?OUU5Vnk1ZkdTa1MxclRGbG5nelZpRGhYSUx4ckR2L04yZm9BVS9J?=\n\t=?gb2312?b?OWJvcm5HQ2psNUZCL2sxNWNnRGZZdWF1YU5ibTZUcDc1Um82NkFi?=\n\t=?gb2312?b?WWhKODJaWGEzRzVPZXJWd3gyYWRyZUlZM0J6RmREd3V6MmU5RlJp?=\n\t=?gb2312?b?TUhveVo3WU9QdlQwcisxa3ZMdzd3bS9QTHVVVjJNNUIrczVXcGxQ?=\n\t=?gb2312?b?eWFLK3NDWGxycTJtMW4zNXY3YmtWZkltN1dHWWRMd3dtTXJOM1lD?=\n\t=?gb2312?b?U0hmOUF6Z2g4anl0ZXhIUFdYVUZOL3k4V3lUZFV2dU9vYlRtM2Zi?=\n\t=?gb2312?b?ODBrdWJZS09ERXU5dTF4a2hnQ1E1NlNmVTIxdlp5M0lPL3phRFBM?=\n\t=?gb2312?b?b0QxbHlMT1NMeStLZWlIaFhpRG5GVEQwMCs2eVF4VkxwYUdRZnFa?=\n\t=?gb2312?b?Nytjd1lJZE5TUTA3SjVTUk94YTNBR2wxUVlMa3V1M0FRUWlqZllN?=\n\t=?gb2312?b?NFg4Q2JnaWZmKzlvRlNNWFRUYWRtVEpyelNRWXRZelVBSXUyVEt3?=\n\t=?gb2312?b?cmhaalROOUNyN3VueTZDdENnc0o5V3c1VDVIK2FyRXhaQXlsVGMv?=\n\t=?gb2312?b?cFN1ME9xVENob29ZS3lTNTdsS3lhcEtZODB1UTMvd3EwRzlUblIz?=\n\t=?gb2312?b?UTg4RU1xTVdjT242OWdxTHhJZUNGZWsxZDFBcnZ1dStUamZ0YUVL?=\n\t=?gb2312?b?elBzVmorUDZEYkkxbHlHekFMYTY4MVBkczgxY1pVYkcvWExmaE1R?=\n\t=?gb2312?b?OWNkWXkzYmVTaEpZRnpOTEkxcFRIODk3NG5GQW5HL0lacjlWeDVE?=\n\t=?gb2312?b?UmlTWmtkaTRSM21tcjVWQTVVVTVjYS9PTkhMRWk5MnJUQUlCR3Qr?=\n\t=?gb2312?b?Z3E3ZjlYcGxRUnVRMHJReDJZNjhOSG5McE16by85NXFKeWVMczlm?=\n\t=?gb2312?b?SnJmY045eDVhWVo4dzdBQUhRbEdKcVlYTldxU0ZFT1V0ZlY0Ukc4?=\n\t=?gb2312?b?SWRnMDlvczU2ZmEwZ0FXWEdGeXltWW1NRzhLTW53TnB0NWN0di9R?=\n\t=?gb2312?b?RFBQWkRtWmpjSDd1cS9PclM0K1ZoSGRROXA4eTc4NkxGOFVDaEpu?=\n\t=?gb2312?b?eE1OMjg5UURkTW85MEp5aENObVNpZkNtYlRqMCt4MUpVT2pXZHFi?=\n\t=?gb2312?b?S2lrZEJyOWhnTERwL2UvdGhBbkt0aFBkam5KL2FhbHdIY2t3emdY?=\n\t=?gb2312?b?cWFCcld2ck82TFFUdGtvdDEwNmN2MExTaXMxQ0xtTWgxR25paG0r?=\n\t=?gb2312?b?dzRBQWlyQ21uYks2aFNLRkw1aWl3UkoyRUs1V1hZYmVva0pjUksx?=\n\t=?gb2312?b?VDlKdUI1eXU2Rzdaby82WmVlUXBzSkErZFhoLzJKLzU2NVp1N1Fl?=\n\t=?gb2312?b?MzJMMDY3eHd2YVMweGpjbWthMm1odHBhbEVhMThBOVVKaUgxTGs4?=\n\t=?gb2312?b?NHgrV3JudGZWVXBGSm5OWll0TFZuVUdvQlhYU3J2NGt5dHZYaklE?=\n\t=?gb2312?b?MEpibElHbjdodWZYdmRyRkZ1Y01yK3hGdUJrMVd6b0RrSlB0b2pn?=\n\t=?gb2312?b?NHd0OFRkVHFZb09ZRnBIbi8rb1Y3TTBMZjY1THc9?=","x-forefront-antispam-report":"CIP:255.255.255.255; CTRY:; LANG:zh-cn; SCL:1; \n\tSRV:; IPV:NLI; SFV:NSPM;\n\tH:PAXPR04MB8285.eurprd04.prod.outlook.com; PTR:; \n\tCAT:NONE; SFS:(13230040)(366016)(1800799024)(376014)(38070700018);\n\tDIR:OUT; SFP:1101; ","x-ms-exchange-antispam-messagedata-chunkcount":"1","x-ms-exchange-antispam-messagedata-0":"=?gb2312?b?czNGNDZtL201YzhNSnowTlRE?=\n\t=?gb2312?b?WWoxRjNNREhaZDQwbUNRZXZCRnJhL3Y3eDZWQ3IrZmlubWM3MDl5?=\n\t=?gb2312?b?NDM3K3hUMTJsS0hqYUpZTEgvd1BzbWJpcmR1U1lGckpqb3A4Rk01?=\n\t=?gb2312?b?RmYrNnJIc0N6aTJxaFV3cXZybGRGMkpCS3ZxZUYzaUgyc0hBNGdn?=\n\t=?gb2312?b?eXNZN2dXbWhMT3NXSnJyMjJzdERsWXdGVzJZbklwVXA4d0RCNzUx?=\n\t=?gb2312?b?cGRoT1MwRE5xNHBBMnJ1cGI3bGQwUXJpcmRUd2k5Y2I5S3UwZGVa?=\n\t=?gb2312?b?alFQc0IyTnh3VHRUR1V0MmQ1T2lGc21uSmhicmxkdk8yZ2Q5aCtM?=\n\t=?gb2312?b?VGRreUE0MUYwcWlDUFZFRnVkYWdPczRsR0dRREo5ZHJTUjdnWm5q?=\n\t=?gb2312?b?SklyS2FIckw5ZUxydnp5VzN1RWduT0ZncE90cDRhRlhHdmhqNzdp?=\n\t=?gb2312?b?WC9odDkwNTV0MEJCNUZqRWVpZXlaTm9scjJyenZIVWdOVkhWbUxy?=\n\t=?gb2312?b?Vmo5bU9vbXh4YkcxNnhMbkhzUEtic01xamJwZEpDMEFiUEY0QnJ6?=\n\t=?gb2312?b?UzFLckhoUkczT2VDSVdFRm1JWDVLc2ppUmJPMzMzWjBUS0ZEbzNq?=\n\t=?gb2312?b?bHNTSXNmdlY2aG9MWUcydGdvQ0k3V0RDTm5OeHB0c1lYaFJLejFJ?=\n\t=?gb2312?b?azVhd3BMb29PMEJVVEYvbkhkckRIR25oQlpsTTZHUlJxNGNSOHBa?=\n\t=?gb2312?b?MldoU3BpR2RaeitRYTg5M3JIdG16QU5xSnVQRUhLNjRGRmRrZ0hr?=\n\t=?gb2312?b?aFlDa0cxSHlpQzNFakZnZnhKRnphL1dQTHhuNnk1LyswUzlZdHpC?=\n\t=?gb2312?b?MnZqb1BvNkt5RGJ0NVVmQ1k2eUVYaHNma1BNQm8xbVdPWVYvYks3?=\n\t=?gb2312?b?bGdqMmIxb2xZVjhFcThycm9xcU1QRWZnTUdtV2o1d2pLRlNacVpB?=\n\t=?gb2312?b?NmFObTNKZWNxeVdLcGkyTDhheWVlR1ZNMlRDc1VEV21jeU92a1lx?=\n\t=?gb2312?b?NGt2RzZ0MHp0TUFDVlRqc3RwTXl1Zk5nd0JiT2JHdDlabXZyT3pM?=\n\t=?gb2312?b?R1NsRlN6cnBQd1YzK0hKQTExNXVkeTc3YkxLdG85L2lkbVhPN0pr?=\n\t=?gb2312?b?MUFpanFPaVZZQlhWa0YrM0U1Z25PaGxnNExUN280TVhHQlkwQVZE?=\n\t=?gb2312?b?RVlPV2JJeHlNdTVkVXFjQ0dHRVozYUxRMU1CcERHUkdQQkVjayty?=\n\t=?gb2312?b?Z09qVERoZWxmMkozTnUvRW1NazZEQTFDeThETmpFQjRCSWtyRXdZ?=\n\t=?gb2312?b?K00wVnVSc2VPYkcxdVZSbEoybnNhNDBYUUJuem54Tk02VEFnUDhp?=\n\t=?gb2312?b?M3hQanRTSGdoRXRiTmNmNnBtc3ZLMmU0ditBMFFDSmxOUjd3RXly?=\n\t=?gb2312?b?dW9mQ0c5MkIvS3hDMFlWeFhqTHdPYkY3d00yVDFWd1IyVUcvc0lT?=\n\t=?gb2312?b?Ky9EL0tqUitwUDVhVzgzZjhRTEJKOXRtZ2tFSXF4R0haMGVTTU90?=\n\t=?gb2312?b?Y002U2JudUhCSGdrK2tvaDFzT3J3N1p2amc2Q1FYbVJUdVFKQWU3?=\n\t=?gb2312?b?OVg3R2NXQWJ2cllYRE9zWnpXSnR0d2YzbjIyZ2U4Yms1aWpvdHBM?=\n\t=?gb2312?b?YmhieHlXMXdoL0I4WDFLTytWR3huclU1dS9uS05WRktBWEVKYnJl?=\n\t=?gb2312?b?OXVCRE8xbk83NWh1MEhVZmJUMStZc3UwZHV1dnYwVytZczB5K3FZ?=\n\t=?gb2312?b?TWJTWDBUeFNhNHZtd2lMSXlVbktDWE9yank1Q1VOZmdOYzV4TTFy?=\n\t=?gb2312?b?czc3ZEt0cjY5UitqaDlsemp1cVhBNEp0Vk55WDUyNER1bW1Kd0Rt?=\n\t=?gb2312?b?aS84TEljSCtDcCtkRjEzR0U5TUJwNE1wK2VmVmY2eHIrRmR5TWhv?=\n\t=?gb2312?b?cnZZeW94NFpQajVRVWx1VTMrV1lrZ3JlNDdkTTBxa3lmZDZlellH?=\n\t=?gb2312?b?K2FnNjdGUWJnLzdwVXV6UHpmUENza01sWGd0bnNIdmIrZXVUeS9q?=\n\t=?gb2312?b?T1R3QVpTK1dkQkJQUEFDZTg5VUNCcWNyNnB3RVR6QXNEdUEvQ05q?=\n\t=?gb2312?b?SElqV1d2a3FvS1JsOTd5YytRSXRLN0ZWQ29tR09DM1RwRWJzZFBJ?=\n\t=?gb2312?b?VmtaeHlVZzBLeEdTUEprUWlwWllrQjljS0Q3RWJSNkFuamZLYjZs?=\n\t=?gb2312?b?VmpNZ2dtR1FRWUFQdFNIZU5NemN5ZU9zNzJXQnZ0aVArWmQyWE9T?=\n\t=?gb2312?b?QT0=?=","Content-Type":"text/plain; charset=\"gb2312\"","Content-Transfer-Encoding":"base64","MIME-Version":"1.0","X-OriginatorOrg":"nxp.com","X-MS-Exchange-CrossTenant-AuthAs":"Internal","X-MS-Exchange-CrossTenant-AuthSource":"PAXPR04MB8285.eurprd04.prod.outlook.com","X-MS-Exchange-CrossTenant-Network-Message-Id":"dc0c037a-acbb-4f6e-7b06-08dd92008044","X-MS-Exchange-CrossTenant-originalarrivaltime":"13 May 2025 09:28:20.5502\n\t(UTC)","X-MS-Exchange-CrossTenant-fromentityheader":"Hosted","X-MS-Exchange-CrossTenant-id":"686ea1d3-bc2b-4c6f-a92c-d99c5c301635","X-MS-Exchange-CrossTenant-mailboxtype":"HOSTED","X-MS-Exchange-CrossTenant-userprincipalname":"/DZI4PpYgKLZwYP/rqw0mxs3l/dXP7skos1EwWYrG8pdGuvrTU5LJUxPZE9xJAyP","X-MS-Exchange-Transport-CrossTenantHeadersStamped":"AS8PR04MB7750","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":34218,"web_url":"https://patchwork.libcamera.org/comment/34218/","msgid":"<PAXPR04MB82859ACC6320C47D9B46BC329796A@PAXPR04MB8285.eurprd04.prod.outlook.com>","date":"2025-05-13T09:29:29","subject":"RE: [EXT] Re: [PATCH v6] gstreamer: Add GstVideoMeta support","submitter":{"id":195,"url":"https://patchwork.libcamera.org/api/people/195/","name":"Qi Hou","email":"qi.hou@nxp.com"},"content":"Hi Nicolas,\n\nThank you for revising my patch line by line. I will submit v7 after I finish coding and testing.\n\nRegards,\nQi Hou\n\n-----Original Message-----\nFrom: Nicolas Dufresne <nicolas@ndufresne.ca> \nSent: 2025年5月13日 5:06\nTo: Qi Hou <qi.hou@nxp.com>; libcamera-devel@lists.libcamera.org\nCc: Jared Hu <jared.hu@nxp.com>; Julien Vuillaumier <julien.vuillaumier@nxp.com>\nSubject: [EXT] Re: [PATCH v6] gstreamer: Add GstVideoMeta support\n\nCaution: This is an external email. Please take care when clicking links or opening attachments. When in doubt, report the message using the 'Report this email' button\n\n\nHi,\n\nLe mercredi 04 décembre 2024 à 17:55 +0900, Hou Qi a écrit :\n> GStreamer video-info calculated stride and offset may differ from \n> those used by the camera.\n>\n> For stride and offset mismatch, this patch adds video meta to buffer \n> if downstream supports VideoMeta through allocation query. Otherwise, \n> create a internal VideoPool using the caps, and copy video frame to \n> this system memory.\n>\n> Signed-off-by: Hou Qi <qi.hou@nxp.com>\n> ---\n>  src/gstreamer/gstlibcamera-utils.cpp |  33 +++++++\n>  src/gstreamer/gstlibcamera-utils.h   |   4 +\n>  src/gstreamer/gstlibcamerapad.cpp    |  31 +++++++\n>  src/gstreamer/gstlibcamerapad.h      |   8 ++\n>  src/gstreamer/gstlibcamerapool.cpp   |  18 +++-\n>  src/gstreamer/gstlibcamerapool.h     |   3 +-\n>  src/gstreamer/gstlibcamerasrc.cpp    | 133 ++++++++++++++++++++++++++-\n>  7 files changed, 226 insertions(+), 4 deletions(-)\n>\n> diff --git a/src/gstreamer/gstlibcamera-utils.cpp \n> b/src/gstreamer/gstlibcamera-utils.cpp\n> index 732987ef..09af9204 100644\n> --- a/src/gstreamer/gstlibcamera-utils.cpp\n> +++ b/src/gstreamer/gstlibcamera-utils.cpp\n> @@ -591,6 +591,39 @@ gst_task_resume(GstTask *task)  }  #endif\n>\n> +#if !GST_CHECK_VERSION(1, 22, 0)\n> +/*\n> + * Copyright (C) <1999> Erik Walthinsen <omega@cse.ogi.edu>\n> + * Library       <2002> Ronald Bultje <rbultje@ronald.bitfreak.net>\n> + * Copyright (C) <2007> David A. Schleef <ds@schleef.org>  */\n> +/* This function has been imported directly from the gstreamer \n> +project to\n> + * support backwards compatibility and should be removed when the \n> +older\n> + * version is no longer supported. */ gint \n> +gst_video_format_info_extrapolate_stride(const GstVideoFormatInfo \n> +*finfo, gint plane, gint stride) {\n> +     gint estride;\n> +     gint comp[GST_VIDEO_MAX_COMPONENTS];\n> +     gint i;\n> +\n> +     /* there is nothing to extrapolate on first plane */\n> +     if (plane == 0)\n> +             return stride;\n> +\n> +     gst_video_format_info_component(finfo, plane, comp);\n> +\n> +     /* For now, all planar formats have a single component on first plane, but\n> +     * if there was a planar format with more, we'd have to make a ratio of the\n> +     * number of component on the first plane against the number of component on\n> +     * the current plane. */\n> +     estride = 0;\n> +     for (i = 0; i < GST_VIDEO_MAX_COMPONENTS && comp[i] >= 0; i++)\n> +             estride += GST_VIDEO_FORMAT_INFO_SCALE_WIDTH(finfo, \n> + comp[i], stride);\n> +\n> +     return estride;\n> +}\n> +#endif\n> +\n>  G_LOCK_DEFINE_STATIC(cm_singleton_lock);\n>  static std::weak_ptr<CameraManager> cm_singleton_ptr;\n>\n> diff --git a/src/gstreamer/gstlibcamera-utils.h \n> b/src/gstreamer/gstlibcamera-utils.h\n> index cab1c814..81149280 100644\n> --- a/src/gstreamer/gstlibcamera-utils.h\n> +++ b/src/gstreamer/gstlibcamera-utils.h\n> @@ -35,6 +35,10 @@ static inline void gst_clear_event(GstEvent \n> **event_ptr)  #if !GST_CHECK_VERSION(1, 17, 1)  gboolean \n> gst_task_resume(GstTask *task);  #endif\n> +\n> +#if !GST_CHECK_VERSION(1, 22, 0)\n> +gint gst_video_format_info_extrapolate_stride(const \n> +GstVideoFormatInfo *finfo, gint plane, gint stride); #endif\n>  std::shared_ptr<libcamera::CameraManager> \n> gst_libcamera_get_camera_manager(int &ret);\n\nnit: I'd like a blank line after endif\n\n>\n>  /**\n> diff --git a/src/gstreamer/gstlibcamerapad.cpp \n> b/src/gstreamer/gstlibcamerapad.cpp\n> index 7b22aebe..7fd11a8f 100644\n> --- a/src/gstreamer/gstlibcamerapad.cpp\n> +++ b/src/gstreamer/gstlibcamerapad.cpp\n> @@ -18,6 +18,8 @@ struct _GstLibcameraPad {\n>       GstPad parent;\n>       StreamRole role;\n>       GstLibcameraPool *pool;\n> +     GstBufferPool *video_pool;\n> +     GstVideoInfo info;\n>       GstClockTime latency;\n>  };\n>\n> @@ -153,6 +155,35 @@ gst_libcamera_pad_set_pool(GstPad *pad, GstLibcameraPool *pool)\n>       self->pool = pool;\n>  }\n>\n> +GstBufferPool *\n> +gst_libcamera_pad_get_video_pool(GstPad *pad) {\n> +     auto *self = GST_LIBCAMERA_PAD(pad);\n> +     return self->video_pool;\n> +}\n> +\n> +void gst_libcamera_pad_set_video_pool(GstPad *pad, GstBufferPool \n> +*video_pool) {\n> +     auto *self = GST_LIBCAMERA_PAD(pad);\n> +\n> +     if (self->video_pool)\n> +             g_object_unref(self->video_pool);\n> +     self->video_pool = video_pool;\n> +}\n> +\n> +GstVideoInfo gst_libcamera_pad_get_video_info(GstPad *pad) {\n> +     auto *self = GST_LIBCAMERA_PAD(pad);\n> +     return self->info;\n> +}\n> +\n> +void gst_libcamera_pad_set_video_info(GstPad *pad, GstVideoInfo info)\n\nVideoInfo, a large structure, is being copied twice, pass a const reference so you only copy it once.\n\n> +{\n> +     auto *self = GST_LIBCAMERA_PAD(pad);\n> +\n> +     self->info = info;\n> +}\n> +\n>  Stream *\n>  gst_libcamera_pad_get_stream(GstPad *pad)  { diff --git \n> a/src/gstreamer/gstlibcamerapad.h b/src/gstreamer/gstlibcamerapad.h \n> index 630c168a..ed0beb1f 100644\n> --- a/src/gstreamer/gstlibcamerapad.h\n> +++ b/src/gstreamer/gstlibcamerapad.h\n> @@ -23,6 +23,14 @@ GstLibcameraPool *gst_libcamera_pad_get_pool(GstPad \n> *pad);\n>\n>  void gst_libcamera_pad_set_pool(GstPad *pad, GstLibcameraPool *pool);\n>\n> +GstBufferPool *gst_libcamera_pad_get_video_pool(GstPad *pad);\n> +\n> +void gst_libcamera_pad_set_video_pool(GstPad *pad, GstBufferPool \n> +*video_pool);\n> +\n> +GstVideoInfo gst_libcamera_pad_get_video_info(GstPad *pad);\n> +\n> +void gst_libcamera_pad_set_video_info(GstPad *pad, GstVideoInfo \n> +info);\n> +\n>  libcamera::Stream *gst_libcamera_pad_get_stream(GstPad *pad);\n>\n>  void gst_libcamera_pad_set_latency(GstPad *pad, GstClockTime \n> latency); diff --git a/src/gstreamer/gstlibcamerapool.cpp \n> b/src/gstreamer/gstlibcamerapool.cpp\n> index 9cd7eccb..471f71da 100644\n> --- a/src/gstreamer/gstlibcamerapool.cpp\n> +++ b/src/gstreamer/gstlibcamerapool.cpp\n> @@ -14,6 +14,9 @@\n>\n>  #include \"gstlibcamera-utils.h\"\n>\n> +#include <gst/gstmeta.h>\n\nnit: gst/gst.h is sufficient and already included. n GStreamer we usually do one header per library. Unlike C++ it has marginal effect on compilation time.\n\n> +\n> +\n>  using namespace libcamera;\n>\n>  enum {\n> @@ -134,8 +137,20 @@ gst_libcamera_pool_class_init(GstLibcameraPoolClass *klass)\n>                                                    G_TYPE_NONE, 0);  }\n>\n> +static void\n> +gst_libcamera_buffer_add_video_meta(GstBuffer *buffer, GstVideoInfo \n> +*info) {\n> +     GstVideoMeta *vmeta;\n> +     vmeta = gst_buffer_add_video_meta_full(buffer, GST_VIDEO_FRAME_FLAG_NONE,\n> +                                            GST_VIDEO_INFO_FORMAT(info), GST_VIDEO_INFO_WIDTH(info),\n> +                                            GST_VIDEO_INFO_HEIGHT(info), GST_VIDEO_INFO_N_PLANES(info),\n> +                                            info->offset, info->stride);\n> +     GST_META_FLAGS(vmeta) = (GstMetaFlags)(GST_META_FLAGS(vmeta) | \n> +GST_META_FLAG_POOLED); }\n> +\n>  GstLibcameraPool *\n> -gst_libcamera_pool_new(GstLibcameraAllocator *allocator, Stream \n> *stream)\n> +gst_libcamera_pool_new(GstLibcameraAllocator *allocator, Stream *stream,\n> +                    GstVideoInfo *info)\n>  {\n>       auto *pool = \n> GST_LIBCAMERA_POOL(g_object_new(GST_TYPE_LIBCAMERA_POOL, nullptr));\n>\n> @@ -145,6 +160,7 @@ gst_libcamera_pool_new(GstLibcameraAllocator *allocator, Stream *stream)\n>       gsize pool_size = gst_libcamera_allocator_get_pool_size(allocator, stream);\n>       for (gsize i = 0; i < pool_size; i++) {\n>               GstBuffer *buffer = gst_buffer_new();\n> +             gst_libcamera_buffer_add_video_meta(buffer, info);\n>               pool->queue->push_back(buffer);\n>       }\n>\n> diff --git a/src/gstreamer/gstlibcamerapool.h \n> b/src/gstreamer/gstlibcamerapool.h\n> index 2a7a9c77..02ee4dd4 100644\n> --- a/src/gstreamer/gstlibcamerapool.h\n> +++ b/src/gstreamer/gstlibcamerapool.h\n> @@ -14,6 +14,7 @@\n>  #include \"gstlibcameraallocator.h\"\n>\n>  #include <gst/gst.h>\n> +#include <gst/video/video.h>\n>\n>  #include <libcamera/stream.h>\n>\n> @@ -21,7 +22,7 @@\n>  G_DECLARE_FINAL_TYPE(GstLibcameraPool, gst_libcamera_pool, \n> GST_LIBCAMERA, POOL, GstBufferPool)\n>\n>  GstLibcameraPool *gst_libcamera_pool_new(GstLibcameraAllocator *allocator,\n> -                                      libcamera::Stream *stream);\n> +                                      libcamera::Stream *stream, \n> + GstVideoInfo *info);\n>\n>  libcamera::Stream *gst_libcamera_pool_get_stream(GstLibcameraPool \n> *self);\n>\n> diff --git a/src/gstreamer/gstlibcamerasrc.cpp \n> b/src/gstreamer/gstlibcamerasrc.cpp\n> index 8efa25f4..82b06eb4 100644\n> --- a/src/gstreamer/gstlibcamerasrc.cpp\n> +++ b/src/gstreamer/gstlibcamerasrc.cpp\n> @@ -268,6 +268,52 @@ GstLibcameraSrcState::requestCompleted(Request *request)\n>       gst_task_resume(src_->task);\n>  }\n>\n> +static void\n> +gst_libcamera_extrapolate_info(GstVideoInfo *info, guint32 stride) {\n> +     guint i, estride;\n> +     gsize offset = 0;\n> +\n> +     /* this should be updated if tiled formats get added in the future. */\n> +     for (i = 0; i < GST_VIDEO_INFO_N_PLANES(info); i++) {\n> +             estride = gst_video_format_info_extrapolate_stride(info->finfo, i, stride);\n> +             info->stride[i] = estride;\n> +             info->offset[i] = offset;\n> +             offset += estride * GST_VIDEO_FORMAT_INFO_SCALE_HEIGHT(info->finfo, i,\n> +                                                                    GST_VIDEO_INFO_HEIGHT(info));\n> +     }\n> +}\n> +\n> +static GstFlowReturn\n> +gst_libcamera_video_frame_copy(GstBuffer *src, GstBuffer *dest, \n> +GstVideoInfo dest_info, guint32 stride)\n\nWe usually avoid copying larger structure, even though valid. So instead we pass a const pointer to VideoInfo.\n\n> +{\n> +     GstVideoInfo src_info = dest_info;\n> +     GstVideoFrame src_frame, dest_frame;\n> +\n> +     gst_libcamera_extrapolate_info(&src_info, stride);\n> +     src_info.size = gst_buffer_get_size(src);\n> +\n> +     if (!gst_video_frame_map(&src_frame, &src_info, src, GST_MAP_READ))\n> +             goto invalid_buffer;\n> +\n> +     if (!gst_video_frame_map(&dest_frame, &dest_info, dest, GST_MAP_WRITE)) {\n> +             gst_video_frame_unmap(&src_frame);\n> +             goto invalid_buffer;\n> +     }\n> +\n> +     gst_video_frame_copy(&dest_frame, &src_frame);\n\nThis can fail, usually a programming error, but its nice to report it.\n\n> +\n> +     gst_video_frame_unmap(&src_frame);\n> +     gst_video_frame_unmap(&dest_frame);\n> +\n> +     return GST_FLOW_OK;\n> +\n> +invalid_buffer : {\n> +     GST_ERROR(\"Could not map buffer\");\n> +     return GST_FLOW_ERROR;\n> +}\n> +}\n> +\n>  /* Must be called with stream_lock held. */  int \n> GstLibcameraSrcState::processRequest()\n>  {\n> @@ -292,11 +338,36 @@ int GstLibcameraSrcState::processRequest()\n>       GstFlowReturn ret = GST_FLOW_OK;\n>       gst_flow_combiner_reset(src_->flow_combiner);\n>\n> -     for (GstPad *srcpad : srcpads_) {\n> +     for (gsize i = 0; i < src_->state->srcpads_.size(); i++) {\nWhy not just                  srcpads_.size() ?\n\nCause I think if \"this\" is not the same as src_->state, we have a problem.\n\n> +             GstPad *srcpad = src_->state->srcpads_[i];\n\nidem.\n\n>               Stream *stream = gst_libcamera_pad_get_stream(srcpad);\n>               GstBuffer *buffer = wrap->detachBuffer(stream);\n>\n>               FrameBuffer *fb = \n> gst_libcamera_buffer_get_frame_buffer(buffer);\n> +             const StreamConfiguration &stream_cfg = src_->state->config_->at(i);\n> +             GstBufferPool *video_pool = \n> + gst_libcamera_pad_get_video_pool(srcpad);\n> +\n> +             if (video_pool) {\n\nPerhaps a comment explaining that we only set a pool when a copy is needed ?\n\n> +                     GstBuffer *copy = NULL;\n> +                     GstVideoInfo info = \n> + gst_libcamera_pad_get_video_info(srcpad);\n> +\n> +                     ret = gst_buffer_pool_acquire_buffer(video_pool, &copy, NULL);\n> +                     if (ret != GST_FLOW_OK) {\n> +                             GST_ERROR(\"Failed to acquire buffer\");\n> +                             gst_buffer_unref(buffer);\n> +                             return -EPIPE;\n\nEPIPE paused the thread, this will lead to stall. If you want this to be fatal you must post an error message on the bus (see GST_ELEMENT_ERROR() macro.\n\n> +                     }\n> +\n> +                     ret = gst_libcamera_video_frame_copy(buffer, copy, info, stream_cfg.stride);\n> +                     gst_buffer_unref(buffer);\n> +                     if (ret != GST_FLOW_OK) {\n> +                             GST_ERROR(\"Failed to copy buffer\");\n> +                             gst_buffer_unref(copy);\n> +                             return -EPIPE;\n\nSame.\n\n> +                     }\n> +\n> +                     buffer = copy;\n> +             }\n>\n>               if (GST_CLOCK_TIME_IS_VALID(wrap->pts_)) {\n>                       GST_BUFFER_PTS(buffer) = wrap->pts_; @@ -497,13 \n> +568,65 @@ gst_libcamera_src_negotiate(GstLibcameraSrc *self)\n>       for (gsize i = 0; i < state->srcpads_.size(); i++) {\n>               GstPad *srcpad = state->srcpads_[i];\n>               const StreamConfiguration &stream_cfg = \n> state->config_->at(i);\n> +             GstBufferPool *video_pool = NULL;\n> +             GstVideoInfo info;\n> +\n> +             g_autoptr(GstCaps) caps = \n> + gst_libcamera_stream_configuration_to_caps(stream_cfg);\n> +\n> +             gst_video_info_init(&info);\n\nNot needed if you use from_caps() below ...\n\n> +             gst_video_info_from_caps(&info, caps);\n> +             gst_libcamera_pad_set_video_info(srcpad, info);\n> +\n> +             /* stride mismatch between camera stride and that calculated by video-info */\n> +             if (static_cast<unsigned int>(info.stride[0]) != stream_cfg.stride &&\n> +                 GST_VIDEO_INFO_FORMAT(&info) != GST_VIDEO_FORMAT_ENCODED) {\n> +                     GstQuery *query = NULL;\n> +                     gboolean need_pool = true;\n\nSince you just use it for readability, can be const.\n\n> +                     gboolean has_video_meta = false;\n> +\n> +                     gst_libcamera_extrapolate_info(&info, \n> + stream_cfg.stride);\n> +\n> +                     query = gst_query_new_allocation(caps, need_pool);\n> +                     if (!gst_pad_peer_query(srcpad, query))\n> +                             GST_DEBUG_OBJECT(self, \"Didn't get downstream ALLOCATION hints\");\n> +                     else\n> +                             has_video_meta = \n> + gst_query_find_allocation_meta(query, GST_VIDEO_META_API_TYPE,\n> NULL);\n> +\n> +                     if (!has_video_meta) {\n> +                             GstBufferPool *pool = NULL;\n> +\n> +                             if (gst_query_get_n_allocation_pools(query) > 0)\n> +                                     \n> + gst_query_parse_nth_allocation_pool(query, 0, &pool, NULL, NULL, \n> + NULL);\n> +\n> +                             if (pool)\n> +                                     video_pool = pool;\n> +                             else {\n> +                                     GstStructure *config;\n> +                                     guint min_buffers = 3;\n> +                                     video_pool = \n> + gst_video_buffer_pool_new();\n> +\n> +                                     config = gst_buffer_pool_get_config(video_pool);\n> +                                     gst_buffer_pool_config_set_params(config, caps, info.size, min_buffers, 0);\n> +                                     gst_buffer_pool_set_config(GST_BUFFER_POOL_CAST(video_pool), config);\n> +                                     GST_DEBUG_OBJECT(self, \"Own pool \n> + config is %\" GST_PTR_FORMAT, config);\n\nMove the trace before the set, since you don't own the pointer afterward, which is risky.\n\n> +                             }\n> +\n> +                             if (!gst_buffer_pool_set_active(video_pool, true)) {\n> +                                     GST_ERROR_OBJECT(self, \"Failed \n> + to active buffer pool\");\n\nUse GST_ELEMENT_ERROR() here.\n\n> +                                     gst_caps_unref(caps);\n> +                                     return false;\n> +                             }\n> +                     }\n> +                     gst_query_unref(query);\n> +             }\n>\n>               GstLibcameraPool *pool = gst_libcamera_pool_new(self->allocator,\n> -                                                             stream_cfg.stream());\n> +                                                             \n> + stream_cfg.stream(), &info);\n>               g_signal_connect_swapped(pool, \"buffer-notify\",\n>                                        G_CALLBACK(gst_task_resume), \n> self->task);\n>\n>               gst_libcamera_pad_set_pool(srcpad, pool);\n> +             gst_libcamera_pad_set_video_pool(srcpad, video_pool);\n\nSee comment about passing a reference. Might be nicer to combine passing the pool and the video info ?\n\n>\n>               /* Clear all reconfigure flags. */\n>               gst_pad_check_reconfigure(srcpad);\n> @@ -920,6 +1043,12 @@ gst_libcamera_src_release_pad(GstElement *element, GstPad *pad)\n>               auto end_iterator = pads.end();\n>               auto pad_iterator = std::find(begin_iterator, \n> end_iterator, pad);\n>\n> +             GstBufferPool *video_pool = gst_libcamera_pad_get_video_pool(pad);\n> +             if (video_pool) {\n> +                     gst_buffer_pool_set_active(video_pool, false);\n> +                     gst_object_unref(video_pool);\n> +             }\n\nWhat about moving this in the pad, and simpy calling gst_libcamera_pad_set_pool(nullptr),\nor pad_unset_pool() ?\n\n> +\n>               if (pad_iterator != end_iterator) {\n>                       g_object_unref(*pad_iterator);\n>                       pads.erase(pad_iterator);","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 65C9DC3200\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 13 May 2025 09:29:33 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2157168B52;\n\tTue, 13 May 2025 11:29:33 +0200 (CEST)","from EUR05-DB8-obe.outbound.protection.outlook.com\n\t(mail-db8eur05on20621.outbound.protection.outlook.com\n\t[IPv6:2a01:111:f403:2614::621])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 35AFC68B51\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 13 May 2025 11:29:31 +0200 (CEST)","from PAXPR04MB8285.eurprd04.prod.outlook.com\n\t(2603:10a6:102:1ca::15)\n\tby AS8PR04MB7750.eurprd04.prod.outlook.com (2603:10a6:20b:2aa::5)\n\twith Microsoft SMTP Server (version=TLS1_2,\n\tcipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.8722.29;\n\tTue, 13 May 2025 09:29:30 +0000","from PAXPR04MB8285.eurprd04.prod.outlook.com\n\t([fe80::e003:8fb:64ea:acfd]) by\n\tPAXPR04MB8285.eurprd04.prod.outlook.com\n\t([fe80::e003:8fb:64ea:acfd%3]) with mapi id 15.20.8722.027;\n\tTue, 13 May 2025 09:29:29 +0000"],"Authentication-Results":["lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=nxp.com header.i=@nxp.com header.b=\"Ns9w0z60\";\n\tdkim-atps=neutral","dkim=none (message not signed)\n\theader.d=none;dmarc=none action=none header.from=nxp.com;"],"ARC-Seal":"i=1; a=rsa-sha256; s=arcselector10001; d=microsoft.com; cv=none;\n\tb=psIi3noh8aP+jHP4NXIdjpPgKSjmwZ0NHJMEG+ahii1EdvJIXo7BGwsYTnnAp2DAFuu0sIUcy1xwBu2LVsNr7H5kz9ok03AntZUGq+k0BODZeyY/cbijlOloV8/OJFB4zS+imfHpNSCNvFtLDNjNIKeQkDdPMbyub7sxSAQShrvu5rzx86UBTNKQh1T9GF8efdMFEoymQHIlTvP+NeGQY/coPz3o57K4Fnrkh9jFtk9ecD5oMAk5nyEEymxUAAUPsGmSxCst1dFcMGV4LFHWVGEpbCuQlWGo57snbxZoWa422KRXpBGMNGYdHGCU3x8KYtnO/QrXQBcItj/Q8lowYQ==","ARC-Message-Signature":"i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com;\n\ts=arcselector10001;\n\th=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1;\n\tbh=3QY/7f4lDon8XkW04cxTyHUu04qkYmS1SzVOa/VLXiw=;\n\tb=Hlw+fgHu3yKhU/KP1iiT/HbyCTd/H6aBENQ9RuI/cCxHLIjehwHegwk4/NGceypmYgSzihX9m0xgKOJHiqwV/wFhM9LbCC9nOPDQ3fD5tvBCPl/iv7u1jVCK0S2omTzoAHa3VgCXHnvtsb6Pr/t0YZBfP6nA+Rl1p2FK8FttfyZTIibX9U8cuGOJu4oMKmql1C8uZrsDJUWr0OJYFfGpjKgQMzgJMLPajjzIejSDFby1JBJFVw6FFDCZd5CPxxHbLc+5OQFot+mWZl3/745HGMqn7PFDmZli1PTTJInouizeFQLm2TyOCQ+ebi7co4ZGpzWLv6f9kZPsWPl1OaBmVQ==","ARC-Authentication-Results":"i=1; mx.microsoft.com 1; spf=pass\n\tsmtp.mailfrom=nxp.com; dmarc=pass action=none header.from=nxp.com;\n\tdkim=pass header.d=nxp.com; arc=none","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=nxp.com; s=selector1;\n\th=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck;\n\tbh=3QY/7f4lDon8XkW04cxTyHUu04qkYmS1SzVOa/VLXiw=;\n\tb=Ns9w0z60Y+a7bo1oNOVBS60gAH8nn75yODA6Adxt5aS28wr8i3kbcDBvui+Exw7iFIofQQVreQkKiiCUR0zsscdyu1VlSPAsM9EjUbva2k0g0lTnrA14I01ROGhT3ppY2yzpfmi5xfCwNByp9f5cp5ck7GeeZAG6b4cGhHBeXgf8ToBOsQL/yzBqx2f7BBbkqiNsBBMBW6XMNLdEODq5lf+so6mrkFvT6uZYop2VVbjL4UzOWi4lWzi5XqFt/OcWelJ4EjmgKyY5I/0c5JIpdg+p8nrCzXAAiKLs6TtoU5cK0QvXn99/gzhGrc55llUp/+mqv1OOUh7sGu3HFfRkdw==","From":"Qi Hou <qi.hou@nxp.com>","To":"Nicolas Dufresne <nicolas@ndufresne.ca>,\n\t\"libcamera-devel@lists.libcamera.org\"\n\t<libcamera-devel@lists.libcamera.org>","CC":"Jared Hu <jared.hu@nxp.com>, Julien Vuillaumier\n\t<julien.vuillaumier@nxp.com>","Subject":"RE: [EXT] Re: [PATCH v6] gstreamer: Add GstVideoMeta support","Thread-Topic":"[EXT] Re: [PATCH v6] gstreamer: Add GstVideoMeta support","Thread-Index":"AQHbRipHNCd3F3VqYU6DyDG//BDktrPQduEAgADPijA=","Date":"Tue, 13 May 2025 09:29:29 +0000","Message-ID":"<PAXPR04MB82859ACC6320C47D9B46BC329796A@PAXPR04MB8285.eurprd04.prod.outlook.com>","References":"<20241204085511.1733434-1-qi.hou@nxp.com>\n\t<f93c135d178d3869c8bfc842309ee63ae2907f99.camel@ndufresne.ca>","In-Reply-To":"<f93c135d178d3869c8bfc842309ee63ae2907f99.camel@ndufresne.ca>","Accept-Language":"zh-CN, en-US","Content-Language":"en-US","X-MS-Has-Attach":"","X-MS-TNEF-Correlator":"","authentication-results":["lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=nxp.com header.i=@nxp.com header.b=\"Ns9w0z60\";\n\tdkim-atps=neutral","dkim=none (message not signed)\n\theader.d=none;dmarc=none action=none header.from=nxp.com;"],"x-ms-publictraffictype":"Email","x-ms-traffictypediagnostic":"PAXPR04MB8285:EE_|AS8PR04MB7750:EE_","x-ms-office365-filtering-correlation-id":"f8921ce0-389b-478b-2244-08dd9200a993","x-ms-exchange-senderadcheck":"1","x-ms-exchange-antispam-relay":"0","x-microsoft-antispam":"BCL:0;\n\tARA:13230040|366016|1800799024|376014|38070700018; ","x-microsoft-antispam-message-info":"=?utf-8?q?KPikl2P8l3M8ERTdaktfqwQ2vm3Y?=\n\t=?utf-8?q?7Zc3ZwL/woL3hpaK3WMVI4RF0zVJvJ67+pKTw+6Bdl6LHlD34tnwKyrf?=\n\t=?utf-8?q?ZWKJ9IXYDfNW59Lz/L2rZ4OPzt70znDf3dh0EzU18dAIIhnbIzRdhD17?=\n\t=?utf-8?q?RHOvUC5LtYISmi/vzyLnXrVAQ/VZ0gigAyR75P5RK0hKZU9D4JkDz86C?=\n\t=?utf-8?q?KFfPiufBqtmw6hqJ0VRAX3FJH59xwGpt7HkPUUhnEZY3pTwRsyPkvqW8?=\n\t=?utf-8?q?7icjPlY4ena8vnGhE5RIrD7etj8piL5rutjfwIpLV9shu7tS6JFr3pqu?=\n\t=?utf-8?q?SsZcN9FIGCqRK0ZlKAxLrnztp2QxqMUGSmMd/SVFXZyHTr9w0tLWY1dy?=\n\t=?utf-8?q?zVKHlxOYmBF4ZUm0OWduLNAjnJIVY8IujEzWTf+imo1drQnNVg905FCF?=\n\t=?utf-8?q?r2DZFsoDccYqUYBLMw8TVFBVo1FnRfwlqfkan74y+GHckVj5Jbc8OXIl?=\n\t=?utf-8?q?/VWv1atF65S912fd6NboAEoN12OjhIV0khcm8OX73DqrZ3/RUL9XSj5O?=\n\t=?utf-8?q?Qy5HcYmFZ0BW4y9VRCOjG6dfgLotOk7X4nHYEwfO9Tf9nWZVDhacSWJf?=\n\t=?utf-8?q?xNsNJRSPbx79d9QW9+AMwwhqMpvc9vi/FNztKu+rvQsMS82LbT49LncO?=\n\t=?utf-8?q?ufnSHAFj0aYcmc3QSNGeRF7BxNcDJMKPk4niV+otX43z5/Mj+wBezPzN?=\n\t=?utf-8?q?ihb1wTtKbpmMIIw/k5HYh9FJ9vw9VqHJn2MydRrICqQFmAZ3SwIrZBzR?=\n\t=?utf-8?q?VnWp3chalgkiuKJ6bzRKvPu6/7f6xbIyfRKUqPqVUROOLXXeXjdjR24R?=\n\t=?utf-8?q?2BbHZ7wHP1TbV72+dYyE/E+xaII9zLFbXHc+1TIsvK3FQkhCWIoVmGWd?=\n\t=?utf-8?q?3WJQcpILqhR0vEjkpyU7M8tinZfOgCtvH2HtM9aSus7owB34I0vXEXgd?=\n\t=?utf-8?q?86Pcg8u2X90gnfeBub9Qq9TTCBDxlDiXFqmuPYLu5zxzJTK2cgJOhBKV?=\n\t=?utf-8?q?zT7uIfTpmR/nyXCgDZIFuZq2Bt97lvYaJ5DFMLNfXJi3cV3nHPFRVcVN?=\n\t=?utf-8?q?Mi81PvXeTQY5REza1SHt+wxF6f7HfuVP0LVOPJ/LrBHTcfv+yXevnGt3?=\n\t=?utf-8?q?ZxdNYZJayTy6iIiSGYR74bl+64Zr3ZFpLeJ6Pn0IV5u7/Q76XnIpCxw/?=\n\t=?utf-8?q?YAJGFXCQ4RPwUTAIP7QIA/Y62fb9JGGC2zK8WmIEvrICoEiHXF0SW1xF?=\n\t=?utf-8?q?o/iK3C8GY/DA+2RZ9hxka2AOincLLWX6qzL4E96GiGUDwZ793mFKlMB/?=\n\t=?utf-8?q?ymeIcZXxNpBC43sS+yiMsc5ViG8TkyJJs8Low8khbfEN/XzlSfWDpDCG?=\n\t=?utf-8?q?BJ/Tr5uySCckA7N8mVE8cnvDkfd6ecbS8P7KzbAj30iUWOeAMLMieIVV?=\n\t=?utf-8?q?heIPtv3cDcj/RqTah3QXboeGccB3emzD+PZgw3pq4yPyidRExonFHH5s?=\n\t=?utf-8?q?/eaJdhcPfMW1EpmvtalbE2FVk189aqaD0bnNvX2X2bUn1cplRrBf9qc5?=\n\t=?utf-8?q?7+itgsXG9A=3D=3D?=","x-forefront-antispam-report":"CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:;\n\tIPV:NLI; SFV:NSPM; H:PAXPR04MB8285.eurprd04.prod.outlook.com; PTR:;\n\tCAT:NONE; \n\tSFS:(13230040)(366016)(1800799024)(376014)(38070700018); DIR:OUT;\n\tSFP:1101; ","x-ms-exchange-antispam-messagedata-chunkcount":"1","x-ms-exchange-antispam-messagedata-0":"=?utf-8?q?kZ1muWZ8dTe4+Wf/LH8a1hek3?=\n\t=?utf-8?q?UiAPiZVUi7EpIJTXUWm0h1U5f3Ip51Fi1hU1lYVeeUbla0H/yI0rtw2V?=\n\t=?utf-8?q?72rneM2OdwFkNgLzxgttoIWmGwV1z8v/USp5TV/h6VTdGz1qA+/lnkOZ?=\n\t=?utf-8?q?vep4mcsELIOo5ygoC7XyS+fonDEyY9gYaL9jFfxaCtsxHy9YCzNBXtLY?=\n\t=?utf-8?q?iok1iED8l6wpUV6b745VHKRmfKHUO3rxXRz2Cb51t106CowrccCbJjRv?=\n\t=?utf-8?q?YZS/b1tAgSK4GSvlvH4ysWYI0qvyLy0RWitiFhBemCqdiF4LfsDPj3Pf?=\n\t=?utf-8?q?Ke5lCJ9DQQsEG1FTtFOR1pwL912+epqQL77mJ9K0By4u6gQiOdnrbCQP?=\n\t=?utf-8?q?z2bcvGy9P8bOSHV8xSxXfVsk/O75ax8H+yIDb0UGIhgzLOkp1MkO4leH?=\n\t=?utf-8?q?sTxYqHTry3RHUTLTKQGLQjKE8lTiWb52fJ9PCKyva/YwreglzmhONjzK?=\n\t=?utf-8?q?44XndvUa6Qz5SN2IcTpbbohQHiqqJ+C/nJ/hcI63co54bsvRXAbR6ASD?=\n\t=?utf-8?q?aRVUdMexP7yX89iHXUCH2ZdS0Bcg1lODxlLXCCWcqVgCYEtHwuUZ8rCE?=\n\t=?utf-8?q?OmLG0yJFJ7cR7Uqp7MPk4lo+4IICORryKbnwEn7qheZCW/J3F46ybB8F?=\n\t=?utf-8?q?KzaTxHwoVN8zbCwmTYvFNi0Q93bw+ukYBDOT2AguszFYvkScvmkIJVZP?=\n\t=?utf-8?q?lkXWPaXYZCIkPSQomPzrrUtW/YjuI4E0SjHrRZHYG08w7kz7nBkw0TXQ?=\n\t=?utf-8?q?1DL84AtVNXYQvifrwXdkY9k4evnzRzSNPEXBI1JtG7qW+dEaqeR47fg6?=\n\t=?utf-8?q?HjgHL/pRxt+2foEBO+AtTRcpH64ARQ4SwtVsA/7Lr3EnTlSZlL370UUU?=\n\t=?utf-8?q?/jUjB1T9R2UWX7Wi6klzqE3gAqFv/CjTQQ+weuX/Crin/H/cQEG6bgtd?=\n\t=?utf-8?q?QsMeT23JG1J3eO7cfidEfLQF9TLQzD7O277KD+STqnTZbHe4/YQGQD1V?=\n\t=?utf-8?q?RXvykT5aTSgOY8t09yCkymdVt1TuNCMrsP1bSMVxmVh5UgMpxcfWt77+?=\n\t=?utf-8?q?mc/FZNPewrZjlKeAwY1vtyKExTs1MG3SpfmEEPYfXPDfPVfWd3p2lyeh?=\n\t=?utf-8?q?Qfa325GWuxmzMnDsA220eYCzNWvcZYN1I5lOK3+PSKcwDRb6jdO9FXTh?=\n\t=?utf-8?q?Sn2PRouXe15XAwmsTMNo5sIFt4/X/vmyFmnKlmd10BrxxGekn8Dene4y?=\n\t=?utf-8?q?mMO7okgkQpKXIIhfX3tuSkwHte8ikHgC4OvSdTXHUNpdZkCkXHcX0MB6?=\n\t=?utf-8?q?qygGlyDYtWKqVfnI5Ff3EHdqAYIxWtPuS6CwMuaSbS32xDoK6UNYRVAm?=\n\t=?utf-8?q?Hk2diujzDjBvfjMQjCFfZXDYIHrkqfKrlNKT0gzo5FaXjMoAgDh8/srk?=\n\t=?utf-8?q?Pg82ABmdYWzrbn1xGMpJ1AX/8sdkGsECMd97wHenDnbi0rxor4jv63Ob?=\n\t=?utf-8?q?vZZFpqCTg34PsfwnhKXPEbDXCL1tvo1YnjiGippGzQkMOEWhGDxcY3WX?=\n\t=?utf-8?q?F+tr12hdL3fKEt68r00DBj2nqZM+o/Y/MWnvOofCjHI8Z4A+TJ90RT+9?=\n\t=?utf-8?q?ErGEHHAC0hmCV0lQUf+728vJ5iTnm/n6tpuR7TcPjc=3D?=","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","MIME-Version":"1.0","X-OriginatorOrg":"nxp.com","X-MS-Exchange-CrossTenant-AuthAs":"Internal","X-MS-Exchange-CrossTenant-AuthSource":"PAXPR04MB8285.eurprd04.prod.outlook.com","X-MS-Exchange-CrossTenant-Network-Message-Id":"f8921ce0-389b-478b-2244-08dd9200a993","X-MS-Exchange-CrossTenant-originalarrivaltime":"13 May 2025 09:29:29.8641\n\t(UTC)","X-MS-Exchange-CrossTenant-fromentityheader":"Hosted","X-MS-Exchange-CrossTenant-id":"686ea1d3-bc2b-4c6f-a92c-d99c5c301635","X-MS-Exchange-CrossTenant-mailboxtype":"HOSTED","X-MS-Exchange-CrossTenant-userprincipalname":"+Iz4AoAKjkQsqT1GgPhT0Yq4FVX6mHelL2Mk3rWARPCh+HqixR4na4VqVw/yUwhQ","X-MS-Exchange-Transport-CrossTenantHeadersStamped":"AS8PR04MB7750","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]