[libcamera-devel] [RFC 02/12] libcamera: pipelines: Explicitly allocate streams
Jacopo Mondi
jacopo at jmondi.org
Wed Nov 6 09:49:27 CET 2019
Hi Niklas,
On Mon, Oct 28, 2019 at 03:25:15AM +0100, Niklas Söderlund wrote:
> Prepare for sub-classing the Stream class with a V4L2 specific
> implementation which will need to be constructed later when knowledge
> about the V4L2 video device is available.
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> ---
> src/libcamera/pipeline/ipu3/ipu3.cpp | 60 +++++++++++++-----------
> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 19 ++++----
> src/libcamera/pipeline/uvcvideo.cpp | 13 +++--
> src/libcamera/pipeline/vimc.cpp | 14 ++++--
> 4 files changed, 60 insertions(+), 46 deletions(-)
>
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index 06ee6ccac641eb41..ff90b729e558c3a3 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -137,8 +137,8 @@ public:
> class IPU3Stream : public Stream
> {
> public:
> - IPU3Stream()
> - : active_(false), device_(nullptr)
> + IPU3Stream(ImgUDevice::ImgUOutput *device, const std::string &name)
> + : active_(false), name_(name), device_(device)
> {
> }
>
> @@ -151,10 +151,16 @@ class IPU3CameraData : public CameraData
> {
> public:
> IPU3CameraData(PipelineHandler *pipe)
> - : CameraData(pipe)
> + : CameraData(pipe), outStream_(nullptr), vfStream_(nullptr)
> {
> }
>
> + ~IPU3CameraData()
> + {
> + delete outStream_;
> + delete vfStream_;
> + }
Have you considered making these and the other stream pointers in the
patch unique_ptr<> to avoid tracking their lifecycle ?
Looking at the usage pattern it's probably not worth it though, as
they're part of CameraData which is a unique_ptr of its own.
The rest of the patch looks good, thanks!
Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>
Thanks
j
> +
> void imguOutputBufferReady(Buffer *buffer);
> void imguInputBufferReady(Buffer *buffer);
> void cio2BufferReady(Buffer *buffer);
> @@ -162,8 +168,8 @@ public:
> CIO2Device cio2_;
> ImgUDevice *imgu_;
>
> - IPU3Stream outStream_;
> - IPU3Stream vfStream_;
> + IPU3Stream *outStream_;
> + IPU3Stream *vfStream_;
>
> std::vector<std::unique_ptr<Buffer>> rawBuffers_;
> };
> @@ -342,8 +348,8 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate()
> * stream otherwise.
> */
> std::set<const IPU3Stream *> availableStreams = {
> - &data_->outStream_,
> - &data_->vfStream_,
> + data_->outStream_,
> + data_->vfStream_,
> };
>
> streams_.clear();
> @@ -356,9 +362,9 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate()
> const IPU3Stream *stream;
>
> if (cfg.size == sensorFormat_.size)
> - stream = &data_->outStream_;
> + stream = data_->outStream_;
> else
> - stream = &data_->vfStream_;
> + stream = data_->vfStream_;
>
> if (availableStreams.find(stream) == availableStreams.end())
> stream = *availableStreams.begin();
> @@ -366,7 +372,7 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate()
> LOG(IPU3, Debug)
> << "Assigned '" << stream->name_ << "' to stream " << i;
>
> - bool scale = stream == &data_->vfStream_;
> + bool scale = stream == data_->vfStream_;
> adjustStream(config_[i], scale);
>
> if (cfg.pixelFormat != pixelFormat || cfg.size != size) {
> @@ -394,8 +400,8 @@ CameraConfiguration *PipelineHandlerIPU3::generateConfiguration(Camera *camera,
> IPU3CameraData *data = cameraData(camera);
> IPU3CameraConfiguration *config;
> std::set<IPU3Stream *> streams = {
> - &data->outStream_,
> - &data->vfStream_,
> + data->outStream_,
> + data->vfStream_,
> };
>
> config = new IPU3CameraConfiguration(camera, data);
> @@ -413,10 +419,10 @@ CameraConfiguration *PipelineHandlerIPU3::generateConfiguration(Camera *camera,
> * and VideoRecording roles are not allowed on
> * the output stream.
> */
> - if (streams.find(&data->outStream_) != streams.end()) {
> - stream = &data->outStream_;
> - } else if (streams.find(&data->vfStream_) != streams.end()) {
> - stream = &data->vfStream_;
> + if (streams.find(data->outStream_) != streams.end()) {
> + stream = data->outStream_;
> + } else if (streams.find(data->vfStream_) != streams.end()) {
> + stream = data->vfStream_;
> } else {
> LOG(IPU3, Error)
> << "No stream available for requested role "
> @@ -446,14 +452,14 @@ CameraConfiguration *PipelineHandlerIPU3::generateConfiguration(Camera *camera,
> * \todo This is an artificial limitation until we
> * figure out the exact capabilities of the hardware.
> */
> - if (streams.find(&data->vfStream_) == streams.end()) {
> + if (streams.find(data->vfStream_) == streams.end()) {
> LOG(IPU3, Error)
> << "No stream available for requested role "
> << role;
> break;
> }
>
> - stream = &data->vfStream_;
> + stream = data->vfStream_;
>
> /*
> * Align the default viewfinder size to the maximum
> @@ -494,8 +500,8 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)
> IPU3CameraConfiguration *config =
> static_cast<IPU3CameraConfiguration *>(c);
> IPU3CameraData *data = cameraData(camera);
> - IPU3Stream *outStream = &data->outStream_;
> - IPU3Stream *vfStream = &data->vfStream_;
> + IPU3Stream *outStream = data->outStream_;
> + IPU3Stream *vfStream = data->vfStream_;
> CIO2Device *cio2 = &data->cio2_;
> ImgUDevice *imgu = data->imgu_;
> int ret;
> @@ -629,8 +635,8 @@ int PipelineHandlerIPU3::allocateBuffers(Camera *camera,
> const std::set<Stream *> &streams)
> {
> IPU3CameraData *data = cameraData(camera);
> - IPU3Stream *outStream = &data->outStream_;
> - IPU3Stream *vfStream = &data->vfStream_;
> + IPU3Stream *outStream = data->outStream_;
> + IPU3Stream *vfStream = data->vfStream_;
> CIO2Device *cio2 = &data->cio2_;
> ImgUDevice *imgu = data->imgu_;
> unsigned int bufferCount;
> @@ -858,8 +864,8 @@ int PipelineHandlerIPU3::registerCameras()
> std::unique_ptr<IPU3CameraData> data =
> utils::make_unique<IPU3CameraData>(this);
> std::set<Stream *> streams = {
> - &data->outStream_,
> - &data->vfStream_,
> + data->outStream_,
> + data->vfStream_,
> };
> CIO2Device *cio2 = &data->cio2_;
>
> @@ -874,10 +880,8 @@ int PipelineHandlerIPU3::registerCameras()
> * second.
> */
> data->imgu_ = numCameras ? &imgu1_ : &imgu0_;
> - data->outStream_.device_ = &data->imgu_->output_;
> - data->outStream_.name_ = "output";
> - data->vfStream_.device_ = &data->imgu_->viewfinder_;
> - data->vfStream_.name_ = "viewfinder";
> + data->outStream_ = new IPU3Stream(&data->imgu_->output_, "output");
> + data->vfStream_ = new IPU3Stream(&data->imgu_->viewfinder_, "viewfinder");
>
> /*
> * Connect video devices' 'bufferReady' signals to their
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index a99df4f8b846d8b7..cefdb54a06d440fb 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -116,19 +116,20 @@ class RkISP1CameraData : public CameraData
> {
> public:
> RkISP1CameraData(PipelineHandler *pipe)
> - : CameraData(pipe), sensor_(nullptr), frame_(0),
> - frameInfo_(pipe)
> + : CameraData(pipe), stream_(nullptr), sensor_(nullptr),
> + frame_(0), frameInfo_(pipe)
> {
> }
>
> ~RkISP1CameraData()
> {
> + delete stream_;
> delete sensor_;
> }
>
> int loadIPA();
>
> - Stream stream_;
> + Stream *stream_;
> CameraSensor *sensor_;
> unsigned int frame_;
> std::vector<IPABuffer> ipaBuffers_;
> @@ -650,7 +651,7 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)
> if (ret)
> return ret;
>
> - cfg.setStream(&data->stream_);
> + cfg.setStream(data->stream_);
>
> return 0;
> }
> @@ -772,8 +773,8 @@ int PipelineHandlerRkISP1::start(Camera *camera)
> /* Inform IPA of stream configuration and sensor controls. */
> std::map<unsigned int, IPAStream> streamConfig;
> streamConfig[0] = {
> - .pixelFormat = data->stream_.configuration().pixelFormat,
> - .size = data->stream_.configuration().size,
> + .pixelFormat = data->stream_->configuration().pixelFormat,
> + .size = data->stream_->configuration().size,
> };
>
> std::map<unsigned int, ControlInfoMap> entityControls;
> @@ -812,7 +813,7 @@ void PipelineHandlerRkISP1::stop(Camera *camera)
> int PipelineHandlerRkISP1::queueRequest(Camera *camera, Request *request)
> {
> RkISP1CameraData *data = cameraData(camera);
> - Stream *stream = &data->stream_;
> + Stream *stream = data->stream_;
>
> RkISP1FrameInfo *info = data->frameInfo_.create(data->frame_, request,
> stream);
> @@ -873,6 +874,8 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)
> std::unique_ptr<RkISP1CameraData> data =
> utils::make_unique<RkISP1CameraData>(this);
>
> + data->stream_ = new Stream();
> +
> ControlInfoMap::Map ctrls;
> ctrls.emplace(std::piecewise_construct,
> std::forward_as_tuple(&controls::AeEnable),
> @@ -889,7 +892,7 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)
> if (ret)
> return ret;
>
> - std::set<Stream *> streams{ &data->stream_ };
> + std::set<Stream *> streams{ data->stream_ };
> std::shared_ptr<Camera> camera =
> Camera::create(this, sensor->name(), streams);
> registerCamera(std::move(camera), std::move(data));
> diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp
> index dc17b456af6987e6..5aa6a8f9e0b2b7a4 100644
> --- a/src/libcamera/pipeline/uvcvideo.cpp
> +++ b/src/libcamera/pipeline/uvcvideo.cpp
> @@ -31,12 +31,13 @@ class UVCCameraData : public CameraData
> {
> public:
> UVCCameraData(PipelineHandler *pipe)
> - : CameraData(pipe), video_(nullptr)
> + : CameraData(pipe), video_(nullptr), stream_(nullptr)
> {
> }
>
> ~UVCCameraData()
> {
> + delete stream_;
> delete video_;
> }
>
> @@ -44,7 +45,7 @@ public:
> void bufferReady(Buffer *buffer);
>
> V4L2VideoDevice *video_;
> - Stream stream_;
> + Stream *stream_;
> };
>
> class UVCCameraConfiguration : public CameraConfiguration
> @@ -187,7 +188,7 @@ int PipelineHandlerUVC::configure(Camera *camera, CameraConfiguration *config)
> format.fourcc != cfg.pixelFormat)
> return -EINVAL;
>
> - cfg.setStream(&data->stream_);
> + cfg.setStream(data->stream_);
>
> return 0;
> }
> @@ -265,7 +266,7 @@ int PipelineHandlerUVC::processControls(UVCCameraData *data, Request *request)
> int PipelineHandlerUVC::queueRequest(Camera *camera, Request *request)
> {
> UVCCameraData *data = cameraData(camera);
> - Buffer *buffer = request->findBuffer(&data->stream_);
> + Buffer *buffer = request->findBuffer(data->stream_);
> if (!buffer) {
> LOG(UVC, Error)
> << "Attempt to queue request with invalid stream";
> @@ -310,7 +311,7 @@ bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator)
> }
>
> /* Create and register the camera. */
> - std::set<Stream *> streams{ &data->stream_ };
> + std::set<Stream *> streams{ data->stream_ };
> std::shared_ptr<Camera> camera = Camera::create(this, media->model(), streams);
> registerCamera(std::move(camera), std::move(data));
>
> @@ -330,6 +331,8 @@ int UVCCameraData::init(MediaEntity *entity)
> if (ret)
> return ret;
>
> + stream_ = new Stream();
> +
> video_->bufferReady.connect(this, &UVCCameraData::bufferReady);
>
> /* Initialise the supported controls. */
> diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp
> index 5f83ae2b85997828..49b850cf0153020f 100644
> --- a/src/libcamera/pipeline/vimc.cpp
> +++ b/src/libcamera/pipeline/vimc.cpp
> @@ -40,7 +40,8 @@ class VimcCameraData : public CameraData
> public:
> VimcCameraData(PipelineHandler *pipe)
> : CameraData(pipe), sensor_(nullptr), debayer_(nullptr),
> - scaler_(nullptr), video_(nullptr), raw_(nullptr)
> + scaler_(nullptr), video_(nullptr), raw_(nullptr),
> + stream_(nullptr)
> {
> }
>
> @@ -51,6 +52,7 @@ public:
> delete scaler_;
> delete video_;
> delete raw_;
> + delete stream_;
> }
>
> int init(MediaDevice *media);
> @@ -61,7 +63,7 @@ public:
> V4L2Subdevice *scaler_;
> V4L2VideoDevice *video_;
> V4L2VideoDevice *raw_;
> - Stream stream_;
> + Stream *stream_;
> };
>
> class VimcCameraConfiguration : public CameraConfiguration
> @@ -253,7 +255,7 @@ int PipelineHandlerVimc::configure(Camera *camera, CameraConfiguration *config)
> if (ret)
> return ret;
>
> - cfg.setStream(&data->stream_);
> + cfg.setStream(data->stream_);
>
> return 0;
> }
> @@ -325,7 +327,7 @@ int PipelineHandlerVimc::processControls(VimcCameraData *data, Request *request)
> int PipelineHandlerVimc::queueRequest(Camera *camera, Request *request)
> {
> VimcCameraData *data = cameraData(camera);
> - Buffer *buffer = request->findBuffer(&data->stream_);
> + Buffer *buffer = request->findBuffer(data->stream_);
> if (!buffer) {
> LOG(VIMC, Error)
> << "Attempt to queue request with invalid stream";
> @@ -375,7 +377,7 @@ bool PipelineHandlerVimc::match(DeviceEnumerator *enumerator)
> return false;
>
> /* Create and register the camera. */
> - std::set<Stream *> streams{ &data->stream_ };
> + std::set<Stream *> streams{ data->stream_ };
> std::shared_ptr<Camera> camera = Camera::create(this, "VIMC Sensor B",
> streams);
> registerCamera(std::move(camera), std::move(data));
> @@ -417,6 +419,8 @@ int VimcCameraData::init(MediaDevice *media)
> if (video_->open())
> return -ENODEV;
>
> + stream_ = new Stream();
> +
> video_->bufferReady.connect(this, &VimcCameraData::bufferReady);
>
> raw_ = new V4L2VideoDevice(media->getEntityByName("Raw Capture 1"));
> --
> 2.23.0
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20191106/951cfda1/attachment-0001.sig>
More information about the libcamera-devel
mailing list