[libcamera-devel] [RFC PATCH v2 07/10] gstreamer: gstlibcameraallocator: Use offset in creating a buffer

[Nicolas Dufresne]

Le mardi 24 août 2021 à 02:14 +0300, Laurent Pinchart a écrit :
> Hi Hiro,
> 
> Thank you for the patch.
> 
> I'm deferring review of this to Nicolas as he knows much better than I
> do :-)
> 
> On Mon, Aug 23, 2021 at 10:12:18PM +0900, Hirokazu Honda wrote:
> > The plane length is the length of the plane size. The buffer length
> > to be allocated for a plane is the offset and the length of
> > FrameBuffer::Plane.
> > 
> > Signed-off-by: Hirokazu Honda <hiroh at chromium.org>
> > ---
> >  src/gstreamer/gstlibcameraallocator.cpp | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/src/gstreamer/gstlibcameraallocator.cpp b/src/gstreamer/gstlibcameraallocator.cpp
> > index 7bd8ba2d..60ead273 100644
> > --- a/src/gstreamer/gstlibcameraallocator.cpp
> > +++ b/src/gstreamer/gstlibcameraallocator.cpp
> > @@ -52,8 +52,10 @@ FrameWrap::FrameWrap(GstAllocator *allocator, FrameBuffer *buffer,
> >  	  outstandingPlanes_(0)
> >  {
> >  	for (const FrameBuffer::Plane &plane : buffer->planes()) {
> > -		GstMemory *mem = gst_fd_allocator_alloc(allocator, plane.fd.fd(), plane.length,
> > +		GstMemory *mem = gst_fd_allocator_alloc(allocator, plane.fd.fd(),
> > +							plane.offset + plane.length,
> >  							GST_FD_MEMORY_FLAG_DONT_CLOSE);
> > +		gst_memory_resize(mem, plane.offset, plane.length);

This looks better indeed. Assuming you have done the inverse (removing the
offset from V4L2 provided value) in the backend.

Reviewed-by: Nicolas Dufresne <nicolas.dufresne at collabora.com>

> >  		gst_mini_object_set_qdata(GST_MINI_OBJECT(mem), getQuark(), this, nullptr);
> >  		GST_MINI_OBJECT(mem)->dispose = gst_libcamera_allocator_release;
> >  		g_object_unref(mem->allocator);
>