| Message ID | 20191126233620.1695316-2-niklas.soderlund@ragnatech.se |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Niklas, On Wed, Nov 27, 2019 at 12:35:51AM +0100, Niklas Söderlund wrote: > Expecting pipeline handler implementations of queueRequest() to call > the base class queueRequest() at the correct point have lead to > different behaviors between the pipelines. > > Fix this by splitting queueRequest() into a base class implementation > which handles the bookkeeping and a new queueRequestHardware() that is > to be implemented by pipeline handlers and only deals with committing the > request to hardware. > Just one minor nit.. > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > --- > src/libcamera/include/pipeline_handler.h | 4 ++- > src/libcamera/pipeline/ipu3/ipu3.cpp | 6 ++-- > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 7 ++--- > src/libcamera/pipeline/uvcvideo.cpp | 6 ++-- > src/libcamera/pipeline/vimc.cpp | 6 ++-- > src/libcamera/pipeline_handler.cpp | 35 +++++++++++++++++------- > 6 files changed, 37 insertions(+), 27 deletions(-) > > diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h > index a02e6e77dc9ce0e7..6ce8ea1c8d31b870 100644 > --- a/src/libcamera/include/pipeline_handler.h > +++ b/src/libcamera/include/pipeline_handler.h > @@ -77,7 +77,7 @@ public: > virtual int start(Camera *camera) = 0; > virtual void stop(Camera *camera) = 0; > > - virtual int queueRequest(Camera *camera, Request *request); > + int queueRequest(Camera *camera, Request *request); > > bool completeBuffer(Camera *camera, Request *request, Buffer *buffer); > void completeRequest(Camera *camera, Request *request); > @@ -89,6 +89,8 @@ protected: > std::unique_ptr<CameraData> data); > void hotplugMediaDevice(MediaDevice *media); > > + virtual int queueRequestHardware(Camera *camera, Request *request) = 0; > + > CameraData *cameraData(const Camera *camera); > > CameraManager *manager_; > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp > index 1c5fccf694289ae9..ad223d9101bdc6ed 100644 > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp > @@ -220,7 +220,7 @@ public: > int start(Camera *camera) override; > void stop(Camera *camera) override; > > - int queueRequest(Camera *camera, Request *request) override; > + int queueRequestHardware(Camera *camera, Request *request) override; > > bool match(DeviceEnumerator *enumerator) override; > > @@ -756,7 +756,7 @@ void PipelineHandlerIPU3::stop(Camera *camera) > data->rawBuffers_.clear(); > } > > -int PipelineHandlerIPU3::queueRequest(Camera *camera, Request *request) > +int PipelineHandlerIPU3::queueRequestHardware(Camera *camera, Request *request) > { > int error = 0; > > @@ -769,8 +769,6 @@ int PipelineHandlerIPU3::queueRequest(Camera *camera, Request *request) > error = ret; > } > > - PipelineHandler::queueRequest(camera, request); > - > return error; > } > > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > index 4a583a7a1d7ef1fd..f75b25c6787e40df 100644 > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > @@ -184,7 +184,7 @@ public: > int start(Camera *camera) override; > void stop(Camera *camera) override; > > - int queueRequest(Camera *camera, Request *request) override; > + int queueRequestHardware(Camera *camera, Request *request) override; > > bool match(DeviceEnumerator *enumerator) override; > > @@ -810,13 +810,12 @@ void PipelineHandlerRkISP1::stop(Camera *camera) > activeCamera_ = nullptr; > } > > -int PipelineHandlerRkISP1::queueRequest(Camera *camera, Request *request) > +int PipelineHandlerRkISP1::queueRequestHardware(Camera *camera, > + Request *request) > { > RkISP1CameraData *data = cameraData(camera); > Stream *stream = &data->stream_; > > - PipelineHandler::queueRequest(camera, request); > - > RkISP1FrameInfo *info = data->frameInfo_.create(data->frame_, request, > stream); > if (!info) > diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp > index 522229241ffb9e8a..3a76653ff6dc2b5e 100644 > --- a/src/libcamera/pipeline/uvcvideo.cpp > +++ b/src/libcamera/pipeline/uvcvideo.cpp > @@ -72,7 +72,7 @@ public: > int start(Camera *camera) override; > void stop(Camera *camera) override; > > - int queueRequest(Camera *camera, Request *request) override; > + int queueRequestHardware(Camera *camera, Request *request) override; > > bool match(DeviceEnumerator *enumerator) override; > > @@ -262,7 +262,7 @@ int PipelineHandlerUVC::processControls(UVCCameraData *data, Request *request) > return ret; > } > > -int PipelineHandlerUVC::queueRequest(Camera *camera, Request *request) > +int PipelineHandlerUVC::queueRequestHardware(Camera *camera, Request *request) > { > UVCCameraData *data = cameraData(camera); > Buffer *buffer = request->findBuffer(&data->stream_); > @@ -281,8 +281,6 @@ int PipelineHandlerUVC::queueRequest(Camera *camera, Request *request) > if (ret < 0) > return ret; > > - PipelineHandler::queueRequest(camera, request); > - > return 0; > } > > diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp > index 06664fed42e796e4..f5550a1723668106 100644 > --- a/src/libcamera/pipeline/vimc.cpp > +++ b/src/libcamera/pipeline/vimc.cpp > @@ -90,7 +90,7 @@ public: > int start(Camera *camera) override; > void stop(Camera *camera) override; > > - int queueRequest(Camera *camera, Request *request) override; > + int queueRequestHardware(Camera *camera, Request *request) override; > > bool match(DeviceEnumerator *enumerator) override; > > @@ -323,7 +323,7 @@ int PipelineHandlerVimc::processControls(VimcCameraData *data, Request *request) > return ret; > } > > -int PipelineHandlerVimc::queueRequest(Camera *camera, Request *request) > +int PipelineHandlerVimc::queueRequestHardware(Camera *camera, Request *request) > { > VimcCameraData *data = cameraData(camera); > Buffer *buffer = request->findBuffer(&data->stream_); > @@ -342,8 +342,6 @@ int PipelineHandlerVimc::queueRequest(Camera *camera, Request *request) > if (ret < 0) > return ret; > > - PipelineHandler::queueRequest(camera, request); > - > return 0; > } > > diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp > index 4349ca8957e403fe..c9e348b98da7b736 100644 > --- a/src/libcamera/pipeline_handler.cpp > +++ b/src/libcamera/pipeline_handler.cpp > @@ -345,27 +345,42 @@ const ControlInfoMap &PipelineHandler::controls(Camera *camera) > * \param[in] request The request to queue > * > * This method queues a capture request to the pipeline handler for processing. > - * The request contains a set of buffers associated with streams and a set of > - * parameters. The pipeline handler shall program the device to ensure that the > - * parameters will be applied to the frames captured in the buffers provided in > - * the request. > - * > - * Pipeline handlers shall override this method. The base implementation in the > - * PipelineHandler class keeps track of queued requests in order to ensure > - * completion of all requests when the pipeline handler is stopped with stop(). > - * Requests completion shall be signalled by the pipeline handler using the > + * The method keeps track of queued requests in order to ensure completion of You are repeating "This method" and "The method" in two consecutive lines. I would just s/The method/It. > + * all requests when the pipeline handler is stopped with stop(). Requests > + * completion shall be signalled by the pipeline handler using the > * completeRequest() method. > * > * \return 0 on success or a negative error code otherwise > */ > int PipelineHandler::queueRequest(Camera *camera, Request *request) > { > + int ret; > + > CameraData *data = cameraData(camera); > data->queuedRequests_.push_back(request); > > - return 0; > + ret = queueRequestHardware(camera, request); You could declare ret here. With these two very minor issues addressed or not: Reviewed-by: Jacopo Mondi <jacopo@jmondi.org> Thanks j > + if (ret) > + data->queuedRequests_.remove(request); > + > + return ret; > } > > +/** > + * \fn PipelineHandler::queueRequestHardware() > + * \brief Queue a request to the hardware > + * \param[in] camera The camera to queue the request to > + * \param[in] request The request to queue > + * > + * This method queues a capture request to the hardware for processing. The > + * request contains a set of buffers associated with streams and a set of > + * parameters. The pipeline handler shall program the device to ensure that the > + * parameters will be applied to the frames captured in the buffers provided in > + * the request. > + * > + * \return 0 on success or a negative error code otherwise > + */ > + > /** > * \brief Complete a buffer for a request > * \param[in] camera The camera the request belongs to > -- > 2.24.0 > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
Hi Niklas, Thank you for the patch. On Wed, Nov 27, 2019 at 12:35:51AM +0100, Niklas Söderlund wrote: > Expecting pipeline handler implementations of queueRequest() to call > the base class queueRequest() at the correct point have lead to s/lead/led/ > different behaviors between the pipelines. > > Fix this by splitting queueRequest() into a base class implementation > which handles the bookkeeping and a new queueRequestHardware() that is > to be implemented by pipeline handlers and only deals with committing the > request to hardware. > > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > --- > src/libcamera/include/pipeline_handler.h | 4 ++- > src/libcamera/pipeline/ipu3/ipu3.cpp | 6 ++-- > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 7 ++--- > src/libcamera/pipeline/uvcvideo.cpp | 6 ++-- > src/libcamera/pipeline/vimc.cpp | 6 ++-- > src/libcamera/pipeline_handler.cpp | 35 +++++++++++++++++------- > 6 files changed, 37 insertions(+), 27 deletions(-) > > diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h > index a02e6e77dc9ce0e7..6ce8ea1c8d31b870 100644 > --- a/src/libcamera/include/pipeline_handler.h > +++ b/src/libcamera/include/pipeline_handler.h > @@ -77,7 +77,7 @@ public: > virtual int start(Camera *camera) = 0; > virtual void stop(Camera *camera) = 0; > > - virtual int queueRequest(Camera *camera, Request *request); > + int queueRequest(Camera *camera, Request *request); I would make this method final. > > bool completeBuffer(Camera *camera, Request *request, Buffer *buffer); > void completeRequest(Camera *camera, Request *request); > @@ -89,6 +89,8 @@ protected: > std::unique_ptr<CameraData> data); > void hotplugMediaDevice(MediaDevice *media); > > + virtual int queueRequestHardware(Camera *camera, Request *request) = 0; > + > CameraData *cameraData(const Camera *camera); > > CameraManager *manager_; > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp > index 1c5fccf694289ae9..ad223d9101bdc6ed 100644 > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp > @@ -220,7 +220,7 @@ public: > int start(Camera *camera) override; > void stop(Camera *camera) override; > > - int queueRequest(Camera *camera, Request *request) override; > + int queueRequestHardware(Camera *camera, Request *request) override; > > bool match(DeviceEnumerator *enumerator) override; > > @@ -756,7 +756,7 @@ void PipelineHandlerIPU3::stop(Camera *camera) > data->rawBuffers_.clear(); > } > > -int PipelineHandlerIPU3::queueRequest(Camera *camera, Request *request) > +int PipelineHandlerIPU3::queueRequestHardware(Camera *camera, Request *request) > { > int error = 0; > > @@ -769,8 +769,6 @@ int PipelineHandlerIPU3::queueRequest(Camera *camera, Request *request) > error = ret; > } > > - PipelineHandler::queueRequest(camera, request); > - > return error; > } > > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > index 4a583a7a1d7ef1fd..f75b25c6787e40df 100644 > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > @@ -184,7 +184,7 @@ public: > int start(Camera *camera) override; > void stop(Camera *camera) override; > > - int queueRequest(Camera *camera, Request *request) override; > + int queueRequestHardware(Camera *camera, Request *request) override; > > bool match(DeviceEnumerator *enumerator) override; > > @@ -810,13 +810,12 @@ void PipelineHandlerRkISP1::stop(Camera *camera) > activeCamera_ = nullptr; > } > > -int PipelineHandlerRkISP1::queueRequest(Camera *camera, Request *request) > +int PipelineHandlerRkISP1::queueRequestHardware(Camera *camera, > + Request *request) > { > RkISP1CameraData *data = cameraData(camera); > Stream *stream = &data->stream_; > > - PipelineHandler::queueRequest(camera, request); > - > RkISP1FrameInfo *info = data->frameInfo_.create(data->frame_, request, > stream); > if (!info) > diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp > index 522229241ffb9e8a..3a76653ff6dc2b5e 100644 > --- a/src/libcamera/pipeline/uvcvideo.cpp > +++ b/src/libcamera/pipeline/uvcvideo.cpp > @@ -72,7 +72,7 @@ public: > int start(Camera *camera) override; > void stop(Camera *camera) override; > > - int queueRequest(Camera *camera, Request *request) override; > + int queueRequestHardware(Camera *camera, Request *request) override; > > bool match(DeviceEnumerator *enumerator) override; > > @@ -262,7 +262,7 @@ int PipelineHandlerUVC::processControls(UVCCameraData *data, Request *request) > return ret; > } > > -int PipelineHandlerUVC::queueRequest(Camera *camera, Request *request) > +int PipelineHandlerUVC::queueRequestHardware(Camera *camera, Request *request) > { > UVCCameraData *data = cameraData(camera); > Buffer *buffer = request->findBuffer(&data->stream_); > @@ -281,8 +281,6 @@ int PipelineHandlerUVC::queueRequest(Camera *camera, Request *request) > if (ret < 0) > return ret; > > - PipelineHandler::queueRequest(camera, request); > - > return 0; > } > > diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp > index 06664fed42e796e4..f5550a1723668106 100644 > --- a/src/libcamera/pipeline/vimc.cpp > +++ b/src/libcamera/pipeline/vimc.cpp > @@ -90,7 +90,7 @@ public: > int start(Camera *camera) override; > void stop(Camera *camera) override; > > - int queueRequest(Camera *camera, Request *request) override; > + int queueRequestHardware(Camera *camera, Request *request) override; > > bool match(DeviceEnumerator *enumerator) override; > > @@ -323,7 +323,7 @@ int PipelineHandlerVimc::processControls(VimcCameraData *data, Request *request) > return ret; > } > > -int PipelineHandlerVimc::queueRequest(Camera *camera, Request *request) > +int PipelineHandlerVimc::queueRequestHardware(Camera *camera, Request *request) > { > VimcCameraData *data = cameraData(camera); > Buffer *buffer = request->findBuffer(&data->stream_); > @@ -342,8 +342,6 @@ int PipelineHandlerVimc::queueRequest(Camera *camera, Request *request) > if (ret < 0) > return ret; > > - PipelineHandler::queueRequest(camera, request); > - > return 0; > } > > diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp > index 4349ca8957e403fe..c9e348b98da7b736 100644 > --- a/src/libcamera/pipeline_handler.cpp > +++ b/src/libcamera/pipeline_handler.cpp > @@ -345,27 +345,42 @@ const ControlInfoMap &PipelineHandler::controls(Camera *camera) > * \param[in] request The request to queue > * > * This method queues a capture request to the pipeline handler for processing. > - * The request contains a set of buffers associated with streams and a set of > - * parameters. The pipeline handler shall program the device to ensure that the > - * parameters will be applied to the frames captured in the buffers provided in > - * the request. > - * > - * Pipeline handlers shall override this method. The base implementation in the > - * PipelineHandler class keeps track of queued requests in order to ensure > - * completion of all requests when the pipeline handler is stopped with stop(). > - * Requests completion shall be signalled by the pipeline handler using the > + * The method keeps track of queued requests in order to ensure completion of > + * all requests when the pipeline handler is stopped with stop(). Requests > + * completion shall be signalled by the pipeline handler using the > * completeRequest() method. I would mention queueRequestHardware() here: * This method queues a capture request to the pipeline handler for processing. * The request is first added to the internal list of queued requests, and * then passed to the pipeline handler with a call to queueRequestHardware(). * * Keeping track of queued requests ensures automatic completion of all requests * when the pipeline handler is stopped with stop(). Request completion shall be * signalled by the pipeline handler using the completeRequest() method. It took me a while to write this paragraph, which is I think a sign that the API isn't very clean, in particular that the queueRequest() and queueRequestHardware() names are confusing (the latter especially because it may queue the request to the pipeline handler first, not immediately to the hardware). I think I would prefer queueRequestDevice() to queueRequestHardware() (with s/hardware/device/ where applicable), but even that isn't perfect. I don't have better names to propose at this moment, so Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > * > * \return 0 on success or a negative error code otherwise > */ > int PipelineHandler::queueRequest(Camera *camera, Request *request) > { > + int ret; > + > CameraData *data = cameraData(camera); > data->queuedRequests_.push_back(request); > > - return 0; > + ret = queueRequestHardware(camera, request); > + if (ret) > + data->queuedRequests_.remove(request); > + > + return ret; > } > > +/** > + * \fn PipelineHandler::queueRequestHardware() > + * \brief Queue a request to the hardware > + * \param[in] camera The camera to queue the request to > + * \param[in] request The request to queue > + * > + * This method queues a capture request to the hardware for processing. The > + * request contains a set of buffers associated with streams and a set of > + * parameters. The pipeline handler shall program the device to ensure that the > + * parameters will be applied to the frames captured in the buffers provided in > + * the request. > + * > + * \return 0 on success or a negative error code otherwise > + */ > + > /** > * \brief Complete a buffer for a request > * \param[in] camera The camera the request belongs to
Hi Laurent, Thanks for your feedback. On 2019-12-09 15:04:06 +0200, Laurent Pinchart wrote: > Hi Niklas, > > Thank you for the patch. > > On Wed, Nov 27, 2019 at 12:35:51AM +0100, Niklas Söderlund wrote: > > Expecting pipeline handler implementations of queueRequest() to call > > the base class queueRequest() at the correct point have lead to > > s/lead/led/ > > > different behaviors between the pipelines. > > > > Fix this by splitting queueRequest() into a base class implementation > > which handles the bookkeeping and a new queueRequestHardware() that is > > to be implemented by pipeline handlers and only deals with committing the > > request to hardware. > > > > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > > --- > > src/libcamera/include/pipeline_handler.h | 4 ++- > > src/libcamera/pipeline/ipu3/ipu3.cpp | 6 ++-- > > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 7 ++--- > > src/libcamera/pipeline/uvcvideo.cpp | 6 ++-- > > src/libcamera/pipeline/vimc.cpp | 6 ++-- > > src/libcamera/pipeline_handler.cpp | 35 +++++++++++++++++------- > > 6 files changed, 37 insertions(+), 27 deletions(-) > > > > diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h > > index a02e6e77dc9ce0e7..6ce8ea1c8d31b870 100644 > > --- a/src/libcamera/include/pipeline_handler.h > > +++ b/src/libcamera/include/pipeline_handler.h > > @@ -77,7 +77,7 @@ public: > > virtual int start(Camera *camera) = 0; > > virtual void stop(Camera *camera) = 0; > > > > - virtual int queueRequest(Camera *camera, Request *request); > > + int queueRequest(Camera *camera, Request *request); > > I would make this method final. To mark this method as final I need to first make it virtual. virtual int queueRequest(Camera *camera, Request *request) final; Looks a but suspicious to me and we have a lot of other methods which should not be overloaded by pipeline implementations. I have keep this as is for now. I'm holding of adding your tag tho until you had a chance to provide your feedback to this. > > > > > bool completeBuffer(Camera *camera, Request *request, Buffer *buffer); > > void completeRequest(Camera *camera, Request *request); > > @@ -89,6 +89,8 @@ protected: > > std::unique_ptr<CameraData> data); > > void hotplugMediaDevice(MediaDevice *media); > > > > + virtual int queueRequestHardware(Camera *camera, Request *request) = 0; > > + > > CameraData *cameraData(const Camera *camera); > > > > CameraManager *manager_; > > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp > > index 1c5fccf694289ae9..ad223d9101bdc6ed 100644 > > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp > > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp > > @@ -220,7 +220,7 @@ public: > > int start(Camera *camera) override; > > void stop(Camera *camera) override; > > > > - int queueRequest(Camera *camera, Request *request) override; > > + int queueRequestHardware(Camera *camera, Request *request) override; > > > > bool match(DeviceEnumerator *enumerator) override; > > > > @@ -756,7 +756,7 @@ void PipelineHandlerIPU3::stop(Camera *camera) > > data->rawBuffers_.clear(); > > } > > > > -int PipelineHandlerIPU3::queueRequest(Camera *camera, Request *request) > > +int PipelineHandlerIPU3::queueRequestHardware(Camera *camera, Request *request) > > { > > int error = 0; > > > > @@ -769,8 +769,6 @@ int PipelineHandlerIPU3::queueRequest(Camera *camera, Request *request) > > error = ret; > > } > > > > - PipelineHandler::queueRequest(camera, request); > > - > > return error; > > } > > > > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > > index 4a583a7a1d7ef1fd..f75b25c6787e40df 100644 > > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp > > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > > @@ -184,7 +184,7 @@ public: > > int start(Camera *camera) override; > > void stop(Camera *camera) override; > > > > - int queueRequest(Camera *camera, Request *request) override; > > + int queueRequestHardware(Camera *camera, Request *request) override; > > > > bool match(DeviceEnumerator *enumerator) override; > > > > @@ -810,13 +810,12 @@ void PipelineHandlerRkISP1::stop(Camera *camera) > > activeCamera_ = nullptr; > > } > > > > -int PipelineHandlerRkISP1::queueRequest(Camera *camera, Request *request) > > +int PipelineHandlerRkISP1::queueRequestHardware(Camera *camera, > > + Request *request) > > { > > RkISP1CameraData *data = cameraData(camera); > > Stream *stream = &data->stream_; > > > > - PipelineHandler::queueRequest(camera, request); > > - > > RkISP1FrameInfo *info = data->frameInfo_.create(data->frame_, request, > > stream); > > if (!info) > > diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp > > index 522229241ffb9e8a..3a76653ff6dc2b5e 100644 > > --- a/src/libcamera/pipeline/uvcvideo.cpp > > +++ b/src/libcamera/pipeline/uvcvideo.cpp > > @@ -72,7 +72,7 @@ public: > > int start(Camera *camera) override; > > void stop(Camera *camera) override; > > > > - int queueRequest(Camera *camera, Request *request) override; > > + int queueRequestHardware(Camera *camera, Request *request) override; > > > > bool match(DeviceEnumerator *enumerator) override; > > > > @@ -262,7 +262,7 @@ int PipelineHandlerUVC::processControls(UVCCameraData *data, Request *request) > > return ret; > > } > > > > -int PipelineHandlerUVC::queueRequest(Camera *camera, Request *request) > > +int PipelineHandlerUVC::queueRequestHardware(Camera *camera, Request *request) > > { > > UVCCameraData *data = cameraData(camera); > > Buffer *buffer = request->findBuffer(&data->stream_); > > @@ -281,8 +281,6 @@ int PipelineHandlerUVC::queueRequest(Camera *camera, Request *request) > > if (ret < 0) > > return ret; > > > > - PipelineHandler::queueRequest(camera, request); > > - > > return 0; > > } > > > > diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp > > index 06664fed42e796e4..f5550a1723668106 100644 > > --- a/src/libcamera/pipeline/vimc.cpp > > +++ b/src/libcamera/pipeline/vimc.cpp > > @@ -90,7 +90,7 @@ public: > > int start(Camera *camera) override; > > void stop(Camera *camera) override; > > > > - int queueRequest(Camera *camera, Request *request) override; > > + int queueRequestHardware(Camera *camera, Request *request) override; > > > > bool match(DeviceEnumerator *enumerator) override; > > > > @@ -323,7 +323,7 @@ int PipelineHandlerVimc::processControls(VimcCameraData *data, Request *request) > > return ret; > > } > > > > -int PipelineHandlerVimc::queueRequest(Camera *camera, Request *request) > > +int PipelineHandlerVimc::queueRequestHardware(Camera *camera, Request *request) > > { > > VimcCameraData *data = cameraData(camera); > > Buffer *buffer = request->findBuffer(&data->stream_); > > @@ -342,8 +342,6 @@ int PipelineHandlerVimc::queueRequest(Camera *camera, Request *request) > > if (ret < 0) > > return ret; > > > > - PipelineHandler::queueRequest(camera, request); > > - > > return 0; > > } > > > > diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp > > index 4349ca8957e403fe..c9e348b98da7b736 100644 > > --- a/src/libcamera/pipeline_handler.cpp > > +++ b/src/libcamera/pipeline_handler.cpp > > @@ -345,27 +345,42 @@ const ControlInfoMap &PipelineHandler::controls(Camera *camera) > > * \param[in] request The request to queue > > * > > * This method queues a capture request to the pipeline handler for processing. > > - * The request contains a set of buffers associated with streams and a set of > > - * parameters. The pipeline handler shall program the device to ensure that the > > - * parameters will be applied to the frames captured in the buffers provided in > > - * the request. > > - * > > - * Pipeline handlers shall override this method. The base implementation in the > > - * PipelineHandler class keeps track of queued requests in order to ensure > > - * completion of all requests when the pipeline handler is stopped with stop(). > > - * Requests completion shall be signalled by the pipeline handler using the > > + * The method keeps track of queued requests in order to ensure completion of > > + * all requests when the pipeline handler is stopped with stop(). Requests > > + * completion shall be signalled by the pipeline handler using the > > * completeRequest() method. > > I would mention queueRequestHardware() here: > > * This method queues a capture request to the pipeline handler for processing. > * The request is first added to the internal list of queued requests, and > * then passed to the pipeline handler with a call to queueRequestHardware(). > * > * Keeping track of queued requests ensures automatic completion of all requests > * when the pipeline handler is stopped with stop(). Request completion shall be > * signalled by the pipeline handler using the completeRequest() method. > > It took me a while to write this paragraph, which is I think a sign that > the API isn't very clean, in particular that the queueRequest() and > queueRequestHardware() names are confusing (the latter especially > because it may queue the request to the pipeline handler first, not > immediately to the hardware). I think I would prefer > queueRequestDevice() to queueRequestHardware() (with s/hardware/device/ > where applicable), but even that isn't perfect. I don't have better > names to propose at this moment, so Tanks! I went with queueRequestDevice() as I too like it more. > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > * > > * \return 0 on success or a negative error code otherwise > > */ > > int PipelineHandler::queueRequest(Camera *camera, Request *request) > > { > > + int ret; > > + > > CameraData *data = cameraData(camera); > > data->queuedRequests_.push_back(request); > > > > - return 0; > > + ret = queueRequestHardware(camera, request); > > + if (ret) > > + data->queuedRequests_.remove(request); > > + > > + return ret; > > } > > > > +/** > > + * \fn PipelineHandler::queueRequestHardware() > > + * \brief Queue a request to the hardware > > + * \param[in] camera The camera to queue the request to > > + * \param[in] request The request to queue > > + * > > + * This method queues a capture request to the hardware for processing. The > > + * request contains a set of buffers associated with streams and a set of > > + * parameters. The pipeline handler shall program the device to ensure that the > > + * parameters will be applied to the frames captured in the buffers provided in > > + * the request. > > + * > > + * \return 0 on success or a negative error code otherwise > > + */ > > + > > /** > > * \brief Complete a buffer for a request > > * \param[in] camera The camera the request belongs to > > -- > Regards, > > Laurent Pinchart
Hi Niklas, On Tue, Dec 10, 2019 at 08:31:24PM +0100, Niklas Söderlund wrote: > On 2019-12-09 15:04:06 +0200, Laurent Pinchart wrote: > > On Wed, Nov 27, 2019 at 12:35:51AM +0100, Niklas Söderlund wrote: > > > Expecting pipeline handler implementations of queueRequest() to call > > > the base class queueRequest() at the correct point have lead to > > > > s/lead/led/ > > > > > different behaviors between the pipelines. > > > > > > Fix this by splitting queueRequest() into a base class implementation > > > which handles the bookkeeping and a new queueRequestHardware() that is > > > to be implemented by pipeline handlers and only deals with committing the > > > request to hardware. > > > > > > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > > > --- > > > src/libcamera/include/pipeline_handler.h | 4 ++- > > > src/libcamera/pipeline/ipu3/ipu3.cpp | 6 ++-- > > > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 7 ++--- > > > src/libcamera/pipeline/uvcvideo.cpp | 6 ++-- > > > src/libcamera/pipeline/vimc.cpp | 6 ++-- > > > src/libcamera/pipeline_handler.cpp | 35 +++++++++++++++++------- > > > 6 files changed, 37 insertions(+), 27 deletions(-) > > > > > > diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h > > > index a02e6e77dc9ce0e7..6ce8ea1c8d31b870 100644 > > > --- a/src/libcamera/include/pipeline_handler.h > > > +++ b/src/libcamera/include/pipeline_handler.h > > > @@ -77,7 +77,7 @@ public: > > > virtual int start(Camera *camera) = 0; > > > virtual void stop(Camera *camera) = 0; > > > > > > - virtual int queueRequest(Camera *camera, Request *request); > > > + int queueRequest(Camera *camera, Request *request); > > > > I would make this method final. > > To mark this method as final I need to first make it virtual. > > virtual int queueRequest(Camera *camera, Request *request) final; > > Looks a but suspicious to me and we have a lot of other methods which > should not be overloaded by pipeline implementations. I have keep this > as is for now. I'm holding of adding your tag tho until you had a chance > to provide your feedback to this. My bad, you're right. > > > > > > bool completeBuffer(Camera *camera, Request *request, Buffer *buffer); > > > void completeRequest(Camera *camera, Request *request); > > > @@ -89,6 +89,8 @@ protected: > > > std::unique_ptr<CameraData> data); > > > void hotplugMediaDevice(MediaDevice *media); > > > > > > + virtual int queueRequestHardware(Camera *camera, Request *request) = 0; > > > + > > > CameraData *cameraData(const Camera *camera); > > > > > > CameraManager *manager_; > > > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp > > > index 1c5fccf694289ae9..ad223d9101bdc6ed 100644 > > > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp > > > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp > > > @@ -220,7 +220,7 @@ public: > > > int start(Camera *camera) override; > > > void stop(Camera *camera) override; > > > > > > - int queueRequest(Camera *camera, Request *request) override; > > > + int queueRequestHardware(Camera *camera, Request *request) override; > > > > > > bool match(DeviceEnumerator *enumerator) override; > > > > > > @@ -756,7 +756,7 @@ void PipelineHandlerIPU3::stop(Camera *camera) > > > data->rawBuffers_.clear(); > > > } > > > > > > -int PipelineHandlerIPU3::queueRequest(Camera *camera, Request *request) > > > +int PipelineHandlerIPU3::queueRequestHardware(Camera *camera, Request *request) > > > { > > > int error = 0; > > > > > > @@ -769,8 +769,6 @@ int PipelineHandlerIPU3::queueRequest(Camera *camera, Request *request) > > > error = ret; > > > } > > > > > > - PipelineHandler::queueRequest(camera, request); > > > - > > > return error; > > > } > > > > > > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > > > index 4a583a7a1d7ef1fd..f75b25c6787e40df 100644 > > > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp > > > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > > > @@ -184,7 +184,7 @@ public: > > > int start(Camera *camera) override; > > > void stop(Camera *camera) override; > > > > > > - int queueRequest(Camera *camera, Request *request) override; > > > + int queueRequestHardware(Camera *camera, Request *request) override; > > > > > > bool match(DeviceEnumerator *enumerator) override; > > > > > > @@ -810,13 +810,12 @@ void PipelineHandlerRkISP1::stop(Camera *camera) > > > activeCamera_ = nullptr; > > > } > > > > > > -int PipelineHandlerRkISP1::queueRequest(Camera *camera, Request *request) > > > +int PipelineHandlerRkISP1::queueRequestHardware(Camera *camera, > > > + Request *request) > > > { > > > RkISP1CameraData *data = cameraData(camera); > > > Stream *stream = &data->stream_; > > > > > > - PipelineHandler::queueRequest(camera, request); > > > - > > > RkISP1FrameInfo *info = data->frameInfo_.create(data->frame_, request, > > > stream); > > > if (!info) > > > diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp > > > index 522229241ffb9e8a..3a76653ff6dc2b5e 100644 > > > --- a/src/libcamera/pipeline/uvcvideo.cpp > > > +++ b/src/libcamera/pipeline/uvcvideo.cpp > > > @@ -72,7 +72,7 @@ public: > > > int start(Camera *camera) override; > > > void stop(Camera *camera) override; > > > > > > - int queueRequest(Camera *camera, Request *request) override; > > > + int queueRequestHardware(Camera *camera, Request *request) override; > > > > > > bool match(DeviceEnumerator *enumerator) override; > > > > > > @@ -262,7 +262,7 @@ int PipelineHandlerUVC::processControls(UVCCameraData *data, Request *request) > > > return ret; > > > } > > > > > > -int PipelineHandlerUVC::queueRequest(Camera *camera, Request *request) > > > +int PipelineHandlerUVC::queueRequestHardware(Camera *camera, Request *request) > > > { > > > UVCCameraData *data = cameraData(camera); > > > Buffer *buffer = request->findBuffer(&data->stream_); > > > @@ -281,8 +281,6 @@ int PipelineHandlerUVC::queueRequest(Camera *camera, Request *request) > > > if (ret < 0) > > > return ret; > > > > > > - PipelineHandler::queueRequest(camera, request); > > > - > > > return 0; > > > } > > > > > > diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp > > > index 06664fed42e796e4..f5550a1723668106 100644 > > > --- a/src/libcamera/pipeline/vimc.cpp > > > +++ b/src/libcamera/pipeline/vimc.cpp > > > @@ -90,7 +90,7 @@ public: > > > int start(Camera *camera) override; > > > void stop(Camera *camera) override; > > > > > > - int queueRequest(Camera *camera, Request *request) override; > > > + int queueRequestHardware(Camera *camera, Request *request) override; > > > > > > bool match(DeviceEnumerator *enumerator) override; > > > > > > @@ -323,7 +323,7 @@ int PipelineHandlerVimc::processControls(VimcCameraData *data, Request *request) > > > return ret; > > > } > > > > > > -int PipelineHandlerVimc::queueRequest(Camera *camera, Request *request) > > > +int PipelineHandlerVimc::queueRequestHardware(Camera *camera, Request *request) > > > { > > > VimcCameraData *data = cameraData(camera); > > > Buffer *buffer = request->findBuffer(&data->stream_); > > > @@ -342,8 +342,6 @@ int PipelineHandlerVimc::queueRequest(Camera *camera, Request *request) > > > if (ret < 0) > > > return ret; > > > > > > - PipelineHandler::queueRequest(camera, request); > > > - > > > return 0; > > > } > > > > > > diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp > > > index 4349ca8957e403fe..c9e348b98da7b736 100644 > > > --- a/src/libcamera/pipeline_handler.cpp > > > +++ b/src/libcamera/pipeline_handler.cpp > > > @@ -345,27 +345,42 @@ const ControlInfoMap &PipelineHandler::controls(Camera *camera) > > > * \param[in] request The request to queue > > > * > > > * This method queues a capture request to the pipeline handler for processing. > > > - * The request contains a set of buffers associated with streams and a set of > > > - * parameters. The pipeline handler shall program the device to ensure that the > > > - * parameters will be applied to the frames captured in the buffers provided in > > > - * the request. > > > - * > > > - * Pipeline handlers shall override this method. The base implementation in the > > > - * PipelineHandler class keeps track of queued requests in order to ensure > > > - * completion of all requests when the pipeline handler is stopped with stop(). > > > - * Requests completion shall be signalled by the pipeline handler using the > > > + * The method keeps track of queued requests in order to ensure completion of > > > + * all requests when the pipeline handler is stopped with stop(). Requests > > > + * completion shall be signalled by the pipeline handler using the > > > * completeRequest() method. > > > > I would mention queueRequestHardware() here: > > > > * This method queues a capture request to the pipeline handler for processing. > > * The request is first added to the internal list of queued requests, and > > * then passed to the pipeline handler with a call to queueRequestHardware(). > > * > > * Keeping track of queued requests ensures automatic completion of all requests > > * when the pipeline handler is stopped with stop(). Request completion shall be > > * signalled by the pipeline handler using the completeRequest() method. > > > > It took me a while to write this paragraph, which is I think a sign that > > the API isn't very clean, in particular that the queueRequest() and > > queueRequestHardware() names are confusing (the latter especially > > because it may queue the request to the pipeline handler first, not > > immediately to the hardware). I think I would prefer > > queueRequestDevice() to queueRequestHardware() (with s/hardware/device/ > > where applicable), but even that isn't perfect. I don't have better > > names to propose at this moment, so > > Tanks! I went with queueRequestDevice() as I too like it more. > > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > > > * > > > * \return 0 on success or a negative error code otherwise > > > */ > > > int PipelineHandler::queueRequest(Camera *camera, Request *request) > > > { > > > + int ret; > > > + > > > CameraData *data = cameraData(camera); > > > data->queuedRequests_.push_back(request); > > > > > > - return 0; > > > + ret = queueRequestHardware(camera, request); > > > + if (ret) > > > + data->queuedRequests_.remove(request); > > > + > > > + return ret; > > > } > > > > > > +/** > > > + * \fn PipelineHandler::queueRequestHardware() > > > + * \brief Queue a request to the hardware > > > + * \param[in] camera The camera to queue the request to > > > + * \param[in] request The request to queue > > > + * > > > + * This method queues a capture request to the hardware for processing. The > > > + * request contains a set of buffers associated with streams and a set of > > > + * parameters. The pipeline handler shall program the device to ensure that the > > > + * parameters will be applied to the frames captured in the buffers provided in > > > + * the request. > > > + * > > > + * \return 0 on success or a negative error code otherwise > > > + */ > > > + > > > /** > > > * \brief Complete a buffer for a request > > > * \param[in] camera The camera the request belongs to
diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h index a02e6e77dc9ce0e7..6ce8ea1c8d31b870 100644 --- a/src/libcamera/include/pipeline_handler.h +++ b/src/libcamera/include/pipeline_handler.h @@ -77,7 +77,7 @@ public: virtual int start(Camera *camera) = 0; virtual void stop(Camera *camera) = 0; - virtual int queueRequest(Camera *camera, Request *request); + int queueRequest(Camera *camera, Request *request); bool completeBuffer(Camera *camera, Request *request, Buffer *buffer); void completeRequest(Camera *camera, Request *request); @@ -89,6 +89,8 @@ protected: std::unique_ptr<CameraData> data); void hotplugMediaDevice(MediaDevice *media); + virtual int queueRequestHardware(Camera *camera, Request *request) = 0; + CameraData *cameraData(const Camera *camera); CameraManager *manager_; diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index 1c5fccf694289ae9..ad223d9101bdc6ed 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -220,7 +220,7 @@ public: int start(Camera *camera) override; void stop(Camera *camera) override; - int queueRequest(Camera *camera, Request *request) override; + int queueRequestHardware(Camera *camera, Request *request) override; bool match(DeviceEnumerator *enumerator) override; @@ -756,7 +756,7 @@ void PipelineHandlerIPU3::stop(Camera *camera) data->rawBuffers_.clear(); } -int PipelineHandlerIPU3::queueRequest(Camera *camera, Request *request) +int PipelineHandlerIPU3::queueRequestHardware(Camera *camera, Request *request) { int error = 0; @@ -769,8 +769,6 @@ int PipelineHandlerIPU3::queueRequest(Camera *camera, Request *request) error = ret; } - PipelineHandler::queueRequest(camera, request); - return error; } diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index 4a583a7a1d7ef1fd..f75b25c6787e40df 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -184,7 +184,7 @@ public: int start(Camera *camera) override; void stop(Camera *camera) override; - int queueRequest(Camera *camera, Request *request) override; + int queueRequestHardware(Camera *camera, Request *request) override; bool match(DeviceEnumerator *enumerator) override; @@ -810,13 +810,12 @@ void PipelineHandlerRkISP1::stop(Camera *camera) activeCamera_ = nullptr; } -int PipelineHandlerRkISP1::queueRequest(Camera *camera, Request *request) +int PipelineHandlerRkISP1::queueRequestHardware(Camera *camera, + Request *request) { RkISP1CameraData *data = cameraData(camera); Stream *stream = &data->stream_; - PipelineHandler::queueRequest(camera, request); - RkISP1FrameInfo *info = data->frameInfo_.create(data->frame_, request, stream); if (!info) diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp index 522229241ffb9e8a..3a76653ff6dc2b5e 100644 --- a/src/libcamera/pipeline/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo.cpp @@ -72,7 +72,7 @@ public: int start(Camera *camera) override; void stop(Camera *camera) override; - int queueRequest(Camera *camera, Request *request) override; + int queueRequestHardware(Camera *camera, Request *request) override; bool match(DeviceEnumerator *enumerator) override; @@ -262,7 +262,7 @@ int PipelineHandlerUVC::processControls(UVCCameraData *data, Request *request) return ret; } -int PipelineHandlerUVC::queueRequest(Camera *camera, Request *request) +int PipelineHandlerUVC::queueRequestHardware(Camera *camera, Request *request) { UVCCameraData *data = cameraData(camera); Buffer *buffer = request->findBuffer(&data->stream_); @@ -281,8 +281,6 @@ int PipelineHandlerUVC::queueRequest(Camera *camera, Request *request) if (ret < 0) return ret; - PipelineHandler::queueRequest(camera, request); - return 0; } diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp index 06664fed42e796e4..f5550a1723668106 100644 --- a/src/libcamera/pipeline/vimc.cpp +++ b/src/libcamera/pipeline/vimc.cpp @@ -90,7 +90,7 @@ public: int start(Camera *camera) override; void stop(Camera *camera) override; - int queueRequest(Camera *camera, Request *request) override; + int queueRequestHardware(Camera *camera, Request *request) override; bool match(DeviceEnumerator *enumerator) override; @@ -323,7 +323,7 @@ int PipelineHandlerVimc::processControls(VimcCameraData *data, Request *request) return ret; } -int PipelineHandlerVimc::queueRequest(Camera *camera, Request *request) +int PipelineHandlerVimc::queueRequestHardware(Camera *camera, Request *request) { VimcCameraData *data = cameraData(camera); Buffer *buffer = request->findBuffer(&data->stream_); @@ -342,8 +342,6 @@ int PipelineHandlerVimc::queueRequest(Camera *camera, Request *request) if (ret < 0) return ret; - PipelineHandler::queueRequest(camera, request); - return 0; } diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp index 4349ca8957e403fe..c9e348b98da7b736 100644 --- a/src/libcamera/pipeline_handler.cpp +++ b/src/libcamera/pipeline_handler.cpp @@ -345,27 +345,42 @@ const ControlInfoMap &PipelineHandler::controls(Camera *camera) * \param[in] request The request to queue * * This method queues a capture request to the pipeline handler for processing. - * The request contains a set of buffers associated with streams and a set of - * parameters. The pipeline handler shall program the device to ensure that the - * parameters will be applied to the frames captured in the buffers provided in - * the request. - * - * Pipeline handlers shall override this method. The base implementation in the - * PipelineHandler class keeps track of queued requests in order to ensure - * completion of all requests when the pipeline handler is stopped with stop(). - * Requests completion shall be signalled by the pipeline handler using the + * The method keeps track of queued requests in order to ensure completion of + * all requests when the pipeline handler is stopped with stop(). Requests + * completion shall be signalled by the pipeline handler using the * completeRequest() method. * * \return 0 on success or a negative error code otherwise */ int PipelineHandler::queueRequest(Camera *camera, Request *request) { + int ret; + CameraData *data = cameraData(camera); data->queuedRequests_.push_back(request); - return 0; + ret = queueRequestHardware(camera, request); + if (ret) + data->queuedRequests_.remove(request); + + return ret; } +/** + * \fn PipelineHandler::queueRequestHardware() + * \brief Queue a request to the hardware + * \param[in] camera The camera to queue the request to + * \param[in] request The request to queue + * + * This method queues a capture request to the hardware for processing. The + * request contains a set of buffers associated with streams and a set of + * parameters. The pipeline handler shall program the device to ensure that the + * parameters will be applied to the frames captured in the buffers provided in + * the request. + * + * \return 0 on success or a negative error code otherwise + */ + /** * \brief Complete a buffer for a request * \param[in] camera The camera the request belongs to
Expecting pipeline handler implementations of queueRequest() to call the base class queueRequest() at the correct point have lead to different behaviors between the pipelines. Fix this by splitting queueRequest() into a base class implementation which handles the bookkeeping and a new queueRequestHardware() that is to be implemented by pipeline handlers and only deals with committing the request to hardware. Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> --- src/libcamera/include/pipeline_handler.h | 4 ++- src/libcamera/pipeline/ipu3/ipu3.cpp | 6 ++-- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 7 ++--- src/libcamera/pipeline/uvcvideo.cpp | 6 ++-- src/libcamera/pipeline/vimc.cpp | 6 ++-- src/libcamera/pipeline_handler.cpp | 35 +++++++++++++++++------- 6 files changed, 37 insertions(+), 27 deletions(-)