| Message ID | 20200314235728.15495-5-laurent.pinchart@ideasonboard.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Hi Laurent, Thanks for your patch. On 2020-03-15 01:57:23 +0200, Laurent Pinchart wrote: > Move the buffer creation code out of allocateBuffers() to a > createBuffers() function. This prepare for the rework of buffer export > and will avoid code duplication. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > --- > src/libcamera/include/v4l2_videodevice.h | 4 +- > src/libcamera/v4l2_videodevice.cpp | 68 +++++++++++++----------- > 2 files changed, 39 insertions(+), 33 deletions(-) > > diff --git a/src/libcamera/include/v4l2_videodevice.h b/src/libcamera/include/v4l2_videodevice.h > index 26f5e5917716..358d89e57414 100644 > --- a/src/libcamera/include/v4l2_videodevice.h > +++ b/src/libcamera/include/v4l2_videodevice.h > @@ -228,7 +228,9 @@ private: > int setSelection(unsigned int target, Rectangle *rect); > > int requestBuffers(unsigned int count, enum v4l2_memory memoryType); > - std::unique_ptr<FrameBuffer> createBuffer(const struct v4l2_buffer &buf); > + int createBuffers(unsigned int count, > + std::vector<std::unique_ptr<FrameBuffer>> *buffers); > + std::unique_ptr<FrameBuffer> createBuffer(unsigned int index); > FileDescriptor exportDmabufFd(unsigned int index, unsigned int plane); > > void bufferAvailable(EventNotifier *notifier); > diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp > index 6911ab024fd7..d02b02ef77d6 100644 > --- a/src/libcamera/v4l2_videodevice.cpp > +++ b/src/libcamera/v4l2_videodevice.cpp > @@ -1052,61 +1052,65 @@ int V4L2VideoDevice::requestBuffers(unsigned int count, > */ > int V4L2VideoDevice::allocateBuffers(unsigned int count, > std::vector<std::unique_ptr<FrameBuffer>> *buffers) > +{ > + int ret = createBuffers(count, buffers); > + if (ret < 0) > + return ret; > + > + cache_ = new V4L2BufferCache(*buffers); > + memoryType_ = V4L2_MEMORY_MMAP; > + > + return ret; > +} > + > +int V4L2VideoDevice::createBuffers(unsigned int count, > + std::vector<std::unique_ptr<FrameBuffer>> *buffers) > { > if (cache_) { > LOG(V4L2, Error) << "Buffers already allocated"; > return -EINVAL; > } > > - memoryType_ = V4L2_MEMORY_MMAP; > - > int ret = requestBuffers(count, V4L2_MEMORY_MMAP); > if (ret < 0) > return ret; > > for (unsigned i = 0; i < count; ++i) { > - struct v4l2_buffer buf = {}; > - struct v4l2_plane planes[VIDEO_MAX_PLANES] = {}; > - > - buf.index = i; > - buf.type = bufferType_; > - buf.memory = memoryType_; > - buf.length = ARRAY_SIZE(planes); > - buf.m.planes = planes; > - > - ret = ioctl(VIDIOC_QUERYBUF, &buf); > - if (ret < 0) { > - LOG(V4L2, Error) > - << "Unable to query buffer " << i << ": " > - << strerror(-ret); > - goto err_buf; > - } > - > - std::unique_ptr<FrameBuffer> buffer = createBuffer(buf); > + std::unique_ptr<FrameBuffer> buffer = createBuffer(i); > if (!buffer) { > LOG(V4L2, Error) << "Unable to create buffer"; > - ret = -EINVAL; > - goto err_buf; > + > + requestBuffers(0, V4L2_MEMORY_MMAP); > + buffers->clear(); > + > + return -EINVAL; > } > > buffers->push_back(std::move(buffer)); > } > > - cache_ = new V4L2BufferCache(*buffers); > - > return count; > +} > > -err_buf: > - requestBuffers(0, V4L2_MEMORY_MMAP); > +std::unique_ptr<FrameBuffer> V4L2VideoDevice::createBuffer(unsigned int index) > +{ > + struct v4l2_plane v4l2Planes[VIDEO_MAX_PLANES] = {}; > + struct v4l2_buffer buf = {}; > > - buffers->clear(); > + buf.index = index; > + buf.type = bufferType_; > + buf.memory = V4L2_MEMORY_MMAP; > + buf.length = ARRAY_SIZE(v4l2Planes); > + buf.m.planes = v4l2Planes; > > - return ret; > -} > + int ret = ioctl(VIDIOC_QUERYBUF, &buf); > + if (ret < 0) { > + LOG(V4L2, Error) > + << "Unable to query buffer " << index << ": " > + << strerror(-ret); > + return nullptr; > + } > > -std::unique_ptr<FrameBuffer> > -V4L2VideoDevice::createBuffer(const struct v4l2_buffer &buf) > -{ > const bool multiPlanar = V4L2_TYPE_IS_MULTIPLANAR(buf.type); > const unsigned int numPlanes = multiPlanar ? buf.length : 1; > > -- > Regards, > > Laurent Pinchart > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
diff --git a/src/libcamera/include/v4l2_videodevice.h b/src/libcamera/include/v4l2_videodevice.h index 26f5e5917716..358d89e57414 100644 --- a/src/libcamera/include/v4l2_videodevice.h +++ b/src/libcamera/include/v4l2_videodevice.h @@ -228,7 +228,9 @@ private: int setSelection(unsigned int target, Rectangle *rect); int requestBuffers(unsigned int count, enum v4l2_memory memoryType); - std::unique_ptr<FrameBuffer> createBuffer(const struct v4l2_buffer &buf); + int createBuffers(unsigned int count, + std::vector<std::unique_ptr<FrameBuffer>> *buffers); + std::unique_ptr<FrameBuffer> createBuffer(unsigned int index); FileDescriptor exportDmabufFd(unsigned int index, unsigned int plane); void bufferAvailable(EventNotifier *notifier); diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp index 6911ab024fd7..d02b02ef77d6 100644 --- a/src/libcamera/v4l2_videodevice.cpp +++ b/src/libcamera/v4l2_videodevice.cpp @@ -1052,61 +1052,65 @@ int V4L2VideoDevice::requestBuffers(unsigned int count, */ int V4L2VideoDevice::allocateBuffers(unsigned int count, std::vector<std::unique_ptr<FrameBuffer>> *buffers) +{ + int ret = createBuffers(count, buffers); + if (ret < 0) + return ret; + + cache_ = new V4L2BufferCache(*buffers); + memoryType_ = V4L2_MEMORY_MMAP; + + return ret; +} + +int V4L2VideoDevice::createBuffers(unsigned int count, + std::vector<std::unique_ptr<FrameBuffer>> *buffers) { if (cache_) { LOG(V4L2, Error) << "Buffers already allocated"; return -EINVAL; } - memoryType_ = V4L2_MEMORY_MMAP; - int ret = requestBuffers(count, V4L2_MEMORY_MMAP); if (ret < 0) return ret; for (unsigned i = 0; i < count; ++i) { - struct v4l2_buffer buf = {}; - struct v4l2_plane planes[VIDEO_MAX_PLANES] = {}; - - buf.index = i; - buf.type = bufferType_; - buf.memory = memoryType_; - buf.length = ARRAY_SIZE(planes); - buf.m.planes = planes; - - ret = ioctl(VIDIOC_QUERYBUF, &buf); - if (ret < 0) { - LOG(V4L2, Error) - << "Unable to query buffer " << i << ": " - << strerror(-ret); - goto err_buf; - } - - std::unique_ptr<FrameBuffer> buffer = createBuffer(buf); + std::unique_ptr<FrameBuffer> buffer = createBuffer(i); if (!buffer) { LOG(V4L2, Error) << "Unable to create buffer"; - ret = -EINVAL; - goto err_buf; + + requestBuffers(0, V4L2_MEMORY_MMAP); + buffers->clear(); + + return -EINVAL; } buffers->push_back(std::move(buffer)); } - cache_ = new V4L2BufferCache(*buffers); - return count; +} -err_buf: - requestBuffers(0, V4L2_MEMORY_MMAP); +std::unique_ptr<FrameBuffer> V4L2VideoDevice::createBuffer(unsigned int index) +{ + struct v4l2_plane v4l2Planes[VIDEO_MAX_PLANES] = {}; + struct v4l2_buffer buf = {}; - buffers->clear(); + buf.index = index; + buf.type = bufferType_; + buf.memory = V4L2_MEMORY_MMAP; + buf.length = ARRAY_SIZE(v4l2Planes); + buf.m.planes = v4l2Planes; - return ret; -} + int ret = ioctl(VIDIOC_QUERYBUF, &buf); + if (ret < 0) { + LOG(V4L2, Error) + << "Unable to query buffer " << index << ": " + << strerror(-ret); + return nullptr; + } -std::unique_ptr<FrameBuffer> -V4L2VideoDevice::createBuffer(const struct v4l2_buffer &buf) -{ const bool multiPlanar = V4L2_TYPE_IS_MULTIPLANAR(buf.type); const unsigned int numPlanes = multiPlanar ? buf.length : 1;
Move the buffer creation code out of allocateBuffers() to a createBuffers() function. This prepare for the rework of buffer export and will avoid code duplication. Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- src/libcamera/include/v4l2_videodevice.h | 4 +- src/libcamera/v4l2_videodevice.cpp | 68 +++++++++++++----------- 2 files changed, 39 insertions(+), 33 deletions(-)