Message ID | 20190418104715.22622-13-jacopo@jmondi.org |
---|---|
State | Superseded |
Headers | show |
Series |
|
Related | show |
Hi Jacopo, Thank you for the patch. On Thu, Apr 18, 2019 at 12:47:13PM +0200, Jacopo Mondi wrote: > The viewfinder and main output require identical logic for buffer and > request completion. Rename the IPU3CameraData::imguOutputBufferReady() > slot to IPU3CameraData::imguCaptureBufferReady() to reflect this, and > connect the viewfinder bufferReady signal to the slot. You don't rename the method anymore in v5, have you forgotten to update the commit message ? > > Update the slot logic to ignore internal buffers that are not part of > the request, and to complete the request only when the last buffer > completes. > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > --- > src/libcamera/pipeline/ipu3/ipu3.cpp | 32 +++++++++++++++++++++++++--- > 1 file changed, 29 insertions(+), 3 deletions(-) > > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp > index ee490a488cf7..71b36ecf6cd9 100644 > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp > @@ -705,6 +705,8 @@ int PipelineHandlerIPU3::registerCameras() > &IPU3CameraData::imguInputBufferReady); > data->imgu_->output_.dev->bufferReady.connect(data.get(), > &IPU3CameraData::imguOutputBufferReady); > + data->imgu_->viewfinder_.dev->bufferReady.connect(data.get(), > + &IPU3CameraData::imguOutputBufferReady); > > /* Initialize and register the Camera and its streams. */ > std::string cameraName = cio2->sensor_->entityName() + " " > @@ -750,10 +752,34 @@ void PipelineHandlerIPU3::IPU3CameraData::imguInputBufferReady(Buffer *buffer) > */ > void PipelineHandlerIPU3::IPU3CameraData::imguOutputBufferReady(Buffer *buffer) > { > - Request *request = queuedRequests_.front(); > + Request *request = buffer->request(); > + if (!request) > + /* Completed buffers not part of a request are ignored. */ > + return; Unless I'm mistaken you were able to get the main output working without needing to queue dummy buffers, is this check still needed ? > + > + if (!pipe_->completeBuffer(camera_, request, buffer)) > + /* Request not completed yet, return here. */ > + return; > + > + /* > + * Complete requests in queuing order: if some other request is > + * pending, post-pone completion. s/post-pone/postpone/ > + */ > + Request *front = queuedRequests_.front(); > + if (front != request) > + return; > > - pipe_->completeBuffer(camera_, request, buffer); > - pipe_->completeRequest(camera_, request); > + /* > + * Complete the current request, and all the other pending ones, > + * in queuing order. > + */ > + while (1) { > + if (front->hasPendingBuffers()) > + break; > + > + pipe_->completeRequest(camera_, front); > + front = queuedRequests_.front(); > + } Would the following be simpler ? /* Complete the pending requests in queuing order. */ while (1) { request = queuedRequests_.front(); if (request->hasPendingBuffers()) break; pipe_->completeRequest(camera_, request); } > } > > /**
diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index ee490a488cf7..71b36ecf6cd9 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -705,6 +705,8 @@ int PipelineHandlerIPU3::registerCameras() &IPU3CameraData::imguInputBufferReady); data->imgu_->output_.dev->bufferReady.connect(data.get(), &IPU3CameraData::imguOutputBufferReady); + data->imgu_->viewfinder_.dev->bufferReady.connect(data.get(), + &IPU3CameraData::imguOutputBufferReady); /* Initialize and register the Camera and its streams. */ std::string cameraName = cio2->sensor_->entityName() + " " @@ -750,10 +752,34 @@ void PipelineHandlerIPU3::IPU3CameraData::imguInputBufferReady(Buffer *buffer) */ void PipelineHandlerIPU3::IPU3CameraData::imguOutputBufferReady(Buffer *buffer) { - Request *request = queuedRequests_.front(); + Request *request = buffer->request(); + if (!request) + /* Completed buffers not part of a request are ignored. */ + return; + + if (!pipe_->completeBuffer(camera_, request, buffer)) + /* Request not completed yet, return here. */ + return; + + /* + * Complete requests in queuing order: if some other request is + * pending, post-pone completion. + */ + Request *front = queuedRequests_.front(); + if (front != request) + return; - pipe_->completeBuffer(camera_, request, buffer); - pipe_->completeRequest(camera_, request); + /* + * Complete the current request, and all the other pending ones, + * in queuing order. + */ + while (1) { + if (front->hasPendingBuffers()) + break; + + pipe_->completeRequest(camera_, front); + front = queuedRequests_.front(); + } } /**
The viewfinder and main output require identical logic for buffer and request completion. Rename the IPU3CameraData::imguOutputBufferReady() slot to IPU3CameraData::imguCaptureBufferReady() to reflect this, and connect the viewfinder bufferReady signal to the slot. Update the slot logic to ignore internal buffers that are not part of the request, and to complete the request only when the last buffer completes. Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> --- src/libcamera/pipeline/ipu3/ipu3.cpp | 32 +++++++++++++++++++++++++--- 1 file changed, 29 insertions(+), 3 deletions(-)