[{"id":3370,"web_url":"https://patchwork.libcamera.org/comment/3370/","msgid":"<20200107192504.GO533370@oden.dyn.berto.se>","date":"2020-01-07T19:25:04","subject":"Re: [libcamera-devel] [PATCH 14/14] libcamera: v4l2: Use\n\tObject::invokeMethod() return value","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Laurent,\n\nThank for your work.\n\nOn 2020-01-04 07:09:47 +0200, Laurent Pinchart wrote:\n> Now that Object::invokeMethod() supports returning a value, use it and\n> drop the return value method argument.\n> \n> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n\nLooks much nicer,\n\nReviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n\n> ---\n>  src/v4l2/v4l2_camera.cpp       | 109 +++++++++++++++------------------\n>  src/v4l2/v4l2_camera.h         |  18 +++---\n>  src/v4l2/v4l2_camera_proxy.cpp |  51 ++++++++-------\n>  3 files changed, 82 insertions(+), 96 deletions(-)\n> \n> diff --git a/src/v4l2/v4l2_camera.cpp b/src/v4l2/v4l2_camera.cpp\n> index 3590730fde08..4545483cf1c7 100644\n> --- a/src/v4l2/v4l2_camera.cpp\n> +++ b/src/v4l2/v4l2_camera.cpp\n> @@ -34,23 +34,21 @@ V4L2Camera::~V4L2Camera()\n>  \tcamera_->release();\n>  }\n>  \n> -void V4L2Camera::open(int *ret)\n> +int V4L2Camera::open()\n>  {\n>  \t/* \\todo Support multiple open. */\n>  \tif (camera_->acquire() < 0) {\n>  \t\tLOG(V4L2Compat, Error) << \"Failed to acquire camera\";\n> -\t\t*ret = -EINVAL;\n> -\t\treturn;\n> +\t\treturn -EINVAL;\n>  \t}\n>  \n>  \tconfig_ = camera_->generateConfiguration({ StreamRole::Viewfinder });\n>  \tif (!config_) {\n>  \t\tcamera_->release();\n> -\t\t*ret = -EINVAL;\n> -\t\treturn;\n> +\t\treturn -EINVAL;\n>  \t}\n>  \n> -\t*ret = 0;\n> +\treturn 0;\n>  }\n>  \n>  void V4L2Camera::close()\n> @@ -92,9 +90,9 @@ void V4L2Camera::requestComplete(Request *request)\n>  \tbufferSema_.release();\n>  }\n>  \n> -void V4L2Camera::configure(int *ret, StreamConfiguration *streamConfigOut,\n> -\t\t\t   const Size &size, PixelFormat pixelformat,\n> -\t\t\t   unsigned int bufferCount)\n> +int V4L2Camera::configure(StreamConfiguration *streamConfigOut,\n> +\t\t\t  const Size &size, PixelFormat pixelformat,\n> +\t\t\t  unsigned int bufferCount)\n>  {\n>  \tStreamConfiguration &streamConfig = config_->at(0);\n>  \tstreamConfig.size.width = size.width;\n> @@ -106,8 +104,7 @@ void V4L2Camera::configure(int *ret, StreamConfiguration *streamConfigOut,\n>  \tCameraConfiguration::Status validation = config_->validate();\n>  \tif (validation == CameraConfiguration::Invalid) {\n>  \t\tLOG(V4L2Compat, Debug) << \"Configuration invalid\";\n> -\t\t*ret = -EINVAL;\n> -\t\treturn;\n> +\t\treturn -EINVAL;\n>  \t}\n>  \tif (validation == CameraConfiguration::Adjusted)\n>  \t\tLOG(V4L2Compat, Debug) << \"Configuration adjusted\";\n> @@ -115,24 +112,25 @@ void V4L2Camera::configure(int *ret, StreamConfiguration *streamConfigOut,\n>  \tLOG(V4L2Compat, Debug) << \"Validated configuration is: \"\n>  \t\t\t      << streamConfig.toString();\n>  \n> -\t*ret = camera_->configure(config_.get());\n> -\tif (*ret < 0)\n> -\t\treturn;\n> +\tint ret = camera_->configure(config_.get());\n> +\tif (ret < 0)\n> +\t\treturn ret;\n>  \n>  \t*streamConfigOut = config_->at(0);\n> +\n> +\treturn 0;\n>  }\n>  \n> -void V4L2Camera::mmap(void **ret, unsigned int index)\n> +void *V4L2Camera::mmap(unsigned int index)\n>  {\n>  \tStream *stream = *camera_->streams().begin();\n> -\t*ret = stream->buffers()[index].planes()[0].mem();\n> +\treturn stream->buffers()[index].planes()[0].mem();\n>  }\n>  \n> -void V4L2Camera::allocBuffers(int *ret, unsigned int count)\n> +int V4L2Camera::allocBuffers(unsigned int count)\n>  {\n> -\t*ret = camera_->allocateBuffers();\n> -\tif (*ret == -EACCES)\n> -\t\t*ret = -EBUSY;\n> +\tint ret = camera_->allocateBuffers();\n> +\treturn ret == -EACCES ? -EBUSY : ret;\n>  }\n>  \n>  void V4L2Camera::freeBuffers()\n> @@ -140,85 +138,76 @@ void V4L2Camera::freeBuffers()\n>  \tcamera_->freeBuffers();\n>  }\n>  \n> -void V4L2Camera::streamOn(int *ret)\n> +int V4L2Camera::streamOn()\n>  {\n> -\t*ret = 0;\n> -\n>  \tif (isRunning_)\n> -\t\treturn;\n> +\t\treturn 0;\n> +\n> +\tint ret = camera_->start();\n> +\tif (ret < 0)\n> +\t\treturn ret == -EACCES ? -EBUSY : ret;\n>  \n> -\t*ret = camera_->start();\n> -\tif (*ret < 0) {\n> -\t\tif (*ret == -EACCES)\n> -\t\t\t*ret = -EBUSY;\n> -\t\treturn;\n> -\t}\n>  \tisRunning_ = true;\n>  \n>  \tfor (std::unique_ptr<Request> &req : pendingRequests_) {\n>  \t\t/* \\todo What should we do if this returns -EINVAL? */\n> -\t\t*ret = camera_->queueRequest(req.release());\n> -\t\tif (*ret < 0) {\n> -\t\t\tif (*ret == -EACCES)\n> -\t\t\t\t*ret = -EBUSY;\n> -\t\t\treturn;\n> -\t\t}\n> +\t\tret = camera_->queueRequest(req.release());\n> +\t\tif (ret < 0)\n> +\t\t\treturn ret == -EACCES ? -EBUSY : ret;\n>  \t}\n>  \n>  \tpendingRequests_.clear();\n> +\n> +\treturn 0;\n>  }\n>  \n> -void V4L2Camera::streamOff(int *ret)\n> +int V4L2Camera::streamOff()\n>  {\n> -\t*ret = 0;\n> -\n>  \t/* \\todo Restore buffers to reqbufs state? */\n>  \tif (!isRunning_)\n> -\t\treturn;\n> +\t\treturn 0;\n> +\n> +\tint ret = camera_->stop();\n> +\tif (ret < 0)\n> +\t\treturn ret == -EACCES ? -EBUSY : ret;\n>  \n> -\t*ret = camera_->stop();\n> -\tif (*ret < 0) {\n> -\t\tif (*ret == -EACCES)\n> -\t\t\t*ret = -EBUSY;\n> -\t\treturn;\n> -\t}\n>  \tisRunning_ = false;\n> +\n> +\treturn 0;\n>  }\n>  \n> -void V4L2Camera::qbuf(int *ret, unsigned int index)\n> +int V4L2Camera::qbuf(unsigned int index)\n>  {\n>  \tStream *stream = config_->at(0).stream();\n>  \tstd::unique_ptr<Buffer> buffer = stream->createBuffer(index);\n>  \tif (!buffer) {\n>  \t\tLOG(V4L2Compat, Error) << \"Can't create buffer\";\n> -\t\t*ret = -ENOMEM;\n> -\t\treturn;\n> +\t\treturn -ENOMEM;\n>  \t}\n>  \n>  \tstd::unique_ptr<Request> request =\n>  \t\tstd::unique_ptr<Request>(camera_->createRequest());\n>  \tif (!request) {\n>  \t\tLOG(V4L2Compat, Error) << \"Can't create request\";\n> -\t\t*ret = -ENOMEM;\n> -\t\treturn;\n> +\t\treturn -ENOMEM;\n>  \t}\n>  \n> -\t*ret = request->addBuffer(std::move(buffer));\n> -\tif (*ret < 0) {\n> +\tint ret = request->addBuffer(std::move(buffer));\n> +\tif (ret < 0) {\n>  \t\tLOG(V4L2Compat, Error) << \"Can't set buffer for request\";\n> -\t\t*ret = -ENOMEM;\n> -\t\treturn;\n> +\t\treturn -ENOMEM;\n>  \t}\n>  \n>  \tif (!isRunning_) {\n>  \t\tpendingRequests_.push_back(std::move(request));\n> -\t\treturn;\n> +\t\treturn 0;\n>  \t}\n>  \n> -\t*ret = camera_->queueRequest(request.release());\n> -\tif (*ret < 0) {\n> +\tret = camera_->queueRequest(request.release());\n> +\tif (ret < 0) {\n>  \t\tLOG(V4L2Compat, Error) << \"Can't queue request\";\n> -\t\tif (*ret == -EACCES)\n> -\t\t\t*ret = -EBUSY;\n> +\t\treturn ret == -EACCES ? -EBUSY : ret;\n>  \t}\n> +\n> +\treturn 0;\n>  }\n> diff --git a/src/v4l2/v4l2_camera.h b/src/v4l2/v4l2_camera.h\n> index d24dbca6aaf4..5a889efdb4a2 100644\n> --- a/src/v4l2/v4l2_camera.h\n> +++ b/src/v4l2/v4l2_camera.h\n> @@ -48,23 +48,23 @@ public:\n>  \tV4L2Camera(std::shared_ptr<Camera> camera);\n>  \t~V4L2Camera();\n>  \n> -\tvoid open(int *ret);\n> +\tint open();\n>  \tvoid close();\n>  \tvoid getStreamConfig(StreamConfiguration *streamConfig);\n>  \tstd::vector<FrameMetadata> completedBuffers();\n>  \n> -\tvoid mmap(void **ret, unsigned int index);\n> +\tvoid *mmap(unsigned int index);\n>  \n> -\tvoid configure(int *ret, StreamConfiguration *streamConfigOut,\n> -\t\t       const Size &size, PixelFormat pixelformat,\n> -\t\t       unsigned int bufferCount);\n> +\tint configure(StreamConfiguration *streamConfigOut,\n> +\t\t      const Size &size, PixelFormat pixelformat,\n> +\t\t      unsigned int bufferCount);\n>  \n> -\tvoid allocBuffers(int *ret, unsigned int count);\n> +\tint allocBuffers(unsigned int count);\n>  \tvoid freeBuffers();\n> -\tvoid streamOn(int *ret);\n> -\tvoid streamOff(int *ret);\n> +\tint streamOn();\n> +\tint streamOff();\n>  \n> -\tvoid qbuf(int *ret, unsigned int index);\n> +\tint qbuf(unsigned int index);\n>  \n>  \tSemaphore bufferSema_;\n>  \n> diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp\n> index 559caba69270..2eeb12396d90 100644\n> --- a/src/v4l2/v4l2_camera_proxy.cpp\n> +++ b/src/v4l2/v4l2_camera_proxy.cpp\n> @@ -40,9 +40,8 @@ int V4L2CameraProxy::open(bool nonBlocking)\n>  {\n>  \tLOG(V4L2Compat, Debug) << \"Servicing open\";\n>  \n> -\tint ret;\n> -\tvcam_->invokeMethod(&V4L2Camera::open, ConnectionTypeBlocking,\n> -\t\t\t    &ret);\n> +\tint ret = vcam_->invokeMethod(&V4L2Camera::open,\n> +\t\t\t\t      ConnectionTypeBlocking);\n>  \tif (ret < 0) {\n>  \t\terrno = -ret;\n>  \t\treturn -1;\n> @@ -91,9 +90,8 @@ void *V4L2CameraProxy::mmap(size_t length, int prot, int flags, off_t offset)\n>  \t\treturn MAP_FAILED;\n>  \t}\n>  \n> -\tvoid *val;\n> -\tvcam_->invokeMethod(&V4L2Camera::mmap, ConnectionTypeBlocking,\n> -\t\t\t    &val, index);\n> +\tvoid *val = vcam_->invokeMethod(&V4L2Camera::mmap,\n> +\t\t\t\t\tConnectionTypeBlocking, index);\n>  \n>  \tbuffers_[index].flags |= V4L2_BUF_FLAG_MAPPED;\n>  \tmmaps_[val] = index;\n> @@ -245,9 +243,11 @@ int V4L2CameraProxy::vidioc_s_fmt(struct v4l2_format *arg)\n>  \t\treturn ret;\n>  \n>  \tSize size(arg->fmt.pix.width, arg->fmt.pix.height);\n> -\tvcam_->invokeMethod(&V4L2Camera::configure, ConnectionTypeBlocking,\n> -\t\t\t    &ret, &streamConfig_, size,\n> -\t\t\t    v4l2ToDrm(arg->fmt.pix.pixelformat), bufferCount_);\n> +\tret = vcam_->invokeMethod(&V4L2Camera::configure,\n> +\t\t\t\t  ConnectionTypeBlocking,\n> +\t\t\t\t  &streamConfig_, size,\n> +\t\t\t\t  v4l2ToDrm(arg->fmt.pix.pixelformat),\n> +\t\t\t\t  bufferCount_);\n>  \tif (ret < 0)\n>  \t\treturn -EINVAL;\n>  \n> @@ -297,9 +297,8 @@ int V4L2CameraProxy::freeBuffers()\n>  {\n>  \tLOG(V4L2Compat, Debug) << \"Freeing libcamera bufs\";\n>  \n> -\tint ret;\n> -\tvcam_->invokeMethod(&V4L2Camera::streamOff,\n> -\t\t\t    ConnectionTypeBlocking, &ret);\n> +\tint ret = vcam_->invokeMethod(&V4L2Camera::streamOff,\n> +\t\t\t\t      ConnectionTypeBlocking);\n>  \tif (ret < 0) {\n>  \t\tLOG(V4L2Compat, Error) << \"Failed to stop stream\";\n>  \t\treturn ret;\n> @@ -327,10 +326,11 @@ int V4L2CameraProxy::vidioc_reqbufs(struct v4l2_requestbuffers *arg)\n>  \t\treturn freeBuffers();\n>  \n>  \tSize size(curV4L2Format_.fmt.pix.width, curV4L2Format_.fmt.pix.height);\n> -\tvcam_->invokeMethod(&V4L2Camera::configure, ConnectionTypeBlocking,\n> -\t\t\t    &ret, &streamConfig_, size,\n> -\t\t\t    v4l2ToDrm(curV4L2Format_.fmt.pix.pixelformat),\n> -\t\t\t    arg->count);\n> +\tret = vcam_->invokeMethod(&V4L2Camera::configure,\n> +\t\t\t\t  ConnectionTypeBlocking,\n> +\t\t\t\t  &streamConfig_, size,\n> +\t\t\t\t  v4l2ToDrm(curV4L2Format_.fmt.pix.pixelformat),\n> +\t\t\t\t  arg->count);\n>  \tif (ret < 0)\n>  \t\treturn -EINVAL;\n>  \n> @@ -343,8 +343,8 @@ int V4L2CameraProxy::vidioc_reqbufs(struct v4l2_requestbuffers *arg)\n>  \targ->count = streamConfig_.bufferCount;\n>  \tbufferCount_ = arg->count;\n>  \n> -\tvcam_->invokeMethod(&V4L2Camera::allocBuffers,\n> -\t\t\t    ConnectionTypeBlocking, &ret, arg->count);\n> +\tret = vcam_->invokeMethod(&V4L2Camera::allocBuffers,\n> +\t\t\t\t  ConnectionTypeBlocking, arg->count);\n>  \tif (ret < 0) {\n>  \t\targ->count = 0;\n>  \t\treturn ret;\n> @@ -392,9 +392,8 @@ int V4L2CameraProxy::vidioc_qbuf(struct v4l2_buffer *arg)\n>  \t    arg->index >= bufferCount_)\n>  \t\treturn -EINVAL;\n>  \n> -\tint ret;\n> -\tvcam_->invokeMethod(&V4L2Camera::qbuf, ConnectionTypeBlocking,\n> -\t\t\t    &ret, arg->index);\n> +\tint ret = vcam_->invokeMethod(&V4L2Camera::qbuf, ConnectionTypeBlocking,\n> +\t\t\t\t      arg->index);\n>  \tif (ret < 0)\n>  \t\treturn ret;\n>  \n> @@ -437,9 +436,8 @@ int V4L2CameraProxy::vidioc_streamon(int *arg)\n>  \tif (!validateBufferType(*arg))\n>  \t\treturn -EINVAL;\n>  \n> -\tint ret;\n> -\tvcam_->invokeMethod(&V4L2Camera::streamOn,\n> -\t\t\t    ConnectionTypeBlocking, &ret);\n> +\tint ret = vcam_->invokeMethod(&V4L2Camera::streamOn,\n> +\t\t\t\t      ConnectionTypeBlocking);\n>  \n>  \treturn ret;\n>  }\n> @@ -451,9 +449,8 @@ int V4L2CameraProxy::vidioc_streamoff(int *arg)\n>  \tif (!validateBufferType(*arg))\n>  \t\treturn -EINVAL;\n>  \n> -\tint ret;\n> -\tvcam_->invokeMethod(&V4L2Camera::streamOff,\n> -\t\t\t    ConnectionTypeBlocking, &ret);\n> +\tint ret = vcam_->invokeMethod(&V4L2Camera::streamOff,\n> +\t\t\t\t      ConnectionTypeBlocking);\n>  \n>  \tfor (struct v4l2_buffer &buf : buffers_)\n>  \t\tbuf.flags &= ~(V4L2_BUF_FLAG_QUEUED | V4L2_BUF_FLAG_DONE);\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":"<niklas.soderlund@ragnatech.se>","Received":["from mail-lj1-x244.google.com (mail-lj1-x244.google.com\n\t[IPv6:2a00:1450:4864:20::244])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9507F6063B\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue,  7 Jan 2020 20:25:06 +0100 (CET)","by mail-lj1-x244.google.com with SMTP id m26so703340ljc.13\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 07 Jan 2020 11:25:06 -0800 (PST)","from localhost (h-93-159.A463.priv.bahnhof.se. [46.59.93.159])\n\tby smtp.gmail.com with ESMTPSA id\n\tq186sm237357ljq.14.2020.01.07.11.25.05\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tTue, 07 Jan 2020 11:25:05 -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\tbh=C/ZT0XU2r26SR6T3FhytdH7X6qXC4Ma+XpNyw1HyroQ=;\n\tb=pBLEUCZ69zizOzuFY2JYsfdO0HnYefVLg49rVJ5FtI0S10BH0HIdc/i9wfJkusAVvy\n\toc6e7ZPzMfXxtL5ZhOtBy3BjksMl8CuxkvDtGLksy5odFw83EpQC3PhKlRriAUuHkS8j\n\tHFbruIl0/XWfpbBisnWnXOW7H3tLoFcUMl9h5DZ+1BZMRyZ6zAR/+onsl3natwE3b3xr\n\t+eoGQkdsjDAZJIKp7MWFkG2VByCWojDyCIy0L1PRHCdbiKU8b2fsTGXmhieLGPg1BXG6\n\tJEay6VEdFdjdOjsjuM7qqVfO033F9gWQT0wqktVOd1oSvbR0szXB7VJLqeiyLkb8aUTp\n\tFvRQ==","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;\n\tbh=C/ZT0XU2r26SR6T3FhytdH7X6qXC4Ma+XpNyw1HyroQ=;\n\tb=knBuM4T03mqUUIDh2A4u5EYqK/4Yan8m+/prEHoRIOZqbF0pfUYEuyc6iIHJ/Oxxmn\n\td1ZV2loRGdFjgpHeMbISKar+3kqaYv3XoO24T8XU4f4Blwb2RJTFjeVn2zBbwGXuPh+o\n\texgWuaHzL3s21VziCiVsrWARvpOcJpWH/HI9ZFyCIT5JPwcV5tQRssxo38gJK1rSZyyT\n\t03/bJF+pAhZJP3fFYRPQ5uegaUR8fIAB/Nl7OGa5vVeuxuFkWJtdTWOAhCUzQZS2Xw9d\n\tNds0dZVTbA2hJUnCz5K0i4McXsF+P7xOL4zibFHToZ1vPFSaEuzPIKNREmGv5rkv/67Z\n\te6HQ==","X-Gm-Message-State":"APjAAAU5ZzLgeHlOwd33DZiMykOXU6typuWzjq5gRoOS3ir3BobijH6+\n\tG/RLt4vXMd6B/Sl6BbB0ktMdJkDEM7V7Ag==","X-Google-Smtp-Source":"APXvYqy14GQTC1w4lyaPyzOGMhSldh15UhkiH/909PPsYJj0fmZbSt8Zv0cggEm7i5JXAhs29l7ezQ==","X-Received":"by 2002:a2e:9f47:: with SMTP id v7mr605856ljk.124.1578425105859; \n\tTue, 07 Jan 2020 11:25:05 -0800 (PST)","Date":"Tue, 7 Jan 2020 20:25:04 +0100","From":"Niklas =?iso-8859-1?q?S=F6derlund?= <niklas.soderlund@ragnatech.se>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200107192504.GO533370@oden.dyn.berto.se>","References":"<20200104050947.7673-1-laurent.pinchart@ideasonboard.com>\n\t<20200104050947.7673-15-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":"<20200104050947.7673-15-laurent.pinchart@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 14/14] libcamera: v4l2: Use\n\tObject::invokeMethod() return value","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Tue, 07 Jan 2020 19:25:06 -0000"}}]