From patchwork Thu Feb 28 16:29:04 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 654 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 28A33610B6 for ; Thu, 28 Feb 2019 17:29:25 +0100 (CET) Received: from pendragon.bb.dnainternet.fi (81-175-216-236.bb.dnainternet.fi [81.175.216.236]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id B58CA67 for ; Thu, 28 Feb 2019 17:29:24 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1551371364; bh=6EeDSKKsb49gVuJmphPuRuJ8UiJkOHnfVhKXfaGXb2k=; h=From:To:Subject:Date:In-Reply-To:References:From; b=uq5AUYYEdecQK+KiuQho0ljJbNkeYpksSDYGA8hAbutGWkP2vLofsCWzt2CHx4fTW I5eEhCuMsPDvV366pzGUdDWZ82PBvJxHmDWfMQVBKuOFBglUkFYzipxXaWTksQxQgx ZaPSCsi96zIY8wvo8nq3r7gZUOlxYXuNJp1jsm48= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Thu, 28 Feb 2019 18:29:04 +0200 Message-Id: <20190228162913.6508-2-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.19.2 In-Reply-To: <20190228162913.6508-1-laurent.pinchart@ideasonboard.com> References: <20190228162913.6508-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 01/10] libcamera: pipeline: Fix double release of media devices X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 28 Feb 2019 16:29:25 -0000 Media devices are acquired in the match() function of pipeline handlers, and explicitly released if no match is found. The pipeline handler is then deleted, which causes a second release of the media device in the destructor. Fix this by removing the explicit release in the match() function. Signed-off-by: Laurent Pinchart Reviewed-by: Niklas Söderlund Acked-by: Jacopo Mondi --- src/libcamera/pipeline/ipu3/ipu3.cpp | 28 +++++++++++----------------- src/libcamera/pipeline/uvcvideo.cpp | 1 - src/libcamera/pipeline/vimc.cpp | 5 +---- 3 files changed, 12 insertions(+), 22 deletions(-) diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index 9694d0ce51ab..cf5c28577393 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -278,19 +278,20 @@ bool PipelineHandlerIPU3::match(DeviceEnumerator *enumerator) imgu_dm.add("ipu3-imgu 1 viewfinder"); imgu_dm.add("ipu3-imgu 1 3a stat"); + /* + * It is safe to acquire both media devices at this point as + * DeviceEnumerator::search() skips the busy ones for us. + */ cio2_ = enumerator->search(cio2_dm); if (!cio2_) return false; + cio2_->acquire(); + imgu_ = enumerator->search(imgu_dm); if (!imgu_) return false; - /* - * It is safe to acquire both media devices at this point as - * DeviceEnumerator::search() skips the busy ones for us. - */ - cio2_->acquire(); imgu_->acquire(); /* @@ -301,25 +302,18 @@ bool PipelineHandlerIPU3::match(DeviceEnumerator *enumerator) * not need to be changed after. */ if (cio2_->open()) - goto error_release_mdev; + return false; - if (cio2_->disableLinks()) - goto error_close_cio2; + if (cio2_->disableLinks()) { + cio2_->close(); + return false; + } registerCameras(); cio2_->close(); return true; - -error_close_cio2: - cio2_->close(); - -error_release_mdev: - cio2_->release(); - imgu_->release(); - - return false; } /* diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp index b7b8ff109109..dd20bb085a29 100644 --- a/src/libcamera/pipeline/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo.cpp @@ -167,7 +167,6 @@ bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator) if (!video_) LOG(UVC, Error) << "Could not find a default video device"; - media_->release(); return false; } diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp index 812777cffbb3..640fca5cb0e7 100644 --- a/src/libcamera/pipeline/vimc.cpp +++ b/src/libcamera/pipeline/vimc.cpp @@ -165,11 +165,8 @@ bool PipelineHandlerVimc::match(DeviceEnumerator *enumerator) media_->acquire(); video_ = new V4L2Device(media_->getEntityByName("Raw Capture 1")); - - if (video_->open()) { - media_->release(); + if (video_->open()) return false; - } std::vector streams{ &stream_ }; std::shared_ptr camera = Camera::create(this, "VIMC Sensor B", From patchwork Thu Feb 28 16:29:05 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 655 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 8CBEE610B6 for ; Thu, 28 Feb 2019 17:29:25 +0100 (CET) Received: from pendragon.bb.dnainternet.fi (81-175-216-236.bb.dnainternet.fi [81.175.216.236]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 1ABF349 for ; Thu, 28 Feb 2019 17:29:25 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1551371365; bh=7FvWZq179Oj/YrYcO2feox0TvkwfyhVFEakflXYIuUM=; h=From:To:Subject:Date:In-Reply-To:References:From; b=G+0+/CjZi3itgwHc9gRc3BDojj8kukj+cvmK65JIVydpZGIdontrWTt+EbBVCNmef xGu561UXf8chutohU7RD6VvB7VYprOUO+Js+5MGXMHHWdt+0KP2Ljr3+R7vhHOlV+6 kB6bLn4lXIeIcihA4gzCNcRavG0GHkD1YqF9RYEM= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Thu, 28 Feb 2019 18:29:05 +0200 Message-Id: <20190228162913.6508-3-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.19.2 In-Reply-To: <20190228162913.6508-1-laurent.pinchart@ideasonboard.com> References: <20190228162913.6508-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 02/10] libcamera: pipeline: uvcvideo: Create UVCCameraData X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 28 Feb 2019 16:29:25 -0000 Subclassing CameraData will become mandatory for pipeline handlers. Create a new UVCCameraData class and instantiate it when creating cameras to prepare for that change. The video_ and stream_ fields of the UVC pipeline handler belong to the camera data, move them there. Signed-off-by: Laurent Pinchart Reviewed-by: Niklas Söderlund --- src/libcamera/pipeline/uvcvideo.cpp | 74 +++++++++++++++++++++-------- 1 file changed, 53 insertions(+), 21 deletions(-) diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp index dd20bb085a29..4ad311163a95 100644 --- a/src/libcamera/pipeline/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo.cpp @@ -13,6 +13,7 @@ #include "log.h" #include "media_device.h" #include "pipeline_handler.h" +#include "utils.h" #include "v4l2_device.h" namespace libcamera { @@ -42,21 +43,38 @@ public: bool match(DeviceEnumerator *enumerator); private: + class UVCCameraData : public CameraData + { + public: + UVCCameraData() + { + } + + ~UVCCameraData() + { + delete video_; + } + + V4L2Device *video_; + Stream stream_; + }; + + UVCCameraData *cameraData(const Camera *camera) + { + return static_cast( + PipelineHandler::cameraData(camera)); + } + std::shared_ptr media_; - V4L2Device *video_; - Stream stream_; }; PipelineHandlerUVC::PipelineHandlerUVC(CameraManager *manager) - : PipelineHandler(manager), media_(nullptr), video_(nullptr) + : PipelineHandler(manager), media_(nullptr) { } PipelineHandlerUVC::~PipelineHandlerUVC() { - if (video_) - delete video_; - if (media_) media_->release(); } @@ -65,8 +83,9 @@ std::map PipelineHandlerUVC::streamConfiguration(Camera *camera, std::vector &streams) { + UVCCameraData *data = cameraData(camera); + std::map configs; - StreamConfiguration config{}; LOG(UVC, Debug) << "Retrieving default format"; @@ -75,7 +94,7 @@ PipelineHandlerUVC::streamConfiguration(Camera *camera, config.pixelFormat = V4L2_PIX_FMT_YUYV; config.bufferCount = 4; - configs[&stream_] = config; + configs[&data->stream_] = config; return configs; } @@ -83,7 +102,8 @@ PipelineHandlerUVC::streamConfiguration(Camera *camera, int PipelineHandlerUVC::configureStreams(Camera *camera, std::map &config) { - StreamConfiguration *cfg = &config[&stream_]; + UVCCameraData *data = cameraData(camera); + StreamConfiguration *cfg = &config[&data->stream_]; int ret; LOG(UVC, Debug) << "Configure the camera for resolution " @@ -94,7 +114,7 @@ int PipelineHandlerUVC::configureStreams(Camera *camera, format.height = cfg->height; format.fourcc = cfg->pixelFormat; - ret = video_->setFormat(&format); + ret = data->video_->setFormat(&format); if (ret) return ret; @@ -108,31 +128,36 @@ int PipelineHandlerUVC::configureStreams(Camera *camera, int PipelineHandlerUVC::allocateBuffers(Camera *camera, Stream *stream) { + UVCCameraData *data = cameraData(camera); const StreamConfiguration &cfg = stream->configuration(); LOG(UVC, Debug) << "Requesting " << cfg.bufferCount << " buffers"; - return video_->exportBuffers(&stream->bufferPool()); + return data->video_->exportBuffers(&stream->bufferPool()); } int PipelineHandlerUVC::freeBuffers(Camera *camera, Stream *stream) { - return video_->releaseBuffers(); + UVCCameraData *data = cameraData(camera); + return data->video_->releaseBuffers(); } int PipelineHandlerUVC::start(const Camera *camera) { - return video_->streamOn(); + UVCCameraData *data = cameraData(camera); + return data->video_->streamOn(); } void PipelineHandlerUVC::stop(const Camera *camera) { - video_->streamOff(); + UVCCameraData *data = cameraData(camera); + data->video_->streamOff(); } int PipelineHandlerUVC::queueRequest(const Camera *camera, Request *request) { - Buffer *buffer = request->findBuffer(&stream_); + UVCCameraData *data = cameraData(camera); + Buffer *buffer = request->findBuffer(&data->stream_); if (!buffer) { LOG(UVC, Error) << "Attempt to queue request with invalid stream"; @@ -140,7 +165,7 @@ int PipelineHandlerUVC::queueRequest(const Camera *camera, Request *request) return -ENOENT; } - video_->queueBuffer(buffer); + data->video_->queueBuffer(buffer); return 0; } @@ -150,29 +175,36 @@ bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator) DeviceMatch dm("uvcvideo"); media_ = enumerator->search(dm); - if (!media_) return false; media_->acquire(); + std::unique_ptr data = utils::make_unique(); + + /* Locate and open the default video node. */ for (MediaEntity *entity : media_->entities()) { if (entity->flags() & MEDIA_ENT_FL_DEFAULT) { - video_ = new V4L2Device(entity); + data->video_ = new V4L2Device(entity); break; } } - if (!video_ || video_->open()) { - if (!video_) + if (!data->video_ || data->video_->open()) { + if (!data->video_) LOG(UVC, Error) << "Could not find a default video device"; return false; } - std::vector streams{ &stream_ }; + /* Create and register the camera. */ + std::vector streams{ &data->stream_ }; std::shared_ptr camera = Camera::create(this, media_->model(), streams); + + setCameraData(camera.get(), std::move(data)); registerCamera(std::move(camera)); + + /* Enable hot-unplug notifications. */ hotplugMediaDevice(media_.get()); return true; From patchwork Thu Feb 28 16:29:06 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 656 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id E0B0D610C5 for ; Thu, 28 Feb 2019 17:29:25 +0100 (CET) Received: from pendragon.bb.dnainternet.fi (81-175-216-236.bb.dnainternet.fi [81.175.216.236]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 7787C67 for ; Thu, 28 Feb 2019 17:29:25 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1551371365; bh=hlkmkfRM9+S1PU/wHE3+cyeS34RAqRsKI2YBLlTpJME=; h=From:To:Subject:Date:In-Reply-To:References:From; b=rSrD91m0dEi2i2gy+eAjbjFzs0BOaTxSsSHhB6kznF0L7kpe6s0RQbpcs5ZpF39Q9 31UFp1munb0iYLt8gEX7fgiwCRFGr9WiCi2sinM00Exd2RXRNf/yb2LdxM/FmnqSUF FpkMVadwDmIx+DH0ntOcfsdtyw/8GH+e5sVdEnXI= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Thu, 28 Feb 2019 18:29:06 +0200 Message-Id: <20190228162913.6508-4-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.19.2 In-Reply-To: <20190228162913.6508-1-laurent.pinchart@ideasonboard.com> References: <20190228162913.6508-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 03/10] libcamera: pipeline: vimc: Create VimcCameraData X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 28 Feb 2019 16:29:26 -0000 Subclassing CameraData will become mandatory for pipeline handlers. Create a new VimcCameraData class and instantiate it when creating cameras to prepare for that change. The video_ and stream_ fields of the VIMC pipeline handler belong to the camera data, move them there. Signed-off-by: Laurent Pinchart Reviewed-by: Niklas Söderlund Reviewed-by: Jacopo Mondi --- src/libcamera/pipeline/vimc.cpp | 73 +++++++++++++++++++++++++-------- 1 file changed, 56 insertions(+), 17 deletions(-) diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp index 640fca5cb0e7..ef47e7f96436 100644 --- a/src/libcamera/pipeline/vimc.cpp +++ b/src/libcamera/pipeline/vimc.cpp @@ -13,6 +13,7 @@ #include "log.h" #include "media_device.h" #include "pipeline_handler.h" +#include "utils.h" #include "v4l2_device.h" namespace libcamera { @@ -42,20 +43,38 @@ public: bool match(DeviceEnumerator *enumerator); private: + class VimcCameraData : public CameraData + { + public: + VimcCameraData() + { + } + + ~VimcCameraData() + { + delete video_; + } + + V4L2Device *video_; + Stream stream_; + }; + + VimcCameraData *cameraData(const Camera *camera) + { + return static_cast( + PipelineHandler::cameraData(camera)); + } + std::shared_ptr media_; - V4L2Device *video_; - Stream stream_; }; PipelineHandlerVimc::PipelineHandlerVimc(CameraManager *manager) - : PipelineHandler(manager), media_(nullptr), video_(nullptr) + : PipelineHandler(manager), media_(nullptr) { } PipelineHandlerVimc::~PipelineHandlerVimc() { - delete video_; - if (media_) media_->release(); } @@ -64,6 +83,7 @@ std::map PipelineHandlerVimc::streamConfiguration(Camera *camera, std::vector &streams) { + VimcCameraData *data = cameraData(camera); std::map configs; StreamConfiguration config{}; @@ -74,7 +94,7 @@ PipelineHandlerVimc::streamConfiguration(Camera *camera, config.pixelFormat = V4L2_PIX_FMT_RGB24; config.bufferCount = 4; - configs[&stream_] = config; + configs[&data->stream_] = config; return configs; } @@ -82,7 +102,8 @@ PipelineHandlerVimc::streamConfiguration(Camera *camera, int PipelineHandlerVimc::configureStreams(Camera *camera, std::map &config) { - StreamConfiguration *cfg = &config[&stream_]; + VimcCameraData *data = cameraData(camera); + StreamConfiguration *cfg = &config[&data->stream_]; int ret; LOG(VIMC, Debug) << "Configure the camera for resolution " @@ -93,7 +114,7 @@ int PipelineHandlerVimc::configureStreams(Camera *camera, format.height = cfg->height; format.fourcc = cfg->pixelFormat; - ret = video_->setFormat(&format); + ret = data->video_->setFormat(&format); if (ret) return ret; @@ -107,31 +128,36 @@ int PipelineHandlerVimc::configureStreams(Camera *camera, int PipelineHandlerVimc::allocateBuffers(Camera *camera, Stream *stream) { + VimcCameraData *data = cameraData(camera); const StreamConfiguration &cfg = stream->configuration(); LOG(VIMC, Debug) << "Requesting " << cfg.bufferCount << " buffers"; - return video_->exportBuffers(&stream->bufferPool()); + return data->video_->exportBuffers(&stream->bufferPool()); } int PipelineHandlerVimc::freeBuffers(Camera *camera, Stream *stream) { - return video_->releaseBuffers(); + VimcCameraData *data = cameraData(camera); + return data->video_->releaseBuffers(); } int PipelineHandlerVimc::start(const Camera *camera) { - return video_->streamOn(); + VimcCameraData *data = cameraData(camera); + return data->video_->streamOn(); } void PipelineHandlerVimc::stop(const Camera *camera) { - video_->streamOff(); + VimcCameraData *data = cameraData(camera); + data->video_->streamOff(); } int PipelineHandlerVimc::queueRequest(const Camera *camera, Request *request) { - Buffer *buffer = request->findBuffer(&stream_); + VimcCameraData *data = cameraData(camera); + Buffer *buffer = request->findBuffer(&data->stream_); if (!buffer) { LOG(VIMC, Error) << "Attempt to queue request with invalid stream"; @@ -139,7 +165,7 @@ int PipelineHandlerVimc::queueRequest(const Camera *camera, Request *request) return -ENOENT; } - video_->queueBuffer(buffer); + data->video_->queueBuffer(buffer); return 0; } @@ -164,13 +190,26 @@ bool PipelineHandlerVimc::match(DeviceEnumerator *enumerator) media_->acquire(); - video_ = new V4L2Device(media_->getEntityByName("Raw Capture 1")); - if (video_->open()) + std::unique_ptr data = utils::make_unique(); + + /* Locate and open the capture video node. */ + MediaEntity *entity = media_->getEntityByName("Raw Capture 1"); + if (!entity) { + LOG(VIMC, Error) + << "Can't find 'Raw Capture 1' entity"; + return false; + } + + data->video_ = new V4L2Device(entity); + if (data->video_->open()) return false; - std::vector streams{ &stream_ }; + /* Create and register the camera. */ + std::vector streams{ &data->stream_ }; std::shared_ptr camera = Camera::create(this, "VIMC Sensor B", streams); + + setCameraData(camera.get(), std::move(data)); registerCamera(std::move(camera)); return true; From patchwork Thu Feb 28 16:29:07 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 657 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 3A8F4610BF for ; Thu, 28 Feb 2019 17:29:26 +0100 (CET) Received: from pendragon.bb.dnainternet.fi (81-175-216-236.bb.dnainternet.fi [81.175.216.236]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id D1DD749 for ; Thu, 28 Feb 2019 17:29:25 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1551371366; bh=KhL0VPGAZ282yG7zRGCD4VmASgKPaOaRe223Y9+kyO8=; h=From:To:Subject:Date:In-Reply-To:References:From; b=mHypCLESj4dDsOIQLUIcklRk4onavSrOdnZb/gTFrlX+LOOeaFNOVFGeLGAx661K1 nRV0NfmzfZfjsjPMWb1iKWswRCJkYXKOjCkpSFNxrk7cOdPIqqDUW/ysnGsj8pqs9p tKUQh+VrWVa2BBzOBdDVpge+jpgSWovB0bhgSaFE= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Thu, 28 Feb 2019 18:29:07 +0200 Message-Id: <20190228162913.6508-5-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.19.2 In-Reply-To: <20190228162913.6508-1-laurent.pinchart@ideasonboard.com> References: <20190228162913.6508-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 04/10] libcamera: pipeline_handler: Pass a non-const Camera to methods X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 28 Feb 2019 16:29:27 -0000 The start(), stop() and queueRequest() methods receive a const pointer to the related Camera object. The stop() request will need to modify the state of the camera, in order to report completion of pending requests. Un-constify the Camera pointer to that method, and update the start() and queueRequest() methods similarly for coherency. Signed-off-by: Laurent Pinchart Reviewed-by: Niklas Söderlund Reviewed-by: Jacopo Mondi --- src/libcamera/include/pipeline_handler.h | 6 +++--- src/libcamera/pipeline/ipu3/ipu3.cpp | 12 ++++++------ src/libcamera/pipeline/uvcvideo.cpp | 12 ++++++------ src/libcamera/pipeline/vimc.cpp | 12 ++++++------ 4 files changed, 21 insertions(+), 21 deletions(-) diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h index 4363dcd8ed8e..45e1f07ffca9 100644 --- a/src/libcamera/include/pipeline_handler.h +++ b/src/libcamera/include/pipeline_handler.h @@ -52,10 +52,10 @@ public: virtual int allocateBuffers(Camera *camera, Stream *stream) = 0; virtual int freeBuffers(Camera *camera, Stream *stream) = 0; - virtual int start(const Camera *camera) = 0; - virtual void stop(const Camera *camera) = 0; + virtual int start(Camera *camera) = 0; + virtual void stop(Camera *camera) = 0; - virtual int queueRequest(const Camera *camera, Request *request) = 0; + virtual int queueRequest(Camera *camera, Request *request) = 0; protected: void registerCamera(std::shared_ptr camera); diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index cf5c28577393..248e921117f4 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -39,10 +39,10 @@ public: int allocateBuffers(Camera *camera, Stream *stream) override; int freeBuffers(Camera *camera, Stream *stream) override; - int start(const Camera *camera) override; - void stop(const Camera *camera) override; + int start(Camera *camera) override; + void stop(Camera *camera) override; - int queueRequest(const Camera *camera, Request *request) override; + int queueRequest(Camera *camera, Request *request) override; bool match(DeviceEnumerator *enumerator); @@ -213,7 +213,7 @@ int PipelineHandlerIPU3::freeBuffers(Camera *camera, Stream *stream) return 0; } -int PipelineHandlerIPU3::start(const Camera *camera) +int PipelineHandlerIPU3::start(Camera *camera) { IPU3CameraData *data = cameraData(camera); int ret; @@ -227,7 +227,7 @@ int PipelineHandlerIPU3::start(const Camera *camera) return 0; } -void PipelineHandlerIPU3::stop(const Camera *camera) +void PipelineHandlerIPU3::stop(Camera *camera) { IPU3CameraData *data = cameraData(camera); @@ -235,7 +235,7 @@ void PipelineHandlerIPU3::stop(const Camera *camera) LOG(IPU3, Info) << "Failed to stop camera " << camera->name(); } -int PipelineHandlerIPU3::queueRequest(const Camera *camera, Request *request) +int PipelineHandlerIPU3::queueRequest(Camera *camera, Request *request) { IPU3CameraData *data = cameraData(camera); Stream *stream = &data->stream_; diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp index 4ad311163a95..cc513513cb34 100644 --- a/src/libcamera/pipeline/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo.cpp @@ -35,10 +35,10 @@ public: int allocateBuffers(Camera *camera, Stream *stream) override; int freeBuffers(Camera *camera, Stream *stream) override; - int start(const Camera *camera) override; - void stop(const Camera *camera) override; + int start(Camera *camera) override; + void stop(Camera *camera) override; - int queueRequest(const Camera *camera, Request *request) override; + int queueRequest(Camera *camera, Request *request) override; bool match(DeviceEnumerator *enumerator); @@ -142,19 +142,19 @@ int PipelineHandlerUVC::freeBuffers(Camera *camera, Stream *stream) return data->video_->releaseBuffers(); } -int PipelineHandlerUVC::start(const Camera *camera) +int PipelineHandlerUVC::start(Camera *camera) { UVCCameraData *data = cameraData(camera); return data->video_->streamOn(); } -void PipelineHandlerUVC::stop(const Camera *camera) +void PipelineHandlerUVC::stop(Camera *camera) { UVCCameraData *data = cameraData(camera); data->video_->streamOff(); } -int PipelineHandlerUVC::queueRequest(const Camera *camera, Request *request) +int PipelineHandlerUVC::queueRequest(Camera *camera, Request *request) { UVCCameraData *data = cameraData(camera); Buffer *buffer = request->findBuffer(&data->stream_); diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp index ef47e7f96436..1d01fa80f8d5 100644 --- a/src/libcamera/pipeline/vimc.cpp +++ b/src/libcamera/pipeline/vimc.cpp @@ -35,10 +35,10 @@ public: int allocateBuffers(Camera *camera, Stream *stream) override; int freeBuffers(Camera *camera, Stream *stream) override; - int start(const Camera *camera) override; - void stop(const Camera *camera) override; + int start(Camera *camera) override; + void stop(Camera *camera) override; - int queueRequest(const Camera *camera, Request *request) override; + int queueRequest(Camera *camera, Request *request) override; bool match(DeviceEnumerator *enumerator); @@ -142,19 +142,19 @@ int PipelineHandlerVimc::freeBuffers(Camera *camera, Stream *stream) return data->video_->releaseBuffers(); } -int PipelineHandlerVimc::start(const Camera *camera) +int PipelineHandlerVimc::start(Camera *camera) { VimcCameraData *data = cameraData(camera); return data->video_->streamOn(); } -void PipelineHandlerVimc::stop(const Camera *camera) +void PipelineHandlerVimc::stop(Camera *camera) { VimcCameraData *data = cameraData(camera); data->video_->streamOff(); } -int PipelineHandlerVimc::queueRequest(const Camera *camera, Request *request) +int PipelineHandlerVimc::queueRequest(Camera *camera, Request *request) { VimcCameraData *data = cameraData(camera); Buffer *buffer = request->findBuffer(&data->stream_); From patchwork Thu Feb 28 16:29:08 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 658 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id D51F0610C9 for ; Thu, 28 Feb 2019 17:29:26 +0100 (CET) Received: from pendragon.bb.dnainternet.fi (81-175-216-236.bb.dnainternet.fi [81.175.216.236]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 389F867 for ; Thu, 28 Feb 2019 17:29:26 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1551371366; bh=EuIVei7PUtsE3JHCMW6RkCvMQSdim9WFfEm+OkLaUkI=; h=From:To:Subject:Date:In-Reply-To:References:From; b=I28k7d44DenBdrtALsfUeFDho+8RKb/C0j4v9j31sik04hHrmMtf4nMaQdtChjwCf OPzNpIohVLfxdha5OefGUGNAHIwBSwfev/C2Xo9r6f9K7vyAKo/HbO6aeUu4t5U60v SPiEWrIMjaLAe7THX/vyFgHeQg+jx2/c4JV+FGFk= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Thu, 28 Feb 2019 18:29:08 +0200 Message-Id: <20190228162913.6508-6-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.19.2 In-Reply-To: <20190228162913.6508-1-laurent.pinchart@ideasonboard.com> References: <20190228162913.6508-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 05/10] libcamera: pipeline_handler: Store pipe and camera in CameraData X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 28 Feb 2019 16:29:27 -0000 Extend the CameraData class with two member variables pipe_ and camera_ that store pointers to the pipeline handler and camera that the CameraData instance is related to. This will be used by pipeline handlers to access the camera and the pipeline in member methods of their CameraData derived classes. Signed-off-by: Laurent Pinchart Reviewed-by: Niklas Söderlund --- src/libcamera/include/pipeline_handler.h | 9 ++++- src/libcamera/pipeline/ipu3/ipu3.cpp | 9 +++-- src/libcamera/pipeline/uvcvideo.cpp | 5 ++- src/libcamera/pipeline/vimc.cpp | 5 ++- src/libcamera/pipeline_handler.cpp | 49 +++++++++++++++++++++--- 5 files changed, 63 insertions(+), 14 deletions(-) diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h index 45e1f07ffca9..bc4da5820ac4 100644 --- a/src/libcamera/include/pipeline_handler.h +++ b/src/libcamera/include/pipeline_handler.h @@ -19,6 +19,7 @@ class Camera; class CameraManager; class DeviceEnumerator; class MediaDevice; +class PipelineHandler; class Request; class Stream; class StreamConfiguration; @@ -26,10 +27,14 @@ class StreamConfiguration; class CameraData { public: + explicit CameraData(PipelineHandler *pipe) + : pipe_(pipe) + { + } virtual ~CameraData() {} -protected: - CameraData() {} + Camera *camera_; + PipelineHandler *pipe_; private: CameraData(const CameraData &) = delete; diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index 248e921117f4..347ee657fddf 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -50,8 +50,11 @@ private: class IPU3CameraData : public CameraData { public: - IPU3CameraData() - : cio2_(nullptr), csi2_(nullptr), sensor_(nullptr) {} + IPU3CameraData(PipelineHandler *pipe) + : CameraData(pipe), cio2_(nullptr), csi2_(nullptr), + sensor_(nullptr) + { + } ~IPU3CameraData() { @@ -365,7 +368,7 @@ void PipelineHandlerIPU3::registerCameras() if (link->setEnabled(true)) continue; - std::unique_ptr data = utils::make_unique(); + std::unique_ptr data = utils::make_unique(this); std::string cameraName = sensor->name() + " " + std::to_string(id); std::vector streams{ &data->stream_ }; diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp index cc513513cb34..9af4838891f4 100644 --- a/src/libcamera/pipeline/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo.cpp @@ -46,7 +46,8 @@ private: class UVCCameraData : public CameraData { public: - UVCCameraData() + UVCCameraData(PipelineHandler *pipe) + : CameraData(pipe) { } @@ -180,7 +181,7 @@ bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator) media_->acquire(); - std::unique_ptr data = utils::make_unique(); + std::unique_ptr data = utils::make_unique(this); /* Locate and open the default video node. */ for (MediaEntity *entity : media_->entities()) { diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp index 1d01fa80f8d5..c4c1eb0dc19f 100644 --- a/src/libcamera/pipeline/vimc.cpp +++ b/src/libcamera/pipeline/vimc.cpp @@ -46,7 +46,8 @@ private: class VimcCameraData : public CameraData { public: - VimcCameraData() + VimcCameraData(PipelineHandler *pipe) + : CameraData(pipe) { } @@ -190,7 +191,7 @@ bool PipelineHandlerVimc::match(DeviceEnumerator *enumerator) media_->acquire(); - std::unique_ptr data = utils::make_unique(); + std::unique_ptr data = utils::make_unique(this); /* Locate and open the capture video node. */ MediaEntity *entity = media_->getEntityByName("Raw Capture 1"); diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp index c440aa8382ff..ac1acea5a318 100644 --- a/src/libcamera/pipeline_handler.cpp +++ b/src/libcamera/pipeline_handler.cpp @@ -11,6 +11,7 @@ #include "log.h" #include "media_device.h" #include "pipeline_handler.h" +#include "utils.h" /** * \file pipeline_handler.h @@ -44,6 +45,35 @@ LOG_DEFINE_CATEGORY(Pipeline) * with cameraData(). */ +/** + * \fn CameraData::CameraData(PipelineHandler *pipe) + * \brief Construct a CameraData instance for the given pipeline handler + * \param[in] pipe The pipeline handler + * + * The reference to the pipeline handler is stored internally, the caller shall + * guarantee that the pointer remains valid as long as the CameraData instance + * exists. + */ + +/** + * \var CameraData::camera_ + * \brief The camera related to this CameraData instance + * + * The camera_ pointer provides access to the Camera object that this instance + * is related to. It is set when the Camera is registered with + * PipelineHandler::registerCamera() and remains valid until the CameraData + * instance is destroyed. + */ + +/** + * \var CameraData::pipe_ + * \brief The pipeline handler related to this CameraData instance + * + * The pipe_ pointer provides access to the PipelineHandler object that this + * instance is related to. It is set when the CameraData instance is created + * and remains valid until the instance is destroyed. + */ + /** * \class PipelineHandler * \brief Create and manage cameras based on a set of media devices @@ -218,11 +248,19 @@ PipelineHandler::~PipelineHandler() * \brief Register a camera to the camera manager and pipeline handler * \param[in] camera The camera to be added * - * This function is called by pipeline handlers to register the cameras they - * handle with the camera manager. + * This method is called by pipeline handlers to register the cameras they + * handle with the camera manager. If no CameraData has been associated with + * the camera with setCameraData() by the pipeline handler, the method creates + * a default CameraData instance for the \a camera. */ void PipelineHandler::registerCamera(std::shared_ptr camera) { + if (!cameraData_.count(camera.get())) { + std::unique_ptr data = utils::make_unique(this); + setCameraData(camera.get(), std::move(data)); + } + + cameraData(camera.get())->camera_ = camera.get(); cameras_.push_back(camera); manager_->addCamera(std::move(camera)); } @@ -313,9 +351,10 @@ CameraData *PipelineHandler::cameraData(const Camera *camera) * information with \a camera. Ownership of \a data is transferred to * the PipelineHandler. * - * Pipeline-specific data can only be set once. Any attempt to call - * this method after the first one with the same camera won't change - * the pipeline-specific data. + * Pipeline-specific data can only be set once, and shall be set before + * registering the camera with registerCamera(). Any attempt to call this + * method more than once for the same camera, or to call it after registering + * the camera, will not change the pipeline-specific data. * * The data can be retrieved by pipeline handlers using the cameraData() method. */ From patchwork Thu Feb 28 16:29:09 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 659 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 29A99610C5 for ; Thu, 28 Feb 2019 17:29:27 +0100 (CET) Received: from pendragon.bb.dnainternet.fi (81-175-216-236.bb.dnainternet.fi [81.175.216.236]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 95CBB988 for ; Thu, 28 Feb 2019 17:29:26 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1551371366; bh=9tcO8ejHe9LiecA5bcaLEI7h/BG/tJna7/U9DP5oIUg=; h=From:To:Subject:Date:In-Reply-To:References:From; b=I4WF+UFaYwcBooVwQQUQHfIlFadIiNhLwbx5FJALnn9oZ0rOeJXOXPwdh3QPNiXgp 2vXMkkPWYx59tzx6qOwsg0Hk7RtW22qxbhKvl3uS+2QeHuGaqQSDHWIuzlN+4g3hQ/ cee3BwL54mhMYItQ91QN3A5+h/XCSQdd8mRx9B3k= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Thu, 28 Feb 2019 18:29:09 +0200 Message-Id: <20190228162913.6508-7-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.19.2 In-Reply-To: <20190228162913.6508-1-laurent.pinchart@ideasonboard.com> References: <20190228162913.6508-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 06/10] libcamera: pipeline_handler: Make pipeline-specific data mandatory X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 28 Feb 2019 16:29:27 -0000 Mandate creationg of pipeline-specific data by pipeline handlers. This allows simplifying the API by passing the pipeline-specific data to the registerCamera() method and removing the separate setCameraData() method. Signed-off-by: Laurent Pinchart Reviewed-by: Niklas Söderlund --- src/libcamera/include/pipeline_handler.h | 4 +- src/libcamera/pipeline/ipu3/ipu3.cpp | 3 +- src/libcamera/pipeline/uvcvideo.cpp | 4 +- src/libcamera/pipeline/vimc.cpp | 4 +- src/libcamera/pipeline_handler.cpp | 56 +++++------------------- 5 files changed, 15 insertions(+), 56 deletions(-) diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h index bc4da5820ac4..b2b9c94cebdc 100644 --- a/src/libcamera/include/pipeline_handler.h +++ b/src/libcamera/include/pipeline_handler.h @@ -63,11 +63,11 @@ public: virtual int queueRequest(Camera *camera, Request *request) = 0; protected: - void registerCamera(std::shared_ptr camera); + void registerCamera(std::shared_ptr camera, + std::unique_ptr data); void hotplugMediaDevice(MediaDevice *media); CameraData *cameraData(const Camera *camera); - void setCameraData(const Camera *camera, std::unique_ptr data); CameraManager *manager_; diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index 347ee657fddf..776b7f07f78d 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -406,8 +406,7 @@ void PipelineHandlerIPU3::registerCameras() if (ret) continue; - setCameraData(camera.get(), std::move(data)); - registerCamera(std::move(camera)); + registerCamera(std::move(camera), std::move(data)); LOG(IPU3, Info) << "Registered Camera[" << numCameras << "] \"" diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp index 9af4838891f4..f121d3c5633e 100644 --- a/src/libcamera/pipeline/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo.cpp @@ -201,9 +201,7 @@ bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator) /* Create and register the camera. */ std::vector streams{ &data->stream_ }; std::shared_ptr camera = Camera::create(this, media_->model(), streams); - - setCameraData(camera.get(), std::move(data)); - registerCamera(std::move(camera)); + registerCamera(std::move(camera), std::move(data)); /* Enable hot-unplug notifications. */ hotplugMediaDevice(media_.get()); diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp index c4c1eb0dc19f..6d022c37eb9f 100644 --- a/src/libcamera/pipeline/vimc.cpp +++ b/src/libcamera/pipeline/vimc.cpp @@ -209,9 +209,7 @@ bool PipelineHandlerVimc::match(DeviceEnumerator *enumerator) std::vector streams{ &data->stream_ }; std::shared_ptr camera = Camera::create(this, "VIMC Sensor B", streams); - - setCameraData(camera.get(), std::move(data)); - registerCamera(std::move(camera)); + registerCamera(std::move(camera), std::move(data)); return true; } diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp index ac1acea5a318..54f237942a48 100644 --- a/src/libcamera/pipeline_handler.cpp +++ b/src/libcamera/pipeline_handler.cpp @@ -247,20 +247,18 @@ PipelineHandler::~PipelineHandler() /** * \brief Register a camera to the camera manager and pipeline handler * \param[in] camera The camera to be added + * \param[in] data Pipeline-specific data for the camera * * This method is called by pipeline handlers to register the cameras they - * handle with the camera manager. If no CameraData has been associated with - * the camera with setCameraData() by the pipeline handler, the method creates - * a default CameraData instance for the \a camera. + * handle with the camera manager. It associates the pipeline-specific \a data + * with the camera, for later retrieval with cameraData(). Ownership of \a data + * is transferred to the PipelineHandler. */ -void PipelineHandler::registerCamera(std::shared_ptr camera) +void PipelineHandler::registerCamera(std::shared_ptr camera, + std::unique_ptr data) { - if (!cameraData_.count(camera.get())) { - std::unique_ptr data = utils::make_unique(this); - setCameraData(camera.get(), std::move(data)); - } - - cameraData(camera.get())->camera_ = camera.get(); + data->camera_ = camera.get(); + cameraData_[camera.get()] = std::move(data); cameras_.push_back(camera); manager_->addCamera(std::move(camera)); } @@ -325,51 +323,17 @@ void PipelineHandler::disconnect() * \brief Retrieve the pipeline-specific data associated with a Camera * \param camera The camera whose data to retrieve * - * \return A pointer to the pipeline-specific data set with setCameraData(). + * \return A pointer to the pipeline-specific data passed to registerCamera(). * The returned pointer is a borrowed reference and is guaranteed to remain * valid until the pipeline handler is destroyed. It shall not be deleted * manually by the caller. */ CameraData *PipelineHandler::cameraData(const Camera *camera) { - if (!cameraData_.count(camera)) { - LOG(Pipeline, Error) - << "Cannot get data associated with camera " - << camera->name(); - return nullptr; - } - + ASSERT(cameraData_.count(camera)); return cameraData_[camera].get(); } -/** - * \brief Set pipeline-specific data for the camera - * \param camera The camera to associate data to - * \param data The pipeline-specific data - * - * This method allows pipeline handlers to associate pipeline-specific - * information with \a camera. Ownership of \a data is transferred to - * the PipelineHandler. - * - * Pipeline-specific data can only be set once, and shall be set before - * registering the camera with registerCamera(). Any attempt to call this - * method more than once for the same camera, or to call it after registering - * the camera, will not change the pipeline-specific data. - * - * The data can be retrieved by pipeline handlers using the cameraData() method. - */ -void PipelineHandler::setCameraData(const Camera *camera, - std::unique_ptr data) -{ - if (cameraData_.find(camera) != cameraData_.end()) { - LOG(Pipeline, Error) - << "Replacing data associated with a camera is forbidden"; - return; - } - - cameraData_[camera] = std::move(data); -} - /** * \var PipelineHandler::manager_ * \brief The Camera manager associated with the pipeline handler From patchwork Thu Feb 28 16:29:10 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 660 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 767B9610B7 for ; Thu, 28 Feb 2019 17:29:27 +0100 (CET) Received: from pendragon.bb.dnainternet.fi (81-175-216-236.bb.dnainternet.fi [81.175.216.236]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 0E11549 for ; Thu, 28 Feb 2019 17:29:26 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1551371367; bh=OiouI0WIP4KokHgWBXtzSeN3x/+3HQnLsvei202zKR4=; h=From:To:Subject:Date:In-Reply-To:References:From; b=OOKIgQCeIGbXDeK9BuuL+WtAbQlITzslL0YQa1juoUAn4MvXVadYUpKN4IO7OyYuM EAie4tSdcvu4d4yYX5tIppEwoA3UsNzLMUz9ErsxRFYIlEwHo+cyFBUXNzFfqSsoai XVdDWHxjV7xLSAzB7sCfhGriB0GJEGpyqymrniMI= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Thu, 28 Feb 2019 18:29:10 +0200 Message-Id: <20190228162913.6508-8-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.19.2 In-Reply-To: <20190228162913.6508-1-laurent.pinchart@ideasonboard.com> References: <20190228162913.6508-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 07/10] libcamera: buffer: Add buffer completion status X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 28 Feb 2019 16:29:27 -0000 Add a new field to the Buffer class to report its completion status, with a new cancel() method to mark the buffer as cancelled. Signed-off-by: Laurent Pinchart Reviewed-by: Niklas Söderlund --- include/libcamera/buffer.h | 10 ++++++++++ src/libcamera/buffer.cpp | 35 +++++++++++++++++++++++++++++++++++ src/libcamera/v4l2_device.cpp | 2 ++ 3 files changed, 47 insertions(+) diff --git a/include/libcamera/buffer.h b/include/libcamera/buffer.h index dc9aaad12a81..f740ade9bb4f 100644 --- a/include/libcamera/buffer.h +++ b/include/libcamera/buffer.h @@ -40,12 +40,19 @@ private: class Buffer final { public: + enum Status { + BufferSuccess, + BufferError, + BufferCancelled, + }; + Buffer(); unsigned int index() const { return index_; } unsigned int bytesused() const { return bytesused_; } uint64_t timestamp() const { return timestamp_; } unsigned int sequence() const { return sequence_; } + Status status() const { return status_; } std::vector &planes() { return planes_; } Signal completed; @@ -54,10 +61,13 @@ private: friend class BufferPool; friend class V4L2Device; + void cancel(); + unsigned int index_; unsigned int bytesused_; uint64_t timestamp_; unsigned int sequence_; + Status status_; std::vector planes_; }; diff --git a/src/libcamera/buffer.cpp b/src/libcamera/buffer.cpp index 80dd9c854a4b..524eb47d4364 100644 --- a/src/libcamera/buffer.cpp +++ b/src/libcamera/buffer.cpp @@ -181,6 +181,20 @@ void *Plane::mem() * objects if the image format is multi-planar. */ +/** + * \enum Buffer::Status + * Buffer completion status + * \var Buffer::BufferSuccess + * The buffer has completed with success and contains valid data. All its other + * metadata (such as bytesused(), timestamp() or sequence() number) are valid. + * \var Buffer::BufferError + * The buffer has completed with an error and doesn't contain valid data. Its + * other metadata are valid. + * \var Buffer::BufferCancelled + * The buffer has been cancelled due to capture stop. Its other metadata are + * invalid and shall not be used. + */ + Buffer::Buffer() : index_(-1) { @@ -229,6 +243,27 @@ Buffer::Buffer() * \return Sequence number of the buffer */ +/** + * \fn Buffer::status() + * \brief Retrieve the buffer status + * + * The buffer status reports whether the buffer has completed successfully + * (BufferSuccess) or if an error occurred (BufferError). + * + * \return The buffer status + */ + +/** + * \brief Mark a buffer as cancel by setting its status to BufferCancelled + */ +void Buffer::cancel() +{ + bytesused_ = 0; + timestamp_ = 0; + sequence_ = 0; + status_ = BufferCancelled; +} + /** * \class BufferPool * \brief A pool of buffers diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp index 31200a6e7d6f..054499e4b888 100644 --- a/src/libcamera/v4l2_device.cpp +++ b/src/libcamera/v4l2_device.cpp @@ -793,6 +793,8 @@ Buffer *V4L2Device::dequeueBuffer() buffer->timestamp_ = buf.timestamp.tv_sec * 1000000000ULL + buf.timestamp.tv_usec * 1000ULL; buffer->sequence_ = buf.sequence; + buffer->status_ = buf.flags & V4L2_BUF_FLAG_ERROR + ? Buffer::BufferError : Buffer::BufferSuccess; return buffer; } From patchwork Thu Feb 28 16:29:11 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 661 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id C5762610B7 for ; Thu, 28 Feb 2019 17:29:27 +0100 (CET) Received: from pendragon.bb.dnainternet.fi (81-175-216-236.bb.dnainternet.fi [81.175.216.236]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 6878067 for ; Thu, 28 Feb 2019 17:29:27 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1551371367; bh=Z3eX5qXbTA306L8rPhCv+xNc6a2yWaBd65cagntz7hc=; h=From:To:Subject:Date:In-Reply-To:References:From; b=u5Cc0/YcXOsiM0eKpiJ6omXsRj8kzOdHYDnyh7Qse/xH/m1BCAvCeU3i7bW0NXUE0 n/bttSMxb4A6YJUnlGZN75ztLbS1enEHWIw3u/SNe2niSZK8YZ/xaGZrG/gqcYb943 RKyuEzAGhlu/v7ILZnzJcJrzvPUPD7aFgxuGVeM0= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Thu, 28 Feb 2019 18:29:11 +0200 Message-Id: <20190228162913.6508-9-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.19.2 In-Reply-To: <20190228162913.6508-1-laurent.pinchart@ideasonboard.com> References: <20190228162913.6508-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 08/10] libcamera: request: Add request completion status X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 28 Feb 2019 16:29:28 -0000 Add a new field to the Request class to report its completion status, and a new complete() method to update the status. Signed-off-by: Laurent Pinchart Reviewed-by: Niklas Söderlund --- include/libcamera/request.h | 12 +++++++++++ src/libcamera/request.cpp | 40 ++++++++++++++++++++++++++++++++++++- 2 files changed, 51 insertions(+), 1 deletion(-) diff --git a/include/libcamera/request.h b/include/libcamera/request.h index ef081177309f..0b75f9d9bd19 100644 --- a/include/libcamera/request.h +++ b/include/libcamera/request.h @@ -18,9 +18,16 @@ class Buffer; class Camera; class Stream; + class Request { public: + enum Status { + RequestPending, + RequestComplete, + RequestCancelled, + }; + explicit Request(Camera *camera); Request(const Request &) = delete; Request &operator=(const Request &) = delete; @@ -28,15 +35,20 @@ public: int setBuffers(const std::map &streamMap); Buffer *findBuffer(Stream *stream) const; + Status status() const { return status_; } + private: friend class Camera; int prepare(); + void complete(Status status); void bufferCompleted(Buffer *buffer); Camera *camera_; std::map bufferMap_; std::unordered_set pending_; + + Status status_; }; } /* namespace libcamera */ diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp index d76db24de0e2..cb170930fbb6 100644 --- a/src/libcamera/request.cpp +++ b/src/libcamera/request.cpp @@ -23,6 +23,17 @@ namespace libcamera { LOG_DEFINE_CATEGORY(Request) +/** + * \enum Request::Status + * Request completion status + * \var Request::RequestPending + * The request hasn't completed yet + * \var Request::RequestComplete + * The request has completed + * \var Request::RequestCancelled + * The request has been cancelled due to capture stop + */ + /** * \class Request * \brief A frame capture request @@ -36,7 +47,7 @@ LOG_DEFINE_CATEGORY(Request) * \param[in] camera The camera that creates the request */ Request::Request(Camera *camera) - : camera_(camera) + : camera_(camera), status_(RequestPending) { } @@ -82,6 +93,19 @@ Buffer *Request::findBuffer(Stream *stream) const return it->second; } +/** + * \fn Request::status() + * \brief Retrieve the request completion status + * + * The request status indicates whether the request has completed successfully + * or with an error. When requests are created and before they complete the + * request status is set to RequestPending, and is updated at completion time + * to RequestComplete. If a request is cancelled at capture stop before it has + * completed, its status is set to RequestCancelled. + * + * \return The request completion status + */ + /** * \brief Prepare the resources for the completion handler */ @@ -96,6 +120,18 @@ int Request::prepare() return 0; } +/** + * \brief Complete a queued request + * \param[in] status The request completion status + * + * Mark the request as complete by updating its status to \a status. + */ +void Request::complete(Status status) +{ + ASSERT(pending_.empty()); + status_ = status; +} + /** * \brief Slot for the buffer completed signal * @@ -117,6 +153,8 @@ void Request::bufferCompleted(Buffer *buffer) if (!pending_.empty()) return; + complete(RequestComplete); + std::map buffers(std::move(bufferMap_)); camera_->requestCompleted.emit(this, buffers); delete this; From patchwork Thu Feb 28 16:29:12 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 662 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 36F73610EF for ; Thu, 28 Feb 2019 17:29:28 +0100 (CET) Received: from pendragon.bb.dnainternet.fi (81-175-216-236.bb.dnainternet.fi [81.175.216.236]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id C1C4C49 for ; Thu, 28 Feb 2019 17:29:27 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1551371367; bh=1/4lmarB6jmPzfh5dpVsG/s3fBP8xRDX8j37UMQLCrU=; h=From:To:Subject:Date:In-Reply-To:References:From; b=mUKSw1CPqWrzHRHAtTMY8GfRXhuL0KOQHCfgL9FnYiyMXIVPT1DO6aQnNoRQ8iZW/ snoHDzEFZtRXo1gUWiz6xsp5VudFjrcjirBNrlOD6EQbpkPIOPXMcoVkZqVSmj5kUc Nb4jZ6nUW3elpJTrAMhUOgFD9JZNIugn9ImeGOEI= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Thu, 28 Feb 2019 18:29:12 +0200 Message-Id: <20190228162913.6508-10-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.19.2 In-Reply-To: <20190228162913.6508-1-laurent.pinchart@ideasonboard.com> References: <20190228162913.6508-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 09/10] libcamera: Handle request completion explicitly in pipeline handlers X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 28 Feb 2019 16:29:29 -0000 Request complete by themselves when all the buffers they contain have completed, connecting the buffer's completed signal to be notified of buffer completion. While this works for now, it prevents pipelines from delaying request completion until all metadata is available, and makes it impossible to ensure that requests complete in the order they are queued. To fix this, make request completion handling explicit in pipeline handlers. The base PipelineHandler class is extended with implementations of the queueRequest() and stop() methods and gets new completeBuffer() and completeRequest() methods to help pipeline handlers tracking requests and buffers. The three existing pipeline handlers connect the bufferReady signal of their capture video node to a slot of their respective camera data instance, where they use the PipelineHandler helpers to notify buffer and request completion. Request completion is handled synchronously with buffer completion as the pipeline handlers don't need to support more advanced use cases, but this paves the road for future work. Signed-off-by: Laurent Pinchart Reviewed-by: Niklas Söderlund --- include/libcamera/buffer.h | 5 +- include/libcamera/camera.h | 3 + include/libcamera/request.h | 4 +- src/libcamera/buffer.cpp | 5 -- src/libcamera/camera.cpp | 21 ++++++ src/libcamera/include/pipeline_handler.h | 10 ++- src/libcamera/pipeline/ipu3/ipu3.cpp | 18 ++++- src/libcamera/pipeline/uvcvideo.cpp | 19 ++++- src/libcamera/pipeline/vimc.cpp | 19 ++++- src/libcamera/pipeline_handler.cpp | 93 +++++++++++++++++++++++- src/libcamera/request.cpp | 30 +++----- src/libcamera/v4l2_device.cpp | 3 - 12 files changed, 192 insertions(+), 38 deletions(-) diff --git a/include/libcamera/buffer.h b/include/libcamera/buffer.h index f740ade9bb4f..0c844d126a27 100644 --- a/include/libcamera/buffer.h +++ b/include/libcamera/buffer.h @@ -10,8 +10,6 @@ #include #include -#include - namespace libcamera { class BufferPool; @@ -55,10 +53,9 @@ public: Status status() const { return status_; } std::vector &planes() { return planes_; } - Signal completed; - private: friend class BufferPool; + friend class PipelineHandler; friend class V4L2Device; void cancel(); diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h index bf70255a6a5e..74eca3d86094 100644 --- a/include/libcamera/camera.h +++ b/include/libcamera/camera.h @@ -34,6 +34,7 @@ public: const std::string &name() const; + Signal bufferCompleted; Signal &> requestCompleted; Signal disconnected; @@ -62,6 +63,8 @@ private: void disconnect(); int exclusiveAccess(); + void requestComplete(Request *request); + std::shared_ptr pipe_; std::string name_; std::vector streams_; diff --git a/include/libcamera/request.h b/include/libcamera/request.h index 0b75f9d9bd19..0dbd425115e8 100644 --- a/include/libcamera/request.h +++ b/include/libcamera/request.h @@ -39,10 +39,12 @@ public: private: friend class Camera; + friend class PipelineHandler; int prepare(); void complete(Status status); - void bufferCompleted(Buffer *buffer); + + bool completeBuffer(Buffer *buffer); Camera *camera_; std::map bufferMap_; diff --git a/src/libcamera/buffer.cpp b/src/libcamera/buffer.cpp index 524eb47d4364..e2d1cf04411e 100644 --- a/src/libcamera/buffer.cpp +++ b/src/libcamera/buffer.cpp @@ -212,11 +212,6 @@ Buffer::Buffer() * \return A reference to a vector holding all Planes within the buffer */ -/** - * \var Buffer::completed - * \brief A Signal to provide notifications that the specific Buffer is ready - */ - /** * \fn Buffer::bytesused() * \brief Retrieve the number of bytes occupied by the data in the buffer diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp index 3789732b95d1..3ef760943ff9 100644 --- a/src/libcamera/camera.cpp +++ b/src/libcamera/camera.cpp @@ -98,6 +98,12 @@ const std::string &Camera::name() const return name_; } +/** + * \var Camera::bufferCompleted + * \brief Signal emitted when a buffer for a request queued to the camera has + * completed + */ + /** * \var Camera::requestCompleted * \brief Signal emitted when a request queued to the camera has completed @@ -421,4 +427,19 @@ int Camera::exclusiveAccess() return 0; } +/** + * \brief Handle request completion and notify application + * \param[in] request The request that has completed + * + * This function is called by the pipeline handler to notify the camera that + * the request has completed. It emits the requestCompleted signal and deletes + * the request. + */ +void Camera::requestComplete(Request *request) +{ + std::map buffers(std::move(request->bufferMap_)); + requestCompleted.emit(request, buffers); + delete request; +} + } /* namespace libcamera */ diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h index b2b9c94cebdc..cbc953696816 100644 --- a/src/libcamera/include/pipeline_handler.h +++ b/src/libcamera/include/pipeline_handler.h @@ -7,6 +7,7 @@ #ifndef __LIBCAMERA_PIPELINE_HANDLER_H__ #define __LIBCAMERA_PIPELINE_HANDLER_H__ +#include #include #include #include @@ -14,6 +15,7 @@ namespace libcamera { +class Buffer; class BufferPool; class Camera; class CameraManager; @@ -35,6 +37,7 @@ public: Camera *camera_; PipelineHandler *pipe_; + std::list queuedRequests_; private: CameraData(const CameraData &) = delete; @@ -58,9 +61,12 @@ public: virtual int freeBuffers(Camera *camera, Stream *stream) = 0; virtual int start(Camera *camera) = 0; - virtual void stop(Camera *camera) = 0; + virtual void stop(Camera *camera); - virtual int queueRequest(Camera *camera, Request *request) = 0; + virtual int queueRequest(Camera *camera, Request *request); + + bool completeBuffer(Camera *camera, Request *request, Buffer *buffer); + void completeRequest(Camera *camera, Request *request); protected: void registerCamera(std::shared_ptr camera, diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index 776b7f07f78d..4695ec99470e 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -63,6 +63,8 @@ private: delete sensor_; } + void bufferReady(Buffer *buffer); + V4L2Device *cio2_; V4L2Subdevice *csi2_; V4L2Subdevice *sensor_; @@ -236,6 +238,8 @@ void PipelineHandlerIPU3::stop(Camera *camera) if (data->cio2_->streamOff()) LOG(IPU3, Info) << "Failed to stop camera " << camera->name(); + + PipelineHandler::stop(camera); } int PipelineHandlerIPU3::queueRequest(Camera *camera, Request *request) @@ -250,7 +254,11 @@ int PipelineHandlerIPU3::queueRequest(Camera *camera, Request *request) return -ENOENT; } - data->cio2_->queueBuffer(buffer); + int ret = data->cio2_->queueBuffer(buffer); + if (ret < 0) + return ret; + + PipelineHandler::queueRequest(camera, request); return 0; } @@ -417,6 +425,14 @@ void PipelineHandlerIPU3::registerCameras() } } +void PipelineHandlerIPU3::IPU3CameraData::bufferReady(Buffer *buffer) +{ + Request *request = queuedRequests_.front(); + + pipe_->completeBuffer(camera_, request, buffer); + pipe_->completeRequest(camera_, request); +} + REGISTER_PIPELINE_HANDLER(PipelineHandlerIPU3); } /* namespace libcamera */ diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp index f121d3c5633e..29c0c25ae7a8 100644 --- a/src/libcamera/pipeline/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo.cpp @@ -56,6 +56,8 @@ private: delete video_; } + void bufferReady(Buffer *buffer); + V4L2Device *video_; Stream stream_; }; @@ -153,6 +155,7 @@ void PipelineHandlerUVC::stop(Camera *camera) { UVCCameraData *data = cameraData(camera); data->video_->streamOff(); + PipelineHandler::stop(camera); } int PipelineHandlerUVC::queueRequest(Camera *camera, Request *request) @@ -166,7 +169,11 @@ int PipelineHandlerUVC::queueRequest(Camera *camera, Request *request) return -ENOENT; } - data->video_->queueBuffer(buffer); + int ret = data->video_->queueBuffer(buffer); + if (ret < 0) + return ret; + + PipelineHandler::queueRequest(camera, request); return 0; } @@ -198,6 +205,8 @@ bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator) return false; } + data->video_->bufferReady.connect(data.get(), &UVCCameraData::bufferReady); + /* Create and register the camera. */ std::vector streams{ &data->stream_ }; std::shared_ptr camera = Camera::create(this, media_->model(), streams); @@ -209,6 +218,14 @@ bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator) return true; } +void PipelineHandlerUVC::UVCCameraData::bufferReady(Buffer *buffer) +{ + Request *request = queuedRequests_.front(); + + pipe_->completeBuffer(camera_, request, buffer); + pipe_->completeRequest(camera_, request); +} + REGISTER_PIPELINE_HANDLER(PipelineHandlerUVC); } /* namespace libcamera */ diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp index 6d022c37eb9f..8c7c2d05828f 100644 --- a/src/libcamera/pipeline/vimc.cpp +++ b/src/libcamera/pipeline/vimc.cpp @@ -56,6 +56,8 @@ private: delete video_; } + void bufferReady(Buffer *buffer); + V4L2Device *video_; Stream stream_; }; @@ -153,6 +155,7 @@ void PipelineHandlerVimc::stop(Camera *camera) { VimcCameraData *data = cameraData(camera); data->video_->streamOff(); + PipelineHandler::stop(camera); } int PipelineHandlerVimc::queueRequest(Camera *camera, Request *request) @@ -166,7 +169,11 @@ int PipelineHandlerVimc::queueRequest(Camera *camera, Request *request) return -ENOENT; } - data->video_->queueBuffer(buffer); + int ret = data->video_->queueBuffer(buffer); + if (ret < 0) + return ret; + + PipelineHandler::queueRequest(camera, request); return 0; } @@ -205,6 +212,8 @@ bool PipelineHandlerVimc::match(DeviceEnumerator *enumerator) if (data->video_->open()) return false; + data->video_->bufferReady.connect(data.get(), &VimcCameraData::bufferReady); + /* Create and register the camera. */ std::vector streams{ &data->stream_ }; std::shared_ptr camera = Camera::create(this, "VIMC Sensor B", @@ -214,6 +223,14 @@ bool PipelineHandlerVimc::match(DeviceEnumerator *enumerator) return true; } +void PipelineHandlerVimc::VimcCameraData::bufferReady(Buffer *buffer) +{ + Request *request = queuedRequests_.front(); + + pipe_->completeBuffer(camera_, request, buffer); + pipe_->completeRequest(camera_, request); +} + REGISTER_PIPELINE_HANDLER(PipelineHandlerVimc); } /* namespace libcamera */ diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp index 54f237942a48..1a858f2638ce 100644 --- a/src/libcamera/pipeline_handler.cpp +++ b/src/libcamera/pipeline_handler.cpp @@ -5,6 +5,7 @@ * pipeline_handler.cpp - Pipeline handler infrastructure */ +#include #include #include @@ -74,6 +75,17 @@ LOG_DEFINE_CATEGORY(Pipeline) * and remains valid until the instance is destroyed. */ +/** + * \var CameraData::queuedRequests_ + * \brief The list of queued and not yet completed request + * + * The list of queued request is used to track requests queued in order to + * ensure completion of all requests when the pipeline handler is stopped. + * + * \sa PipelineHandler::queueRequest(), PipelineHandler::stop(), + * PipelineHandler::completeRequest() + */ + /** * \class PipelineHandler * \brief Create and manage cameras based on a set of media devices @@ -226,8 +238,30 @@ PipelineHandler::~PipelineHandler() * This method stops capturing and processing requests immediately. All pending * requests are cancelled and complete immediately in an error state. * - * \todo Complete the pending requests immediately + * Pipeline handlers shall override this method to stop the pipeline, ensure + * that all pending request completion signaled through completeRequest() have + * returned, and call the base implementation of the stop() method as the last + * step of their implementation. The base implementation cancels all requests + * queued but not yet complete. */ +void PipelineHandler::stop(Camera *camera) +{ + CameraData *data = cameraData(camera); + + while (!data->queuedRequests_.empty()) { + Request *request = data->queuedRequests_.front(); + data->queuedRequests_.pop_front(); + + while (!request->pending_.empty()) { + Buffer *buffer = *request->pending_.begin(); + buffer->cancel(); + completeBuffer(camera, request, buffer); + } + + request->complete(Request::RequestCancelled); + camera->requestComplete(request); + } +} /** * \fn PipelineHandler::queueRequest() @@ -241,8 +275,65 @@ PipelineHandler::~PipelineHandler() * 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 signaled 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) +{ + CameraData *data = cameraData(camera); + data->queuedRequests_.push_back(request); + + return 0; +} + +/** + * \brief Complete a buffer for a request + * \param[in] camera The camera the request belongs to + * \param[in] request The request the buffer belongs to + * \param[in] buffer The buffer that has completed + * + * This method shall be called by pipeline handlers to signal completion of the + * \a buffer part of the \a request. It notifies applications of buffer + * completion and updates the request's internal buffer tracking. The request + * is not completed automatically when the last buffer completes, pipeline + * handlers shall complete requests explicitly with completeRequest(). + * + * \return True if all buffers contained in the request have completed, false + * otherwise + */ +bool PipelineHandler::completeBuffer(Camera *camera, Request *request, + Buffer *buffer) +{ + camera->bufferCompleted.emit(request, buffer); + return request->completeBuffer(buffer); +} + +/** + * \brief Signal request completion + * \param[in] camera The camera that the request belongs to + * \param[in] request The request that has completed + * + * The pipeline handler shall call this method to notify the \a camera that the + * request request has complete. The request is deleted and shall not be + * accessed once this method returns. + * + * The pipeline handler shall ensure that requests complete in the same order + * they are submitted. + */ +void PipelineHandler::completeRequest(Camera *camera, Request *request) +{ + CameraData *data = cameraData(camera); + ASSERT(request == data->queuedRequests_.front()); + data->queuedRequests_.pop_front(); + + request->complete(Request::RequestComplete); + camera->requestComplete(request); +} /** * \brief Register a camera to the camera manager and pipeline handler diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp index cb170930fbb6..e0e77e972411 100644 --- a/src/libcamera/request.cpp +++ b/src/libcamera/request.cpp @@ -113,7 +113,6 @@ int Request::prepare() { for (auto const &pair : bufferMap_) { Buffer *buffer = pair.second; - buffer->completed.connect(this, &Request::bufferCompleted); pending_.insert(buffer); } @@ -133,31 +132,24 @@ void Request::complete(Status status) } /** - * \brief Slot for the buffer completed signal + * \brief Complete a buffer for the request + * \param[in] buffer The buffer that has completed * - * The bufferCompleted method serves as slot where to connect the - * Buffer::completed signal that is emitted when a buffer has available - * data. + * A request tracks the status of all buffers it contains through a set of + * pending buffers. This function removes the \a buffer from the set to mark it + * as complete. All buffers associate with the request shall be marked as + * complete by calling this function once and once only before reporting the + * request as complete with the complete() method. * - * The request completes when all the buffers it contains are ready to be - * presented to the application. It then emits the Camera::requestCompleted - * signal and is automatically deleted. + * \return True if all buffers contained in the request have completed, false + * otherwise */ -void Request::bufferCompleted(Buffer *buffer) +bool Request::completeBuffer(Buffer *buffer) { - buffer->completed.disconnect(this, &Request::bufferCompleted); - int ret = pending_.erase(buffer); ASSERT(ret == 1); - if (!pending_.empty()) - return; - - complete(RequestComplete); - - std::map buffers(std::move(bufferMap_)); - camera_->requestCompleted.emit(this, buffers); - delete this; + return pending_.empty(); } } /* namespace libcamera */ diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp index 054499e4b888..52167a3e047f 100644 --- a/src/libcamera/v4l2_device.cpp +++ b/src/libcamera/v4l2_device.cpp @@ -819,9 +819,6 @@ void V4L2Device::bufferAvailable(EventNotifier *notifier) /* Notify anyone listening to the device. */ bufferReady.emit(buffer); - - /* Notify anyone listening to the buffer specifically. */ - buffer->completed.emit(buffer); } /** From patchwork Thu Feb 28 16:29:13 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 663 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 8FF1E610F5 for ; Thu, 28 Feb 2019 17:29:28 +0100 (CET) Received: from pendragon.bb.dnainternet.fi (81-175-216-236.bb.dnainternet.fi [81.175.216.236]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 2A61E67 for ; Thu, 28 Feb 2019 17:29:28 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1551371368; bh=2544lptDAQRB451qgD7SxR37VsqdziLTuQ1XKbpMrmU=; h=From:To:Subject:Date:In-Reply-To:References:From; b=hxhMiVr2J2J5/L60pEN2nSPmFWRemmem1GIc8FFeVN6LlfSl15h4pcLDgSBozi7TY JP76WkYcYZO8kb9hfv6f/LEBS8M92RKx15LqNa9eGXMNj9TI676P4QlvGKHIqmc4jC Xvh7Qv7nHg9BFHZIP0++5npXrf14310Y/8VuL0Dc= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Thu, 28 Feb 2019 18:29:13 +0200 Message-Id: <20190228162913.6508-11-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.19.2 In-Reply-To: <20190228162913.6508-1-laurent.pinchart@ideasonboard.com> References: <20190228162913.6508-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 10/10] cam: Don't requeue requests when stopping stream X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 28 Feb 2019 16:29:29 -0000 When stopping the stream all pending requests are cancelled, resulting in the request completion signal being emitted with the request status set appropriately. Check the request status in the request completion slot and skip requeuing the request if it has been cancelled. Signed-off-by: Laurent Pinchart Reviewed-by: Niklas Söderlund --- src/cam/main.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/cam/main.cpp b/src/cam/main.cpp index 4c2df583fe8e..c8e673e30c0b 100644 --- a/src/cam/main.cpp +++ b/src/cam/main.cpp @@ -102,6 +102,9 @@ static void requestComplete(Request *request, const std::map { static uint64_t last = 0; + if (request->status() == Request::RequestCancelled) + return; + Buffer *buffer = buffers.begin()->second; double fps = buffer->timestamp() - last;