[libcamera-devel] [PATCH v2] gst: use the streams of CameraConfiguration when allocating buffers

[Laurent Pinchart]

Hi Dafna,

Thank you for the patch.

On Thu, Feb 25, 2021 at 05:11:06PM +0100, Dafna Hirschfeld wrote:
> Currently, when allocating buffers, the streams of
> the Camera object are used. Instead the streams of
> the CameraConfiguration object should be used.
> This is because the Camera object holds all available
> streams while the CameraConfiguration holds only the streams
> associated with the current configuration.
> 
> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld at collabora.com>
> Reviewed-by: Nicolas Dufresne <nicolas.dufresne at collabora.com>
> ---
> Changes from v1: change the prefix to 'gst:' and extend
> the commit log.
> 
>  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>

With the header in alphabetical order,

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

No need to resend, I'll change this when applying.

>  
>  #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"),

-- 
Regards,

Laurent Pinchart