[libcamera-devel] [PATCH 02/13] gstreamer: Inline gst_libcamera_buffer_get_frame_buffer()

[Nicolas Dufresne]

Hi Laurent,

thanks for the patch.

Le vendredi 24 juin 2022 à 02:21 +0300, Laurent Pinchart a écrit :
> The gst_libcamera_buffer_get_frame_buffer() function retrieves a
> FrameBuffer corresponding to the GstBuffer. While the GstLibcameraPool
> class allocates the GstBuffer instances, associating them to a
> FrameBuffer is handled by GstLibcameraAllocator. The
> gst_libcamera_buffer_get_frame_buffer() function is ill-placed in
> gstlibcamerapool.cpp. Move it to gstlibcameraallocator.cpp, which allows
> inlining it in its single caller, simplifying the code flow.

Encapsulation wise, I think it was more appropriate before this change. The
details that the FrameBuffer is stored in GstMemory as a Quark data, is internal
implementation of the allocator class, while the detail of the GstMemory being
only memory from our allocator is internal implementation detail of the pool
class (since the pool is the buffer factory.

As you are seeking for an optimized call stack, you could perhaps must inline
these two helpers into their respective headers ?

regards,
Nicolas

> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
>  src/gstreamer/gstlibcameraallocator.cpp | 3 ++-
>  src/gstreamer/gstlibcameraallocator.h   | 2 +-
>  src/gstreamer/gstlibcamerapool.cpp      | 7 -------
>  src/gstreamer/gstlibcamerapool.h        | 2 --
>  4 files changed, 3 insertions(+), 11 deletions(-)
> 
> diff --git a/src/gstreamer/gstlibcameraallocator.cpp b/src/gstreamer/gstlibcameraallocator.cpp
> index c740b8fc82a8..53693834eeff 100644
> --- a/src/gstreamer/gstlibcameraallocator.cpp
> +++ b/src/gstreamer/gstlibcameraallocator.cpp
> @@ -252,8 +252,9 @@ gst_libcamera_allocator_get_pool_size(GstLibcameraAllocator *self,
>  }
>  
>  FrameBuffer *
> -gst_libcamera_memory_get_frame_buffer(GstMemory *mem)
> +gst_libcamera_buffer_get_frame_buffer(GstBuffer *buffer)
>  {
> +	GstMemory *mem = gst_buffer_peek_memory(buffer, 0);
>  	auto *frame = reinterpret_cast<FrameWrap *>(gst_mini_object_get_qdata(GST_MINI_OBJECT_CAST(mem),
>  									      FrameWrap::getQuark()));
>  	return frame->buffer_;
> diff --git a/src/gstreamer/gstlibcameraallocator.h b/src/gstreamer/gstlibcameraallocator.h
> index 0a08c3bb3bbe..b1c038c50257 100644
> --- a/src/gstreamer/gstlibcameraallocator.h
> +++ b/src/gstreamer/gstlibcameraallocator.h
> @@ -28,4 +28,4 @@ bool gst_libcamera_allocator_prepare_buffer(GstLibcameraAllocator *self,
>  gsize gst_libcamera_allocator_get_pool_size(GstLibcameraAllocator *allocator,
>  					    libcamera::Stream *stream);
>  
> -libcamera::FrameBuffer *gst_libcamera_memory_get_frame_buffer(GstMemory *mem);
> +libcamera::FrameBuffer *gst_libcamera_buffer_get_frame_buffer(GstBuffer *buffer);
> diff --git a/src/gstreamer/gstlibcamerapool.cpp b/src/gstreamer/gstlibcamerapool.cpp
> index 1fde42135119..118bc6db7067 100644
> --- a/src/gstreamer/gstlibcamerapool.cpp
> +++ b/src/gstreamer/gstlibcamerapool.cpp
> @@ -140,10 +140,3 @@ gst_libcamera_buffer_get_stream(GstBuffer *buffer)
>  	auto *self = (GstLibcameraPool *)buffer->pool;
>  	return self->stream;
>  }
> -
> -FrameBuffer *
> -gst_libcamera_buffer_get_frame_buffer(GstBuffer *buffer)
> -{
> -	GstMemory *mem = gst_buffer_peek_memory(buffer, 0);
> -	return gst_libcamera_memory_get_frame_buffer(mem);
> -}
> diff --git a/src/gstreamer/gstlibcamerapool.h b/src/gstreamer/gstlibcamerapool.h
> index 05795d21223e..06b38cb296fc 100644
> --- a/src/gstreamer/gstlibcamerapool.h
> +++ b/src/gstreamer/gstlibcamerapool.h
> @@ -26,5 +26,3 @@ GstLibcameraPool *gst_libcamera_pool_new(GstLibcameraAllocator *allocator,
>  libcamera::Stream *gst_libcamera_pool_get_stream(GstLibcameraPool *self);
>  
>  libcamera::Stream *gst_libcamera_buffer_get_stream(GstBuffer *buffer);
> -
> -libcamera::FrameBuffer *gst_libcamera_buffer_get_frame_buffer(GstBuffer *buffer);