[EXT] Re: [PATCH v2] gstreamer: Add GstVideoMeta support
Nicolas Dufresne
nicolas at ndufresne.ca
Tue Nov 12 19:27:12 CET 2024
Le mardi 12 novembre 2024 à 07:33 +0200, Laurent Pinchart a écrit :
> Hi Qi,
>
> 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. 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.
> 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.
Nicolas
>
> > 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;
> > > > +
> > > > + 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);
> > > >
>
More information about the libcamera-devel
mailing list