| Message ID | 20260618123844.656396-26-barnabas.pocze@ideasonboard.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
On Thu, Jun 18, 2026 at 02:38:42PM +0200, Barnabás Pőcze wrote: > This expresses the meaning better. ok.. Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > > Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > --- > src/v4l2/v4l2_camera.cpp | 2 +- > src/v4l2/v4l2_camera.h | 8 ++++---- > src/v4l2/v4l2_camera_proxy.cpp | 4 ++-- > 3 files changed, 7 insertions(+), 7 deletions(-) > > diff --git a/src/v4l2/v4l2_camera.cpp b/src/v4l2/v4l2_camera.cpp > index 7dd0ea1ef9..e72b73a22a 100644 > --- a/src/v4l2/v4l2_camera.cpp > +++ b/src/v4l2/v4l2_camera.cpp > @@ -68,7 +68,7 @@ void V4L2Camera::unbind() > efd_ = -1; > } > > -std::vector<V4L2Camera::Buffer> V4L2Camera::completedBuffers() > +std::vector<V4L2Camera::CompletedBuffer> V4L2Camera::completedBuffers() > { > MutexLocker lock(bufferLock_); > std::vector v(std::move_iterator(completedBuffers_.begin()), > diff --git a/src/v4l2/v4l2_camera.h b/src/v4l2/v4l2_camera.h > index 8a58169d89..1d90eb43e4 100644 > --- a/src/v4l2/v4l2_camera.h > +++ b/src/v4l2/v4l2_camera.h > @@ -23,8 +23,8 @@ > class V4L2Camera > { > public: > - struct Buffer { > - Buffer(unsigned int index, const libcamera::FrameMetadata &data) > + struct CompletedBuffer { > + CompletedBuffer(unsigned int index, const libcamera::FrameMetadata &data) > : index_(index), data_(data) > { > } > @@ -41,7 +41,7 @@ public: > void bind(int efd); > void unbind(); > > - std::vector<Buffer> completedBuffers() LIBCAMERA_TSA_EXCLUDES(bufferLock_); > + std::vector<CompletedBuffer> completedBuffers() LIBCAMERA_TSA_EXCLUDES(bufferLock_); > > int configure(libcamera::StreamConfiguration *streamConfigOut, > const libcamera::Size &size, > @@ -85,7 +85,7 @@ private: > std::vector<std::unique_ptr<libcamera::Request>> requestPool_; > > std::deque<libcamera::Request *> pendingRequests_; > - std::deque<Buffer> completedBuffers_ > + std::deque<CompletedBuffer> completedBuffers_ > LIBCAMERA_TSA_GUARDED_BY(bufferLock_); > > int efd_; > diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp > index 5281f10552..820a417a9c 100644 > --- a/src/v4l2/v4l2_camera_proxy.cpp > +++ b/src/v4l2/v4l2_camera_proxy.cpp > @@ -243,8 +243,8 @@ void V4L2CameraProxy::querycap(std::shared_ptr<Camera> camera) > > void V4L2CameraProxy::updateBuffers() > { > - std::vector<V4L2Camera::Buffer> completedBuffers = vcam_->completedBuffers(); > - for (const V4L2Camera::Buffer &buffer : completedBuffers) { > + std::vector<V4L2Camera::CompletedBuffer> completedBuffers = vcam_->completedBuffers(); > + for (const V4L2Camera::CompletedBuffer &buffer : completedBuffers) { > const FrameMetadata &fmd = buffer.data_; > struct v4l2_buffer &buf = buffers_[buffer.index_]; > > -- > 2.54.0 >
On Thu, Jun 18, 2026 at 02:38:42PM +0200, Barnabás Pőcze wrote: > This expresses the meaning better. Not entirely convinced. If you need to free the type name Buffer to introduce another type in the same class I won't object to this, but otherwise I'd drop this patch. > Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > --- > src/v4l2/v4l2_camera.cpp | 2 +- > src/v4l2/v4l2_camera.h | 8 ++++---- > src/v4l2/v4l2_camera_proxy.cpp | 4 ++-- > 3 files changed, 7 insertions(+), 7 deletions(-) > > diff --git a/src/v4l2/v4l2_camera.cpp b/src/v4l2/v4l2_camera.cpp > index 7dd0ea1ef9..e72b73a22a 100644 > --- a/src/v4l2/v4l2_camera.cpp > +++ b/src/v4l2/v4l2_camera.cpp > @@ -68,7 +68,7 @@ void V4L2Camera::unbind() > efd_ = -1; > } > > -std::vector<V4L2Camera::Buffer> V4L2Camera::completedBuffers() > +std::vector<V4L2Camera::CompletedBuffer> V4L2Camera::completedBuffers() > { > MutexLocker lock(bufferLock_); > std::vector v(std::move_iterator(completedBuffers_.begin()), > diff --git a/src/v4l2/v4l2_camera.h b/src/v4l2/v4l2_camera.h > index 8a58169d89..1d90eb43e4 100644 > --- a/src/v4l2/v4l2_camera.h > +++ b/src/v4l2/v4l2_camera.h > @@ -23,8 +23,8 @@ > class V4L2Camera > { > public: > - struct Buffer { > - Buffer(unsigned int index, const libcamera::FrameMetadata &data) > + struct CompletedBuffer { > + CompletedBuffer(unsigned int index, const libcamera::FrameMetadata &data) > : index_(index), data_(data) > { > } > @@ -41,7 +41,7 @@ public: > void bind(int efd); > void unbind(); > > - std::vector<Buffer> completedBuffers() LIBCAMERA_TSA_EXCLUDES(bufferLock_); > + std::vector<CompletedBuffer> completedBuffers() LIBCAMERA_TSA_EXCLUDES(bufferLock_); > > int configure(libcamera::StreamConfiguration *streamConfigOut, > const libcamera::Size &size, > @@ -85,7 +85,7 @@ private: > std::vector<std::unique_ptr<libcamera::Request>> requestPool_; > > std::deque<libcamera::Request *> pendingRequests_; > - std::deque<Buffer> completedBuffers_ > + std::deque<CompletedBuffer> completedBuffers_ > LIBCAMERA_TSA_GUARDED_BY(bufferLock_); > > int efd_; > diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp > index 5281f10552..820a417a9c 100644 > --- a/src/v4l2/v4l2_camera_proxy.cpp > +++ b/src/v4l2/v4l2_camera_proxy.cpp > @@ -243,8 +243,8 @@ void V4L2CameraProxy::querycap(std::shared_ptr<Camera> camera) > > void V4L2CameraProxy::updateBuffers() > { > - std::vector<V4L2Camera::Buffer> completedBuffers = vcam_->completedBuffers(); > - for (const V4L2Camera::Buffer &buffer : completedBuffers) { > + std::vector<V4L2Camera::CompletedBuffer> completedBuffers = vcam_->completedBuffers(); > + for (const V4L2Camera::CompletedBuffer &buffer : completedBuffers) { > const FrameMetadata &fmd = buffer.data_; > struct v4l2_buffer &buf = buffers_[buffer.index_]; >
Quoting Laurent Pinchart (2026-06-24 08:05:00) > On Thu, Jun 18, 2026 at 02:38:42PM +0200, Barnabás Pőcze wrote: > > This expresses the meaning better. > > Not entirely convinced. If you need to free the type name Buffer to > introduce another type in the same class I won't object to this, but > otherwise I'd drop this patch. I'm convinced :) I think what probably happened was that while I was hacking I was going to use it as a multipurpose buffer, but then it ended up being only used as a temporary container for completed buffers. So imo CompletedBuffer would be a more appropriate name. Since we're cleaning up v4l2-compat we might as well pull it in. Reviewed-by: Paul Elder <paul.elder@ideasonboard.com> > > > Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > > --- > > src/v4l2/v4l2_camera.cpp | 2 +- > > src/v4l2/v4l2_camera.h | 8 ++++---- > > src/v4l2/v4l2_camera_proxy.cpp | 4 ++-- > > 3 files changed, 7 insertions(+), 7 deletions(-) > > > > diff --git a/src/v4l2/v4l2_camera.cpp b/src/v4l2/v4l2_camera.cpp > > index 7dd0ea1ef9..e72b73a22a 100644 > > --- a/src/v4l2/v4l2_camera.cpp > > +++ b/src/v4l2/v4l2_camera.cpp > > @@ -68,7 +68,7 @@ void V4L2Camera::unbind() > > efd_ = -1; > > } > > > > -std::vector<V4L2Camera::Buffer> V4L2Camera::completedBuffers() > > +std::vector<V4L2Camera::CompletedBuffer> V4L2Camera::completedBuffers() > > { > > MutexLocker lock(bufferLock_); > > std::vector v(std::move_iterator(completedBuffers_.begin()), > > diff --git a/src/v4l2/v4l2_camera.h b/src/v4l2/v4l2_camera.h > > index 8a58169d89..1d90eb43e4 100644 > > --- a/src/v4l2/v4l2_camera.h > > +++ b/src/v4l2/v4l2_camera.h > > @@ -23,8 +23,8 @@ > > class V4L2Camera > > { > > public: > > - struct Buffer { > > - Buffer(unsigned int index, const libcamera::FrameMetadata &data) > > + struct CompletedBuffer { > > + CompletedBuffer(unsigned int index, const libcamera::FrameMetadata &data) > > : index_(index), data_(data) > > { > > } > > @@ -41,7 +41,7 @@ public: > > void bind(int efd); > > void unbind(); > > > > - std::vector<Buffer> completedBuffers() LIBCAMERA_TSA_EXCLUDES(bufferLock_); > > + std::vector<CompletedBuffer> completedBuffers() LIBCAMERA_TSA_EXCLUDES(bufferLock_); > > > > int configure(libcamera::StreamConfiguration *streamConfigOut, > > const libcamera::Size &size, > > @@ -85,7 +85,7 @@ private: > > std::vector<std::unique_ptr<libcamera::Request>> requestPool_; > > > > std::deque<libcamera::Request *> pendingRequests_; > > - std::deque<Buffer> completedBuffers_ > > + std::deque<CompletedBuffer> completedBuffers_ > > LIBCAMERA_TSA_GUARDED_BY(bufferLock_); > > > > int efd_; > > diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp > > index 5281f10552..820a417a9c 100644 > > --- a/src/v4l2/v4l2_camera_proxy.cpp > > +++ b/src/v4l2/v4l2_camera_proxy.cpp > > @@ -243,8 +243,8 @@ void V4L2CameraProxy::querycap(std::shared_ptr<Camera> camera) > > > > void V4L2CameraProxy::updateBuffers() > > { > > - std::vector<V4L2Camera::Buffer> completedBuffers = vcam_->completedBuffers(); > > - for (const V4L2Camera::Buffer &buffer : completedBuffers) { > > + std::vector<V4L2Camera::CompletedBuffer> completedBuffers = vcam_->completedBuffers(); > > + for (const V4L2Camera::CompletedBuffer &buffer : completedBuffers) { > > const FrameMetadata &fmd = buffer.data_; > > struct v4l2_buffer &buf = buffers_[buffer.index_]; > > > > -- > Regards, > > Laurent Pinchart
2026. 06. 24. 9:06 keltezéssel, Paul Elder írta: > Quoting Laurent Pinchart (2026-06-24 08:05:00) >> On Thu, Jun 18, 2026 at 02:38:42PM +0200, Barnabás Pőcze wrote: >>> This expresses the meaning better. >> >> Not entirely convinced. If you need to free the type name Buffer to >> introduce another type in the same class I won't object to this, but >> otherwise I'd drop this patch. I think I wanted the `Buffer` name for something, but now that I check all my changes, it's nowhere to be found. So it is not needed, it seems. > > I'm convinced :) > > I think what probably happened was that while I was hacking I was going to use > it as a multipurpose buffer, but then it ended up being only used as a > temporary container for completed buffers. So imo CompletedBuffer would be a > more appropriate name. Since we're cleaning up v4l2-compat we might as well > pull it in. Okay then I'll keep this for the time being. > > Reviewed-by: Paul Elder <paul.elder@ideasonboard.com> > >> >>> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> >>> --- >>> src/v4l2/v4l2_camera.cpp | 2 +- >>> src/v4l2/v4l2_camera.h | 8 ++++---- >>> src/v4l2/v4l2_camera_proxy.cpp | 4 ++-- >>> 3 files changed, 7 insertions(+), 7 deletions(-) >>> >>> diff --git a/src/v4l2/v4l2_camera.cpp b/src/v4l2/v4l2_camera.cpp >>> index 7dd0ea1ef9..e72b73a22a 100644 >>> --- a/src/v4l2/v4l2_camera.cpp >>> +++ b/src/v4l2/v4l2_camera.cpp >>> @@ -68,7 +68,7 @@ void V4L2Camera::unbind() >>> efd_ = -1; >>> } >>> >>> -std::vector<V4L2Camera::Buffer> V4L2Camera::completedBuffers() >>> +std::vector<V4L2Camera::CompletedBuffer> V4L2Camera::completedBuffers() >>> { >>> MutexLocker lock(bufferLock_); >>> std::vector v(std::move_iterator(completedBuffers_.begin()), >>> diff --git a/src/v4l2/v4l2_camera.h b/src/v4l2/v4l2_camera.h >>> index 8a58169d89..1d90eb43e4 100644 >>> --- a/src/v4l2/v4l2_camera.h >>> +++ b/src/v4l2/v4l2_camera.h >>> @@ -23,8 +23,8 @@ >>> class V4L2Camera >>> { >>> public: >>> - struct Buffer { >>> - Buffer(unsigned int index, const libcamera::FrameMetadata &data) >>> + struct CompletedBuffer { >>> + CompletedBuffer(unsigned int index, const libcamera::FrameMetadata &data) >>> : index_(index), data_(data) >>> { >>> } >>> @@ -41,7 +41,7 @@ public: >>> void bind(int efd); >>> void unbind(); >>> >>> - std::vector<Buffer> completedBuffers() LIBCAMERA_TSA_EXCLUDES(bufferLock_); >>> + std::vector<CompletedBuffer> completedBuffers() LIBCAMERA_TSA_EXCLUDES(bufferLock_); >>> >>> int configure(libcamera::StreamConfiguration *streamConfigOut, >>> const libcamera::Size &size, >>> @@ -85,7 +85,7 @@ private: >>> std::vector<std::unique_ptr<libcamera::Request>> requestPool_; >>> >>> std::deque<libcamera::Request *> pendingRequests_; >>> - std::deque<Buffer> completedBuffers_ >>> + std::deque<CompletedBuffer> completedBuffers_ >>> LIBCAMERA_TSA_GUARDED_BY(bufferLock_); >>> >>> int efd_; >>> diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp >>> index 5281f10552..820a417a9c 100644 >>> --- a/src/v4l2/v4l2_camera_proxy.cpp >>> +++ b/src/v4l2/v4l2_camera_proxy.cpp >>> @@ -243,8 +243,8 @@ void V4L2CameraProxy::querycap(std::shared_ptr<Camera> camera) >>> >>> void V4L2CameraProxy::updateBuffers() >>> { >>> - std::vector<V4L2Camera::Buffer> completedBuffers = vcam_->completedBuffers(); >>> - for (const V4L2Camera::Buffer &buffer : completedBuffers) { >>> + std::vector<V4L2Camera::CompletedBuffer> completedBuffers = vcam_->completedBuffers(); >>> + for (const V4L2Camera::CompletedBuffer &buffer : completedBuffers) { >>> const FrameMetadata &fmd = buffer.data_; >>> struct v4l2_buffer &buf = buffers_[buffer.index_]; >>> >> >> -- >> Regards, >> >> Laurent Pinchart
On Wed, Jun 24, 2026 at 04:06:29PM +0900, Paul Elder wrote: > Quoting Laurent Pinchart (2026-06-24 08:05:00) > > On Thu, Jun 18, 2026 at 02:38:42PM +0200, Barnabás Pőcze wrote: > > > This expresses the meaning better. > > > > Not entirely convinced. If you need to free the type name Buffer to > > introduce another type in the same class I won't object to this, but > > otherwise I'd drop this patch. > > I'm convinced :) I'll submit to your maintainer decision :-) > I think what probably happened was that while I was hacking I was going to use > it as a multipurpose buffer, but then it ended up being only used as a > temporary container for completed buffers. So imo CompletedBuffer would be a > more appropriate name. Since we're cleaning up v4l2-compat we might as well > pull it in. > > Reviewed-by: Paul Elder <paul.elder@ideasonboard.com> > > > > Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > > > --- > > > src/v4l2/v4l2_camera.cpp | 2 +- > > > src/v4l2/v4l2_camera.h | 8 ++++---- > > > src/v4l2/v4l2_camera_proxy.cpp | 4 ++-- > > > 3 files changed, 7 insertions(+), 7 deletions(-) > > > > > > diff --git a/src/v4l2/v4l2_camera.cpp b/src/v4l2/v4l2_camera.cpp > > > index 7dd0ea1ef9..e72b73a22a 100644 > > > --- a/src/v4l2/v4l2_camera.cpp > > > +++ b/src/v4l2/v4l2_camera.cpp > > > @@ -68,7 +68,7 @@ void V4L2Camera::unbind() > > > efd_ = -1; > > > } > > > > > > -std::vector<V4L2Camera::Buffer> V4L2Camera::completedBuffers() > > > +std::vector<V4L2Camera::CompletedBuffer> V4L2Camera::completedBuffers() > > > { > > > MutexLocker lock(bufferLock_); > > > std::vector v(std::move_iterator(completedBuffers_.begin()), > > > diff --git a/src/v4l2/v4l2_camera.h b/src/v4l2/v4l2_camera.h > > > index 8a58169d89..1d90eb43e4 100644 > > > --- a/src/v4l2/v4l2_camera.h > > > +++ b/src/v4l2/v4l2_camera.h > > > @@ -23,8 +23,8 @@ > > > class V4L2Camera > > > { > > > public: > > > - struct Buffer { > > > - Buffer(unsigned int index, const libcamera::FrameMetadata &data) > > > + struct CompletedBuffer { > > > + CompletedBuffer(unsigned int index, const libcamera::FrameMetadata &data) > > > : index_(index), data_(data) > > > { > > > } > > > @@ -41,7 +41,7 @@ public: > > > void bind(int efd); > > > void unbind(); > > > > > > - std::vector<Buffer> completedBuffers() LIBCAMERA_TSA_EXCLUDES(bufferLock_); > > > + std::vector<CompletedBuffer> completedBuffers() LIBCAMERA_TSA_EXCLUDES(bufferLock_); > > > > > > int configure(libcamera::StreamConfiguration *streamConfigOut, > > > const libcamera::Size &size, > > > @@ -85,7 +85,7 @@ private: > > > std::vector<std::unique_ptr<libcamera::Request>> requestPool_; > > > > > > std::deque<libcamera::Request *> pendingRequests_; > > > - std::deque<Buffer> completedBuffers_ > > > + std::deque<CompletedBuffer> completedBuffers_ > > > LIBCAMERA_TSA_GUARDED_BY(bufferLock_); > > > > > > int efd_; > > > diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp > > > index 5281f10552..820a417a9c 100644 > > > --- a/src/v4l2/v4l2_camera_proxy.cpp > > > +++ b/src/v4l2/v4l2_camera_proxy.cpp > > > @@ -243,8 +243,8 @@ void V4L2CameraProxy::querycap(std::shared_ptr<Camera> camera) > > > > > > void V4L2CameraProxy::updateBuffers() > > > { > > > - std::vector<V4L2Camera::Buffer> completedBuffers = vcam_->completedBuffers(); > > > - for (const V4L2Camera::Buffer &buffer : completedBuffers) { > > > + std::vector<V4L2Camera::CompletedBuffer> completedBuffers = vcam_->completedBuffers(); > > > + for (const V4L2Camera::CompletedBuffer &buffer : completedBuffers) { > > > const FrameMetadata &fmd = buffer.data_; > > > struct v4l2_buffer &buf = buffers_[buffer.index_]; > > >
diff --git a/src/v4l2/v4l2_camera.cpp b/src/v4l2/v4l2_camera.cpp index 7dd0ea1ef9..e72b73a22a 100644 --- a/src/v4l2/v4l2_camera.cpp +++ b/src/v4l2/v4l2_camera.cpp @@ -68,7 +68,7 @@ void V4L2Camera::unbind() efd_ = -1; } -std::vector<V4L2Camera::Buffer> V4L2Camera::completedBuffers() +std::vector<V4L2Camera::CompletedBuffer> V4L2Camera::completedBuffers() { MutexLocker lock(bufferLock_); std::vector v(std::move_iterator(completedBuffers_.begin()), diff --git a/src/v4l2/v4l2_camera.h b/src/v4l2/v4l2_camera.h index 8a58169d89..1d90eb43e4 100644 --- a/src/v4l2/v4l2_camera.h +++ b/src/v4l2/v4l2_camera.h @@ -23,8 +23,8 @@ class V4L2Camera { public: - struct Buffer { - Buffer(unsigned int index, const libcamera::FrameMetadata &data) + struct CompletedBuffer { + CompletedBuffer(unsigned int index, const libcamera::FrameMetadata &data) : index_(index), data_(data) { } @@ -41,7 +41,7 @@ public: void bind(int efd); void unbind(); - std::vector<Buffer> completedBuffers() LIBCAMERA_TSA_EXCLUDES(bufferLock_); + std::vector<CompletedBuffer> completedBuffers() LIBCAMERA_TSA_EXCLUDES(bufferLock_); int configure(libcamera::StreamConfiguration *streamConfigOut, const libcamera::Size &size, @@ -85,7 +85,7 @@ private: std::vector<std::unique_ptr<libcamera::Request>> requestPool_; std::deque<libcamera::Request *> pendingRequests_; - std::deque<Buffer> completedBuffers_ + std::deque<CompletedBuffer> completedBuffers_ LIBCAMERA_TSA_GUARDED_BY(bufferLock_); int efd_; diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp index 5281f10552..820a417a9c 100644 --- a/src/v4l2/v4l2_camera_proxy.cpp +++ b/src/v4l2/v4l2_camera_proxy.cpp @@ -243,8 +243,8 @@ void V4L2CameraProxy::querycap(std::shared_ptr<Camera> camera) void V4L2CameraProxy::updateBuffers() { - std::vector<V4L2Camera::Buffer> completedBuffers = vcam_->completedBuffers(); - for (const V4L2Camera::Buffer &buffer : completedBuffers) { + std::vector<V4L2Camera::CompletedBuffer> completedBuffers = vcam_->completedBuffers(); + for (const V4L2Camera::CompletedBuffer &buffer : completedBuffers) { const FrameMetadata &fmd = buffer.data_; struct v4l2_buffer &buf = buffers_[buffer.index_];
This expresses the meaning better. Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> --- src/v4l2/v4l2_camera.cpp | 2 +- src/v4l2/v4l2_camera.h | 8 ++++---- src/v4l2/v4l2_camera_proxy.cpp | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-)