| Message ID | 20200314235728.15495-9-laurent.pinchart@ideasonboard.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Hi Laurent, Thanks for your patch. On 2020-03-15 01:57:27 +0200, Laurent Pinchart wrote: > There's no need anymore to have the Camera object control how and when > pipeline handlers allocate and free the buffers for the > application-facing video devices. Fold those operations, currently > performed by importFrameBuffers() and freeFrameBuffers(), into the > start() and stop() functions. This simplifies the pipeline handler API, > its implementation, and the implementation of the Camera class. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > --- > src/libcamera/camera.cpp | 11 -- > src/libcamera/include/pipeline_handler.h | 2 - > src/libcamera/pipeline/ipu3/ipu3.cpp | 146 +++++++++++------------ > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 20 ++-- > src/libcamera/pipeline/uvcvideo.cpp | 28 ++--- > src/libcamera/pipeline/vimc.cpp | 28 ++--- > src/libcamera/pipeline_handler.cpp | 41 +------ > 7 files changed, 101 insertions(+), 175 deletions(-) > > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp > index 3192dfb42d01..5593c1b317a0 100644 > --- a/src/libcamera/camera.cpp > +++ b/src/libcamera/camera.cpp > @@ -915,13 +915,6 @@ int Camera::start() > > LOG(Camera, Debug) << "Starting capture"; > > - for (Stream *stream : p_->activeStreams_) { > - ret = p_->pipe_->invokeMethod(&PipelineHandler::importFrameBuffers, > - ConnectionTypeDirect, this, stream); > - if (ret < 0) > - return ret; > - } > - > ret = p_->pipe_->invokeMethod(&PipelineHandler::start, > ConnectionTypeBlocking, this); > if (ret) > @@ -959,10 +952,6 @@ int Camera::stop() > p_->pipe_->invokeMethod(&PipelineHandler::stop, ConnectionTypeBlocking, > this); > > - for (Stream *stream : p_->activeStreams_) > - p_->pipe_->invokeMethod(&PipelineHandler::freeFrameBuffers, > - ConnectionTypeBlocking, this, stream); > - > return 0; > } > > diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h > index db6c3104d812..3fcfeda4bfee 100644 > --- a/src/libcamera/include/pipeline_handler.h > +++ b/src/libcamera/include/pipeline_handler.h > @@ -76,8 +76,6 @@ public: > > virtual int exportFrameBuffers(Camera *camera, Stream *stream, > std::vector<std::unique_ptr<FrameBuffer>> *buffers) = 0; > - virtual int importFrameBuffers(Camera *camera, Stream *stream) = 0; > - virtual void freeFrameBuffers(Camera *camera, Stream *stream) = 0; > > virtual int start(Camera *camera) = 0; > virtual void stop(Camera *camera) = 0; > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp > index b6db8d567ea4..6b93c50978a7 100644 > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp > @@ -72,6 +72,7 @@ public: > int configureOutput(ImgUOutput *output, > const StreamConfiguration &cfg); > > + int allocateBuffers(IPU3CameraData *data, unsigned int bufferCount); > void freeBuffers(IPU3CameraData *data); > > int start(); > @@ -208,8 +209,6 @@ public: > > int exportFrameBuffers(Camera *camera, Stream *stream, > std::vector<std::unique_ptr<FrameBuffer>> *buffers) override; > - int importFrameBuffers(Camera *camera, Stream *stream) override; > - void freeFrameBuffers(Camera *camera, Stream *stream) override; > > int start(Camera *camera) override; > void stop(Camera *camera) override; > @@ -625,23 +624,6 @@ int PipelineHandlerIPU3::exportFrameBuffers(Camera *camera, Stream *stream, > return video->exportBuffers(count, buffers); > } > > -int PipelineHandlerIPU3::importFrameBuffers(Camera *camera, Stream *stream) > -{ > - IPU3Stream *ipu3stream = static_cast<IPU3Stream *>(stream); > - V4L2VideoDevice *video = ipu3stream->device_->dev; > - unsigned int count = stream->configuration().bufferCount; > - > - return video->importBuffers(count); > -} > - > -void PipelineHandlerIPU3::freeFrameBuffers(Camera *camera, Stream *stream) > -{ > - IPU3Stream *ipu3stream = static_cast<IPU3Stream *>(stream); > - V4L2VideoDevice *video = ipu3stream->device_->dev; > - > - video->releaseBuffers(); > -} > - > /** > * \todo Clarify if 'viewfinder' and 'stat' nodes have to be set up and > * started even if not in use. As of now, if not properly configured and > @@ -653,69 +635,24 @@ void PipelineHandlerIPU3::freeFrameBuffers(Camera *camera, Stream *stream) > int PipelineHandlerIPU3::allocateBuffers(Camera *camera) > { > IPU3CameraData *data = cameraData(camera); > - IPU3Stream *outStream = &data->outStream_; > - IPU3Stream *vfStream = &data->vfStream_; > CIO2Device *cio2 = &data->cio2_; > ImgUDevice *imgu = data->imgu_; > unsigned int bufferCount; > int ret; > > - /* Share buffers between CIO2 output and ImgU input. */ > ret = cio2->allocateBuffers(); > if (ret < 0) > return ret; > > bufferCount = ret; > > - ret = imgu->input_->importBuffers(bufferCount); > - if (ret) { > - LOG(IPU3, Error) << "Failed to import ImgU input buffers"; > - goto error; > - } > - > - /* > - * Use for the stat's internal pool the same number of buffers as for > - * the input pool. > - * \todo To be revised when we'll actually use the stat node. > - */ > - ret = imgu->stat_.dev->allocateBuffers(bufferCount, &imgu->stat_.buffers); > + ret = imgu->allocateBuffers(data, bufferCount); > if (ret < 0) { > - LOG(IPU3, Error) << "Failed to allocate ImgU stat buffers"; > - goto error; > - } > - > - /* > - * Allocate buffers also on non-active outputs; use the same number > - * of buffers as the active ones. > - */ > - if (!outStream->active_) { > - ImgUDevice::ImgUOutput *output = outStream->device_; > - > - ret = output->dev->allocateBuffers(bufferCount, &output->buffers); > - if (ret < 0) { > - LOG(IPU3, Error) << "Failed to allocate ImgU " > - << output->name << " buffers"; > - goto error; > - } > - } > - > - if (!vfStream->active_) { > - ImgUDevice::ImgUOutput *output = vfStream->device_; > - > - ret = output->dev->allocateBuffers(bufferCount, &output->buffers); > - if (ret < 0) { > - LOG(IPU3, Error) << "Failed to allocate ImgU " > - << output->name << " buffers"; > - goto error; > - } > + cio2->freeBuffers(); > + return ret; > } > > return 0; > - > -error: > - freeBuffers(camera); > - > - return ret; > } > > int PipelineHandlerIPU3::freeBuffers(Camera *camera) > @@ -1156,6 +1093,65 @@ int ImgUDevice::configureOutput(ImgUOutput *output, > return 0; > } > > +/** > + * \brief Allocate buffers for all the ImgU video devices > + */ > +int ImgUDevice::allocateBuffers(IPU3CameraData *data, unsigned int bufferCount) > +{ > + IPU3Stream *outStream = &data->outStream_; > + IPU3Stream *vfStream = &data->vfStream_; > + > + /* Share buffers between CIO2 output and ImgU input. */ > + int ret = input_->importBuffers(bufferCount); > + if (ret) { > + LOG(IPU3, Error) << "Failed to import ImgU input buffers"; > + return ret; > + } > + > + /* > + * Use for the stat's internal pool the same number of buffers as for > + * the input pool. > + * \todo To be revised when we'll actually use the stat node. > + */ > + ret = stat_.dev->allocateBuffers(bufferCount, &stat_.buffers); > + if (ret < 0) { > + LOG(IPU3, Error) << "Failed to allocate ImgU stat buffers"; > + goto error; > + } > + > + /* > + * Allocate buffers for both outputs. If an output is active, prepare > + * for buffer import, otherwise allocate internal buffers. Use the same > + * number of buffers in either case. > + */ > + if (outStream->active_) > + ret = output_.dev->importBuffers(bufferCount); > + else > + ret = output_.dev->allocateBuffers(bufferCount, > + &output_.buffers); > + if (ret < 0) { > + LOG(IPU3, Error) << "Failed to allocate ImgU output buffers"; > + goto error; > + } > + > + if (vfStream->active_) > + ret = viewfinder_.dev->importBuffers(bufferCount); > + else > + ret = viewfinder_.dev->allocateBuffers(bufferCount, > + &viewfinder_.buffers); > + if (ret < 0) { > + LOG(IPU3, Error) << "Failed to allocate ImgU viewfinder buffers"; > + goto error; > + } > + > + return 0; > + > +error: > + freeBuffers(data); > + > + return ret; > +} > + > /** > * \brief Release buffers for all the ImgU video devices > */ > @@ -1163,21 +1159,17 @@ void ImgUDevice::freeBuffers(IPU3CameraData *data) > { > int ret; > > - if (!data->outStream_.active_) { > - ret = output_.dev->releaseBuffers(); > - if (ret) > - LOG(IPU3, Error) << "Failed to release ImgU output buffers"; > - } > + ret = output_.dev->releaseBuffers(); > + if (ret) > + LOG(IPU3, Error) << "Failed to release ImgU output buffers"; > > ret = stat_.dev->releaseBuffers(); > if (ret) > LOG(IPU3, Error) << "Failed to release ImgU stat buffers"; > > - if (!data->vfStream_.active_) { > - ret = viewfinder_.dev->releaseBuffers(); > - if (ret) > - LOG(IPU3, Error) << "Failed to release ImgU viewfinder buffers"; > - } > + ret = viewfinder_.dev->releaseBuffers(); > + if (ret) > + LOG(IPU3, Error) << "Failed to release ImgU viewfinder buffers"; > > ret = input_->releaseBuffers(); > if (ret) > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > index 1ad53fbde112..01977ad697a9 100644 > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > @@ -175,8 +175,6 @@ public: > > int exportFrameBuffers(Camera *camera, Stream *stream, > std::vector<std::unique_ptr<FrameBuffer>> *buffers) override; > - int importFrameBuffers(Camera *camera, Stream *stream) override; > - void freeFrameBuffers(Camera *camera, Stream *stream) override; > > int start(Camera *camera) override; > void stop(Camera *camera) override; > @@ -668,17 +666,6 @@ int PipelineHandlerRkISP1::exportFrameBuffers(Camera *camera, Stream *stream, > return video_->exportBuffers(count, buffers); > } > > -int PipelineHandlerRkISP1::importFrameBuffers(Camera *camera, Stream *stream) > -{ > - unsigned int count = stream->configuration().bufferCount; > - return video_->importBuffers(count); > -} > - > -void PipelineHandlerRkISP1::freeFrameBuffers(Camera *camera, Stream *stream) > -{ > - video_->releaseBuffers(); > -} > - > int PipelineHandlerRkISP1::allocateBuffers(Camera *camera) > { > RkISP1CameraData *data = cameraData(camera); > @@ -689,6 +676,10 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera) > for (const Stream *s : camera->streams()) > maxBuffers = std::max(maxBuffers, s->configuration().bufferCount); > > + ret = video_->importBuffers(count); > + if (ret < 0) > + goto error; > + > ret = param_->allocateBuffers(maxBuffers, ¶mBuffers_); > if (ret < 0) > goto error; > @@ -749,6 +740,9 @@ int PipelineHandlerRkISP1::freeBuffers(Camera *camera) > if (stat_->releaseBuffers()) > LOG(RkISP1, Error) << "Failed to release stat buffers"; > > + if (video_->releaseBuffers()) > + LOG(RkISP1, Error) << "Failed to release video buffers"; > + > return 0; > } > > diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp > index 29afb121aa46..40cc3ee7d098 100644 > --- a/src/libcamera/pipeline/uvcvideo.cpp > +++ b/src/libcamera/pipeline/uvcvideo.cpp > @@ -67,8 +67,6 @@ public: > > int exportFrameBuffers(Camera *camera, Stream *stream, > std::vector<std::unique_ptr<FrameBuffer>> *buffers) override; > - int importFrameBuffers(Camera *camera, Stream *stream) override; > - void freeFrameBuffers(Camera *camera, Stream *stream) override; > > int start(Camera *camera) override; > void stop(Camera *camera) override; > @@ -202,31 +200,29 @@ int PipelineHandlerUVC::exportFrameBuffers(Camera *camera, Stream *stream, > return data->video_->exportBuffers(count, buffers); > } > > -int PipelineHandlerUVC::importFrameBuffers(Camera *camera, Stream *stream) > +int PipelineHandlerUVC::start(Camera *camera) > { > UVCCameraData *data = cameraData(camera); > - unsigned int count = stream->configuration().bufferCount; > + unsigned int count = data->stream_.configuration().bufferCount; > > - return data->video_->importBuffers(count); > -} > - > -void PipelineHandlerUVC::freeFrameBuffers(Camera *camera, Stream *stream) > -{ > - UVCCameraData *data = cameraData(camera); > + int ret = data->video_->importBuffers(count); > + if (ret < 0) > + return ret; > > - data->video_->releaseBuffers(); > -} > + ret = data->video_->streamOn(); > + if (ret < 0) { > + data->video_->releaseBuffers(); > + return ret; > + } > > -int PipelineHandlerUVC::start(Camera *camera) > -{ > - UVCCameraData *data = cameraData(camera); > - return data->video_->streamOn(); > + return 0; > } > > void PipelineHandlerUVC::stop(Camera *camera) > { > UVCCameraData *data = cameraData(camera); > data->video_->streamOff(); > + data->video_->releaseBuffers(); > } > > int PipelineHandlerUVC::processControls(UVCCameraData *data, Request *request) > diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp > index 5d3d12fef30b..eceb16d5586a 100644 > --- a/src/libcamera/pipeline/vimc.cpp > +++ b/src/libcamera/pipeline/vimc.cpp > @@ -84,8 +84,6 @@ public: > > int exportFrameBuffers(Camera *camera, Stream *stream, > std::vector<std::unique_ptr<FrameBuffer>> *buffers) override; > - int importFrameBuffers(Camera *camera, Stream *stream) override; > - void freeFrameBuffers(Camera *camera, Stream *stream) override; > > int start(Camera *camera) override; > void stop(Camera *camera) override; > @@ -268,31 +266,29 @@ int PipelineHandlerVimc::exportFrameBuffers(Camera *camera, Stream *stream, > return data->video_->exportBuffers(count, buffers); > } > > -int PipelineHandlerVimc::importFrameBuffers(Camera *camera, Stream *stream) > +int PipelineHandlerVimc::start(Camera *camera) > { > VimcCameraData *data = cameraData(camera); > - unsigned int count = stream->configuration().bufferCount; > + unsigned int count = data->stream_.configuration().bufferCount; > > - return data->video_->importBuffers(count); > -} > - > -void PipelineHandlerVimc::freeFrameBuffers(Camera *camera, Stream *stream) > -{ > - VimcCameraData *data = cameraData(camera); > + int ret = data->video_->importBuffers(count); > + if (ret < 0) > + return ret; > > - data->video_->releaseBuffers(); > -} > + ret = data->video_->streamOn(); > + if (ret < 0) { > + data->video_->releaseBuffers(); > + return ret; > + } > > -int PipelineHandlerVimc::start(Camera *camera) > -{ > - VimcCameraData *data = cameraData(camera); > - return data->video_->streamOn(); > + return 0; > } > > void PipelineHandlerVimc::stop(Camera *camera) > { > VimcCameraData *data = cameraData(camera); > data->video_->streamOff(); > + data->video_->releaseBuffers(); > } > > int PipelineHandlerVimc::processControls(VimcCameraData *data, Request *request) > diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp > index e5034c54e2fb..254d341fb8a4 100644 > --- a/src/libcamera/pipeline_handler.cpp > +++ b/src/libcamera/pipeline_handler.cpp > @@ -325,7 +325,7 @@ const ControlList &PipelineHandler::properties(Camera *camera) > > /** > * \fn PipelineHandler::exportFrameBuffers() > - * \brief Allocate buffers for \a stream > + * \brief Allocate and export buffers for \a stream > * \param[in] camera The camera > * \param[in] stream The stream to allocate buffers for > * \param[out] buffers Array of buffers successfully allocated > @@ -347,45 +347,6 @@ const ControlList &PipelineHandler::properties(Camera *camera) > * otherwise > */ > > -/** > - * \fn PipelineHandler::importFrameBuffers() > - * \brief Prepare \a stream to use external buffers > - * \param[in] camera The camera > - * \param[in] stream The stream to prepare for import > - * > - * This method prepares the pipeline handler to use buffers provided by the > - * application for the \a stream. > - * > - * The method may only be called after the Camera has been configured and before > - * it gets started, or after it gets stopped. It shall be called only for > - * streams that are part of the active camera configuration, and at most once > - * per stream until buffers for the stream are freed with freeFrameBuffers(). > - * > - * importFrameBuffers() shall also allocate all other resources required by the > - * pipeline handler for the stream to prepare for starting the Camera. > - * > - * The only intended caller is Camera::start(). > - * > - * \context This function is called from the CameraManager thread. > - * > - * \return 0 on success or a negative error code otherwise > - */ > - > -/** > - * \fn PipelineHandler::freeFrameBuffers() > - * \brief Free buffers allocated from the stream > - * \param[in] camera The camera > - * \param[in] stream The stream to free buffers for > - * > - * This method shall release all resources allocated for the \a stream by > - * importFrameBuffers(). It shall be called only after a successful call that > - * method, and only once per stream. > - * > - * The only intended callers are Camera::stop() and Camera::freeFrameBuffers(). > - * > - * \context This function is called from the CameraManager thread. > - */ > - > /** > * \fn PipelineHandler::start() > * \brief Start capturing from a group of streams > -- > Regards, > > Laurent Pinchart > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp index 3192dfb42d01..5593c1b317a0 100644 --- a/src/libcamera/camera.cpp +++ b/src/libcamera/camera.cpp @@ -915,13 +915,6 @@ int Camera::start() LOG(Camera, Debug) << "Starting capture"; - for (Stream *stream : p_->activeStreams_) { - ret = p_->pipe_->invokeMethod(&PipelineHandler::importFrameBuffers, - ConnectionTypeDirect, this, stream); - if (ret < 0) - return ret; - } - ret = p_->pipe_->invokeMethod(&PipelineHandler::start, ConnectionTypeBlocking, this); if (ret) @@ -959,10 +952,6 @@ int Camera::stop() p_->pipe_->invokeMethod(&PipelineHandler::stop, ConnectionTypeBlocking, this); - for (Stream *stream : p_->activeStreams_) - p_->pipe_->invokeMethod(&PipelineHandler::freeFrameBuffers, - ConnectionTypeBlocking, this, stream); - return 0; } diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h index db6c3104d812..3fcfeda4bfee 100644 --- a/src/libcamera/include/pipeline_handler.h +++ b/src/libcamera/include/pipeline_handler.h @@ -76,8 +76,6 @@ public: virtual int exportFrameBuffers(Camera *camera, Stream *stream, std::vector<std::unique_ptr<FrameBuffer>> *buffers) = 0; - virtual int importFrameBuffers(Camera *camera, Stream *stream) = 0; - virtual void freeFrameBuffers(Camera *camera, Stream *stream) = 0; virtual int start(Camera *camera) = 0; virtual void stop(Camera *camera) = 0; diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index b6db8d567ea4..6b93c50978a7 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -72,6 +72,7 @@ public: int configureOutput(ImgUOutput *output, const StreamConfiguration &cfg); + int allocateBuffers(IPU3CameraData *data, unsigned int bufferCount); void freeBuffers(IPU3CameraData *data); int start(); @@ -208,8 +209,6 @@ public: int exportFrameBuffers(Camera *camera, Stream *stream, std::vector<std::unique_ptr<FrameBuffer>> *buffers) override; - int importFrameBuffers(Camera *camera, Stream *stream) override; - void freeFrameBuffers(Camera *camera, Stream *stream) override; int start(Camera *camera) override; void stop(Camera *camera) override; @@ -625,23 +624,6 @@ int PipelineHandlerIPU3::exportFrameBuffers(Camera *camera, Stream *stream, return video->exportBuffers(count, buffers); } -int PipelineHandlerIPU3::importFrameBuffers(Camera *camera, Stream *stream) -{ - IPU3Stream *ipu3stream = static_cast<IPU3Stream *>(stream); - V4L2VideoDevice *video = ipu3stream->device_->dev; - unsigned int count = stream->configuration().bufferCount; - - return video->importBuffers(count); -} - -void PipelineHandlerIPU3::freeFrameBuffers(Camera *camera, Stream *stream) -{ - IPU3Stream *ipu3stream = static_cast<IPU3Stream *>(stream); - V4L2VideoDevice *video = ipu3stream->device_->dev; - - video->releaseBuffers(); -} - /** * \todo Clarify if 'viewfinder' and 'stat' nodes have to be set up and * started even if not in use. As of now, if not properly configured and @@ -653,69 +635,24 @@ void PipelineHandlerIPU3::freeFrameBuffers(Camera *camera, Stream *stream) int PipelineHandlerIPU3::allocateBuffers(Camera *camera) { IPU3CameraData *data = cameraData(camera); - IPU3Stream *outStream = &data->outStream_; - IPU3Stream *vfStream = &data->vfStream_; CIO2Device *cio2 = &data->cio2_; ImgUDevice *imgu = data->imgu_; unsigned int bufferCount; int ret; - /* Share buffers between CIO2 output and ImgU input. */ ret = cio2->allocateBuffers(); if (ret < 0) return ret; bufferCount = ret; - ret = imgu->input_->importBuffers(bufferCount); - if (ret) { - LOG(IPU3, Error) << "Failed to import ImgU input buffers"; - goto error; - } - - /* - * Use for the stat's internal pool the same number of buffers as for - * the input pool. - * \todo To be revised when we'll actually use the stat node. - */ - ret = imgu->stat_.dev->allocateBuffers(bufferCount, &imgu->stat_.buffers); + ret = imgu->allocateBuffers(data, bufferCount); if (ret < 0) { - LOG(IPU3, Error) << "Failed to allocate ImgU stat buffers"; - goto error; - } - - /* - * Allocate buffers also on non-active outputs; use the same number - * of buffers as the active ones. - */ - if (!outStream->active_) { - ImgUDevice::ImgUOutput *output = outStream->device_; - - ret = output->dev->allocateBuffers(bufferCount, &output->buffers); - if (ret < 0) { - LOG(IPU3, Error) << "Failed to allocate ImgU " - << output->name << " buffers"; - goto error; - } - } - - if (!vfStream->active_) { - ImgUDevice::ImgUOutput *output = vfStream->device_; - - ret = output->dev->allocateBuffers(bufferCount, &output->buffers); - if (ret < 0) { - LOG(IPU3, Error) << "Failed to allocate ImgU " - << output->name << " buffers"; - goto error; - } + cio2->freeBuffers(); + return ret; } return 0; - -error: - freeBuffers(camera); - - return ret; } int PipelineHandlerIPU3::freeBuffers(Camera *camera) @@ -1156,6 +1093,65 @@ int ImgUDevice::configureOutput(ImgUOutput *output, return 0; } +/** + * \brief Allocate buffers for all the ImgU video devices + */ +int ImgUDevice::allocateBuffers(IPU3CameraData *data, unsigned int bufferCount) +{ + IPU3Stream *outStream = &data->outStream_; + IPU3Stream *vfStream = &data->vfStream_; + + /* Share buffers between CIO2 output and ImgU input. */ + int ret = input_->importBuffers(bufferCount); + if (ret) { + LOG(IPU3, Error) << "Failed to import ImgU input buffers"; + return ret; + } + + /* + * Use for the stat's internal pool the same number of buffers as for + * the input pool. + * \todo To be revised when we'll actually use the stat node. + */ + ret = stat_.dev->allocateBuffers(bufferCount, &stat_.buffers); + if (ret < 0) { + LOG(IPU3, Error) << "Failed to allocate ImgU stat buffers"; + goto error; + } + + /* + * Allocate buffers for both outputs. If an output is active, prepare + * for buffer import, otherwise allocate internal buffers. Use the same + * number of buffers in either case. + */ + if (outStream->active_) + ret = output_.dev->importBuffers(bufferCount); + else + ret = output_.dev->allocateBuffers(bufferCount, + &output_.buffers); + if (ret < 0) { + LOG(IPU3, Error) << "Failed to allocate ImgU output buffers"; + goto error; + } + + if (vfStream->active_) + ret = viewfinder_.dev->importBuffers(bufferCount); + else + ret = viewfinder_.dev->allocateBuffers(bufferCount, + &viewfinder_.buffers); + if (ret < 0) { + LOG(IPU3, Error) << "Failed to allocate ImgU viewfinder buffers"; + goto error; + } + + return 0; + +error: + freeBuffers(data); + + return ret; +} + /** * \brief Release buffers for all the ImgU video devices */ @@ -1163,21 +1159,17 @@ void ImgUDevice::freeBuffers(IPU3CameraData *data) { int ret; - if (!data->outStream_.active_) { - ret = output_.dev->releaseBuffers(); - if (ret) - LOG(IPU3, Error) << "Failed to release ImgU output buffers"; - } + ret = output_.dev->releaseBuffers(); + if (ret) + LOG(IPU3, Error) << "Failed to release ImgU output buffers"; ret = stat_.dev->releaseBuffers(); if (ret) LOG(IPU3, Error) << "Failed to release ImgU stat buffers"; - if (!data->vfStream_.active_) { - ret = viewfinder_.dev->releaseBuffers(); - if (ret) - LOG(IPU3, Error) << "Failed to release ImgU viewfinder buffers"; - } + ret = viewfinder_.dev->releaseBuffers(); + if (ret) + LOG(IPU3, Error) << "Failed to release ImgU viewfinder buffers"; ret = input_->releaseBuffers(); if (ret) diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index 1ad53fbde112..01977ad697a9 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -175,8 +175,6 @@ public: int exportFrameBuffers(Camera *camera, Stream *stream, std::vector<std::unique_ptr<FrameBuffer>> *buffers) override; - int importFrameBuffers(Camera *camera, Stream *stream) override; - void freeFrameBuffers(Camera *camera, Stream *stream) override; int start(Camera *camera) override; void stop(Camera *camera) override; @@ -668,17 +666,6 @@ int PipelineHandlerRkISP1::exportFrameBuffers(Camera *camera, Stream *stream, return video_->exportBuffers(count, buffers); } -int PipelineHandlerRkISP1::importFrameBuffers(Camera *camera, Stream *stream) -{ - unsigned int count = stream->configuration().bufferCount; - return video_->importBuffers(count); -} - -void PipelineHandlerRkISP1::freeFrameBuffers(Camera *camera, Stream *stream) -{ - video_->releaseBuffers(); -} - int PipelineHandlerRkISP1::allocateBuffers(Camera *camera) { RkISP1CameraData *data = cameraData(camera); @@ -689,6 +676,10 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera) for (const Stream *s : camera->streams()) maxBuffers = std::max(maxBuffers, s->configuration().bufferCount); + ret = video_->importBuffers(count); + if (ret < 0) + goto error; + ret = param_->allocateBuffers(maxBuffers, ¶mBuffers_); if (ret < 0) goto error; @@ -749,6 +740,9 @@ int PipelineHandlerRkISP1::freeBuffers(Camera *camera) if (stat_->releaseBuffers()) LOG(RkISP1, Error) << "Failed to release stat buffers"; + if (video_->releaseBuffers()) + LOG(RkISP1, Error) << "Failed to release video buffers"; + return 0; } diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp index 29afb121aa46..40cc3ee7d098 100644 --- a/src/libcamera/pipeline/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo.cpp @@ -67,8 +67,6 @@ public: int exportFrameBuffers(Camera *camera, Stream *stream, std::vector<std::unique_ptr<FrameBuffer>> *buffers) override; - int importFrameBuffers(Camera *camera, Stream *stream) override; - void freeFrameBuffers(Camera *camera, Stream *stream) override; int start(Camera *camera) override; void stop(Camera *camera) override; @@ -202,31 +200,29 @@ int PipelineHandlerUVC::exportFrameBuffers(Camera *camera, Stream *stream, return data->video_->exportBuffers(count, buffers); } -int PipelineHandlerUVC::importFrameBuffers(Camera *camera, Stream *stream) +int PipelineHandlerUVC::start(Camera *camera) { UVCCameraData *data = cameraData(camera); - unsigned int count = stream->configuration().bufferCount; + unsigned int count = data->stream_.configuration().bufferCount; - return data->video_->importBuffers(count); -} - -void PipelineHandlerUVC::freeFrameBuffers(Camera *camera, Stream *stream) -{ - UVCCameraData *data = cameraData(camera); + int ret = data->video_->importBuffers(count); + if (ret < 0) + return ret; - data->video_->releaseBuffers(); -} + ret = data->video_->streamOn(); + if (ret < 0) { + data->video_->releaseBuffers(); + return ret; + } -int PipelineHandlerUVC::start(Camera *camera) -{ - UVCCameraData *data = cameraData(camera); - return data->video_->streamOn(); + return 0; } void PipelineHandlerUVC::stop(Camera *camera) { UVCCameraData *data = cameraData(camera); data->video_->streamOff(); + data->video_->releaseBuffers(); } int PipelineHandlerUVC::processControls(UVCCameraData *data, Request *request) diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp index 5d3d12fef30b..eceb16d5586a 100644 --- a/src/libcamera/pipeline/vimc.cpp +++ b/src/libcamera/pipeline/vimc.cpp @@ -84,8 +84,6 @@ public: int exportFrameBuffers(Camera *camera, Stream *stream, std::vector<std::unique_ptr<FrameBuffer>> *buffers) override; - int importFrameBuffers(Camera *camera, Stream *stream) override; - void freeFrameBuffers(Camera *camera, Stream *stream) override; int start(Camera *camera) override; void stop(Camera *camera) override; @@ -268,31 +266,29 @@ int PipelineHandlerVimc::exportFrameBuffers(Camera *camera, Stream *stream, return data->video_->exportBuffers(count, buffers); } -int PipelineHandlerVimc::importFrameBuffers(Camera *camera, Stream *stream) +int PipelineHandlerVimc::start(Camera *camera) { VimcCameraData *data = cameraData(camera); - unsigned int count = stream->configuration().bufferCount; + unsigned int count = data->stream_.configuration().bufferCount; - return data->video_->importBuffers(count); -} - -void PipelineHandlerVimc::freeFrameBuffers(Camera *camera, Stream *stream) -{ - VimcCameraData *data = cameraData(camera); + int ret = data->video_->importBuffers(count); + if (ret < 0) + return ret; - data->video_->releaseBuffers(); -} + ret = data->video_->streamOn(); + if (ret < 0) { + data->video_->releaseBuffers(); + return ret; + } -int PipelineHandlerVimc::start(Camera *camera) -{ - VimcCameraData *data = cameraData(camera); - return data->video_->streamOn(); + return 0; } void PipelineHandlerVimc::stop(Camera *camera) { VimcCameraData *data = cameraData(camera); data->video_->streamOff(); + data->video_->releaseBuffers(); } int PipelineHandlerVimc::processControls(VimcCameraData *data, Request *request) diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp index e5034c54e2fb..254d341fb8a4 100644 --- a/src/libcamera/pipeline_handler.cpp +++ b/src/libcamera/pipeline_handler.cpp @@ -325,7 +325,7 @@ const ControlList &PipelineHandler::properties(Camera *camera) /** * \fn PipelineHandler::exportFrameBuffers() - * \brief Allocate buffers for \a stream + * \brief Allocate and export buffers for \a stream * \param[in] camera The camera * \param[in] stream The stream to allocate buffers for * \param[out] buffers Array of buffers successfully allocated @@ -347,45 +347,6 @@ const ControlList &PipelineHandler::properties(Camera *camera) * otherwise */ -/** - * \fn PipelineHandler::importFrameBuffers() - * \brief Prepare \a stream to use external buffers - * \param[in] camera The camera - * \param[in] stream The stream to prepare for import - * - * This method prepares the pipeline handler to use buffers provided by the - * application for the \a stream. - * - * The method may only be called after the Camera has been configured and before - * it gets started, or after it gets stopped. It shall be called only for - * streams that are part of the active camera configuration, and at most once - * per stream until buffers for the stream are freed with freeFrameBuffers(). - * - * importFrameBuffers() shall also allocate all other resources required by the - * pipeline handler for the stream to prepare for starting the Camera. - * - * The only intended caller is Camera::start(). - * - * \context This function is called from the CameraManager thread. - * - * \return 0 on success or a negative error code otherwise - */ - -/** - * \fn PipelineHandler::freeFrameBuffers() - * \brief Free buffers allocated from the stream - * \param[in] camera The camera - * \param[in] stream The stream to free buffers for - * - * This method shall release all resources allocated for the \a stream by - * importFrameBuffers(). It shall be called only after a successful call that - * method, and only once per stream. - * - * The only intended callers are Camera::stop() and Camera::freeFrameBuffers(). - * - * \context This function is called from the CameraManager thread. - */ - /** * \fn PipelineHandler::start() * \brief Start capturing from a group of streams
There's no need anymore to have the Camera object control how and when pipeline handlers allocate and free the buffers for the application-facing video devices. Fold those operations, currently performed by importFrameBuffers() and freeFrameBuffers(), into the start() and stop() functions. This simplifies the pipeline handler API, its implementation, and the implementation of the Camera class. Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- src/libcamera/camera.cpp | 11 -- src/libcamera/include/pipeline_handler.h | 2 - src/libcamera/pipeline/ipu3/ipu3.cpp | 146 +++++++++++------------ src/libcamera/pipeline/rkisp1/rkisp1.cpp | 20 ++-- src/libcamera/pipeline/uvcvideo.cpp | 28 ++--- src/libcamera/pipeline/vimc.cpp | 28 ++--- src/libcamera/pipeline_handler.cpp | 41 +------ 7 files changed, 101 insertions(+), 175 deletions(-)