| Message ID | 20210823131221.1034059-8-hiroh@chromium.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
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@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); > 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);
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@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@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); >
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); 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);
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@chromium.org> --- src/gstreamer/gstlibcameraallocator.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)