[{"id":950,"web_url":"https://patchwork.libcamera.org/comment/950/","msgid":"<20190228165958.GF899@bigcity.dyn.berto.se>","date":"2019-02-28T16:59:58","subject":"Re: [libcamera-devel] [PATCH 03/10] libcamera: pipeline: vimc:\n\tCreate VimcCameraData","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Laurent,\n\nThanks for your work.\n\nOn 2019-02-28 18:29:06 +0200, Laurent Pinchart wrote:\n> Subclassing CameraData will become mandatory for pipeline handlers.\n> Create a new VimcCameraData class and instantiate it when creating\n> cameras to prepare for that change. The video_ and stream_ fields of the\n> VIMC pipeline handler belong to the camera data, move them there.\n> \n> Signed-off-by: Laurent Pinchart \n> ---\n> src/libcamera/pipeline/vimc.cpp | 73 +++++++++++++++++++++++++--------\n> 1 file changed, 56 insertions(+), 17 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp\n> index 640fca5cb0e7..ef47e7f96436 100644\n> --- a/src/libcamera/pipeline/vimc.cpp\n> +++ b/src/libcamera/pipeline/vimc.cpp\n> @@ -13,6 +13,7 @@\n> #include \"log.h\"\n> #include \"media_device.h\"\n> #include \"pipeline_handler.h\"\n> +#include \"utils.h\"\n> #include \"v4l2_device.h\"\n> \n> namespace libcamera {\n> @@ -42,20 +43,38 @@ public:\n> \tbool match(DeviceEnumerator *enumerator);\n> \n> private:\n> +\tclass VimcCameraData : public CameraData\n> +\t{\n> +\tpublic:\n> +\t\tVimcCameraData()\n> +\t\t{\n> +\t\t}\n> +\n> +\t\t~VimcCameraData()\n> +\t\t{\n> +\t\t\tdelete video_;\n> +\t\t}\n> +\n> +\t\tV4L2Device *video_;\n> +\t\tStream stream_;\n> +\t};\n> +\n> +\tVimcCameraData *cameraData(const Camera *camera)\n> +\t{\n> +\t\treturn static_cast(\n> +\t\t\tPipelineHandler::cameraData(camera));\n> +\t}\n> +\n> \tstd::shared_ptr media_;\n> -\tV4L2Device *video_;\n> -\tStream stream_;\n> };\n> \n> PipelineHandlerVimc::PipelineHandlerVimc(CameraManager *manager)\n> -\t: PipelineHandler(manager), media_(nullptr), video_(nullptr)\n> +\t: PipelineHandler(manager), media_(nullptr)\n> {\n> }\n> \n> PipelineHandlerVimc::~PipelineHandlerVimc()\n> {\n> -\tdelete video_;\n> -\n> \tif (media_)\n> \t\tmedia_->release();\n> }\n> @@ -64,6 +83,7 @@ std::map\n> PipelineHandlerVimc::streamConfiguration(Camera *camera,\n> \t\t\t\t std::vector &streams)\n> {\n> +\tVimcCameraData *data = cameraData(camera);\n> \tstd::map configs;\n> \n> \tStreamConfiguration config{};\n> @@ -74,7 +94,7 @@ PipelineHandlerVimc::streamConfiguration(Camera *camera,\n> \tconfig.pixelFormat = V4L2_PIX_FMT_RGB24;\n> \tconfig.bufferCount = 4;\n> \n> -\tconfigs[&stream_] = config;\n> +\tconfigs[&data->stream_] = config;\n> \n> \treturn configs;\n> }\n> @@ -82,7 +102,8 @@ PipelineHandlerVimc::streamConfiguration(Camera *camera,\n> int PipelineHandlerVimc::configureStreams(Camera *camera,\n> \t\t\t\t std::map &config)\n> {\n> -\tStreamConfiguration *cfg = &config[&stream_];\n> +\tVimcCameraData *data = cameraData(camera);\n> +\tStreamConfiguration *cfg = &config[&data->stream_];\n> \tint ret;\n> \n> \tLOG(VIMC, Debug) << \"Configure the camera for resolution \"\n> @@ -93,7 +114,7 @@ int PipelineHandlerVimc::configureStreams(Camera *camera,\n> \tformat.height = cfg->height;\n> \tformat.fourcc = cfg->pixelFormat;\n> \n> -\tret = video_->setFormat(&format);\n> +\tret = data->video_->setFormat(&format);\n> \tif (ret)\n> \t\treturn ret;\n> \n> @@ -107,31 +128,36 @@ int PipelineHandlerVimc::configureStreams(Camera *camera,\n> \n> int PipelineHandlerVimc::allocateBuffers(Camera *camera, Stream *stream)\n> {\n> +\tVimcCameraData *data = cameraData(camera);\n> \tconst StreamConfiguration &cfg = stream->configuration();\n> \n> \tLOG(VIMC, Debug) << \"Requesting \" << cfg.bufferCount << \" buffers\";\n> \n> -\treturn video_->exportBuffers(&stream->bufferPool());\n> +\treturn data->video_->exportBuffers(&stream->bufferPool());\n> }\n> \n> int PipelineHandlerVimc::freeBuffers(Camera *camera, Stream *stream)\n> {\n> -\treturn video_->releaseBuffers();\n> +\tVimcCameraData *data = cameraData(camera);\n> +\treturn data->video_->releaseBuffers();\n> }\n> \n> int PipelineHandlerVimc::start(const Camera *camera)\n> {\n> -\treturn video_->streamOn();\n> +\tVimcCameraData *data = cameraData(camera);\n> +\treturn data->video_->streamOn();\n> }\n> \n> void PipelineHandlerVimc::stop(const Camera *camera)\n> {\n> -\tvideo_->streamOff();\n> +\tVimcCameraData *data = cameraData(camera);\n> +\tdata->video_->streamOff();\n> }\n> \n> int PipelineHandlerVimc::queueRequest(const Camera *camera, Request *request)\n> {\n> -\tBuffer *buffer = request->findBuffer(&stream_);\n> +\tVimcCameraData *data = cameraData(camera);\n> +\tBuffer *buffer = request->findBuffer(&data->stream_);\n> \tif (!buffer) {\n> \t\tLOG(VIMC, Error)\n> \t\t\t<< \"Attempt to queue request with invalid stream\";\n> @@ -139,7 +165,7 @@ int PipelineHandlerVimc::queueRequest(const Camera *camera, Request *request)\n> \t\treturn -ENOENT;\n> \t}\n> \n> -\tvideo_->queueBuffer(buffer);\n> +\tdata->video_->queueBuffer(buffer);\n> \n> \treturn 0;\n> }\n> @@ -164,13 +190,26 @@ bool PipelineHandlerVimc::match(DeviceEnumerator *enumerator)\n> \n> \tmedia_->acquire();\n> \n> -\tvideo_ = new V4L2Device(media_->getEntityByName(\"Raw Capture 1\"));\n> -\tif (video_->open())\n> +\tstd::unique_ptr data = utils::make_unique();\n> +\n> +\t/* Locate and open the capture video node. */\n> +\tMediaEntity *entity = media_->getEntityByName(\"Raw Capture 1\");\n> +\tif (!entity) {\n> +\t\tLOG(VIMC, Error)\n> +\t\t\t<< \"Can't find 'Raw Capture 1' entity\";\n> +\t\treturn false;\n> +\t}\n\nThis check is redundant as we add \"Raw Capture 1\" to the device matcher \nwe are guaranteed it exists in the media graph. With this check dropped,\n\nReviewed-by: Niklas Söderlund \n\n> +\n> +\tdata->video_ = new V4L2Device(entity);\n> +\tif (data->video_->open())\n> \t\treturn false;\n> \n> -\tstd::vector streams{ &stream_ };\n> +\t/* Create and register the camera. */\n> +\tstd::vector streams{ &data->stream_ };\n> \tstd::shared_ptr camera = Camera::create(this, \"VIMC Sensor B\",\n> \t\t\t\t\t\t\tstreams);\n> +\n> +\tsetCameraData(camera.get(), std::move(data));\n> \tregisterCamera(std::move(camera));\n> \n> \treturn true;\n> -- \n> Regards,\n> \n> Laurent Pinchart\n> \n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"","Received":["from mail-lj1-x22e.google.com (mail-lj1-x22e.google.com\n\t[IPv6:2a00:1450:4864:20::22e])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id ED699610B6\n\tfor ;\n\tThu, 28 Feb 2019 18:00:00 +0100 (CET)","by mail-lj1-x22e.google.com with SMTP id q128so17755758ljb.11\n\tfor ;\n\tThu, 28 Feb 2019 09:00:00 -0800 (PST)","from localhost (89-233-230-99.cust.bredband2.com. [89.233.230.99])\n\tby smtp.gmail.com with ESMTPSA id\n\th101sm793468ljh.80.2019.02.28.08.59.59\n\t(version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);\n\tThu, 28 Feb 2019 08:59:59 -0800 (PST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to\n\t:user-agent; bh=Orb2V2RC2KOrS1K1tFWC0cRg5m95I+gkWswPv/6yNwA=;\n\tb=m8uB4UuPGpCUTiYk3l7Sjypo7VNx5LqFzu+50UccXY8t4YVasx2bONASS55KBgwReY\n\twA1Lctapk5kxSGoz0OEoOZeKTPSpptjKnVe+91dCf1aIJDQtTSAtt+I2IGMSNS8K190E\n\t93Q7WDs4+qGxCxP5BTqKJrKx6RJDtnjWfbR53dsasXrGTCX7DFpZWx6lWKZTG3EjxEza\n\tJAVa1/UlBp1++Y0js23gcITW4AQExNE3hDMkwbMiaDvUT350R3RNlRo5v1Sux2zZ1IUA\n\tatLFlkYq15/a4HHycVueY5xHDRvWx8LsOw1aROrn5c81suJTxa6u9sPu805x+qz1mVq2\n\tabmA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to:user-agent;\n\tbh=Orb2V2RC2KOrS1K1tFWC0cRg5m95I+gkWswPv/6yNwA=;\n\tb=Zn5DHIRkna4QnknvDV5WzoMv9FdcqqUDnih90H4VUSNQ7Ok+GctEnspPN41SLHocXJ\n\te4wzcNPOg6YAR96Fl2vImfppjCz3dkJ7OHX5AoO5ZX0uf1xbM4NX6ue0KytbkLdQEseT\n\tx51WGif4goIeXpVMQP8PLxd7PjMCGDS2AYcr2mhGgxU8qwlpL5/tj1nwHimNp4bR9rjc\n\tBO/8jN7cqLT8LieCUu+Mznq8lQcyF2gDfMz3/ALFa8IEoHvqpDtysW+gcrasZDK0WR/Q\n\tDSVOSNCCHGQD4d4JJQpDKTtv8Py6g+4CCGpmhRGR+7CU5AJoj7vGKp6wIddmyHVsgzSZ\n\th4Qw==","X-Gm-Message-State":"AHQUAuYFG3LIrHymBp7hW8wkwXpqPdvASJp5pPae+zl5zsTsac317PlP\n\t8EF9F9RvEH1wl2sZc2T2HKHEPoK2ouw=","X-Google-Smtp-Source":"APXvYqxHN1HuisKfZaJ0Ef3/ssShpE0TIbd5DuoDYaeG8tfPYxotLaVxw4OTapqmBYhFWhxMW/AGzA==","X-Received":"by 2002:a2e:9793:: with SMTP id\n\ty19mr5247538lji.194.1551373200215; \n\tThu, 28 Feb 2019 09:00:00 -0800 (PST)","Date":"Thu, 28 Feb 2019 17:59:58 +0100","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190228165958.GF899@bigcity.dyn.berto.se>","References":"<20190228162913.6508-1-laurent.pinchart@ideasonboard.com>\n\t<20190228162913.6508-4-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20190228162913.6508-4-laurent.pinchart@ideasonboard.com>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH 03/10] libcamera: pipeline: vimc:\n\tCreate VimcCameraData","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Thu, 28 Feb 2019 17:00:01 -0000"}},{"id":998,"web_url":"https://patchwork.libcamera.org/comment/998/","msgid":"<20190301130917.cypft7wyfcalq6yx@uno.localdomain>","date":"2019-03-01T13:09:17","subject":"Re: [libcamera-devel] [PATCH 03/10] libcamera: pipeline: vimc:\n\tCreate VimcCameraData","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"HI Laurent,\n\nOn Thu, Feb 28, 2019 at 06:29:06PM +0200, Laurent Pinchart wrote:\n> Subclassing CameraData will become mandatory for pipeline handlers.\n> Create a new VimcCameraData class and instantiate it when creating\n> cameras to prepare for that change. The video_ and stream_ fields of the\n> VIMC pipeline handler belong to the camera data, move them there.\n>\n> Signed-off-by: Laurent Pinchart \n\nWith Niklas' comment addressed:\nReviewed-by: Jacopo Mondi \n\nThanks\n j\n> ---\n> src/libcamera/pipeline/vimc.cpp | 73 +++++++++++++++++++++++++--------\n> 1 file changed, 56 insertions(+), 17 deletions(-)\n>\n> diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp\n> index 640fca5cb0e7..ef47e7f96436 100644\n> --- a/src/libcamera/pipeline/vimc.cpp\n> +++ b/src/libcamera/pipeline/vimc.cpp\n> @@ -13,6 +13,7 @@\n> #include \"log.h\"\n> #include \"media_device.h\"\n> #include \"pipeline_handler.h\"\n> +#include \"utils.h\"\n> #include \"v4l2_device.h\"\n>\n> namespace libcamera {\n> @@ -42,20 +43,38 @@ public:\n> \tbool match(DeviceEnumerator *enumerator);\n>\n> private:\n> +\tclass VimcCameraData : public CameraData\n> +\t{\n> +\tpublic:\n> +\t\tVimcCameraData()\n> +\t\t{\n> +\t\t}\n> +\n> +\t\t~VimcCameraData()\n> +\t\t{\n> +\t\t\tdelete video_;\n> +\t\t}\n> +\n> +\t\tV4L2Device *video_;\n> +\t\tStream stream_;\n> +\t};\n> +\n> +\tVimcCameraData *cameraData(const Camera *camera)\n> +\t{\n> +\t\treturn static_cast(\n> +\t\t\tPipelineHandler::cameraData(camera));\n> +\t}\n> +\n> \tstd::shared_ptr media_;\n> -\tV4L2Device *video_;\n> -\tStream stream_;\n> };\n>\n> PipelineHandlerVimc::PipelineHandlerVimc(CameraManager *manager)\n> -\t: PipelineHandler(manager), media_(nullptr), video_(nullptr)\n> +\t: PipelineHandler(manager), media_(nullptr)\n> {\n> }\n>\n> PipelineHandlerVimc::~PipelineHandlerVimc()\n> {\n> -\tdelete video_;\n> -\n> \tif (media_)\n> \t\tmedia_->release();\n> }\n> @@ -64,6 +83,7 @@ std::map\n> PipelineHandlerVimc::streamConfiguration(Camera *camera,\n> \t\t\t\t std::vector &streams)\n> {\n> +\tVimcCameraData *data = cameraData(camera);\n> \tstd::map configs;\n>\n> \tStreamConfiguration config{};\n> @@ -74,7 +94,7 @@ PipelineHandlerVimc::streamConfiguration(Camera *camera,\n> \tconfig.pixelFormat = V4L2_PIX_FMT_RGB24;\n> \tconfig.bufferCount = 4;\n>\n> -\tconfigs[&stream_] = config;\n> +\tconfigs[&data->stream_] = config;\n>\n> \treturn configs;\n> }\n> @@ -82,7 +102,8 @@ PipelineHandlerVimc::streamConfiguration(Camera *camera,\n> int PipelineHandlerVimc::configureStreams(Camera *camera,\n> \t\t\t\t std::map &config)\n> {\n> -\tStreamConfiguration *cfg = &config[&stream_];\n> +\tVimcCameraData *data = cameraData(camera);\n> +\tStreamConfiguration *cfg = &config[&data->stream_];\n> \tint ret;\n>\n> \tLOG(VIMC, Debug) << \"Configure the camera for resolution \"\n> @@ -93,7 +114,7 @@ int PipelineHandlerVimc::configureStreams(Camera *camera,\n> \tformat.height = cfg->height;\n> \tformat.fourcc = cfg->pixelFormat;\n>\n> -\tret = video_->setFormat(&format);\n> +\tret = data->video_->setFormat(&format);\n> \tif (ret)\n> \t\treturn ret;\n>\n> @@ -107,31 +128,36 @@ int PipelineHandlerVimc::configureStreams(Camera *camera,\n>\n> int PipelineHandlerVimc::allocateBuffers(Camera *camera, Stream *stream)\n> {\n> +\tVimcCameraData *data = cameraData(camera);\n> \tconst StreamConfiguration &cfg = stream->configuration();\n>\n> \tLOG(VIMC, Debug) << \"Requesting \" << cfg.bufferCount << \" buffers\";\n>\n> -\treturn video_->exportBuffers(&stream->bufferPool());\n> +\treturn data->video_->exportBuffers(&stream->bufferPool());\n> }\n>\n> int PipelineHandlerVimc::freeBuffers(Camera *camera, Stream *stream)\n> {\n> -\treturn video_->releaseBuffers();\n> +\tVimcCameraData *data = cameraData(camera);\n> +\treturn data->video_->releaseBuffers();\n> }\n>\n> int PipelineHandlerVimc::start(const Camera *camera)\n> {\n> -\treturn video_->streamOn();\n> +\tVimcCameraData *data = cameraData(camera);\n> +\treturn data->video_->streamOn();\n> }\n>\n> void PipelineHandlerVimc::stop(const Camera *camera)\n> {\n> -\tvideo_->streamOff();\n> +\tVimcCameraData *data = cameraData(camera);\n> +\tdata->video_->streamOff();\n> }\n>\n> int PipelineHandlerVimc::queueRequest(const Camera *camera, Request *request)\n> {\n> -\tBuffer *buffer = request->findBuffer(&stream_);\n> +\tVimcCameraData *data = cameraData(camera);\n> +\tBuffer *buffer = request->findBuffer(&data->stream_);\n> \tif (!buffer) {\n> \t\tLOG(VIMC, Error)\n> \t\t\t<< \"Attempt to queue request with invalid stream\";\n> @@ -139,7 +165,7 @@ int PipelineHandlerVimc::queueRequest(const Camera *camera, Request *request)\n> \t\treturn -ENOENT;\n> \t}\n>\n> -\tvideo_->queueBuffer(buffer);\n> +\tdata->video_->queueBuffer(buffer);\n>\n> \treturn 0;\n> }\n> @@ -164,13 +190,26 @@ bool PipelineHandlerVimc::match(DeviceEnumerator *enumerator)\n>\n> \tmedia_->acquire();\n>\n> -\tvideo_ = new V4L2Device(media_->getEntityByName(\"Raw Capture 1\"));\n> -\tif (video_->open())\n> +\tstd::unique_ptr data = utils::make_unique();\n> +\n> +\t/* Locate and open the capture video node. */\n> +\tMediaEntity *entity = media_->getEntityByName(\"Raw Capture 1\");\n> +\tif (!entity) {\n> +\t\tLOG(VIMC, Error)\n> +\t\t\t<< \"Can't find 'Raw Capture 1' entity\";\n> +\t\treturn false;\n> +\t}\n> +\n> +\tdata->video_ = new V4L2Device(entity);\n> +\tif (data->video_->open())\n> \t\treturn false;\n>\n> -\tstd::vector streams{ &stream_ };\n> +\t/* Create and register the camera. */\n> +\tstd::vector streams{ &data->stream_ };\n> \tstd::shared_ptr camera = Camera::create(this, \"VIMC Sensor B\",\n> \t\t\t\t\t\t\tstreams);\n> +\n> +\tsetCameraData(camera.get(), std::move(data));\n> \tregisterCamera(std::move(camera));\n>\n> \treturn true;\n> --\n> Regards,\n>\n> Laurent Pinchart\n>\n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"","Received":["from relay1-d.mail.gandi.net (relay1-d.mail.gandi.net\n\t[217.70.183.193])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5E1B7610BF\n\tfor ;\n\tFri, 1 Mar 2019 14:08:49 +0100 (CET)","from uno.localdomain (2-224-242-101.ip172.fastwebnet.it\n\t[2.224.242.101]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay1-d.mail.gandi.net (Postfix) with ESMTPSA id CD0F4240006;\n\tFri, 1 Mar 2019 13:08:47 +0000 (UTC)"],"X-Originating-IP":"2.224.242.101","Date":"Fri, 1 Mar 2019 14:09:17 +0100","From":"Jacopo Mondi ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190301130917.cypft7wyfcalq6yx@uno.localdomain>","References":"<20190228162913.6508-1-laurent.pinchart@ideasonboard.com>\n\t<20190228162913.6508-4-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"iuzchcrsj2cmsylg\"","Content-Disposition":"inline","In-Reply-To":"<20190228162913.6508-4-laurent.pinchart@ideasonboard.com>","User-Agent":"NeoMutt/20180716","Subject":"Re: [libcamera-devel] [PATCH 03/10] libcamera: pipeline: vimc:\n\tCreate VimcCameraData","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Fri, 01 Mar 2019 13:08:49 -0000"}}]