[EXT] Re: [PATCH v2] gstreamer: Add GstVideoMeta support
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Tue Nov 12 19:43:10 CET 2024
Hi Nicolas,
On Tue, Nov 12, 2024 at 01:27:12PM -0500, Nicolas Dufresne wrote:
> Le mardi 12 novembre 2024 à 07:33 +0200, Laurent Pinchart a écrit :
> > On Tue, Nov 12, 2024 at 02:06:44AM +0000, Qi Hou wrote:
> > > Hi Kieran,
> > >
> > > "gst_video_format_info_extrapolate_stride" is defined since gstreamer
> > > 1.22. I think it causes build fail on old compiler with gstreamer
> > > version lower than 1.22. Do I need to add GST_CHECK_VERSION(1, 22, 0)
> > > to uses this function for gstreamer version >=1.22.0, and fallback to
> > > uses other functions for gstreamer version <1.22.0 ?
> >
> > I'd like to keep supporting 1.18 if possible as it's still shipped by
> > Debian Bullseye. A version check is fine, it will clearly mark the
>
> Fun fact, Debian Bullseye will never be updated to ship a newer GStreamer.
> Debian never updates major GStreamer version in stable.
I know. I didn't imply they would update.
> Though, it seems very
> up-to-date on security fixes, which I can see in some of the logs (these are
> fixes that upstream did not backport and release, because we only go back 2
> versions):
>
> https://metadata.ftp-master.debian.org/changelogs//main/g/gst-plugins-good1.0/gst-plugins-good1.0_1.18.4-2+deb11u2_changelog
>
> My understanding is that this is EOL 5 years after the .0 release, so in August
> 2026. Is it your plan to drop it at the time ? Its quite unusable to see
> libraries keeping backward support for that long. But it would be nice to have
> this officially stated somewhere.
We try to support the current and previous versions of major
distributions when possible. For Debian, this is also due to using
bullseye in CI to test compilation with gcc 9 and gcc 10.
> > backward compatibility code, and allow us to remove it once we'll bump
> > the minimum GStreamer version.
>
> That function only depends on 1.18 internally, so yes, that's an option:
>
> #if < 1.22
>
> gint
> gst_video_format_info_extrapolate_stride (const GstVideoFormatInfo * finfo,
> gint plane, gint stride)
> {
> gint estride;
> gint comp[GST_VIDEO_MAX_COMPONENTS];
> gint i;
>
> /* there is nothing to extrapolate on first plane */
> if (plane == 0)
> return stride;
>
> gst_video_format_info_component (finfo, plane, comp);
>
> /* For now, all planar formats have a single component on first plane, but
> * if there was a planar format with more, we'd have to make a ratio of the
> * number of component on the first plane against the number of component on
> * the current plane. */
> estride = 0;
> for (i = 0; i < GST_VIDEO_MAX_COMPONENTS && comp[i] >= 0; i++)
> estride += GST_VIDEO_FORMAT_INFO_SCALE_WIDTH (finfo, comp[i], stride);
>
> return estride;
> }
> #endif
>
> Don't forget to refer it and copy over the copyright:
>
>
> * Copyright (C) <1999> Erik Walthinsen <omega at cse.ogi.edu>
> * Library <2002> Ronald Bultje <rbultje at ronald.bitfreak.net>
> * Copyright (C) 2007 David A. Schleef <ds at schleef.org>
>
> This code is LGPL V2.
>
> > > On 2024年11月11日 17:30, Kieran Bingham wrote:
> > > > Quoting Hou Qi (2024-11-08 09:21:13)
> > > > > GStreamer video-info calculated stride and offset may differ from
> > > > > those used by the camera.
> > > > >
> > > > > This patch enhances downstream plugin's support for videometa by
> > > > > adding videometa support for libcamerasrc. It ensures that when
> > > > > downstream plugin supports videometa by allocation query, libcamerasrc
> > > > > also attaches videometa to buffer, preventing discrepancies between
> > > > > the stride and offset calculated by video-info and camera.
> > > >
> > > > Thanks for the v2, this looks like interesting work.
> > > >
> > > > Unfortunately the CI isn't clear on this one yet:
> > > >
> > > > https://gitlab.freedesktop.org/camera/libcamera/-/jobs/66327653
> > > >
> > > > Any idea what's happening here? It looks like this only failed on one
> > > > compiler:
> > > >
> > > > [443/641] Generating src/py/libcamera/py_gen_formats with a custom command [444/641] Compiling C++ object src/gstreamer/libgstlibcamera.so.p/gstlibcamerapool.cpp.o
> > > > FAILED: src/gstreamer/libgstlibcamera.so.p/gstlibcamerapool.cpp.o
> > > > g++-9 -Isrc/gstreamer/libgstlibcamera.so.p -Isrc/gstreamer
> > > > g++-I../src/gstreamer -Iinclude -I../include -Iinclude/libcamera
> > > > g++-I/usr/include/gstreamer-1.0 -I/usr/include/orc-0.4
> > > > g++-I/usr/include/x86_64-linux-gnu -I/usr/include/glib-2.0
> > > > g++-I/usr/lib/x86_64-linux-gnu/glib-2.0/include
> > > > g++-fdiagnostics-color=always -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch
> > > > g++-Wextra -Werror -std=c++17 -O0 -g -Wmissing-declarations -Wshadow
> > > > g++-include /builds/camera/libcamera/build/config.h -fPIC -pthread
> > > > g++'-DVERSION="0.3.2+1-edf89091-nvm"' '-DPACKAGE="libcamera"'
> > > > g++-DGLIB_VERSION_MIN_REQUIRED=GLIB_VERSION_2_40 -MD -MQ
> > > > g++src/gstreamer/libgstlibcamera.so.p/gstlibcamerapool.cpp.o -MF
> > > > g++src/gstreamer/libgstlibcamera.so.p/gstlibcamerapool.cpp.o.d -o
> > > > g++src/gstreamer/libgstlibcamera.so.p/gstlibcamerapool.cpp.o -c
> > > > g++../src/gstreamer/gstlibcamerapool.cpp
> > > > ../src/gstreamer/gstlibcamerapool.cpp: In function ‘GstLibcameraPool* gst_libcamera_pool_new(GstLibcameraAllocator*, const libcamera::StreamConfiguration&, GstCaps*, gboolean)’:
> > > > ../src/gstreamer/gstlibcamerapool.cpp:151:13: error: ‘gst_video_format_info_extrapolate_stride’ was not declared in this scope; did you mean ‘gst_video_format_info_component’?
> > > > 151 | stride = gst_video_format_info_extrapolate_stride(info.finfo, k, stream_cfg.stride);
> > > > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > > | gst_video_format_info_component
> > > > [445/641] Compiling C++ object src/apps/qcam/qcam.p/viewfinder_qt.cpp.o
> > > >
> > > > So perhaps we're hitting a requirement to have a specific gstreamer version that isn't being catered for ?
> > > >
> > > > --
> > > > Kieran
> > > >
> > > > > Signed-off-by: Hou Qi <qi.hou at nxp.com>
> > > > > ---
> > > > > src/gstreamer/gstlibcamerapool.cpp | 28 ++++++++++++++++++++++++----
> > > > > src/gstreamer/gstlibcamerapool.h | 2 +-
> > > > > src/gstreamer/gstlibcamerasrc.cpp | 14 +++++++++++++-
> > > > > 3 files changed, 38 insertions(+), 6 deletions(-)
> > > > >
> > > > > diff --git a/src/gstreamer/gstlibcamerapool.cpp
> > > > > b/src/gstreamer/gstlibcamerapool.cpp
> > > > > index 9cd7eccb..6593b3ca 100644
> > > > > --- a/src/gstreamer/gstlibcamerapool.cpp
> > > > > +++ b/src/gstreamer/gstlibcamerapool.cpp
> > > > > @@ -135,16 +135,36 @@
> > > > > gst_libcamera_pool_class_init(GstLibcameraPoolClass *klass) }
> > > > >
> > > > > GstLibcameraPool *
> > > > > -gst_libcamera_pool_new(GstLibcameraAllocator *allocator, Stream *stream)
> > > > > +gst_libcamera_pool_new(GstLibcameraAllocator *allocator, const StreamConfiguration &stream_cfg,
> > > > > + GstCaps *caps, gboolean add_video_meta)
> > > > > {
> > > > > auto *pool = GST_LIBCAMERA_POOL(g_object_new(GST_TYPE_LIBCAMERA_POOL, nullptr));
> > > > > + GstVideoInfo info;
> > > > >
> > > > > pool->allocator = GST_LIBCAMERA_ALLOCATOR(g_object_ref(allocator));
> > > > > - pool->stream = stream;
> > > > > -
> > > > > - gsize pool_size = gst_libcamera_allocator_get_pool_size(allocator, stream);
> > > > > + pool->stream = stream_cfg.stream();
> > > > > +
> > > > > + if (caps && gst_video_info_from_caps(&info, caps)) {
> > > > > + guint k, stride;
> > > > > + gsize offset = 0;
> > > > > + for (k = 0; k < GST_VIDEO_INFO_N_PLANES(&info); k++) {
> > > > > + stride = gst_video_format_info_extrapolate_stride(info.finfo, k, stream_cfg.stride);
> > > > > + info.stride[k] = stride;
> > > > > + info.offset[k] = offset;
> > > > > + offset += stride * GST_VIDEO_FORMAT_INFO_SCALE_HEIGHT(info.finfo, k, GST_VIDEO_INFO_HEIGHT(&info));
> > > > > + }
> > > > > + } else
> > > > > + add_video_meta = false;
I think there are a few coding style issues that checkstyle.py could
have caught. See https://libcamera.org/coding-style.html#tools for more
information, especially the last paragraph that explains how to use the
git commit hook.
> > > > > +
> > > > > + gsize pool_size = gst_libcamera_allocator_get_pool_size(allocator, stream_cfg.stream());
> > > > > for (gsize i = 0; i < pool_size; i++) {
> > > > > GstBuffer *buffer = gst_buffer_new();
> > > > > + if (add_video_meta) {
> > > > > + gst_buffer_add_video_meta_full(buffer, GST_VIDEO_FRAME_FLAG_NONE,
> > > > > + GST_VIDEO_INFO_FORMAT(&info), GST_VIDEO_INFO_WIDTH(&info),
> > > > > + GST_VIDEO_INFO_HEIGHT(&info), GST_VIDEO_INFO_N_PLANES(&info),
> > > > > + info.offset, info.stride);
> > > > > + }
> > > > > pool->queue->push_back(buffer);
> > > > > }
> > > > >
> > > > > diff --git a/src/gstreamer/gstlibcamerapool.h
> > > > > b/src/gstreamer/gstlibcamerapool.h
> > > > > index 2a7a9c77..8ad87cab 100644
> > > > > --- a/src/gstreamer/gstlibcamerapool.h
> > > > > +++ b/src/gstreamer/gstlibcamerapool.h
> > > > > @@ -21,7 +21,7 @@
> > > > > G_DECLARE_FINAL_TYPE(GstLibcameraPool, gst_libcamera_pool, GST_LIBCAMERA, POOL, GstBufferPool)
> > > > >
> > > > > GstLibcameraPool *gst_libcamera_pool_new(GstLibcameraAllocator *allocator,
> > > > > - libcamera::Stream *stream);
> > > > > + const
> > > > > + libcamera::StreamConfiguration &stream_cfg, GstCaps *caps, gboolean
> > > > > + add_video_meta);
> > > > >
> > > > > libcamera::Stream *gst_libcamera_pool_get_stream(GstLibcameraPool *self);
> > > > >
> > > > > diff --git a/src/gstreamer/gstlibcamerasrc.cpp
> > > > > b/src/gstreamer/gstlibcamerasrc.cpp
> > > > > index 8efa25f4..c05a31e7 100644
> > > > > --- a/src/gstreamer/gstlibcamerasrc.cpp
> > > > > +++ b/src/gstreamer/gstlibcamerasrc.cpp
> > > > > @@ -497,9 +497,21 @@ gst_libcamera_src_negotiate(GstLibcameraSrc *self)
> > > > > for (gsize i = 0; i < state->srcpads_.size(); i++) {
> > > > > GstPad *srcpad = state->srcpads_[i];
> > > > > const StreamConfiguration &stream_cfg = state->config_->at(i);
> > > > > + GstQuery *query = NULL;
> > > > > + gboolean add_video_meta = false;
> > > > > +
> > > > > + g_autoptr(GstCaps) caps = gst_libcamera_stream_configuration_to_caps(stream_cfg);
> > > > > + gst_libcamera_framerate_to_caps(caps, element_caps);
> > > > > +
> > > > > + query = gst_query_new_allocation(caps, false);
> > > > > + if (!gst_pad_peer_query(srcpad, query))
> > > > > + GST_DEBUG_OBJECT(self, "didn't get downstream ALLOCATION hints");
> > > > > + else
> > > > > + add_video_meta = gst_query_find_allocation_meta(query, GST_VIDEO_META_API_TYPE, NULL);
> > > > > + gst_query_unref(query);
> > > > >
> > > > > GstLibcameraPool *pool = gst_libcamera_pool_new(self->allocator,
> > > > > - stream_cfg.stream());
> > > > > +
> > > > > + stream_cfg, caps, add_video_meta);
> > > > > g_signal_connect_swapped(pool, "buffer-notify",
> > > > > G_CALLBACK(gst_task_resume), self->task);
> > > > >
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list