| Message ID | 20200206150504.24204-2-kieran.bingham@ideasonboard.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Kieran, Thank you for the patch. On Thu, Feb 06, 2020 at 03:04:59PM +0000, Kieran Bingham wrote: > The FrameBufferAllocator must be deleted and reconstructed before performing > any reconfiguration of the stream. > > Construct the allocator at startCapture, and destroy it during stopCapture so > that we can successfully stop and restart the stream. > > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > --- > src/qcam/main_window.cpp | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp > index df51fa888342..38bc04a23b86 100644 > --- a/src/qcam/main_window.cpp > +++ b/src/qcam/main_window.cpp > @@ -37,7 +37,6 @@ MainWindow::MainWindow(CameraManager *cm, const OptionsParser::Options &options) > > ret = openCamera(cm); > if (!ret) { > - allocator_ = FrameBufferAllocator::create(camera_); > ret = startCapture(); > } You can remove the braces. Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > @@ -50,7 +49,6 @@ MainWindow::~MainWindow() > { > if (camera_) { > stopCapture(); > - delete allocator_; > camera_->release(); > camera_.reset(); > } > @@ -171,6 +169,7 @@ int MainWindow::startCapture() > > adjustSize(); > > + allocator_ = FrameBufferAllocator::create(camera_); > ret = allocator_->allocate(stream); > if (ret < 0) { > std::cerr << "Failed to allocate capture buffers" << std::endl; > @@ -255,6 +254,8 @@ void MainWindow::stopCapture() > } > mappedBuffers_.clear(); > > + delete allocator_; > + > isCapturing_ = false; > > config_.reset();
Le jeu. 6 févr. 2020 10 h 05, Kieran Bingham < kieran.bingham@ideasonboard.com> a écrit : > The FrameBufferAllocator must be deleted and reconstructed before > performing > any reconfiguration of the stream. > > Construct the allocator at startCapture, and destroy it during stopCapture > so > that we can successfully stop and restart the stream. > Can you explain why adding this limitation. We have fixed this issue in v4l with buffer orphaning, so I don't understand why introducing it again in software. Not being able to orphan the buffer pool actually prevents zero-copy renegotiation. Consider the case where the framebuffer is being used as your display scan out. You need to reconfigure and produce a new buffer before this framebuffer can be returned. > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > --- > src/qcam/main_window.cpp | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp > index df51fa888342..38bc04a23b86 100644 > --- a/src/qcam/main_window.cpp > +++ b/src/qcam/main_window.cpp > @@ -37,7 +37,6 @@ MainWindow::MainWindow(CameraManager *cm, const > OptionsParser::Options &options) > > ret = openCamera(cm); > if (!ret) { > - allocator_ = FrameBufferAllocator::create(camera_); > ret = startCapture(); > } > > @@ -50,7 +49,6 @@ MainWindow::~MainWindow() > { > if (camera_) { > stopCapture(); > - delete allocator_; > camera_->release(); > camera_.reset(); > } > @@ -171,6 +169,7 @@ int MainWindow::startCapture() > > adjustSize(); > > + allocator_ = FrameBufferAllocator::create(camera_); > ret = allocator_->allocate(stream); > if (ret < 0) { > std::cerr << "Failed to allocate capture buffers" << > std::endl; > @@ -255,6 +254,8 @@ void MainWindow::stopCapture() > } > mappedBuffers_.clear(); > > + delete allocator_; > + > isCapturing_ = false; > > config_.reset(); > -- > 2.20.1 > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel >
diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp index df51fa888342..38bc04a23b86 100644 --- a/src/qcam/main_window.cpp +++ b/src/qcam/main_window.cpp @@ -37,7 +37,6 @@ MainWindow::MainWindow(CameraManager *cm, const OptionsParser::Options &options) ret = openCamera(cm); if (!ret) { - allocator_ = FrameBufferAllocator::create(camera_); ret = startCapture(); } @@ -50,7 +49,6 @@ MainWindow::~MainWindow() { if (camera_) { stopCapture(); - delete allocator_; camera_->release(); camera_.reset(); } @@ -171,6 +169,7 @@ int MainWindow::startCapture() adjustSize(); + allocator_ = FrameBufferAllocator::create(camera_); ret = allocator_->allocate(stream); if (ret < 0) { std::cerr << "Failed to allocate capture buffers" << std::endl; @@ -255,6 +254,8 @@ void MainWindow::stopCapture() } mappedBuffers_.clear(); + delete allocator_; + isCapturing_ = false; config_.reset();
The FrameBufferAllocator must be deleted and reconstructed before performing any reconfiguration of the stream. Construct the allocator at startCapture, and destroy it during stopCapture so that we can successfully stop and restart the stream. Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> --- src/qcam/main_window.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)