[{"id":32154,"web_url":"https://patchwork.libcamera.org/comment/32154/","msgid":"<13945eb57fb8706d491bc27b6886dc3668020e5a.camel@ndufresne.ca>","date":"2024-11-13T19:19:51","subject":"Re: [PATCH v3] 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 13 novembre 2024 à 16:29 +0900, Hou Qi a écrit :\n> GStreamer video-info calculated stride and offset may differ from\n> those used by the camera.\n> \n> This patch enhances downstream plugin's support for videometa by\n> adding videometa support for libcamerasrc. It ensures that when\n> downstream plugin supports videometa by allocation query, libcamerasrc\n> also attaches videometa to buffer, preventing discrepancies\n> between the stride and offset calculated by video-info and camera.\n> \n> Signed-off-by: Hou Qi <qi.hou@nxp.com>\n> ---\n>  src/gstreamer/gstlibcamera-utils.cpp | 28 ++++++++++++++++++++++++\n>  src/gstreamer/gstlibcamera-utils.h   |  7 ++++++\n>  src/gstreamer/gstlibcamerapool.cpp   | 32 ++++++++++++++++++++++++----\n>  src/gstreamer/gstlibcamerapool.h     |  2 +-\n>  src/gstreamer/gstlibcamerasrc.cpp    | 14 +++++++++++-\n>  5 files changed, 77 insertions(+), 6 deletions(-)\n> \n> diff --git a/src/gstreamer/gstlibcamera-utils.cpp b/src/gstreamer/gstlibcamera-utils.cpp\n> index 732987ef..91de1d44 100644\n> --- a/src/gstreamer/gstlibcamera-utils.cpp\n> +++ b/src/gstreamer/gstlibcamera-utils.cpp\n> @@ -2,6 +2,9 @@\n>  /*\n>   * Copyright (C) 2020, Collabora Ltd.\n>   *     Author: Nicolas Dufresne <nicolas.dufresne@collabora.com>\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\nI'll leave that to the upper maintainers to evaluate, but it would be nice to\nidentify that these are only needed as long as\ngst_libcamera_extrapolate_stride() copy is there. As soon as Trixie becomes\nstable, we'll bump to GStreamer 1.22 and remove it, would be nice to remove the\nspurious copyright.\n\n>   *\n>   * GStreamer libcamera Utility Function\n>   */\n> @@ -591,6 +594,31 @@ gst_task_resume(GstTask *task)\n>  }\n>  #endif\n>  \n> +#if !GST_CHECK_VERSION(1, 22, 0)\n> +gint gst_libcamera_extrapolate_stride(const GstVideoFormatInfo *finfo, gint plane, gint stride)\n\nBecause its under an ifdef, it is guarantied not to clash at built time, and the\nlinker will always resolve it locally. Keep the mainline namespace, it will\nsimplify the code later.\n\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..14bf6664 100644\n> --- a/src/gstreamer/gstlibcamera-utils.h\n> +++ b/src/gstreamer/gstlibcamera-utils.h\n> @@ -2,6 +2,9 @@\n>  /*\n>   * Copyright (C) 2020, Collabora Ltd.\n>   *     Author: Nicolas Dufresne <nicolas.dufresne@collabora.com>\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\nNot needed.\n\n>   *\n>   * GStreamer libcamera Utility Functions\n>   */\n> @@ -35,6 +38,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_libcamera_extrapolate_stride(const GstVideoFormatInfo *finfo, gint plane, gint stride);\n\nApply namespace comment here too.\n\n> +#endif\n>  std::shared_ptr<libcamera::CameraManager> gst_libcamera_get_camera_manager(int &ret);\n>  \n>  /**\n> diff --git a/src/gstreamer/gstlibcamerapool.cpp b/src/gstreamer/gstlibcamerapool.cpp\n> index 9cd7eccb..315f08ac 100644\n> --- a/src/gstreamer/gstlibcamerapool.cpp\n> +++ b/src/gstreamer/gstlibcamerapool.cpp\n> @@ -135,16 +135,40 @@ gst_libcamera_pool_class_init(GstLibcameraPoolClass *klass)\n>  }\n>  \n>  GstLibcameraPool *\n> -gst_libcamera_pool_new(GstLibcameraAllocator *allocator, Stream *stream)\n> +gst_libcamera_pool_new(GstLibcameraAllocator *allocator, const StreamConfiguration &stream_cfg,\n> +\t\t       GstCaps *caps, gboolean add_video_meta)\n>  {\n>  \tauto *pool = GST_LIBCAMERA_POOL(g_object_new(GST_TYPE_LIBCAMERA_POOL, nullptr));\n> +\tGstVideoInfo info;\n>  \n>  \tpool->allocator = GST_LIBCAMERA_ALLOCATOR(g_object_ref(allocator));\n> -\tpool->stream = stream;\n> -\n> -\tgsize pool_size = gst_libcamera_allocator_get_pool_size(allocator, stream);\n> +\tpool->stream = stream_cfg.stream();\n\nnit: Is pool->stream const ? C++ and C are pretty permissive, but storing a non-\nconst  pointer obtained from a const object is a bit of a stretch.\n\n> +\n> +\tif (caps && gst_video_info_from_caps(&info, caps)) {\n\nThis needs some work, caps will never be null. Now, gst_video_info_from_caps()\nwill work for image/ and video/ type, in that case the format is set to ENCODED\nand a video meta must not be appended. That includes bayer format, for which we\nonly support matching display and storage dimensions (since arbitrary cropping\nof bayer is a bad idea).\n\nIn short, don't get caps, and be verbose if gst_video_info_from_caps() fails,\nthat is not expected.\n\n> +\t\tguint k, stride;\n> +\t\tgsize offset = 0;\n> +\t\tfor (k = 0; k < GST_VIDEO_INFO_N_PLANES(&info); k++) {\n> +#if !GST_CHECK_VERSION(1, 22, 0)\n\nNot needed with the suggestion I just made.\n\n> +\t\t\tstride = gst_libcamera_extrapolate_stride(info.finfo, k, stream_cfg.stride);\n> +#else\n> +\t\t\tstride = gst_video_format_info_extrapolate_stride(info.finfo, k, stream_cfg.stride);\n> +#endif\n> +\t\t\tinfo.stride[k] = stride;\n> +\t\t\tinfo.offset[k] = offset;\n> +\t\t\toffset += stride * GST_VIDEO_FORMAT_INFO_SCALE_HEIGHT(info.finfo, k, GST_VIDEO_INFO_HEIGHT(&info));\n\nPlease add a comment that this should be updated if tiled formats get added in\nthe future. For anything single plane we don't care of course.\n\n> +\t\t}\n> +\t} else\n> +\t\tadd_video_meta = false;\n\nNot sure yet why sometimes you don't want to add it.\n\n> +\n> +\tgsize pool_size = gst_libcamera_allocator_get_pool_size(allocator, stream_cfg.stream());\n>  \tfor (gsize i = 0; i < pool_size; i++) {\n>  \t\tGstBuffer *buffer = gst_buffer_new();\n> +\t\tif (add_video_meta) {\n> +\t\t\tgst_buffer_add_video_meta_full(buffer, GST_VIDEO_FRAME_FLAG_NONE,\n> +\t\t\t\t\t\t       GST_VIDEO_INFO_FORMAT(&info), GST_VIDEO_INFO_WIDTH(&info),\n> +\t\t\t\t\t\t       GST_VIDEO_INFO_HEIGHT(&info), GST_VIDEO_INFO_N_PLANES(&info),\n> +\t\t\t\t\t\t       info.offset, info.stride);\n> +\t\t}\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..8ad87cab 100644\n> --- a/src/gstreamer/gstlibcamerapool.h\n> +++ b/src/gstreamer/gstlibcamerapool.h\n> @@ -21,7 +21,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 const libcamera::StreamConfiguration &stream_cfg, GstCaps *caps, gboolean add_video_meta);\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..c05a31e7 100644\n> --- a/src/gstreamer/gstlibcamerasrc.cpp\n> +++ b/src/gstreamer/gstlibcamerasrc.cpp\n> @@ -497,9 +497,21 @@ 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\tGstQuery *query = NULL;\n> +\t\tgboolean add_video_meta = false;\n> +\n> +\t\tg_autoptr(GstCaps) caps = gst_libcamera_stream_configuration_to_caps(stream_cfg);\n> +\t\tgst_libcamera_framerate_to_caps(caps, element_caps);\n> +\n> +\t\tquery = gst_query_new_allocation(caps, false);\n\nMy hate for boolean parameters is strong enough that I'd like to suggest:\n\n\t\tbool need_pool = false;\n\t\tquery = gst_query_new_allocation(caps, need_pool);\n\n> +\t\tif (!gst_pad_peer_query(srcpad, query))\n> +\t\t\tGST_DEBUG_OBJECT(self, \"didn't get downstream ALLOCATION hints\");\n> +\t\telse\n> +\t\t\tadd_video_meta = gst_query_find_allocation_meta(query, GST_VIDEO_META_API_TYPE, \n> NULL);\n\nIts not sufficient to not add video meta. If downstream does not support\nVideoMeta and the stride does not match what you from just turning the caps into\nVideoInfo, you must bounce these buffers to system memory. For that reason, you\nlikely want to handle the stride extrapolation here and pass VideoInfo to the\npool instead. If the stride miss-match, you can then create a generic VideoPool\nusing the caps, and later own you will bounce the memory using\ngst_video_frame_copy() (which can handle stride miss-match).\n\n> +\t\tgst_query_unref(query);\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, caps, add_video_meta);\n>  \t\tg_signal_connect_swapped(pool, \"buffer-notify\",\n>  \t\t\t\t\t G_CALLBACK(gst_task_resume), self->task);\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 E72BEBE173\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 13 Nov 2024 19:19:56 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id CE94065825;\n\tWed, 13 Nov 2024 20:19:55 +0100 (CET)","from mail-qk1-x72c.google.com (mail-qk1-x72c.google.com\n\t[IPv6:2607:f8b0:4864:20::72c])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 09DD7657CF\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 13 Nov 2024 20:19:54 +0100 (CET)","by mail-qk1-x72c.google.com with SMTP id\n\taf79cd13be357-7b35b1ca0c2so1796085a.0\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 13 Nov 2024 11:19:53 -0800 (PST)","from nicolas-tpx395.localdomain ([2606:6d00:15:862e::580])\n\tby smtp.gmail.com with ESMTPSA id\n\taf79cd13be357-7b32ac2daa6sm723317085a.14.2024.11.13.11.19.52\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tWed, 13 Nov 2024 11:19:52 -0800 (PST)"],"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=\"dFb/wKON\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ndufresne-ca.20230601.gappssmtp.com; s=20230601; t=1731525593;\n\tx=1732130393; 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=WTRIr/q10JGhoF4Hw3AX5TqVfn+/vDdHKXqw8fhnZ0c=;\n\tb=dFb/wKONfjaalWQDxPV3TX5jFhT7ooxw3LIQmiAcBzNCMYgIhypvK3Oa7SG5BQIiXM\n\tkV7Ro88b5ENyZeRAeJIW5KXbOrm0w9PKj/rTqLzcMxHfn0nfL38L1Gt9RnD4i+Q3baIb\n\txz0bTxQ8WNsDHD0BSgq2AOeMzIfmHRTm5cya22xhyuN0y1OqiKdYbATJ7VxGUBA63lR5\n\tQNYqlcxLWanMYZqK2whTxyQ3J8BGahD3F3Kj77NsNuiNalc0Ve7hTgboLHZw2+D5Auhi\n\tKJ8hwnUCT/6XD5fzrMk/B46MqkdT+b0AfbhB9b2AWYJFf909OxC1jGCXpJ7kpVvCOw62\n\tp74Q==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1731525593; x=1732130393;\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=WTRIr/q10JGhoF4Hw3AX5TqVfn+/vDdHKXqw8fhnZ0c=;\n\tb=YOygWPVGGB5FTrNxlMkKjTKusOBW46yZLpng9Uv46LAbNT/TL/VU0KH+Sf1lbmgDiM\n\tfIL2QX3UW3xKbIB3/zz1xmwhdKg5QzfFSMqk446ddNmBOZUhuFsSA5y9PXfajLgW8M5t\n\tirebUJmY4wkG9xOBelBOdPVzJLgmHaKU3oqbN8ZzfBPpzJL7e87sEa11AwF+cCznMog6\n\tBwqE6yXhA4gfRtCPJb+bQ1ITG4+Mkt98BqDD+G8zML8iGNIYRtd/LSMtA05Mg+tKZ9HO\n\tq30a0eLVslWvZRIR17ZHxpm+7cQHr2jNRLu8yEDikQxlruBJzMnPr3UcGK605146EFHG\n\t31nA==","X-Forwarded-Encrypted":"i=1;\n\tAJvYcCWMjsBnIqxS2YwlwI9mHNoHFxxCOGX4e84oGw6xVEvA7L9eWAhn2cOEBqaeiDJsK4WYjh12hrIeZDoO0JRpqr8=@lists.libcamera.org","X-Gm-Message-State":"AOJu0YyHrzlkjI0fyFsoP5iiBPToVWsVylt8Hghlgtq9OvJP6/32yhTn\n\t8XWNJV5OnzpKaDhQ1sZtloKW+Lsz3uDAGcD2b7PZn2OcHRti0ih/O5rFpjfrtfI=","X-Google-Smtp-Source":"AGHT+IE1+HzsuAgVHfhC2bcsXm33eFUdGiFTnXQ8ghVsYcDmZ2i256JsLvByoYaaUyE2IMqWYyFlJg==","X-Received":"by 2002:a05:620a:2492:b0:7af:ce25:392c with SMTP id\n\taf79cd13be357-7b35a4690dcmr91523585a.4.1731525592748; \n\tWed, 13 Nov 2024 11:19:52 -0800 (PST)","Message-ID":"<13945eb57fb8706d491bc27b6886dc3668020e5a.camel@ndufresne.ca>","Subject":"Re: [PATCH v3] 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":"Wed, 13 Nov 2024 14:19:51 -0500","In-Reply-To":"<20241113072947.1579075-1-qi.hou@nxp.com>","References":"<20241113072947.1579075-1-qi.hou@nxp.com>","Content-Type":"text/plain; charset=\"UTF-8\"","Content-Transfer-Encoding":"quoted-printable","User-Agent":"Evolution 3.54.1 (3.54.1-1.fc41) ","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":32164,"web_url":"https://patchwork.libcamera.org/comment/32164/","msgid":"<173157611957.3831226.1409354371390888803@ping.linuxembedded.co.uk>","date":"2024-11-14T09:21:59","subject":"Re: [PATCH v3] 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 Nicolas Dufresne (2024-11-13 19:19:51)\n> Hi,\n> \n> Le mercredi 13 novembre 2024 à 16:29 +0900, Hou Qi a écrit :\n> > GStreamer video-info calculated stride and offset may differ from\n> > those used by the camera.\n> > \n> > This patch enhances downstream plugin's support for videometa by\n> > adding videometa support for libcamerasrc. It ensures that when\n> > downstream plugin supports videometa by allocation query, libcamerasrc\n> > also attaches videometa to buffer, preventing discrepancies\n> > between the stride and offset calculated by video-info and camera.\n> > \n> > Signed-off-by: Hou Qi <qi.hou@nxp.com>\n> > ---\n> >  src/gstreamer/gstlibcamera-utils.cpp | 28 ++++++++++++++++++++++++\n> >  src/gstreamer/gstlibcamera-utils.h   |  7 ++++++\n> >  src/gstreamer/gstlibcamerapool.cpp   | 32 ++++++++++++++++++++++++----\n> >  src/gstreamer/gstlibcamerapool.h     |  2 +-\n> >  src/gstreamer/gstlibcamerasrc.cpp    | 14 +++++++++++-\n> >  5 files changed, 77 insertions(+), 6 deletions(-)\n> > \n> > diff --git a/src/gstreamer/gstlibcamera-utils.cpp b/src/gstreamer/gstlibcamera-utils.cpp\n> > index 732987ef..91de1d44 100644\n> > --- a/src/gstreamer/gstlibcamera-utils.cpp\n> > +++ b/src/gstreamer/gstlibcamera-utils.cpp\n> > @@ -2,6 +2,9 @@\n> >  /*\n> >   * Copyright (C) 2020, Collabora Ltd.\n> >   *     Author: Nicolas Dufresne <nicolas.dufresne@collabora.com>\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> I'll leave that to the upper maintainers to evaluate, but it would be nice to\n> identify that these are only needed as long as\n> gst_libcamera_extrapolate_stride() copy is there. As soon as Trixie becomes\n> stable, we'll bump to GStreamer 1.22 and remove it, would be nice to remove the\n> spurious copyright.\n\nWhat about putting it in a comment banner directly adjacent to the\nfunction with an explanation that the function has been imported\ndirectly from the gstreamer project to support backwards compatibility\nand will be removed (the function and the comment banner) when the the\nolder version is no longer supported ?\n\nThen the copyright notice is directly adjacent to the code it relates\nto, and it's easy to see and remove it all in one go.\n\nAnother alternative would be to create a new\ngstlibcamera-compatibility.cpp and put it there on it's own - but I\nthink that's overkill, so keeping it in -utils is fine with me.\n\n> \n> >   *\n> >   * GStreamer libcamera Utility Function\n> >   */\n> > @@ -591,6 +594,31 @@ gst_task_resume(GstTask *task)\n> >  }\n> >  #endif\n> >  \n> > +#if !GST_CHECK_VERSION(1, 22, 0)\n> > +gint gst_libcamera_extrapolate_stride(const GstVideoFormatInfo *finfo, gint plane, gint stride)\n> \n> Because its under an ifdef, it is guarantied not to clash at built time, and the\n> linker will always resolve it locally. Keep the mainline namespace, it will\n> simplify the code later.\n\nYes, I agree - lets keep this as\ngst_video_format_info_extrapolate_stride please.\n\n> \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> >  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..14bf6664 100644\n> > --- a/src/gstreamer/gstlibcamera-utils.h\n> > +++ b/src/gstreamer/gstlibcamera-utils.h\n> > @@ -2,6 +2,9 @@\n> >  /*\n> >   * Copyright (C) 2020, Collabora Ltd.\n> >   *     Author: Nicolas Dufresne <nicolas.dufresne@collabora.com>\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> Not needed.\n> \n> >   *\n> >   * GStreamer libcamera Utility Functions\n> >   */\n> > @@ -35,6 +38,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_libcamera_extrapolate_stride(const GstVideoFormatInfo *finfo, gint plane, gint stride);\n> \n> Apply namespace comment here too.\n> \n> > +#endif\n> >  std::shared_ptr<libcamera::CameraManager> gst_libcamera_get_camera_manager(int &ret);\n> >  \n> >  /**\n> > diff --git a/src/gstreamer/gstlibcamerapool.cpp b/src/gstreamer/gstlibcamerapool.cpp\n> > index 9cd7eccb..315f08ac 100644\n> > --- a/src/gstreamer/gstlibcamerapool.cpp\n> > +++ b/src/gstreamer/gstlibcamerapool.cpp\n> > @@ -135,16 +135,40 @@ gst_libcamera_pool_class_init(GstLibcameraPoolClass *klass)\n> >  }\n> >  \n> >  GstLibcameraPool *\n> > -gst_libcamera_pool_new(GstLibcameraAllocator *allocator, Stream *stream)\n> > +gst_libcamera_pool_new(GstLibcameraAllocator *allocator, const StreamConfiguration &stream_cfg,\n> > +                    GstCaps *caps, gboolean add_video_meta)\n> >  {\n> >       auto *pool = GST_LIBCAMERA_POOL(g_object_new(GST_TYPE_LIBCAMERA_POOL, nullptr));\n> > +     GstVideoInfo info;\n> >  \n> >       pool->allocator = GST_LIBCAMERA_ALLOCATOR(g_object_ref(allocator));\n> > -     pool->stream = stream;\n> > -\n> > -     gsize pool_size = gst_libcamera_allocator_get_pool_size(allocator, stream);\n> > +     pool->stream = stream_cfg.stream();\n> \n> nit: Is pool->stream const ? C++ and C are pretty permissive, but storing a non-\n> const  pointer obtained from a const object is a bit of a stretch.\n> \n> > +\n> > +     if (caps && gst_video_info_from_caps(&info, caps)) {\n> \n> This needs some work, caps will never be null. Now, gst_video_info_from_caps()\n> will work for image/ and video/ type, in that case the format is set to ENCODED\n> and a video meta must not be appended. That includes bayer format, for which we\n> only support matching display and storage dimensions (since arbitrary cropping\n> of bayer is a bad idea).\n> \n> In short, don't get caps, and be verbose if gst_video_info_from_caps() fails,\n> that is not expected.\n> \n> > +             guint k, stride;\n> > +             gsize offset = 0;\n> > +             for (k = 0; k < GST_VIDEO_INFO_N_PLANES(&info); k++) {\n> > +#if !GST_CHECK_VERSION(1, 22, 0)\n> \n> Not needed with the suggestion I just made.\n> \n> > +                     stride = gst_libcamera_extrapolate_stride(info.finfo, k, stream_cfg.stride);\n> > +#else\n> > +                     stride = gst_video_format_info_extrapolate_stride(info.finfo, k, stream_cfg.stride);\n> > +#endif\n> > +                     info.stride[k] = stride;\n> > +                     info.offset[k] = offset;\n> > +                     offset += stride * GST_VIDEO_FORMAT_INFO_SCALE_HEIGHT(info.finfo, k, GST_VIDEO_INFO_HEIGHT(&info));\n> \n> Please add a comment that this should be updated if tiled formats get added in\n> the future. For anything single plane we don't care of course.\n> \n> > +             }\n> > +     } else\n> > +             add_video_meta = false;\n> \n> Not sure yet why sometimes you don't want to add it.\n> \n> > +\n> > +     gsize pool_size = gst_libcamera_allocator_get_pool_size(allocator, stream_cfg.stream());\n> >       for (gsize i = 0; i < pool_size; i++) {\n> >               GstBuffer *buffer = gst_buffer_new();\n> > +             if (add_video_meta) {\n> > +                     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> > +             }\n> >               pool->queue->push_back(buffer);\n> >       }\n> >  \n> > diff --git a/src/gstreamer/gstlibcamerapool.h b/src/gstreamer/gstlibcamerapool.h\n> > index 2a7a9c77..8ad87cab 100644\n> > --- a/src/gstreamer/gstlibcamerapool.h\n> > +++ b/src/gstreamer/gstlibcamerapool.h\n> > @@ -21,7 +21,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> > +                                      const libcamera::StreamConfiguration &stream_cfg, GstCaps *caps, gboolean add_video_meta);\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..c05a31e7 100644\n> > --- a/src/gstreamer/gstlibcamerasrc.cpp\n> > +++ b/src/gstreamer/gstlibcamerasrc.cpp\n> > @@ -497,9 +497,21 @@ 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> > +             GstQuery *query = NULL;\n> > +             gboolean add_video_meta = false;\n> > +\n> > +             g_autoptr(GstCaps) caps = gst_libcamera_stream_configuration_to_caps(stream_cfg);\n> > +             gst_libcamera_framerate_to_caps(caps, element_caps);\n> > +\n> > +             query = gst_query_new_allocation(caps, false);\n> \n> My hate for boolean parameters is strong enough that I'd like to suggest:\n> \n>                 bool need_pool = false;\n>                 query = gst_query_new_allocation(caps, need_pool);\n\n\tbool kieran_agrees = true;\n\tret = send_response(kieran_agrees);\n\n\n> > +             if (!gst_pad_peer_query(srcpad, query))\n> > +                     GST_DEBUG_OBJECT(self, \"didn't get downstream ALLOCATION hints\");\n> > +             else\n> > +                     add_video_meta = gst_query_find_allocation_meta(query, GST_VIDEO_META_API_TYPE, \n> > NULL);\n> \n> Its not sufficient to not add video meta. If downstream does not support\n> VideoMeta and the stride does not match what you from just turning the caps into\n> VideoInfo, you must bounce these buffers to system memory. For that reason, you\n> likely want to handle the stride extrapolation here and pass VideoInfo to the\n> pool instead. If the stride miss-match, you can then create a generic VideoPool\n> using the caps, and later own you will bounce the memory using\n> gst_video_frame_copy() (which can handle stride miss-match).\n> \n> > +             gst_query_unref(query);\n> >  \n> >               GstLibcameraPool *pool = gst_libcamera_pool_new(self->allocator,\n> > -                                                             stream_cfg.stream());\n> > +                                                             stream_cfg, caps, add_video_meta);\n> >               g_signal_connect_swapped(pool, \"buffer-notify\",\n> >                                        G_CALLBACK(gst_task_resume), self->task);\n> >  \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 05907C324C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 14 Nov 2024 09:22:05 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id EC6A765834;\n\tThu, 14 Nov 2024 10:22:03 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A79EA65831\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 14 Nov 2024 10:22:02 +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 C3D5D502;\n\tThu, 14 Nov 2024 10:21:48 +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=\"eGKN3T+V\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1731576108;\n\tbh=cPrAwPm1jQt8ke55beg+HCknDNMMgGXu/Kd+kZHpfGE=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=eGKN3T+VFlFkk69uYSauquNZggqat6JD0UR9OzW1oMaePJ/T1ww/kNYOuGUKWS1iU\n\tnwpbuz2woiCCe4VbUSwYXcZVpSazph7K+gFf6E2xl0V5YXJywZtJQx+qUr8mdplJmV\n\tktsdwvCdgIEwVYK3M5g5WcFjrQzbr/E7G7uXwpd4=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<13945eb57fb8706d491bc27b6886dc3668020e5a.camel@ndufresne.ca>","References":"<20241113072947.1579075-1-qi.hou@nxp.com>\n\t<13945eb57fb8706d491bc27b6886dc3668020e5a.camel@ndufresne.ca>","Subject":"Re: [PATCH v3] gstreamer: Add GstVideoMeta support","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"jared.hu@nxp.com, julien.vuillaumier@nxp.com","To":"Hou Qi <qi.hou@nxp.com>, Nicolas Dufresne <nicolas@ndufresne.ca>,\n\tlibcamera-devel@lists.libcamera.org","Date":"Thu, 14 Nov 2024 09:21:59 +0000","Message-ID":"<173157611957.3831226.1409354371390888803@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":32168,"web_url":"https://patchwork.libcamera.org/comment/32168/","msgid":"<20241114105648.GD26171@pendragon.ideasonboard.com>","date":"2024-11-14T10:56:48","subject":"Re: [PATCH v3] gstreamer: Add GstVideoMeta support","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Thu, Nov 14, 2024 at 09:21:59AM +0000, Kieran Bingham wrote:\n> Quoting Nicolas Dufresne (2024-11-13 19:19:51)\n> > Hi,\n> > \n> > Le mercredi 13 novembre 2024 à 16:29 +0900, Hou Qi a écrit :\n> > > GStreamer video-info calculated stride and offset may differ from\n> > > those used by the camera.\n> > > \n> > > This patch enhances downstream plugin's support for videometa by\n> > > adding videometa support for libcamerasrc. It ensures that when\n> > > downstream plugin supports videometa by allocation query, libcamerasrc\n> > > also attaches videometa to buffer, preventing discrepancies\n> > > between the stride and offset calculated by video-info and camera.\n> > > \n> > > Signed-off-by: Hou Qi <qi.hou@nxp.com>\n> > > ---\n> > >  src/gstreamer/gstlibcamera-utils.cpp | 28 ++++++++++++++++++++++++\n> > >  src/gstreamer/gstlibcamera-utils.h   |  7 ++++++\n> > >  src/gstreamer/gstlibcamerapool.cpp   | 32 ++++++++++++++++++++++++----\n> > >  src/gstreamer/gstlibcamerapool.h     |  2 +-\n> > >  src/gstreamer/gstlibcamerasrc.cpp    | 14 +++++++++++-\n> > >  5 files changed, 77 insertions(+), 6 deletions(-)\n> > > \n> > > diff --git a/src/gstreamer/gstlibcamera-utils.cpp b/src/gstreamer/gstlibcamera-utils.cpp\n> > > index 732987ef..91de1d44 100644\n> > > --- a/src/gstreamer/gstlibcamera-utils.cpp\n> > > +++ b/src/gstreamer/gstlibcamera-utils.cpp\n> > > @@ -2,6 +2,9 @@\n> > >  /*\n> > >   * Copyright (C) 2020, Collabora Ltd.\n> > >   *     Author: Nicolas Dufresne <nicolas.dufresne@collabora.com>\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> > I'll leave that to the upper maintainers to evaluate, but it would be nice to\n> > identify that these are only needed as long as\n> > gst_libcamera_extrapolate_stride() copy is there. As soon as Trixie becomes\n> > stable, we'll bump to GStreamer 1.22 and remove it, would be nice to remove the\n> > spurious copyright.\n> \n> What about putting it in a comment banner directly adjacent to the\n> function with an explanation that the function has been imported\n> directly from the gstreamer project to support backwards compatibility\n> and will be removed (the function and the comment banner) when the the\n> older version is no longer supported ?\n> \n> Then the copyright notice is directly adjacent to the code it relates\n> to, and it's easy to see and remove it all in one go.\n> \n> Another alternative would be to create a new\n> gstlibcamera-compatibility.cpp and put it there on it's own - but I\n> think that's overkill, so keeping it in -utils is fine with me.\n\nAgreed, a new file is overkill, we can just add the copyright comment\ndirectly above the function, within the #ifdef version guard.\n\n> > >   *\n> > >   * GStreamer libcamera Utility Function\n> > >   */\n> > > @@ -591,6 +594,31 @@ gst_task_resume(GstTask *task)\n> > >  }\n> > >  #endif\n> > >  \n> > > +#if !GST_CHECK_VERSION(1, 22, 0)\n> > > +gint gst_libcamera_extrapolate_stride(const GstVideoFormatInfo *finfo, gint plane, gint stride)\n> > \n> > Because its under an ifdef, it is guarantied not to clash at built time, and the\n> > linker will always resolve it locally. Keep the mainline namespace, it will\n> > simplify the code later.\n> \n> Yes, I agree - lets keep this as\n> gst_video_format_info_extrapolate_stride please.\n> \n> > \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> > >  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..14bf6664 100644\n> > > --- a/src/gstreamer/gstlibcamera-utils.h\n> > > +++ b/src/gstreamer/gstlibcamera-utils.h\n> > > @@ -2,6 +2,9 @@\n> > >  /*\n> > >   * Copyright (C) 2020, Collabora Ltd.\n> > >   *     Author: Nicolas Dufresne <nicolas.dufresne@collabora.com>\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> > Not needed.\n> > \n> > >   *\n> > >   * GStreamer libcamera Utility Functions\n> > >   */\n> > > @@ -35,6 +38,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_libcamera_extrapolate_stride(const GstVideoFormatInfo *finfo, gint plane, gint stride);\n> > \n> > Apply namespace comment here too.\n> > \n> > > +#endif\n> > >  std::shared_ptr<libcamera::CameraManager> gst_libcamera_get_camera_manager(int &ret);\n> > >  \n> > >  /**\n> > > diff --git a/src/gstreamer/gstlibcamerapool.cpp b/src/gstreamer/gstlibcamerapool.cpp\n> > > index 9cd7eccb..315f08ac 100644\n> > > --- a/src/gstreamer/gstlibcamerapool.cpp\n> > > +++ b/src/gstreamer/gstlibcamerapool.cpp\n> > > @@ -135,16 +135,40 @@ gst_libcamera_pool_class_init(GstLibcameraPoolClass *klass)\n> > >  }\n> > >  \n> > >  GstLibcameraPool *\n> > > -gst_libcamera_pool_new(GstLibcameraAllocator *allocator, Stream *stream)\n> > > +gst_libcamera_pool_new(GstLibcameraAllocator *allocator, const StreamConfiguration &stream_cfg,\n> > > +                    GstCaps *caps, gboolean add_video_meta)\n> > >  {\n> > >       auto *pool = GST_LIBCAMERA_POOL(g_object_new(GST_TYPE_LIBCAMERA_POOL, nullptr));\n> > > +     GstVideoInfo info;\n> > >  \n> > >       pool->allocator = GST_LIBCAMERA_ALLOCATOR(g_object_ref(allocator));\n> > > -     pool->stream = stream;\n> > > -\n> > > -     gsize pool_size = gst_libcamera_allocator_get_pool_size(allocator, stream);\n> > > +     pool->stream = stream_cfg.stream();\n> > \n> > nit: Is pool->stream const ? C++ and C are pretty permissive, but storing a non-\n> > const  pointer obtained from a const object is a bit of a stretch.\n> > \n> > > +\n> > > +     if (caps && gst_video_info_from_caps(&info, caps)) {\n> > \n> > This needs some work, caps will never be null. Now, gst_video_info_from_caps()\n> > will work for image/ and video/ type, in that case the format is set to ENCODED\n> > and a video meta must not be appended. That includes bayer format, for which we\n> > only support matching display and storage dimensions (since arbitrary cropping\n> > of bayer is a bad idea).\n> > \n> > In short, don't get caps, and be verbose if gst_video_info_from_caps() fails,\n> > that is not expected.\n> > \n> > > +             guint k, stride;\n> > > +             gsize offset = 0;\n> > > +             for (k = 0; k < GST_VIDEO_INFO_N_PLANES(&info); k++) {\n> > > +#if !GST_CHECK_VERSION(1, 22, 0)\n> > \n> > Not needed with the suggestion I just made.\n> > \n> > > +                     stride = gst_libcamera_extrapolate_stride(info.finfo, k, stream_cfg.stride);\n> > > +#else\n> > > +                     stride = gst_video_format_info_extrapolate_stride(info.finfo, k, stream_cfg.stride);\n> > > +#endif\n> > > +                     info.stride[k] = stride;\n> > > +                     info.offset[k] = offset;\n> > > +                     offset += stride * GST_VIDEO_FORMAT_INFO_SCALE_HEIGHT(info.finfo, k, GST_VIDEO_INFO_HEIGHT(&info));\n> > \n> > Please add a comment that this should be updated if tiled formats get added in\n> > the future. For anything single plane we don't care of course.\n> > \n> > > +             }\n> > > +     } else\n> > > +             add_video_meta = false;\n> > \n> > Not sure yet why sometimes you don't want to add it.\n> > \n> > > +\n> > > +     gsize pool_size = gst_libcamera_allocator_get_pool_size(allocator, stream_cfg.stream());\n> > >       for (gsize i = 0; i < pool_size; i++) {\n> > >               GstBuffer *buffer = gst_buffer_new();\n> > > +             if (add_video_meta) {\n> > > +                     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> > > +             }\n> > >               pool->queue->push_back(buffer);\n> > >       }\n> > >  \n> > > diff --git a/src/gstreamer/gstlibcamerapool.h b/src/gstreamer/gstlibcamerapool.h\n> > > index 2a7a9c77..8ad87cab 100644\n> > > --- a/src/gstreamer/gstlibcamerapool.h\n> > > +++ b/src/gstreamer/gstlibcamerapool.h\n> > > @@ -21,7 +21,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> > > +                                      const libcamera::StreamConfiguration &stream_cfg, GstCaps *caps, gboolean add_video_meta);\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..c05a31e7 100644\n> > > --- a/src/gstreamer/gstlibcamerasrc.cpp\n> > > +++ b/src/gstreamer/gstlibcamerasrc.cpp\n> > > @@ -497,9 +497,21 @@ 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> > > +             GstQuery *query = NULL;\n> > > +             gboolean add_video_meta = false;\n> > > +\n> > > +             g_autoptr(GstCaps) caps = gst_libcamera_stream_configuration_to_caps(stream_cfg);\n> > > +             gst_libcamera_framerate_to_caps(caps, element_caps);\n> > > +\n> > > +             query = gst_query_new_allocation(caps, false);\n> > \n> > My hate for boolean parameters is strong enough that I'd like to suggest:\n> > \n> >                 bool need_pool = false;\n> >                 query = gst_query_new_allocation(caps, need_pool);\n> \n> \tbool kieran_agrees = true;\n> \tret = send_response(kieran_agrees);\n> \n> \n> > > +             if (!gst_pad_peer_query(srcpad, query))\n> > > +                     GST_DEBUG_OBJECT(self, \"didn't get downstream ALLOCATION hints\");\n> > > +             else\n> > > +                     add_video_meta = gst_query_find_allocation_meta(query, GST_VIDEO_META_API_TYPE, \n> > > NULL);\n> > \n> > Its not sufficient to not add video meta. If downstream does not support\n> > VideoMeta and the stride does not match what you from just turning the caps into\n> > VideoInfo, you must bounce these buffers to system memory. For that reason, you\n> > likely want to handle the stride extrapolation here and pass VideoInfo to the\n> > pool instead. If the stride miss-match, you can then create a generic VideoPool\n> > using the caps, and later own you will bounce the memory using\n> > gst_video_frame_copy() (which can handle stride miss-match).\n> > \n> > > +             gst_query_unref(query);\n> > >  \n> > >               GstLibcameraPool *pool = gst_libcamera_pool_new(self->allocator,\n> > > -                                                             stream_cfg.stream());\n> > > +                                                             stream_cfg, caps, add_video_meta);\n> > >               g_signal_connect_swapped(pool, \"buffer-notify\",\n> > >                                        G_CALLBACK(gst_task_resume), self->task);\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 568E3C0F1B\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 14 Nov 2024 10:56:59 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 87334657DA;\n\tThu, 14 Nov 2024 11:56:58 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7D680657DA\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 14 Nov 2024 11:56:56 +0100 (CET)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 65C01502;\n\tThu, 14 Nov 2024 11:56:42 +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=\"KGivTkJo\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1731581802;\n\tbh=wsmnn3Oq779B/tyvd+9XGHI9wAILoWucqYOUeWSFFrI=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=KGivTkJo1Au4IxWdZ9mBKhYZJ+sU9JBbDQRz3ibDrCl/0zt+C41/PivIc67UCuXNa\n\tILQ12cikjFPEyBxYsZPV0aAN+Au6OuKqFFP9wMJKpndU84j/lLsW6xHoNcyHoK3D8q\n\tu2tSMwzxAVFXPKoLJlDV/FJ33YvLAWDzM0G/62xE=","Date":"Thu, 14 Nov 2024 12:56:48 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"Hou Qi <qi.hou@nxp.com>, Nicolas Dufresne <nicolas@ndufresne.ca>,\n\tlibcamera-devel@lists.libcamera.org, jared.hu@nxp.com,\n\tjulien.vuillaumier@nxp.com","Subject":"Re: [PATCH v3] gstreamer: Add GstVideoMeta support","Message-ID":"<20241114105648.GD26171@pendragon.ideasonboard.com>","References":"<20241113072947.1579075-1-qi.hou@nxp.com>\n\t<13945eb57fb8706d491bc27b6886dc3668020e5a.camel@ndufresne.ca>\n\t<173157611957.3831226.1409354371390888803@ping.linuxembedded.co.uk>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<173157611957.3831226.1409354371390888803@ping.linuxembedded.co.uk>","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":32301,"web_url":"https://patchwork.libcamera.org/comment/32301/","msgid":"<PAXPR04MB82854BFA32D199F843B0309E97212@PAXPR04MB8285.eurprd04.prod.outlook.com>","date":"2024-11-20T10:04:54","subject":"RE: [EXT] Re: [PATCH v3] 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,\n\nThanks for all of your review. Suggestions are useful. I make a v4 patch. Please help review that version. I am also doing more local test.\n\nRegards,\nQi Hou\n\n-----Original Message-----\nFrom: Nicolas Dufresne <nicolas@ndufresne.ca> \nSent: 2024年11月14日 3:20\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 v3] 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 13 novembre 2024 à 16:29 +0900, Hou Qi a écrit :\n> GStreamer video-info calculated stride and offset may differ from \n> those used by the camera.\n>\n> This patch enhances downstream plugin's support for videometa by \n> adding videometa support for libcamerasrc. It ensures that when \n> downstream plugin supports videometa by allocation query, libcamerasrc \n> also attaches videometa to buffer, preventing discrepancies between \n> the stride and offset calculated by video-info and camera.\n>\n> Signed-off-by: Hou Qi <qi.hou@nxp.com>\n> ---\n>  src/gstreamer/gstlibcamera-utils.cpp | 28 ++++++++++++++++++++++++\n>  src/gstreamer/gstlibcamera-utils.h   |  7 ++++++\n>  src/gstreamer/gstlibcamerapool.cpp   | 32 ++++++++++++++++++++++++----\n>  src/gstreamer/gstlibcamerapool.h     |  2 +-\n>  src/gstreamer/gstlibcamerasrc.cpp    | 14 +++++++++++-\n>  5 files changed, 77 insertions(+), 6 deletions(-)\n>\n> diff --git a/src/gstreamer/gstlibcamera-utils.cpp \n> b/src/gstreamer/gstlibcamera-utils.cpp\n> index 732987ef..91de1d44 100644\n> --- a/src/gstreamer/gstlibcamera-utils.cpp\n> +++ b/src/gstreamer/gstlibcamera-utils.cpp\n> @@ -2,6 +2,9 @@\n>  /*\n>   * Copyright (C) 2020, Collabora Ltd.\n>   *     Author: Nicolas Dufresne <nicolas.dufresne@collabora.com>\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\nI'll leave that to the upper maintainers to evaluate, but it would be nice to identify that these are only needed as long as\ngst_libcamera_extrapolate_stride() copy is there. As soon as Trixie becomes stable, we'll bump to GStreamer 1.22 and remove it, would be nice to remove the spurious copyright.\n\n>   *\n>   * GStreamer libcamera Utility Function\n>   */\n> @@ -591,6 +594,31 @@ gst_task_resume(GstTask *task)  }  #endif\n>\n> +#if !GST_CHECK_VERSION(1, 22, 0)\n> +gint gst_libcamera_extrapolate_stride(const GstVideoFormatInfo \n> +*finfo, gint plane, gint stride)\n\nBecause its under an ifdef, it is guarantied not to clash at built time, and the linker will always resolve it locally. Keep the mainline namespace, it will simplify the code later.\n\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, \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..14bf6664 100644\n> --- a/src/gstreamer/gstlibcamera-utils.h\n> +++ b/src/gstreamer/gstlibcamera-utils.h\n> @@ -2,6 +2,9 @@\n>  /*\n>   * Copyright (C) 2020, Collabora Ltd.\n>   *     Author: Nicolas Dufresne <nicolas.dufresne@collabora.com>\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\nNot needed.\n\n>   *\n>   * GStreamer libcamera Utility Functions\n>   */\n> @@ -35,6 +38,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_libcamera_extrapolate_stride(const GstVideoFormatInfo \n> +*finfo, gint plane, gint stride);\n\nApply namespace comment here too.\n\n> +#endif\n>  std::shared_ptr<libcamera::CameraManager> \n> gst_libcamera_get_camera_manager(int &ret);\n>\n>  /**\n> diff --git a/src/gstreamer/gstlibcamerapool.cpp \n> b/src/gstreamer/gstlibcamerapool.cpp\n> index 9cd7eccb..315f08ac 100644\n> --- a/src/gstreamer/gstlibcamerapool.cpp\n> +++ b/src/gstreamer/gstlibcamerapool.cpp\n> @@ -135,16 +135,40 @@ \n> gst_libcamera_pool_class_init(GstLibcameraPoolClass *klass)  }\n>\n>  GstLibcameraPool *\n> -gst_libcamera_pool_new(GstLibcameraAllocator *allocator, Stream \n> *stream)\n> +gst_libcamera_pool_new(GstLibcameraAllocator *allocator, const StreamConfiguration &stream_cfg,\n> +                    GstCaps *caps, gboolean add_video_meta)\n>  {\n>       auto *pool = \n> GST_LIBCAMERA_POOL(g_object_new(GST_TYPE_LIBCAMERA_POOL, nullptr));\n> +     GstVideoInfo info;\n>\n>       pool->allocator = GST_LIBCAMERA_ALLOCATOR(g_object_ref(allocator));\n> -     pool->stream = stream;\n> -\n> -     gsize pool_size = gst_libcamera_allocator_get_pool_size(allocator, stream);\n> +     pool->stream = stream_cfg.stream();\n\nnit: Is pool->stream const ? C++ and C are pretty permissive, but storing a non- const  pointer obtained from a const object is a bit of a stretch.\n\n> +\n> +     if (caps && gst_video_info_from_caps(&info, caps)) {\n\nThis needs some work, caps will never be null. Now, gst_video_info_from_caps() will work for image/ and video/ type, in that case the format is set to ENCODED and a video meta must not be appended. That includes bayer format, for which we only support matching display and storage dimensions (since arbitrary cropping of bayer is a bad idea).\n\nIn short, don't get caps, and be verbose if gst_video_info_from_caps() fails, that is not expected.\n\n> +             guint k, stride;\n> +             gsize offset = 0;\n> +             for (k = 0; k < GST_VIDEO_INFO_N_PLANES(&info); k++) { \n> +#if !GST_CHECK_VERSION(1, 22, 0)\n\nNot needed with the suggestion I just made.\n\n> +                     stride = \n> +gst_libcamera_extrapolate_stride(info.finfo, k, stream_cfg.stride); #else\n> +                     stride = \n> +gst_video_format_info_extrapolate_stride(info.finfo, k, stream_cfg.stride); #endif\n> +                     info.stride[k] = stride;\n> +                     info.offset[k] = offset;\n> +                     offset += stride * \n> +GST_VIDEO_FORMAT_INFO_SCALE_HEIGHT(info.finfo, k, \n> +GST_VIDEO_INFO_HEIGHT(&info));\n\nPlease add a comment that this should be updated if tiled formats get added in the future. For anything single plane we don't care of course.\n\n> +             }\n> +     } else\n> +             add_video_meta = false;\n\nNot sure yet why sometimes you don't want to add it.\n\n> +\n> +     gsize pool_size = \n> + gst_libcamera_allocator_get_pool_size(allocator, \n> + stream_cfg.stream());\n>       for (gsize i = 0; i < pool_size; i++) {\n>               GstBuffer *buffer = gst_buffer_new();\n> +             if (add_video_meta) {\n> +                     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> +             }\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..8ad87cab 100644\n> --- a/src/gstreamer/gstlibcamerapool.h\n> +++ b/src/gstreamer/gstlibcamerapool.h\n> @@ -21,7 +21,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> +                                      const \n> + libcamera::StreamConfiguration &stream_cfg, GstCaps *caps, gboolean \n> + add_video_meta);\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..c05a31e7 100644\n> --- a/src/gstreamer/gstlibcamerasrc.cpp\n> +++ b/src/gstreamer/gstlibcamerasrc.cpp\n> @@ -497,9 +497,21 @@ 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> +             GstQuery *query = NULL;\n> +             gboolean add_video_meta = false;\n> +\n> +             g_autoptr(GstCaps) caps = gst_libcamera_stream_configuration_to_caps(stream_cfg);\n> +             gst_libcamera_framerate_to_caps(caps, element_caps);\n> +\n> +             query = gst_query_new_allocation(caps, false);\n\nMy hate for boolean parameters is strong enough that I'd like to suggest:\n\n                bool need_pool = false;\n                query = gst_query_new_allocation(caps, need_pool);\n\n> +             if (!gst_pad_peer_query(srcpad, query))\n> +                     GST_DEBUG_OBJECT(self, \"didn't get downstream ALLOCATION hints\");\n> +             else\n> +                     add_video_meta = \n> + gst_query_find_allocation_meta(query, GST_VIDEO_META_API_TYPE,\n> NULL);\n\nIts not sufficient to not add video meta. If downstream does not support VideoMeta and the stride does not match what you from just turning the caps into VideoInfo, you must bounce these buffers to system memory. For that reason, you likely want to handle the stride extrapolation here and pass VideoInfo to the pool instead. If the stride miss-match, you can then create a generic VideoPool using the caps, and later own you will bounce the memory using\ngst_video_frame_copy() (which can handle stride miss-match).\n\n> +             gst_query_unref(query);\n>\n>               GstLibcameraPool *pool = gst_libcamera_pool_new(self->allocator,\n> -                                                             stream_cfg.stream());\n> +                                                             \n> + stream_cfg, caps, add_video_meta);\n>               g_signal_connect_swapped(pool, \"buffer-notify\",\n>                                        G_CALLBACK(gst_task_resume), \n> self->task);\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 A709AC0F2A\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 20 Nov 2024 10:04:58 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5193B65F3E;\n\tWed, 20 Nov 2024 11:04:58 +0100 (CET)","from EUR05-VI1-obe.outbound.protection.outlook.com\n\t(mail-vi1eur05on20610.outbound.protection.outlook.com\n\t[IPv6:2a01:111:f403:2613::610])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4228565F3B\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 20 Nov 2024 11:04:56 +0100 (CET)","from PAXPR04MB8285.eurprd04.prod.outlook.com\n\t(2603:10a6:102:1ca::15)\n\tby AM8PR04MB7779.eurprd04.prod.outlook.com (2603:10a6:20b:24b::14)\n\twith Microsoft SMTP Server (version=TLS1_2,\n\tcipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.8158.23;\n\tWed, 20 Nov 2024 10:04:54 +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.8158.023;\n\tWed, 20 Nov 2024 10:04:54 +0000"],"Authentication-Results":["lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=nxp.com header.i=@nxp.com header.b=\"d1VwxGPo\";\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=Ol4MVVSd/TZrtrWIQUbnRFwWj7Tk/SmjZ9MfBbYmUuemZ+48I5GeLpzlScDOTT3tQKBW2SSWxIkht2J3E3izQuBb4nUD+bTUHLJcD71eJ3HF5Rt6nASB2pGsPiJmoNZQjpY5M785dt5szkkcvsdUvt3S9z5DRImYTXM7RPOjo4Xj9rURgp2A/7qZVYDdp1zdtgJMJ/12//G4QiNFTz6YtVZpm8KcXx8R2PGtqeJIq7A12SS54/segMvJusqT2ffgCy9rIv53jbbpTHXyk39f5rv/1N1qhMJT/6N/CKTRyuS/Q3jD77912oDLdCaVcpG12tifICBmzaN8Xduou2FxKA==","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=6NW94bY9dPWwAF27Uy0tskRmdQH50k9iZNINh7+MTFc=;\n\tb=M50Nd1tL6YCBJFEr1M8VQtfsTEH5tSbizgJ6vBJ7+FoZIBXPMOjayjFjq0Y0AMscT6V1lRL93mGig9mlWW4efifW2jLE0YrvSQ4T8nkC+mcPGkEK32RhtPzT5Rr7MVSSxdYQQu3TXxo4QDR2ub83WQCj1s5zV21l6Qo1h6aOFJPQ/AJKsi1y50gapcI7742Bk0TvpF/4uS2xx48+Qfzhm0Do6rdC1TPYb4CdyMEX/aJXCruuUDPhe9rSUEPVo80C9OW6NncxwkvrPRd/6EnXU0gkibRkWhagv/yoi77WvbiV4hrMPYw7n+q1CZxDirr+FsIACOjFKPSVpwH9k4i2QQ==","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=6NW94bY9dPWwAF27Uy0tskRmdQH50k9iZNINh7+MTFc=;\n\tb=d1VwxGPop0FfJUnSwfXVbPb3C+Azk1jZGcjAnB2s9I/sSmTs8dv4Q8qyr2pKEGP+9nlWsC+snJ/2OTbt/pW0PQP8FXx7ySvLJagSddxA/L8sdRDrtgBfMwfeFvhUcWFs/SoPLR+kbuhfgnZQt8QXuUxfoBRl6HyWkh4PHDkcfFAwcKS8HfAMhF/ykbnHuLt+yEIZZgVpvNf3Sn7qlwuiM63t5XAXMkrBxW+q9729/pIDMgh1njHbHrv6NfvkJVhLK7TyE9TRnDnrOHgVhYiqBO+/5V4qgfGMi2Qp7kkHRo7BaScU4SDFRITElbdj4+zPQiTBQAzObuW2mmwIdiARNg==","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 v3] gstreamer: Add GstVideoMeta support","Thread-Topic":"[EXT] Re: [PATCH v3] gstreamer: Add GstVideoMeta support","Thread-Index":"AQHbNZ3sv/QpC3nHBk62ThY2R9GmBbK1lsqAgApibAA=","Date":"Wed, 20 Nov 2024 10:04:54 +0000","Message-ID":"<PAXPR04MB82854BFA32D199F843B0309E97212@PAXPR04MB8285.eurprd04.prod.outlook.com>","References":"<20241113072947.1579075-1-qi.hou@nxp.com>\n\t<13945eb57fb8706d491bc27b6886dc3668020e5a.camel@ndufresne.ca>","In-Reply-To":"<13945eb57fb8706d491bc27b6886dc3668020e5a.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=\"d1VwxGPo\";\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_|AM8PR04MB7779:EE_","x-ms-office365-filtering-correlation-id":"4db6a724-c011-4acb-fca6-08dd094ac816","x-ms-exchange-senderadcheck":"1","x-ms-exchange-antispam-relay":"0","x-microsoft-antispam":"BCL:0;\n\tARA:13230040|1800799024|366016|376014|38070700018; ","x-microsoft-antispam-message-info":"=?utf-8?q?myMq2qzgFv4f4wt15+mo8rcVRV/x?=\n\t=?utf-8?q?vAczMAvfJyKspHHFcDKxxIJK9dfhv415j0Qc8qvDknkd6Hlrh12BjLqW?=\n\t=?utf-8?q?IjsNYprfXq3uRJglg6n8j5vH/E4qzTcWUcBfO2bRpFampyi/bsV74gt5?=\n\t=?utf-8?q?myB1qM0CviyZJhI2rBqtz1QmngDqNjfMViubJ0xcfRXnXxF86lWaOGFW?=\n\t=?utf-8?q?frejbTqYq88gIl3pXKskViP/uRThTWg17b06538JFDhY9WoyeOJY7EIA?=\n\t=?utf-8?q?8NUGKiP0ae9ragO0yvPeDAabQD6qQce9pmi2EVLbayXBcpAvqyVfA0z4?=\n\t=?utf-8?q?XdqUXWw599/bFAGmLiy0kszN0CTrOOoKPtnGTarAlFphcMzET+KYbi7t?=\n\t=?utf-8?q?LZtk5z9rsAN2M30b8DMyjJr/NiWwm4QbsWwwpxKEKrXqZ3D0g7F6kLT7?=\n\t=?utf-8?q?C8JZRw7x9p5wvcBkWhrk/mSnJZA4+k1VhtA2CZQZX4uAwSd9+9L2rN+T?=\n\t=?utf-8?q?Q+jeym6vOW/G+X3K6kOyHij2NLwaMqaZfFo1OE9lhRqr+piOoiKSVR8+?=\n\t=?utf-8?q?zUEmRBoXocx28sNRCMQErU+5OL6evadoVebrja5BcKeXK6wFMmeH0+JX?=\n\t=?utf-8?q?nazngI098r0BP4Sn26jow/u4JzFESDRz2JOrhm/K3Lil98q1FYR4R5P6?=\n\t=?utf-8?q?AB4CMdZNMKSBeV0m3H3uDJp4o/wxOcqq4IPyZ+z3Hx4A2yLm6xqkbVHx?=\n\t=?utf-8?q?hse3uxsG+ytzppM6ax8Hg1yNomhDDJVvgnXTmeeHWPZaE6mOACOMJsNE?=\n\t=?utf-8?q?LOZU9TEZ7knUjftzOYxdaYup0RX1NHixfXY9CDb5zSFUi0dcdw/QPh0a?=\n\t=?utf-8?q?ggAZF589U12e8cnSZzljKWeahFMt1HhgU4tSRh8Mtv5MNmD+7NmLjkom?=\n\t=?utf-8?q?2g9IRSeYo3WekYQ/NTZVDVbWaA0HrMaBPPMqsd0zJMBV+e9oYeOspbGW?=\n\t=?utf-8?q?aDkI9l4ZpZsHpoNqWBIkwOZ6T2VuF5KrdPKAuELjpvw3yjWb5ntIJSkC?=\n\t=?utf-8?q?BkaaWB+Ut7035w8j+TuMaUM7fXkjVO5FrH52vitMTI6bEidzTJh54+Q6?=\n\t=?utf-8?q?kVb5LSz7XTvG2/yc+wXIMkQDNuUD7nO3dMVkvhgS+KSo+marnua+stxe?=\n\t=?utf-8?q?ZyajMlfAPJno+zvupA0+WcW1XBofhf7SsxwjuZXqQLpjH5GoQ9esF4A+?=\n\t=?utf-8?q?0kcVWPXnufwJkV8pfpASx/R+Np5po2Ww9GQ5+bxJNj6OO3eYSIgy3KzL?=\n\t=?utf-8?q?7XeMNyUj+Fu0bRXrxmwcxjYXd8AljXAuEkwa21BprrCttnKjZ0XZGG2U?=\n\t=?utf-8?q?eNv4Asc36P802qNzGPSZti3nVILau1LR/FHYuc3H/cc7Y5tNJyI9fSDt?=\n\t=?utf-8?q?TQEADhSTWjR2bt+sFO7x5HfcwuQeO1nT+lUsB7hS9CuH9gEC5c9Xtagf?=\n\t=?utf-8?q?Skhv4dLz0DJRYZM=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)(1800799024)(366016)(376014)(38070700018); DIR:OUT;\n\tSFP:1101; ","x-ms-exchange-antispam-messagedata-chunkcount":"1","x-ms-exchange-antispam-messagedata-0":"=?utf-8?q?rIQjbgvzfiLtfOWBasSVsTDaU?=\n\t=?utf-8?q?ozpW+Tdqc4rjf5FZ4LFpjQpD4Mi4D2AuDhqajtuKGPcBV2w2t7RBDuFb?=\n\t=?utf-8?q?zNRgWM/oHdVK8Gi1tKUd6kwzY1yb+i7qR99anRYgFrxWwvlT562Mp9fx?=\n\t=?utf-8?q?COjcg8OI0WdXbmy36L+kK0dtuURkuR9o4ZeZKEVFQKkAe+Nt5Tj9Oy/4?=\n\t=?utf-8?q?SaxAOWVjZTZd1S4djvQtBaNOgrreL2QNHjudCskEixuadbRZC+jAAqGw?=\n\t=?utf-8?q?W4Y5ibTeVqXgx0q7Q18KlHRbACwI0H2zBtNS2khLS25ExHN2ObOYUsgS?=\n\t=?utf-8?q?oY6GNRYVQwMPEBFVhlmDOKhhZ+IJMMR8V10AgZnQCWt4sDAq7Vba/nzR?=\n\t=?utf-8?q?aWSkliKPvcjaovC1/wdldcbAqDhx2+9H1uklDOsZSy0OL+6ClXSCBZBc?=\n\t=?utf-8?q?sOrhXz7ynOcQoaQzRzt3Y+iQzMiTPEJLTBnoGMlaKduI31EiJkLrbAFf?=\n\t=?utf-8?q?QZZ/R4Sl5jO23khs0gb4nxOvtvEDLbvHSXjrJsnz702uAT2T3k8N4URX?=\n\t=?utf-8?q?d0d8pwY7iqhHs3p0sO8scN6AQvvfg7xs/pBR58aAA+64gLYxjsYOWbIF?=\n\t=?utf-8?q?uQZU4syRCxm7LNuvXv0au4MEgT/gZKFVylY5g+r9OVxuHq+KND8m+RgA?=\n\t=?utf-8?q?LaMI0D6AeUdsyfsTp0GbtquTj7qv93+xuH40YJobmePaKqLS6XJApT99?=\n\t=?utf-8?q?059HIgAcHfAhPAFlUFT+K9qfPrurXWuWw2QBefPXxE5H5AjBoukdURbH?=\n\t=?utf-8?q?GfM5X7VS8bIB0VRGZNjc3XxgCOiw23SGPCi/Y+xO6TiIwNjQ5GRczckH?=\n\t=?utf-8?q?KhQVwcvy8OkIRjjyp4HZgdiqWuIckpkN+sSM+QchNNYB7HGc/meMgvbQ?=\n\t=?utf-8?q?OkqgvErCPlWLaBYozdFJRz48RoSI58viJBvg3ZbZdaIOAh18j4g0V561?=\n\t=?utf-8?q?KTAnOm72suqeDto5IdzbclfiJTr7wLN9oBtCbCVQWw/yLQXfGLgqr478?=\n\t=?utf-8?q?nzOhQXOhbYR910fvPRlxu2+cS70oDmhcd7SDpt9+Df0pYOTthrpNn0Vn?=\n\t=?utf-8?q?wA/FJHGTqHpMShkJwtVJeVuaQTVMRQv2xeAPvXm9HcyjHRxNSAjL29p4?=\n\t=?utf-8?q?B+kg96XZpgJ+Mm3I06bdAw7XDSgQE9g5wBnB1RyLByw5E6BihmCzQLqs?=\n\t=?utf-8?q?a2pAX8qmxhMsHN716/PGdRPkaDFZlcqdQAqUEDf3Bt6yJp4sPyFpF1Wu?=\n\t=?utf-8?q?nEd357TBKIdPv2Km5m9Y0azPyaCyruisqTIxmAhOM1YGhCINp3fsIQBn?=\n\t=?utf-8?q?sQMNNzPsMprreseY2+Ifv6H8oR1vcqoaXmCt+vDIh3kuZlQWdpJvZdR9?=\n\t=?utf-8?q?Cdsiqc/yLU2T4tOvEq4BZno2/wCGlFxu///tRpAWOu0ZvEIhBbvxLqSy?=\n\t=?utf-8?q?VE/rJDClBdJLJdRiyAS2mL37Wp07pEaeYPNEAFR5zTR8ZY7jKdAertq0?=\n\t=?utf-8?q?v/EOWjDMM4R8jolXujHtKYpTEBEfTlBiACT7thHL/KZNUGXVd60gNWJE?=\n\t=?utf-8?q?ipPSlJ1hvysz5MV1lnTZ/ZLSwKRv30H2XO+i+PF6RWPrD1BQ15JIqSlA?=\n\t=?utf-8?q?rJfLs7/iHTdUKTT+QUCN4g/etxDYVRjtDKln01PmEQ=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":"4db6a724-c011-4acb-fca6-08dd094ac816","X-MS-Exchange-CrossTenant-originalarrivaltime":"20 Nov 2024 10:04:54.5425\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":"tUhZD8SAQD/PQurGAoGkWe+3bwnwE7KglxNxyjwGyuQ3Pynr0Kts2Ful12LLOqHr","X-MS-Exchange-Transport-CrossTenantHeadersStamped":"AM8PR04MB7779","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>"}}]