| Message ID | 20210131224702.8838-9-laurent.pinchart@ideasonboard.com |
|---|---|
| State | Accepted |
| Delegated to: | Laurent Pinchart |
| Headers | show |
| Series |
|
| Related | show |
On 31/01/2021 22:46, Laurent Pinchart wrote: > The SimpleConverter API signals completion of input and output buffer > pairs. This unnecessarily delays requeueing the input buffer to the > video capture queue until the output buffer completes, and also delays > signalling request completion until the input buffer completes. While > this shouldn't cause large delays in practice, it will also not scale > when multi-stream support will be added to the converter class. > > To address the current issue and prepare for the future, decouple > signalling of input and output buffers completion. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > src/libcamera/pipeline/simple/converter.cpp | 24 +++++-------------- > src/libcamera/pipeline/simple/converter.h | 11 ++++----- > src/libcamera/pipeline/simple/simple.cpp | 26 +++++++++++++-------- > 3 files changed, 26 insertions(+), 35 deletions(-) > > diff --git a/src/libcamera/pipeline/simple/converter.cpp b/src/libcamera/pipeline/simple/converter.cpp > index f782fbc63b09..8324baedc198 100644 > --- a/src/libcamera/pipeline/simple/converter.cpp > +++ b/src/libcamera/pipeline/simple/converter.cpp > @@ -45,8 +45,8 @@ SimpleConverter::SimpleConverter(MediaDevice *media) > return; > } > > - m2m_->output()->bufferReady.connect(this, &SimpleConverter::outputBufferReady); > - m2m_->capture()->bufferReady.connect(this, &SimpleConverter::captureBufferReady); > + m2m_->output()->bufferReady.connect(this, &SimpleConverter::m2mInputBufferReady); > + m2m_->capture()->bufferReady.connect(this, &SimpleConverter::m2mOutputBufferReady); > } > > std::vector<PixelFormat> SimpleConverter::formats(PixelFormat input) > @@ -247,26 +247,14 @@ int SimpleConverter::queueBuffers(FrameBuffer *input, FrameBuffer *output) > return 0; > } > > -void SimpleConverter::captureBufferReady(FrameBuffer *buffer) > +void SimpleConverter::m2mInputBufferReady(FrameBuffer *buffer) > { > - if (!outputDoneQueue_.empty()) { > - FrameBuffer *other = outputDoneQueue_.front(); > - outputDoneQueue_.pop(); > - bufferReady.emit(other, buffer); > - } else { > - captureDoneQueue_.push(buffer); > - } > + inputBufferReady.emit(buffer); > } > > -void SimpleConverter::outputBufferReady(FrameBuffer *buffer) > +void SimpleConverter::m2mOutputBufferReady(FrameBuffer *buffer) > { > - if (!captureDoneQueue_.empty()) { > - FrameBuffer *other = captureDoneQueue_.front(); > - captureDoneQueue_.pop(); > - bufferReady.emit(buffer, other); > - } else { > - outputDoneQueue_.push(buffer); > - } > + outputBufferReady.emit(buffer); > } > > } /* namespace libcamera */ > diff --git a/src/libcamera/pipeline/simple/converter.h b/src/libcamera/pipeline/simple/converter.h > index 780bfa8f7832..739b24df0200 100644 > --- a/src/libcamera/pipeline/simple/converter.h > +++ b/src/libcamera/pipeline/simple/converter.h > @@ -9,7 +9,6 @@ > #define __LIBCAMERA_PIPELINE_SIMPLE_CONVERTER_H__ > > #include <memory> > -#include <queue> > #include <tuple> > #include <vector> > > @@ -47,17 +46,15 @@ public: > > int queueBuffers(FrameBuffer *input, FrameBuffer *output); > > - Signal<FrameBuffer *, FrameBuffer *> bufferReady; > + Signal<FrameBuffer *> inputBufferReady; > + Signal<FrameBuffer *> outputBufferReady; > > private: > - void captureBufferReady(FrameBuffer *buffer); > - void outputBufferReady(FrameBuffer *buffer); > + void m2mInputBufferReady(FrameBuffer *buffer); > + void m2mOutputBufferReady(FrameBuffer *buffer); > > std::unique_ptr<V4L2M2MDevice> m2m_; > > - std::queue<FrameBuffer *> captureDoneQueue_; > - std::queue<FrameBuffer *> outputDoneQueue_; > - > unsigned int inputBufferCount_; > unsigned int outputBufferCount_; > }; > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp > index 20a4ebca94fd..7f9c57234256 100644 > --- a/src/libcamera/pipeline/simple/simple.cpp > +++ b/src/libcamera/pipeline/simple/simple.cpp > @@ -144,7 +144,8 @@ private: > } > > void bufferReady(FrameBuffer *buffer); > - void converterDone(FrameBuffer *input, FrameBuffer *output); > + void converterInputDone(FrameBuffer *buffer); > + void converterOutputDone(FrameBuffer *buffer); > > MediaDevice *media_; > std::map<const MediaEntity *, std::unique_ptr<V4L2VideoDevice>> videos_; > @@ -768,7 +769,8 @@ bool SimplePipelineHandler::match(DeviceEnumerator *enumerator) > << "Failed to create converter, disabling format conversion"; > converter_.reset(); > } else { > - converter_->bufferReady.connect(this, &SimplePipelineHandler::converterDone); > + converter_->inputBufferReady.connect(this, &SimplePipelineHandler::converterInputDone); > + converter_->outputBufferReady.connect(this, &SimplePipelineHandler::converterOutputDone); > } > } > > @@ -925,19 +927,23 @@ void SimplePipelineHandler::bufferReady(FrameBuffer *buffer) > completeRequest(request); > } > > -void SimplePipelineHandler::converterDone(FrameBuffer *input, > - FrameBuffer *output) > +void SimplePipelineHandler::converterInputDone(FrameBuffer *buffer) > { > ASSERT(activeCamera_); > SimpleCameraData *data = cameraData(activeCamera_); > > - /* Complete the request. */ > - Request *request = output->request(); > - completeBuffer(request, output); > - completeRequest(request); > - > /* Queue the input buffer back for capture. */ > - data->video_->queueBuffer(input); > + data->video_->queueBuffer(buffer); > +} > + > +void SimplePipelineHandler::converterOutputDone(FrameBuffer *buffer) > +{ > + ASSERT(activeCamera_); > + > + /* Complete the request. */ > + Request *request = buffer->request(); > + completeBuffer(request, buffer); > + completeRequest(request); > } I assume we still expect to maintain a one-in-one-out principle on the convertor though (no interlacing/de-interacing type buffers though here). Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > REGISTER_PIPELINE_HANDLER(SimplePipelineHandler) >
Hi Kieran, On Fri, Feb 19, 2021 at 05:33:23PM +0000, Kieran Bingham wrote: > On 31/01/2021 22:46, Laurent Pinchart wrote: > > The SimpleConverter API signals completion of input and output buffer > > pairs. This unnecessarily delays requeueing the input buffer to the > > video capture queue until the output buffer completes, and also delays > > signalling request completion until the input buffer completes. While > > this shouldn't cause large delays in practice, it will also not scale > > when multi-stream support will be added to the converter class. > > > > To address the current issue and prepare for the future, decouple > > signalling of input and output buffers completion. > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > --- > > src/libcamera/pipeline/simple/converter.cpp | 24 +++++-------------- > > src/libcamera/pipeline/simple/converter.h | 11 ++++----- > > src/libcamera/pipeline/simple/simple.cpp | 26 +++++++++++++-------- > > 3 files changed, 26 insertions(+), 35 deletions(-) > > > > diff --git a/src/libcamera/pipeline/simple/converter.cpp b/src/libcamera/pipeline/simple/converter.cpp > > index f782fbc63b09..8324baedc198 100644 > > --- a/src/libcamera/pipeline/simple/converter.cpp > > +++ b/src/libcamera/pipeline/simple/converter.cpp > > @@ -45,8 +45,8 @@ SimpleConverter::SimpleConverter(MediaDevice *media) > > return; > > } > > > > - m2m_->output()->bufferReady.connect(this, &SimpleConverter::outputBufferReady); > > - m2m_->capture()->bufferReady.connect(this, &SimpleConverter::captureBufferReady); > > + m2m_->output()->bufferReady.connect(this, &SimpleConverter::m2mInputBufferReady); > > + m2m_->capture()->bufferReady.connect(this, &SimpleConverter::m2mOutputBufferReady); > > } > > > > std::vector<PixelFormat> SimpleConverter::formats(PixelFormat input) > > @@ -247,26 +247,14 @@ int SimpleConverter::queueBuffers(FrameBuffer *input, FrameBuffer *output) > > return 0; > > } > > > > -void SimpleConverter::captureBufferReady(FrameBuffer *buffer) > > +void SimpleConverter::m2mInputBufferReady(FrameBuffer *buffer) > > { > > - if (!outputDoneQueue_.empty()) { > > - FrameBuffer *other = outputDoneQueue_.front(); > > - outputDoneQueue_.pop(); > > - bufferReady.emit(other, buffer); > > - } else { > > - captureDoneQueue_.push(buffer); > > - } > > + inputBufferReady.emit(buffer); > > } > > > > -void SimpleConverter::outputBufferReady(FrameBuffer *buffer) > > +void SimpleConverter::m2mOutputBufferReady(FrameBuffer *buffer) > > { > > - if (!captureDoneQueue_.empty()) { > > - FrameBuffer *other = captureDoneQueue_.front(); > > - captureDoneQueue_.pop(); > > - bufferReady.emit(buffer, other); > > - } else { > > - outputDoneQueue_.push(buffer); > > - } > > + outputBufferReady.emit(buffer); > > } > > > > } /* namespace libcamera */ > > diff --git a/src/libcamera/pipeline/simple/converter.h b/src/libcamera/pipeline/simple/converter.h > > index 780bfa8f7832..739b24df0200 100644 > > --- a/src/libcamera/pipeline/simple/converter.h > > +++ b/src/libcamera/pipeline/simple/converter.h > > @@ -9,7 +9,6 @@ > > #define __LIBCAMERA_PIPELINE_SIMPLE_CONVERTER_H__ > > > > #include <memory> > > -#include <queue> > > #include <tuple> > > #include <vector> > > > > @@ -47,17 +46,15 @@ public: > > > > int queueBuffers(FrameBuffer *input, FrameBuffer *output); > > > > - Signal<FrameBuffer *, FrameBuffer *> bufferReady; > > + Signal<FrameBuffer *> inputBufferReady; > > + Signal<FrameBuffer *> outputBufferReady; > > > > private: > > - void captureBufferReady(FrameBuffer *buffer); > > - void outputBufferReady(FrameBuffer *buffer); > > + void m2mInputBufferReady(FrameBuffer *buffer); > > + void m2mOutputBufferReady(FrameBuffer *buffer); > > > > std::unique_ptr<V4L2M2MDevice> m2m_; > > > > - std::queue<FrameBuffer *> captureDoneQueue_; > > - std::queue<FrameBuffer *> outputDoneQueue_; > > - > > unsigned int inputBufferCount_; > > unsigned int outputBufferCount_; > > }; > > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp > > index 20a4ebca94fd..7f9c57234256 100644 > > --- a/src/libcamera/pipeline/simple/simple.cpp > > +++ b/src/libcamera/pipeline/simple/simple.cpp > > @@ -144,7 +144,8 @@ private: > > } > > > > void bufferReady(FrameBuffer *buffer); > > - void converterDone(FrameBuffer *input, FrameBuffer *output); > > + void converterInputDone(FrameBuffer *buffer); > > + void converterOutputDone(FrameBuffer *buffer); > > > > MediaDevice *media_; > > std::map<const MediaEntity *, std::unique_ptr<V4L2VideoDevice>> videos_; > > @@ -768,7 +769,8 @@ bool SimplePipelineHandler::match(DeviceEnumerator *enumerator) > > << "Failed to create converter, disabling format conversion"; > > converter_.reset(); > > } else { > > - converter_->bufferReady.connect(this, &SimplePipelineHandler::converterDone); > > + converter_->inputBufferReady.connect(this, &SimplePipelineHandler::converterInputDone); > > + converter_->outputBufferReady.connect(this, &SimplePipelineHandler::converterOutputDone); > > } > > } > > > > @@ -925,19 +927,23 @@ void SimplePipelineHandler::bufferReady(FrameBuffer *buffer) > > completeRequest(request); > > } > > > > -void SimplePipelineHandler::converterDone(FrameBuffer *input, > > - FrameBuffer *output) > > +void SimplePipelineHandler::converterInputDone(FrameBuffer *buffer) > > { > > ASSERT(activeCamera_); > > SimpleCameraData *data = cameraData(activeCamera_); > > > > - /* Complete the request. */ > > - Request *request = output->request(); > > - completeBuffer(request, output); > > - completeRequest(request); > > - > > /* Queue the input buffer back for capture. */ > > - data->video_->queueBuffer(input); > > + data->video_->queueBuffer(buffer); > > +} > > + > > +void SimplePipelineHandler::converterOutputDone(FrameBuffer *buffer) > > +{ > > + ASSERT(activeCamera_); > > + > > + /* Complete the request. */ > > + Request *request = buffer->request(); > > + completeBuffer(request, buffer); > > + completeRequest(request); > > } > > I assume we still expect to maintain a one-in-one-out principle on the > convertor though (no interlacing/de-interacing type buffers though here). Correct. Until someone needs this feature of couse :-) > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > > > REGISTER_PIPELINE_HANDLER(SimplePipelineHandler) > >
Hi Laurent, On Mon, Feb 01, 2021 at 12:46:50AM +0200, Laurent Pinchart wrote: > The SimpleConverter API signals completion of input and output buffer > pairs. This unnecessarily delays requeueing the input buffer to the > video capture queue until the output buffer completes, and also delays > signalling request completion until the input buffer completes. While > this shouldn't cause large delays in practice, it will also not scale > when multi-stream support will be added to the converter class. > > To address the current issue and prepare for the future, decouple > signalling of input and output buffers completion. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Reviewed-by: Paul Elder <paul.elder@ideasonboard.com> > --- > src/libcamera/pipeline/simple/converter.cpp | 24 +++++-------------- > src/libcamera/pipeline/simple/converter.h | 11 ++++----- > src/libcamera/pipeline/simple/simple.cpp | 26 +++++++++++++-------- > 3 files changed, 26 insertions(+), 35 deletions(-) > > diff --git a/src/libcamera/pipeline/simple/converter.cpp b/src/libcamera/pipeline/simple/converter.cpp > index f782fbc63b09..8324baedc198 100644 > --- a/src/libcamera/pipeline/simple/converter.cpp > +++ b/src/libcamera/pipeline/simple/converter.cpp > @@ -45,8 +45,8 @@ SimpleConverter::SimpleConverter(MediaDevice *media) > return; > } > > - m2m_->output()->bufferReady.connect(this, &SimpleConverter::outputBufferReady); > - m2m_->capture()->bufferReady.connect(this, &SimpleConverter::captureBufferReady); > + m2m_->output()->bufferReady.connect(this, &SimpleConverter::m2mInputBufferReady); > + m2m_->capture()->bufferReady.connect(this, &SimpleConverter::m2mOutputBufferReady); > } > > std::vector<PixelFormat> SimpleConverter::formats(PixelFormat input) > @@ -247,26 +247,14 @@ int SimpleConverter::queueBuffers(FrameBuffer *input, FrameBuffer *output) > return 0; > } > > -void SimpleConverter::captureBufferReady(FrameBuffer *buffer) > +void SimpleConverter::m2mInputBufferReady(FrameBuffer *buffer) > { > - if (!outputDoneQueue_.empty()) { > - FrameBuffer *other = outputDoneQueue_.front(); > - outputDoneQueue_.pop(); > - bufferReady.emit(other, buffer); > - } else { > - captureDoneQueue_.push(buffer); > - } > + inputBufferReady.emit(buffer); > } > > -void SimpleConverter::outputBufferReady(FrameBuffer *buffer) > +void SimpleConverter::m2mOutputBufferReady(FrameBuffer *buffer) > { > - if (!captureDoneQueue_.empty()) { > - FrameBuffer *other = captureDoneQueue_.front(); > - captureDoneQueue_.pop(); > - bufferReady.emit(buffer, other); > - } else { > - outputDoneQueue_.push(buffer); > - } > + outputBufferReady.emit(buffer); > } > > } /* namespace libcamera */ > diff --git a/src/libcamera/pipeline/simple/converter.h b/src/libcamera/pipeline/simple/converter.h > index 780bfa8f7832..739b24df0200 100644 > --- a/src/libcamera/pipeline/simple/converter.h > +++ b/src/libcamera/pipeline/simple/converter.h > @@ -9,7 +9,6 @@ > #define __LIBCAMERA_PIPELINE_SIMPLE_CONVERTER_H__ > > #include <memory> > -#include <queue> > #include <tuple> > #include <vector> > > @@ -47,17 +46,15 @@ public: > > int queueBuffers(FrameBuffer *input, FrameBuffer *output); > > - Signal<FrameBuffer *, FrameBuffer *> bufferReady; > + Signal<FrameBuffer *> inputBufferReady; > + Signal<FrameBuffer *> outputBufferReady; > > private: > - void captureBufferReady(FrameBuffer *buffer); > - void outputBufferReady(FrameBuffer *buffer); > + void m2mInputBufferReady(FrameBuffer *buffer); > + void m2mOutputBufferReady(FrameBuffer *buffer); > > std::unique_ptr<V4L2M2MDevice> m2m_; > > - std::queue<FrameBuffer *> captureDoneQueue_; > - std::queue<FrameBuffer *> outputDoneQueue_; > - > unsigned int inputBufferCount_; > unsigned int outputBufferCount_; > }; > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp > index 20a4ebca94fd..7f9c57234256 100644 > --- a/src/libcamera/pipeline/simple/simple.cpp > +++ b/src/libcamera/pipeline/simple/simple.cpp > @@ -144,7 +144,8 @@ private: > } > > void bufferReady(FrameBuffer *buffer); > - void converterDone(FrameBuffer *input, FrameBuffer *output); > + void converterInputDone(FrameBuffer *buffer); > + void converterOutputDone(FrameBuffer *buffer); > > MediaDevice *media_; > std::map<const MediaEntity *, std::unique_ptr<V4L2VideoDevice>> videos_; > @@ -768,7 +769,8 @@ bool SimplePipelineHandler::match(DeviceEnumerator *enumerator) > << "Failed to create converter, disabling format conversion"; > converter_.reset(); > } else { > - converter_->bufferReady.connect(this, &SimplePipelineHandler::converterDone); > + converter_->inputBufferReady.connect(this, &SimplePipelineHandler::converterInputDone); > + converter_->outputBufferReady.connect(this, &SimplePipelineHandler::converterOutputDone); > } > } > > @@ -925,19 +927,23 @@ void SimplePipelineHandler::bufferReady(FrameBuffer *buffer) > completeRequest(request); > } > > -void SimplePipelineHandler::converterDone(FrameBuffer *input, > - FrameBuffer *output) > +void SimplePipelineHandler::converterInputDone(FrameBuffer *buffer) > { > ASSERT(activeCamera_); > SimpleCameraData *data = cameraData(activeCamera_); > > - /* Complete the request. */ > - Request *request = output->request(); > - completeBuffer(request, output); > - completeRequest(request); > - > /* Queue the input buffer back for capture. */ > - data->video_->queueBuffer(input); > + data->video_->queueBuffer(buffer); > +} > + > +void SimplePipelineHandler::converterOutputDone(FrameBuffer *buffer) > +{ > + ASSERT(activeCamera_); > + > + /* Complete the request. */ > + Request *request = buffer->request(); > + completeBuffer(request, buffer); > + completeRequest(request); > } > > REGISTER_PIPELINE_HANDLER(SimplePipelineHandler) > -- > Regards, > > Laurent Pinchart > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
diff --git a/src/libcamera/pipeline/simple/converter.cpp b/src/libcamera/pipeline/simple/converter.cpp index f782fbc63b09..8324baedc198 100644 --- a/src/libcamera/pipeline/simple/converter.cpp +++ b/src/libcamera/pipeline/simple/converter.cpp @@ -45,8 +45,8 @@ SimpleConverter::SimpleConverter(MediaDevice *media) return; } - m2m_->output()->bufferReady.connect(this, &SimpleConverter::outputBufferReady); - m2m_->capture()->bufferReady.connect(this, &SimpleConverter::captureBufferReady); + m2m_->output()->bufferReady.connect(this, &SimpleConverter::m2mInputBufferReady); + m2m_->capture()->bufferReady.connect(this, &SimpleConverter::m2mOutputBufferReady); } std::vector<PixelFormat> SimpleConverter::formats(PixelFormat input) @@ -247,26 +247,14 @@ int SimpleConverter::queueBuffers(FrameBuffer *input, FrameBuffer *output) return 0; } -void SimpleConverter::captureBufferReady(FrameBuffer *buffer) +void SimpleConverter::m2mInputBufferReady(FrameBuffer *buffer) { - if (!outputDoneQueue_.empty()) { - FrameBuffer *other = outputDoneQueue_.front(); - outputDoneQueue_.pop(); - bufferReady.emit(other, buffer); - } else { - captureDoneQueue_.push(buffer); - } + inputBufferReady.emit(buffer); } -void SimpleConverter::outputBufferReady(FrameBuffer *buffer) +void SimpleConverter::m2mOutputBufferReady(FrameBuffer *buffer) { - if (!captureDoneQueue_.empty()) { - FrameBuffer *other = captureDoneQueue_.front(); - captureDoneQueue_.pop(); - bufferReady.emit(buffer, other); - } else { - outputDoneQueue_.push(buffer); - } + outputBufferReady.emit(buffer); } } /* namespace libcamera */ diff --git a/src/libcamera/pipeline/simple/converter.h b/src/libcamera/pipeline/simple/converter.h index 780bfa8f7832..739b24df0200 100644 --- a/src/libcamera/pipeline/simple/converter.h +++ b/src/libcamera/pipeline/simple/converter.h @@ -9,7 +9,6 @@ #define __LIBCAMERA_PIPELINE_SIMPLE_CONVERTER_H__ #include <memory> -#include <queue> #include <tuple> #include <vector> @@ -47,17 +46,15 @@ public: int queueBuffers(FrameBuffer *input, FrameBuffer *output); - Signal<FrameBuffer *, FrameBuffer *> bufferReady; + Signal<FrameBuffer *> inputBufferReady; + Signal<FrameBuffer *> outputBufferReady; private: - void captureBufferReady(FrameBuffer *buffer); - void outputBufferReady(FrameBuffer *buffer); + void m2mInputBufferReady(FrameBuffer *buffer); + void m2mOutputBufferReady(FrameBuffer *buffer); std::unique_ptr<V4L2M2MDevice> m2m_; - std::queue<FrameBuffer *> captureDoneQueue_; - std::queue<FrameBuffer *> outputDoneQueue_; - unsigned int inputBufferCount_; unsigned int outputBufferCount_; }; diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp index 20a4ebca94fd..7f9c57234256 100644 --- a/src/libcamera/pipeline/simple/simple.cpp +++ b/src/libcamera/pipeline/simple/simple.cpp @@ -144,7 +144,8 @@ private: } void bufferReady(FrameBuffer *buffer); - void converterDone(FrameBuffer *input, FrameBuffer *output); + void converterInputDone(FrameBuffer *buffer); + void converterOutputDone(FrameBuffer *buffer); MediaDevice *media_; std::map<const MediaEntity *, std::unique_ptr<V4L2VideoDevice>> videos_; @@ -768,7 +769,8 @@ bool SimplePipelineHandler::match(DeviceEnumerator *enumerator) << "Failed to create converter, disabling format conversion"; converter_.reset(); } else { - converter_->bufferReady.connect(this, &SimplePipelineHandler::converterDone); + converter_->inputBufferReady.connect(this, &SimplePipelineHandler::converterInputDone); + converter_->outputBufferReady.connect(this, &SimplePipelineHandler::converterOutputDone); } } @@ -925,19 +927,23 @@ void SimplePipelineHandler::bufferReady(FrameBuffer *buffer) completeRequest(request); } -void SimplePipelineHandler::converterDone(FrameBuffer *input, - FrameBuffer *output) +void SimplePipelineHandler::converterInputDone(FrameBuffer *buffer) { ASSERT(activeCamera_); SimpleCameraData *data = cameraData(activeCamera_); - /* Complete the request. */ - Request *request = output->request(); - completeBuffer(request, output); - completeRequest(request); - /* Queue the input buffer back for capture. */ - data->video_->queueBuffer(input); + data->video_->queueBuffer(buffer); +} + +void SimplePipelineHandler::converterOutputDone(FrameBuffer *buffer) +{ + ASSERT(activeCamera_); + + /* Complete the request. */ + Request *request = buffer->request(); + completeBuffer(request, buffer); + completeRequest(request); } REGISTER_PIPELINE_HANDLER(SimplePipelineHandler)
The SimpleConverter API signals completion of input and output buffer pairs. This unnecessarily delays requeueing the input buffer to the video capture queue until the output buffer completes, and also delays signalling request completion until the input buffer completes. While this shouldn't cause large delays in practice, it will also not scale when multi-stream support will be added to the converter class. To address the current issue and prepare for the future, decouple signalling of input and output buffers completion. Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- src/libcamera/pipeline/simple/converter.cpp | 24 +++++-------------- src/libcamera/pipeline/simple/converter.h | 11 ++++----- src/libcamera/pipeline/simple/simple.cpp | 26 +++++++++++++-------- 3 files changed, 26 insertions(+), 35 deletions(-)