[libcamera-devel] [PATCH] src: gstreamer: use the streams of CameraConfiguration when allocating buffers

[Nicolas Dufresne]

Le jeudi 25 février 2021 à 16:41 +0100, Dafna Hirschfeld a écrit :
> Hi,
> 
> On 24.02.21 19:33, Nicolas Dufresne wrote:
> > Le mercredi 24 février 2021 à 18:19 +0100, Dafna Hirschfeld a écrit :
> > > Currently, when allocating buffers, the streams of
> > > the Camera object are used. Instead the streams of
> > > the CameraConfiguration object should be used.
> > 
> > The changes looks fine, but perhaps you could extend, I wrote this, still I
> > don't understand why enumerating the streams in the configuration would make
> > any
> > difference after the camera has been configured.
> 
> The Camera holds a list of all available streams.
> For rkisp1 those are mainpath and selfpath.
> Then only subset of those streams are chosen which are
> the stream kept in the CameraConfiguration object.
> Specifically using gstreamer with rkisp1 I got:
> 
> [3:59:32.003159780] [2499] ERROR Allocator framebuffer_allocator.cpp:97 Stream
> is not part of /base/i2c at ff3d0000/sensor at 10 active configuration
> 
> > 
> > Minor comment, other GStreamer patches commit message was simply "gst:
> > Description", just mentioning for consistency, but perhaps there is new
> > rules.
> > 
> 
> Ill send v2,

Thanks for the explanation, and my review by still holds ;-P

> 
> Thanks,
> Dafna
> 
> > Reviewed-by: Nicolas Dufresne <nicolas.dufresne at collabora.com>
> > 
> > > 
> > > Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld at collabora.com>
> > > ---
> > >   src/gstreamer/gstlibcameraallocator.cpp | 6 ++++--
> > >   src/gstreamer/gstlibcameraallocator.h   | 4 +++-
> > >   src/gstreamer/gstlibcamerasrc.cpp       | 2 +-
> > >   3 files changed, 8 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/src/gstreamer/gstlibcameraallocator.cpp
> > > b/src/gstreamer/gstlibcameraallocator.cpp
> > > index 13c6b493..7bd8ba2d 100644
> > > --- a/src/gstreamer/gstlibcameraallocator.cpp
> > > +++ b/src/gstreamer/gstlibcameraallocator.cpp
> > > @@ -183,13 +183,15 @@
> > > gst_libcamera_allocator_class_init(GstLibcameraAllocatorClass *klass)
> > >   }
> > >   
> > >   GstLibcameraAllocator *
> > > -gst_libcamera_allocator_new(std::shared_ptr<Camera> camera)
> > > +gst_libcamera_allocator_new(std::shared_ptr<Camera> camera,
> > > +                           CameraConfiguration *config_)
> > >   {
> > >          auto *self =
> > > GST_LIBCAMERA_ALLOCATOR(g_object_new(GST_TYPE_LIBCAMERA_ALLOCATOR,
> > >                                                            nullptr));
> > >   
> > >          self->fb_allocator = new FrameBufferAllocator(camera);
> > > -       for (Stream *stream : camera->streams()) {
> > > +       for (StreamConfiguration &streamCfg : *config_) {
> > > +               Stream *stream = streamCfg.stream();
> > >                  gint ret;
> > >   
> > >                  ret = self->fb_allocator->allocate(stream);
> > > diff --git a/src/gstreamer/gstlibcameraallocator.h
> > > b/src/gstreamer/gstlibcameraallocator.h
> > > index befdcad6..f7fa6acd 100644
> > > --- a/src/gstreamer/gstlibcameraallocator.h
> > > +++ b/src/gstreamer/gstlibcameraallocator.h
> > > @@ -13,12 +13,14 @@
> > >   #include <gst/allocators/allocators.h>
> > >   
> > >   #include <libcamera/stream.h>
> > > +#include <libcamera/camera.h>
> > >   
> > >   #define GST_TYPE_LIBCAMERA_ALLOCATOR gst_libcamera_allocator_get_type()
> > >   G_DECLARE_FINAL_TYPE(GstLibcameraAllocator, gst_libcamera_allocator,
> > >                       GST_LIBCAMERA, ALLOCATOR, GstDmaBufAllocator)
> > >   
> > > -GstLibcameraAllocator
> > > *gst_libcamera_allocator_new(std::shared_ptr<libcamera::Camera> camera);
> > > +GstLibcameraAllocator
> > > *gst_libcamera_allocator_new(std::shared_ptr<libcamera::Camera> camera,
> > > +
> > > libcamera::CameraConfiguration *config_);
> > >   
> > >   bool gst_libcamera_allocator_prepare_buffer(GstLibcameraAllocator *self,
> > >                                              libcamera::Stream *stream,
> > > diff --git a/src/gstreamer/gstlibcamerasrc.cpp
> > > b/src/gstreamer/gstlibcamerasrc.cpp
> > > index 636c14df..7b13667b 100644
> > > --- a/src/gstreamer/gstlibcamerasrc.cpp
> > > +++ b/src/gstreamer/gstlibcamerasrc.cpp
> > > @@ -429,7 +429,7 @@ gst_libcamera_src_task_enter(GstTask *task,
> > > [[maybe_unused]] GThread *thread,
> > >                  return;
> > >          }
> > >   
> > > -       self->allocator = gst_libcamera_allocator_new(state->cam_);
> > > +       self->allocator = gst_libcamera_allocator_new(state->cam_, state-
> > > > config_.get());
> > >          if (!self->allocator) {
> > >                  GST_ELEMENT_ERROR(self, RESOURCE, NO_SPACE_LEFT,
> > >                                    ("Failed to allocate memory"),
> > 
> > 
>