| Message ID | 20220527093440.953377-3-paul.elder@ideasonboard.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Jacopo/Paul, On 5/27/22 11:34, Paul Elder via libcamera-devel wrote: > From: Jacopo Mondi <jacopo@jmondi.org> > > With the introduction of PlatformBufferAllocator all CameraStream can > be used to allocate buffers on-demand. > > Create CameraStream::allocator_ and the associated mutex for all types > of stream. > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> Looks good! > > --- > No change in v2 > > If we want to make CameraStream::mutex_ into non-pointer (as Hiro > suggested), it should be done on top. Do we want to do that? (I suppose > it doesn't affect this patch itself) Yes, I guess would be best on top, explaining the benefits/tradeoffs Reviewed-by: Umang Jain <umang.jain@ideasonboard.com> > --- > src/android/camera_stream.cpp | 6 ++---- > 1 file changed, 2 insertions(+), 4 deletions(-) > > diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp > index 154e088e..045e6006 100644 > --- a/src/android/camera_stream.cpp > +++ b/src/android/camera_stream.cpp > @@ -128,10 +128,8 @@ int CameraStream::configure() > worker_->start(); > } > > - if (type_ == Type::Internal) { > - allocator_ = std::make_unique<PlatformFrameBufferAllocator>(cameraDevice_); > - mutex_ = std::make_unique<Mutex>(); > - } > + allocator_ = std::make_unique<PlatformFrameBufferAllocator>(cameraDevice_); > + mutex_ = std::make_unique<Mutex>(); > > camera3Stream_->max_buffers = configuration().bufferCount; >
Hi Paul, On Fri, May 27, 2022 at 06:34:37PM +0900, Paul Elder wrote: > From: Jacopo Mondi <jacopo@jmondi.org> > > With the introduction of PlatformBufferAllocator all CameraStream can > be used to allocate buffers on-demand. > > Create CameraStream::allocator_ and the associated mutex for all types > of stream. > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > > --- > No change in v2 > > If we want to make CameraStream::mutex_ into non-pointer (as Hiro > suggested), it should be done on top. Do we want to do that? (I suppose > it doesn't affect this patch itself) Here or on top seems equivalent to me. > --- > src/android/camera_stream.cpp | 6 ++---- > 1 file changed, 2 insertions(+), 4 deletions(-) > > diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp > index 154e088e..045e6006 100644 > --- a/src/android/camera_stream.cpp > +++ b/src/android/camera_stream.cpp > @@ -128,10 +128,8 @@ int CameraStream::configure() > worker_->start(); > } > > - if (type_ == Type::Internal) { > - allocator_ = std::make_unique<PlatformFrameBufferAllocator>(cameraDevice_); > - mutex_ = std::make_unique<Mutex>(); > - } > + allocator_ = std::make_unique<PlatformFrameBufferAllocator>(cameraDevice_); > + mutex_ = std::make_unique<Mutex>(); > > camera3Stream_->max_buffers = configuration().bufferCount; > > -- > 2.30.2 >
Hi Paul and Jacopo, Thank you for the patch. On Fri, May 27, 2022 at 06:34:37PM +0900, Paul Elder via libcamera-devel wrote: > From: Jacopo Mondi <jacopo@jmondi.org> > > With the introduction of PlatformBufferAllocator all CameraStream can > be used to allocate buffers on-demand. It's been a while, so I don't recall how the two are related. It would be nice to extend the commit message to explain this better. > Create CameraStream::allocator_ and the associated mutex for all types > of stream. > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > > --- > No change in v2 > > If we want to make CameraStream::mutex_ into non-pointer (as Hiro > suggested), it should be done on top. Do we want to do that? (I suppose > it doesn't affect this patch itself) It would be nice to do so on top indeed. Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > src/android/camera_stream.cpp | 6 ++---- > 1 file changed, 2 insertions(+), 4 deletions(-) > > diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp > index 154e088e..045e6006 100644 > --- a/src/android/camera_stream.cpp > +++ b/src/android/camera_stream.cpp > @@ -128,10 +128,8 @@ int CameraStream::configure() > worker_->start(); > } > > - if (type_ == Type::Internal) { > - allocator_ = std::make_unique<PlatformFrameBufferAllocator>(cameraDevice_); > - mutex_ = std::make_unique<Mutex>(); > - } > + allocator_ = std::make_unique<PlatformFrameBufferAllocator>(cameraDevice_); > + mutex_ = std::make_unique<Mutex>(); > > camera3Stream_->max_buffers = configuration().bufferCount; >
diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp index 154e088e..045e6006 100644 --- a/src/android/camera_stream.cpp +++ b/src/android/camera_stream.cpp @@ -128,10 +128,8 @@ int CameraStream::configure() worker_->start(); } - if (type_ == Type::Internal) { - allocator_ = std::make_unique<PlatformFrameBufferAllocator>(cameraDevice_); - mutex_ = std::make_unique<Mutex>(); - } + allocator_ = std::make_unique<PlatformFrameBufferAllocator>(cameraDevice_); + mutex_ = std::make_unique<Mutex>(); camera3Stream_->max_buffers = configuration().bufferCount;